Closed Bug 1620348 Opened 6 years ago Closed 6 years ago

ubuntu1804 - uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 145] false == true

Categories

(Firefox :: File Handling, defect, P5)

Product:
Firefox
Component:
File Handling
Type:
defect

Tracking

()

Status:
RESOLVED INVALID

People

(Reporter: intermittent-bug-filer, Unassigned)

References

Details

Filed by: egao [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=291762519&repo=try
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/cv2L6MdmQY-UDC8OGte2pg/runs/0/artifacts/public/logs/live_backing.log

suite: xpcshell

context:
[task 2020-03-05T06:58:34.223Z] 06:58:34 WARNING - TEST-UNEXPECTED-FAIL | uriloader/exthandler/tests/unit/test_handlerService.js | xpcshell return code: 0
[task 2020-03-05T06:58:34.223Z] 06:58:34 INFO - TEST-INFO took 181ms
[task 2020-03-05T06:58:34.227Z] 06:58:34 INFO - >>>>>>>
[task 2020-03-05T06:58:34.227Z] 06:58:34 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2020-03-05T06:58:34.227Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 97] true == true
[task 2020-03-05T06:58:34.227Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 99] "nonexistent/type" == "nonexistent/type"
[task 2020-03-05T06:58:34.227Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 102] "nonexistent/type" == "nonexistent/type"
[task 2020-03-05T06:58:34.227Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 105] 0 == 0
[task 2020-03-05T06:58:34.229Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 106] null == null
[task 2020-03-05T06:58:34.229Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 107] 0 == 0
[task 2020-03-05T06:58:34.229Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 108] true == true
[task 2020-03-05T06:58:34.229Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 112] "" == ""
[task 2020-03-05T06:58:34.230Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 113] false == false
[task 2020-03-05T06:58:34.230Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 114] "" == ""
[task 2020-03-05T06:58:34.230Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 136] 1 == 1
[task 2020-03-05T06:58:34.234Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 137] true == true
[task 2020-03-05T06:58:34.234Z] 06:58:34 INFO - TEST-PASS | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 144] 0 == 0
[task 2020-03-05T06:58:34.234Z] 06:58:34 WARNING - TEST-UNEXPECTED-FAIL | uriloader/exthandler/tests/unit/test_handlerService.js | run_test - [run_test : 145] false == true
[task 2020-03-05T06:58:34.234Z] 06:58:34 INFO - /builds/worker/workspace/build/tests/xpcshell/tests/uriloader/exthandler/tests/unit/test_handlerService.js:run_test:145
[task 2020-03-05T06:58:34.234Z] 06:58:34 INFO - /builds/worker/workspace/build/tests/xpcshell/head.js:_execute_test:567
[task 2020-03-05T06:58:34.234Z] 06:58:34 INFO - -e:null:1
[task 2020-03-05T06:58:34.234Z] 06:58:34 INFO - exiting test
[task 2020-03-05T06:58:34.234Z] 06:58:34 INFO - <<<<<<<

try push: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&tier=1%2C2%2C3&revision=626ba877eaa03e587b0c9acf2ef96f7b2d93c3f8

I have been experimenting with removal of a bunch of preinstalled packages that come with the default ubuntu:18.04 docker image to reduce its size.

For a baseline reference, the now-deprecated ubuntu1604-based system has both firefox and thunderbird installed in the image.

For the ubuntu1804-based image, removal of thunderbird does not appear to cause any unexpected failures in CI.
However, removal of firefox package causes this one failure to crop up in xpcshell which I think is directly related to the OS losing the default method for handling the opening of a webpage.

Specifically, the logs above point to this as being the issue:
https://searchfox.org/mozilla-central/source/uriloader/exthandler/tests/unit/test_handlerService.js#139-145

I suspect that this test could use a review, since I don't think the test should fail even if firefox is not installed on the system by default.

:Gijs - I don't know who to ping for this specific test. Could you redirect my ni to someone that could comment on this, thanks.

Flags: needinfo?(gijskruitbosch+bugs)

Furthermore, this failure on test_handlerService.js only appears on webrender when the installed package firefox is removed. Non-webrender xpcshell does not experience the same issue.

