727842 - httpd server in mozmill stops after Firefox restarts in restart tests

Status: RESOLVED WORKSFORME

(Testing Graveyard :: Mozmill, defect)

Description

[Remus Pop (:RemusPop)]

Attachment: testcase1

The httpd server works only if a testfile calls the collector.addHttpResource(). After a restart it will still work if another call to addHttpResource is made.

STR: apply testcase1 patch on a clean mozmill-tests repo and from inside run mozmill-restart -t tests/functional/restartTests/testAddons_installUninstallBlocklistedExtension/ -b /path/to/firefox/nightly --show-all

Actual: You will see in terminal/console, the output of the blocklist:
*** Blocklist::notify: Requesting http://localhost:43336/addons/blocklist/softblock_extension/blocklist.xml
*** Blocklist:onError: There was an error loading the blocklist file

Expected: The test will end with no failures.

In testcase1 addHttpResource() call is only in test1.js. Also in test1 we set the pref extensions.blocklist.url to a file in localhost, so in test2, when we ping the blocklist, the file is no longer available.

In testcase2, I've moved const BLOCKLIST_URL in order to call addHttpResource(), so this testcase should not fail when run with instructions from STR.

Notes: 
* after applying testcase1 you can also add a sleep(60000) (1 minute) in test2 before we ping the the blocklist and try to load http://localhost:43336/addons/blocklist/softblock_extension/blocklist.xml in the browser you currently use.
* this testcases currently include blocklist.xml, but this will be available after bug 727835 lands

Comment 1

[Remus Pop (:RemusPop)]

Attachment: testcase1

Sorry, wrong code.

Comment 2

[Remus Pop (:RemusPop)]

Attachment: testcase2

Actually in this patch I've only added:
const LOCAL_TEST_FOLDER = collector.addHttpResource("../../../../data/");
because I think that is where the httpd starts.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Why has this bug been filed under MozQA automation? It should have been put under Testing / Mozmill because it's a core issue.

Remus, instead of attaching two testcases can you please attach a patch instead? Thanks.

Comment 4

[Remus Pop (:RemusPop)]

Attachment: testcase

Forget what I said previously. I hope this will be easier to understand.

Minimising the tescases I found out that if you create a url (with addHttpResource) and you do not use it (e.g. open/access) you won't be able to open/access the link after a restart.

How this works:
In test1.js we set a pref to a custom url that will be fetched in test2.js. We also enable extensions logging in order to see the status of addons.
In test2.js we try to fetch the url through utils.updateBlocklist() and in case you missed the logging in the console we also try to open the same url in a tab.

How will this ever work? Uncomment the last lines in test1.js. So accessing a link makes that available in all the tests.

How to test: qimport the patch to a clean repo. qpush it. Run mozmill-restart -t tests/functional/restartTests/testHttpd/ -b /path/to/firefox --show-all

Comment 5

[cmtalbert]

Attachment: transforming the test to 2.0

So, I believe that we may have fixed this issue along the way in 2.0.  I've modified your test to work on mozmill 2.0 and it seems to work.

(It passes the test with the two lines in "test one" commented out). So, I think we are OK here.

--> Resolve as WFM?

Comment 6

[cmtalbert]

Because we are about to do some triage, I'm going to go ahead and push this to WFM status since it seems that the issue is resolved. Feel free to re-open if you disagree.

Comment 7

[Remus Pop (:RemusPop)]

Clint, I was referring to mozmill-restart, so my test is still applicable.
I've ran my test with mozmill 2.0 but the same failure occurs.

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Remus: Can you attach a patch with the test updated for Mozmill 2.0? With Mozmill 2.0 we will be using controller.restartApplication(aTestMethod) where aTestMethod is the name of the next test method.

Comment 9

[Remus Pop (:RemusPop)]

Dave, Clint already attached the updated testcase. I wasn't aware of how mozmill 2.0 used the restart application, so my bad here. Please resolve fix this bug if his patch is what we want or leave a comment if more is needed.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: testcase (2.0)

So this is still a problem in Mozmill 2.0. The transformed testcase for 2.0 simply hasn't followed all the steps and scopes from the original testcase.

As it looks like the collector always wants to be initialized again after a restart within the same restart module. That seems to be wrong to me. It should really happen only once for each test module.

The same workaround can be done here by simply placing the assignment of the data folder into the global scope or setupTest so that it will run each time the application gets restarted. That's not a good way because it will leave gaps behind and can cause data corruption in some of our tests.

Comment 11

[cmtalbert]

(In reply to Henrik Skupin (:whimboo) from comment #10)
> Created attachment 627889 [details]
> testcase (2.0)
> 
> So this is still a problem in Mozmill 2.0. The transformed testcase for 2.0
> simply hasn't followed all the steps and scopes from the original testcase.
> 
> As it looks like the collector always wants to be initialized again after a
> restart within the same restart module. That seems to be wrong to me. It
> should really happen only once for each test module.
That's exactly why Remus's and your test here have this problem. Collector is a *javascript* object. It *must* be recreated each time you restart, so its instantiation must be in a setupTest step. It cannot be in a setupmodule step because this test restarts. 

There is no bug here, this test is just wrong.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Well, sort of. So the final solution here without having workarounds in the tests would be to wait for the mozhttpd server? See bug 754847 for reference.

Comment 13

[cmtalbert]

Because we have a method to work with our current httpd.js file, we can close this WFM.

As Henrik notes, optimally mozmill would use a single http server for the entire lifetime of a test and then you wouldn't need to create the http server during your test, so once we shift to mozhttpd we will have a single instance of the web server running during the lifetime of a test and this will solve this issue.

So WFM now, and bug 754847 will be the final solution.