mozmill-2.0 functional testrun does not restart between restartTests This is the actual problem for bug 860670

Mozmill 2.0 does not do restart tests in the same way. As I understand it, these will all be refactored for Mozmill 2.0 compatibility.

Please never file something as blocking mozmill-2.0. All bugs have to go through the triage process. As we have discussed in one of our first triage meetings any restart test is not in our focus for now. So it may not block 2.0. But we will see. Also I think it may be a dupe of a bug which is already on file.

Sorry for that. Filed it as blocking for mozmill-2.0 because it blocks 860670 (and that is set to block mozmill-2.0). Will bring these up in today's meeting.

Changed restart tests to manually restart if we have that option (mozmill 2.0) Didn't ask for review since I couldn't get a positive run on Linux with mozmill 1.5 (reports and error messages below) Patch not applied - reference ============================= Mozmill 2.0 OSX: http://mozmill-crowd.blargon7.com/#/functional/report/452ec32f8deec0960aea87aa065973ed Mozmill 1.5 Linux: TEST-PASS | /tmp/tmp5gR6xc.mozmill-tests/tests/functional/restartTests/testAddons_installUninstallHardBlocklistedExtension/test2.js | test2.js::testBlocklistsExtension Sorry, cannot connect to jsbridge extension, port 24242 *** Removing repository '/tmp/tmp5gR6xc.mozmill-tests' Patch applied ============= Mozmill 2.0 OSX: http://mozmill-crowd.blargon7.com/#/functional/report/452ec32f8deec0960aea87aa06576b83 Windows: http://mozmill-crowd.blargon7.com/#/functional/report/452ec32f8deec0960aea87aa0686124d Linux: http://mozmill-crowd.blargon7.com/#/functional/report/452ec32f8deec0960aea87aa068f36e9 Mozmill 1.5 OSX: http://mozmill-crowd.blargon7.com/#/functional/report/452ec32f8deec0960aea87aa0655a641 Windows: http://mozmill-crowd.blargon7.com/#/functional/report/452ec32f8deec0960aea87aa06863f41 Linux. No testrun TEST-PASS | /tmp/tmp8BgEXs.mozmill-tests/tests/functional/testInstallation/testBreakpadInstalled.js | testBreakpadInstalled.js::testBreakpadInstalled NOTE: child process received `Goodbye', closing down WARNING: waitpid failed pid:3696 errno:10: file /builds/slave/m-cen-lx-ntly-0000000000000000/build/ipc/chromium/src/base/process_util_posix.cc, line 260 WARNING: waitpid failed pid:3696 errno:10: file /builds/slave/m-cen-lx-ntly-0000000000000000/build/ipc/chromium/src/base/process_util_posix.cc, line 260 WARNING: Failed to deliver SIGKILL to 3696!(3).: file /builds/slave/m-cen-lx-ntly-0000000000000000/build/ipc/chromium/src/chrome/common/process_watcher_posix_sigchld.cc, line 118 INFO Passed: 210 INFO Failed: 1 INFO Skipped: 13 Report document created at 'http://mozmill-crowd.blargon7.com/db/452ec32f8deec0960aea87aa06906616' Sorry, cannot connect to jsbridge extension, port 24242

This is not a Mozmill issue but in our tests. Moving to the right component.

1. We manually restart between each restart test and 2. We also reset the profile between each restart suits Mozmill 1.5 ----------- OSX: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c20b1c1f Linux: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c210cd4b Windows: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c2278a1e Mozmill 2.0 ----------- OSX: http://mozmill-crowd.blargon7.com/#/functional/report/ea82256a8ae9808d91b7e8145ee27b5e Linux: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c2167318 ** I'm having a hard time getting a complete functional testrun for mozmill 2.0 under windows, keep getting "IO Completion Port unexpectedly closed" which seem to lead to a Application disconnect error. Raised bug 872414 for that.

Oh, forgot to add one additional note: I had to also skip the teardown in > testAddons_uninstallExtension/test5.js which was skipped for bug 783484 (but wrongfully I missed the teardown skip there) And it would cras: teardown would ran; since we need the controller which is instantiated in setup which is skipped. I've added that here. Maybe should have a different patch for that in bug 783484 ?