(In reply to Edwin Takahashi (:egao) from comment #1)

Specifically, the logs above point to this as being the issue:
https://searchfox.org/mozilla-central/source/uriloader/exthandler/tests/unit/test_handlerService.js#139-145

I suspect that this test could use a review, since I don't think the test should fail even if firefox is not installed on the system by default.

:Gijs - I don't know who to ping for this specific test. Could you redirect my ni to someone that could comment on this, thanks.

Yeah, this test relies on the OS having a default handler for the http protocol. That seems like a pretty reasonable expectation in that all the major OSes do have these (Windows, macOS). The fact that you can uninstall all the browsers on Linux systems is interesting, but not very realistic...

Basically, if we want to avoid having the firefox package, someone needs to fix the test. To fix it, you could add a noWeb variable at https://searchfox.org/mozilla-central/rev/13b081a62d3f3e3e3120f95564529257b0bf451c/uriloader/exthandler/tests/unit/test_handlerService.js#43-55 and do the same thing as we do for mailto, and then on https://searchfox.org/mozilla-central/source/uriloader/exthandler/tests/unit/test_handlerService.js#145, Assert.equal(noWeb, protoInfo.alwaysAskBeforeHandling, "If there is a default handler, don't ask"). The comment at https://searchfox.org/mozilla-central/rev/13b081a62d3f3e3e3120f95564529257b0bf451c/uriloader/exthandler/tests/unit/test_handlerService.js#139 will also want updating.

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(egao)

(In reply to Edwin Takahashi (:egao) from comment #2)

Furthermore, this failure on test_handlerService.js only appears on fission when the installed package firefox is removed. Non-fission xpcshell does not experience the same issue.

Uh, can you elaborate? I don't think we run xpcshell with/without fission - https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&selectedJob=291960136&tier=1%2C2%2C3&searchStr=xpcshell doesn't show any xpcshell jobs other than "regular" xpcshell.

(In reply to :Gijs (he/him) from comment #4)

(In reply to Edwin Takahashi (:egao) from comment #2)

Furthermore, this failure on test_handlerService.js only appears on fission when the installed package firefox is removed. Non-fission xpcshell does not experience the same issue.

Uh, can you elaborate? I don't think we run xpcshell with/without fission - https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&selectedJob=291960136&tier=1%2C2%2C3&searchStr=xpcshell doesn't show any xpcshell jobs other than "regular" xpcshell.

Argh, sorry, I made a mistake when typing that out - conflated fission with webrender. I've updated the comment with the correct terminologies.

(In reply to :Gijs (he/him) from comment #3)

(In reply to Edwin Takahashi (:egao) from comment #1)

Specifically, the logs above point to this as being the issue:
https://searchfox.org/mozilla-central/source/uriloader/exthandler/tests/unit/test_handlerService.js#139-145

I suspect that this test could use a review, since I don't think the test should fail even if firefox is not installed on the system by default.

:Gijs - I don't know who to ping for this specific test. Could you redirect my ni to someone that could comment on this, thanks.

Yeah, this test relies on the OS having a default handler for the http protocol. That seems like a pretty reasonable expectation in that all the major OSes do have these (Windows, macOS). The fact that you can uninstall all the browsers on Linux systems is interesting, but not very realistic...

Thanks. I agree that having a browser installed on the system is a realistic expectation, so I don't think I'll pursue the path of having the test revisited. The preinstalled Firefox, while consuming a good chunk of space, isn't worth requiring a developer to spend some hours on looking at this.

With that said, I am surprised to find that this particular test is exercising the operating system defaults, not the Firefox version that is being tested in CI. If this particular xpcshelltest was conceived as a test of the operating system handler (not specifically a test of Firefox) then I could understand the value, but otherwise this seems like something that belongs more in the unit/integration test of the operating system developers themselves.

Alas, this is just one test, and unless this test proves to be a problem sometime down the line, I won't really spend any more time on it. Thanks for your kind explanation, it has been quite helpful to understand.

Basically, if we want to avoid having the firefox package, someone needs to fix the test. To fix it, you could add a noWeb variable at https://searchfox.org/mozilla-central/rev/13b081a62d3f3e3e3120f95564529257b0bf451c/uriloader/exthandler/tests/unit/test_handlerService.js#43-55 and do the same thing as we do for mailto, and then on https://searchfox.org/mozilla-central/source/uriloader/exthandler/tests/unit/test_handlerService.js#145, Assert.equal(noWeb, protoInfo.alwaysAskBeforeHandling, "If there is a default handler, don't ask"). The comment at https://searchfox.org/mozilla-central/rev/13b081a62d3f3e3e3120f95564529257b0bf451c/uriloader/exthandler/tests/unit/test_handlerService.js#139 will also want updating.

Good reference, thank you. I'll keep this in mind should I decide to revisit this test.

Flags: needinfo?(egao)

Closing as invalid - won't work on it. If the default handler behavior changes in the future, reopen this and take a look.

Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID

(In reply to Edwin Takahashi (:egao) from comment #5)

Thanks. I agree that having a browser installed on the system is a realistic expectation, so I don't think I'll pursue the path of having the test revisited. The preinstalled Firefox, while consuming a good chunk of space, isn't worth requiring a developer to spend some hours on looking at this.

With that said, I am surprised to find that this particular test is exercising the operating system defaults, not the Firefox version that is being tested in CI. If this particular xpcshelltest was conceived as a test of the operating system handler (not specifically a test of Firefox) then I could understand the value, but otherwise this seems like something that belongs more in the unit/integration test of the operating system developers themselves.

It's less that the intent is to test the OS, more that we want to test whether we ask the user about opening a protocol if an internal pref to warn/ask is not set and there is an OS default handler available. The test checks for mailto: as well, for which we have some web-based defaults built-in (gmail and yahoo mail in the en-US case). I guess it checked for http as a protocol because it was a "safe" bet - every machine has a default browser - doesn't matter what it is.

Of course, the "pure" way to test this would be to mock the OS's availability information, but that is much more difficult than relying on the (now no longer safe...) assumption that there'll be an http handler somewhere...

You need to log in before you can comment on or make changes to this bug.