Comment on attachment 749709 [details] [diff] [review] patch v2 Review of attachment 749709 [details] [diff] [review]: ----------------------------------------------------------------- ::: tests/functional/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test1.js @@ +68,5 @@ > var addonIsInstalled = addonsManager.isAddonInstalled({addon: anAddon}); > > assert.ok(addonIsInstalled, ADDON.id + " is successfully installed"); > + > + if ("restartApplication" in controller) Please add a comment to all of those cases which says why we need this if condition. Also reference this bug. We should remove it once transitioned to Mozmill 2.0.

Comment on attachment 749709 [details] [diff] [review] patch v2 Review of attachment 749709 [details] [diff] [review]: ----------------------------------------------------------------- Based on Henrik's comment.

Added comments referencing this bug and reminding to remove the condition once we transition to mozmill 2.0

Comment on attachment 749839 [details] [diff] [review] patch v3 Review of attachment 749839 [details] [diff] [review]: ----------------------------------------------------------------- We're getting closer, one more change needed. ::: tests/functional/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test1.js @@ +68,5 @@ > var addonIsInstalled = addonsManager.isAddonInstalled({addon: anAddon}); > > assert.ok(addonIsInstalled, ADDON.id + " is successfully installed"); > + > + // XXX Bug 867217: Mozmill 1.5 does not have the restartApplication method We no longer use XXX and now we also have a bug to reference. Also please move the description below the bug no.

Changed the comment as asked. *As a sidenote I opened bug 872918 since these comments are not consistent and we should fix that.

Fixed a problem in testAddons_uninstallExtension. Since tests 4 and 5 are skipped we need to reset the profile in test3. Also added a comment to not forget to remove that when we unskip the last 2 tests.

Comment on attachment 750419 [details] [diff] [review] patch 5 Review of attachment 750419 [details] [diff] [review]: ----------------------------------------------------------------- There is still something wrong with, I keep getting this window with "Firefox is already running, but is not responding.. ", when ran with 2.0, after different tests (is not one specific that creates this issue). Can you further investigate? We can't have this error as it is blocking the testrun to continue.

I have no problem running testruns under Mozmill 2.0: OSX: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c29dc4d7 Linux: http://mozmill-crowd.blargon7.com/#/functional/report/14f8bc4e22e61353662cded4c2940afd

Updated patch (would not apply cleanly anymore). I did manage to have a failed run (out of about 6), as you reported: we get a message that a Firefox instance is already running, then we hang with a Disconnect Error. Opened bug 874856 for that However that doesn't seem related to this bug, since most runs don't fail that way. More testruns: Linux (Ubuntu 12.04): http://mozmill-crowd.blargon7.com/#/functional/report/90a0b225333a4e0c868fcb2b442d4247 http://mozmill-crowd.blargon7.com/#/functional/report/90a0b225333a4e0c868fcb2b442d76e9 OSX: http://mozmill-crowd.blargon7.com/#/functional/report/90a0b225333a4e0c868fcb2b442d7372

Comment on attachment 752701 [details] [diff] [review] patch v6 Review of attachment 752701 [details] [diff] [review]: ----------------------------------------------------------------- This is no longer applying cleanly and also I get this failure on both 1.5 and 2.0, while with a clean repository that test passes: http://mozmill-crowd.blargon7.com/#/functional/report/90a0b225333a4e0c868fcb2b44e96eb9 Please add reports when uploading the new patch. ::: tests/functional/restartTests/testAddons_RestartlessExtensionWorksAfterRestart/test1.js @@ +73,5 @@ > + > + // Bug 867217 > + // Mozmill 1.5 does not have the restartApplication method on the controller. > + // Remove condition when transitioned to 2.0 > + if ("restartApplication" in controller) Please add brackets to all files.

Added curly braces for all if clauses. Out guide recommends omitting them for 1 liners if its still readable: https://developer.mozilla.org/en-US/docs/Mozmill_Tests/Mozmill_Style_Guide#Conditionals We should probably update that if it no longer holds. Also each restart call lives now in teardownModule(), add the teardownModule function where there wasn't one just for this call. Makes code more cleaner to have this consistent. Have no Linux testruns for 2.0 because of bug 877101 Mozmill 1.5 =========== OSX: http://mozmill-crowd.blargon7.com/#/functional/report/c93c51b0c0b1af1b05623929160ca84e http://mozmill-crowd.blargon7.com/#/functional/report/c93c51b0c0b1af1b05623929160d2e9c Windows: http://mozmill-crowd.blargon7.com/#/functional/report/c93c51b0c0b1af1b05623929161aeb65 http://mozmill-crowd.blargon7.com/#/functional/report/c93c51b0c0b1af1b05623929161b50f7 Linux: http://mozmill-crowd.blargon7.com/#/functional/report/c93c51b0c0b1af1b0562392916133fd6 http://mozmill-crowd.blargon7.com/#/functional/report/c93c51b0c0b1af1b0562392916134e9a Mozmill 2.0 =========== OSX: http://mozmill-crowd.blargon7.com/#/functional/report/c93c51b0c0b1af1b05623929160fc7ec Windows: http://mozmill-crowd.blargon7.com/#/functional/report/c93c51b0c0b1af1b05623929161ae365

Comment on attachment 755317 [details] [diff] [review] patch v7 Review of attachment 755317 [details] [diff] [review]: ----------------------------------------------------------------- Landed: http://hg.mozilla.org/qa/mozmill-tests/rev/b8a22c8c9a3b (default) Lets see how our testrun looks like in 2.0 since this and select() have been fixed.

Comment on attachment 755317 [details] [diff] [review] patch v7 Review of attachment 755317 [details] [diff] [review]: ----------------------------------------------------------------- ::: tests/functional/restartTests/testSoftwareUpdateAutoProxy/test1.js @@ +31,5 @@ > + // Bug 867217 > + // Mozmill 1.5 does not have the restartApplication method on the controller. > + // Remove condition when transitioned to 2.0 > + if ("restartApplication" in aModule.controller) { > + aModule.controller.restartApplication(null, true); This should not reset the profile here! This will cause the test to misbehave. Please re-check all of the instances before we get this backported. A follow-up is necessary here.

It is really weird how this regressed. I could not find any instance of the regressed code in any locally saved patches. I have no clue on how I managed to botch that. Anyway here is a followup patch which fixes the problem. Also rechecked all restartTests to make sure no other regressions are found.

Comment on attachment 763458 [details] [diff] [review] Followup patch 1 Review of attachment 763458 [details] [diff] [review]: ----------------------------------------------------------------- I need this test for my local io port testing, so I would like to land it now.

Comment on attachment 763458 [details] [diff] [review] Followup patch 1 Review of attachment 763458 [details] [diff] [review]: ----------------------------------------------------------------- Landed on default: http://hg.mozilla.org/qa/mozmill-tests/rev/cf49310c4232 Please check the other branches, so that we can get this backported.

Backport for Aurora

Testruns for all backports

Release and beta are the same at the moment. Why do we need different patches?

mozilla-release and mozilla-beta are not the same at the moment.

What's different? I have merged beta into release 2 days ago and nothing else has been landed on beta yet. This is strange.

So the merge has already been made? Maybe I am misunderstanding how this should work. Here is a diff between 2 branches: mozilla-release vs mozilla-beta Anyway, retested now and the patch for beta now applies cleanly on the release branch. I might have started working on them right before you did the merge. Marking that Release patch as obsolete.

Comment on attachment 765344 [details] mozilla-release vs mozilla-beta Yeah, this diff is before you did the last pull.

Comment on attachment 765259 [details] [diff] [review] Backport Aurora Review of attachment 765259 [details] [diff] [review]: ----------------------------------------------------------------- Due to the merges, aurora patch is now on beta: http://hg.mozilla.org/qa/mozmill-tests/rev/58861ba75b0a (beta) http://hg.mozilla.org/qa/mozmill-tests/rev/50710a8ef5f5 (release) http://hg.mozilla.org/qa/mozmill-tests/rev/8521d0f4c211 (esr17)

Updating summary so it's clear what has been done here without to confuse people with user shutdown modes.