567108 - Mozmill should allow an user initiated exit of the browser

Status: VERIFIED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

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

In the current state Mozmill is automatically shutting down the browser. There is no way for a test itself to specify the way how it get shutdown. But for a couple of tests we need this feature, e.g. installing an add-on and clicking the restart button to check it's functionality. Also for Session Restore it is very important.

We should have a solution here for the next minor release.

Clint, would be great if your team could make this!

Comment 1

[cmtalbert]

(In reply to comment #0)
> In the current state Mozmill is automatically shutting down the browser. There
> is no way for a test itself to specify the way how it get shutdown. But for a
> couple of tests we need this feature, e.g. installing an add-on and clicking
> the restart button to check it's functionality. Also for Session Restore it is
> very important.
> 
> We should have a solution here for the next minor release.
> 
> Clint, would be great if your team could make this!
When?  Is this a 1.4.2 feature or a Mozmill.next?  I think Mozmill.next.

Comment 2

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

(In reply to comment #1)
> When?  Is this a 1.4.2 feature or a Mozmill.next?  I think Mozmill.next.

This one slipped through my list but I have seen it while checking the Session Store tests on Tuesday. This subgroup is targeted for this quarter. So if it's not that difficult to fix it would really help us. Otherwise most of the tests are blocked by this bug for now. But it's really up to your time.

Anthony, can you please check your created bugs and add the relevant bugs to the blocking list? Thanks.

Comment 3

[u279076]

All Session Store bugs added as blocked by this bug.  Here's a query if you are interested: http://bit.ly/dnGiBM

Comment 4

[Andrew Halberstadt [:ahal]]

This can be implemented for 1.4.2.
There are two things we'll need to do due to the fact that any mouse click or keypress could now potentially restart FFx:

1) Differentiate between expected restarts and unexpected restarts/crashes.
2) Determine when a restart was expected but failed to happen.

I propose a "restart timeout" which will need to be set immediately before an expected restart of FFx.  If no restart is detected during this timeout mozmill would throw a failure and resume execution of the test (which would then presumably restart Firefox in the current manner).  On the other hand if a restart happens and no timeout was set, mozmill will exit as it normally does.

Comment 5

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

Sounds like a plan for me. Can we get this wrapped into a controller function, i.e. controller.startUserShutdown(timeout)?

Comment 6

[Andrew Halberstadt [:ahal]]

Attachment: Allows user initiated shutdown and restart of mozmill

You can now call controller.setUserShutdown(timeout, restart) where timeout is the number of milliseconds mozmill will allow a user shutdown and restart is a boolean indicating whether a restart or shutdown is expected.

This will work for both mozmill and mozmill-restart (although note that you shouldn't use the restart option with mozmill tests)

Comment 7

[cmtalbert]

Comment on attachment 458005 [details] [diff] [review]
Allows user initiated shutdown and restart of mozmill

>diff --git a/mozmill/mozmill/__init__.py b/mozmill/mozmill/__init__.py
>index add2b1a..11eee84 100644
>--- a/mozmill/mozmill/__init__.py
>+++ b/mozmill/mozmill/__init__.py
>@@ -137,6 +137,9 @@ class MozMill(object):
> 
>         self.persisted = {}
>         self.endRunnerCalled = False
>+        self.shutdownModes = self.enum('default', 'user_shutdown', 'user_restart')
>+        self.currentShutdownMode = self.shutdownModes.default
>+        self.userShutdownEnabled = False
>         #self.zombieDetector = ZombieDetector(self.stop)
>         self.global_listeners = []
>         self.listeners = []
>@@ -144,6 +147,10 @@ class MozMill(object):
>         self.add_listener(self.endTest_listener, eventType='mozmill.endTest')
>         self.add_listener(self.endRunner_listener, eventType='mozmill.endRunner')
> 
>+    def enum(*sequential, **named):
>+        enums = dict(zip(sequential, range(len(sequential))), **named)
>+        return type('Enum', (), enums)
>+
Isn't this going to associate 'default' == 0, 'user_shutdown' == 1, and 'user_restart == 2'?
But elsewhere, inside controller, you depend on default = 1, shutdown = 2, and restart = 3.  Which one is right? Or am I missing something?

>-        self.runner.start()
>+        if self.currentShutdownMode != self.shutdownModes.user_restart:
>+            self.runner.start()
Why does this only check for != restart?  Shouldn't it check for != shutdown too?

>@@ -472,9 +492,9 @@ class MozMillRestart(MozMill):
>-        self.add_listener(self.endTest_listener, eventType='mozmill.endTest')
>+        #self.add_listener(self.endTest_listener, eventType='mozmill.endTest')
>         self.add_listener(self.firePythonCallback_listener, eventType='mozmill.firePythonCallback')
>-        # self.add_listener(self.endRunner_listener, eventType='mozmill.endRunner')
>+        #self.add_listener(self.endRunner_listener, eventType='mozmill.endRunner')
Not sure I understand what's up here.  Why is the endTest listener no longer needed?  If it is no longer needed then we can just remove these commented out lines.  I think the mozmill base class still uses it, but does the restart class no longer need it?
> 
>+MozMillController.prototype.startUserShutdown = function (timeout, restart) {
>+  // 1 = default shutdown, 2 = user shutdown, 3 = user restart
>+  this.fireEvent('userShutdown', restart ? 3 : 2);
>+  this.window.setTimeout(this.fireEvent, timeout, 'userShutdown', 1);
>+}
This seems like it should be 0, 1, 2.  I'd change the name restart to shouldRestart so it's obvious that's a variable.  Also, since this changes the controller api we'll need to change the documentation on MDC.  Please file a follow on bug for that.

>diff --git a/mozmill/mozmill/extension/resource/modules/frame.js b/mozmill/mozmill/extension/resource/modules/frame.js
>index 7ec29c1..043e10b 100644
>--- a/mozmill/mozmill/extension/resource/modules/frame.js
>+++ b/mozmill/mozmill/extension/resource/modules/frame.js

>diff --git a/mozmill/test/restart/test_user_restart/test1.js b/mozmill/test/restart/test_user_restart/test1.js
>new file mode 100644
>index 0000000..c1645ed
>--- /dev/null
>+++ b/mozmill/test/restart/test_user_restart/test1.js
>@@ -0,0 +1,13 @@
>+var setupModule = function(module) {
>+  module.controller = mozmill.getBrowserController();
>+}
>+
>+/**
>+ * This test should pass
>+ */
>+var testRestartBeforeTimeout = function() {
>+  controller.startUserShutdown(2000, true);
>+  controller.sleep(1000);
>+  controller.window.Application.restart();
>+  controller.window.alert("Should not see this");
>+}
Thanks for these tests.  Are they standard mozmill restart tests?  Or are there necessary manual steps that must be done?  If there are other steps, please add a comment on test1 outlining what those are.

Very good patch.  Please address those questions, and I think we're good to go.
r=ctalbert with the questions answered and follow up bugs filed.

Comment 8

[Andrew Halberstadt [:ahal]]

(In reply to comment #7)
> Comment on attachment 458005 [details] [diff] [review]
> Allows user initiated shutdown and restart of mozmill
> 
> >+    def enum(*sequential, **named):
> >+        enums = dict(zip(sequential, range(len(sequential))), **named)
> >+        return type('Enum', (), enums)
> >+
> Isn't this going to associate 'default' == 0, 'user_shutdown' == 1, and
> 'user_restart == 2'?
> But elsewhere, inside controller, you depend on default = 1, shutdown = 2, and
> restart = 3.  Which one is right? Or am I missing something?
I looked into it and for some reason it is 0 based when run standalone, but 1 based when integrated with Mozmill.  I'll hardcode the numbers to avoid weird bugs in the future.

> >-        self.runner.start()
> >+        if self.currentShutdownMode != self.shutdownModes.user_restart:
> >+            self.runner.start()
> Why does this only check for != restart?  Shouldn't it check for != shutdown
> too?
We still need mozmill to start the browser for the case that we shutdown  during a mozmill-restart test but still need to continue.

> >@@ -472,9 +492,9 @@ class MozMillRestart(MozMill):
> >-        self.add_listener(self.endTest_listener, eventType='mozmill.endTest')
> >+        #self.add_listener(self.endTest_listener, eventType='mozmill.endTest')
> >         self.add_listener(self.firePythonCallback_listener, eventType='mozmill.firePythonCallback')
> >-        # self.add_listener(self.endRunner_listener, eventType='mozmill.endRunner')
> >+        #self.add_listener(self.endRunner_listener, eventType='mozmill.endRunner')
> Not sure I understand what's up here.  Why is the endTest listener no longer
> needed?  If it is no longer needed then we can just remove these commented out
> lines.  I think the mozmill base class still uses it, but does the restart
> class no longer need it?
This was an unrelated bug with a one line fix. We were already adding an endTest listener in the mozmill class constructor. The mozmill-restart class was adding a second one so output was being printed twice (i.e two tests reported as passed instead of one).

> > 
> >+MozMillController.prototype.startUserShutdown = function (timeout, restart) {
> >+  // 1 = default shutdown, 2 = user shutdown, 3 = user restart
> >+  this.fireEvent('userShutdown', restart ? 3 : 2);
> >+  this.window.setTimeout(this.fireEvent, timeout, 'userShutdown', 1);
> >+}
> This seems like it should be 0, 1, 2.  I'd change the name restart to
> shouldRestart so it's obvious that's a variable.  Also, since this changes the
> controller api we'll need to change the documentation on MDC.  Please file a
> follow on bug for that.
Will file a documentation bug.

Comment 9

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

(In reply to comment #8)
> > This seems like it should be 0, 1, 2.  I'd change the name restart to
> > shouldRestart so it's obvious that's a variable.  Also, since this changes the
> > controller api we'll need to change the documentation on MDC.  Please file a
> > follow on bug for that.
> Will file a documentation bug.

Not necessary. We keep the documentation work in the whiteboard entry.

Comment 10

[Andrew Halberstadt [:ahal]]

I added the required documentation to the controller api section in MDC.

Comment 11

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

(In reply to comment #10)
> I added the required documentation to the controller api section in MDC.

Please add documentation once we have released the new version. Otherwise people will be confused. Also it would be nice if you could paste the link into the bug.

But thanks for updating the docs Andrew!

Comment 12

[Andrew Halberstadt [:ahal]]

Here is the link: https://developer.mozilla.org/en/Mozmill/Mozmill_Controller_Object#startUserShutdown.28.29

I added a note saying that this is only available for Mozmill 1.4.2 and later.  This should be included in the beta 2 which will be released later this week.

Comment 13

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

The patch checked-in doesn't work for me on OS X 10.6. During the shutdown of the browser I see the dialog "Should not see this". Also the Python process hangs and I have to kill it manually. Haven't tested more here because it still fails on the first supplied test.

Btw, are we using the global timeout here too, to make sure we kill Firefox if something is wrong after the user initiated quit command? Right now it hangs forever.

Here the Python stack:

  File "/Volumes/data/testing/envs/dev/bin/mozmill-restart", line 8, in <module>
    load_entry_point('mozmill==1.4.1', 'console_scripts', 'mozmill-restart')()
  File "/Volumes/data/build/tools/mozmill/mozmill/mozmill/__init__.py", line 688, in restart_cli
    RestartCLI().run()
  File "/Volumes/data/build/tools/mozmill/mozmill/mozmill/__init__.py", line 671, in run
    CLI.run(self, *args, **kwargs)
  File "/Volumes/data/build/tools/mozmill/mozmill/mozmill/__init__.py", line 655, in run
    self._run()
  File "/Volumes/data/build/tools/mozmill/mozmill/mozmill/__init__.py", line 625, in _run
    self.mozmill.run_tests(self.options.test, self.options.report)
  File "/Volumes/data/build/tools/mozmill/mozmill/mozmill/__init__.py", line 541, in run_tests
    self.run_dir(d, report, sleeptime)
  File "/Volumes/data/build/tools/mozmill/mozmill/mozmill/__init__.py", line 491, in run_dir
    frame.runTestFile(test)
  File "/Volumes/data/build/tools/mozmill/jsbridge/jsbridge/jsobjects.py", line 126, in __call__
    response = self._bridge_.execFunction(self._name_, args)
  File "/Volumes/data/build/tools/mozmill/jsbridge/jsbridge/network.py", line 219, in execFunction
    return self.run(_uuid, 'bridge.execFunction('+ ', '.join(exec_args)+')', interval)
  File "/Volumes/data/build/tools/mozmill/jsbridge/jsbridge/network.py", line 198, in run
    sleep(interval)

Comment 14

[Andrew Halberstadt [:ahal]]

If you look at the test, the line immediately before the 'Should not see this' dialog simply calls Application.restart().  Is Application.restart() not available for Mac?  If so I apologize as I did not know this.

Calling controller.startUserShutdown() does nothing but flip a boolean which tells Mozmill not to throw an error if the browser gets shutdown or restarted.  So if the process is hanging it almost certainly has nothing to do with controller.startUserShutdown(), but rather the fact that Application.restart() isn't restarting the browser.

Comment 15

[Andrew Halberstadt [:ahal]]

Just to clarify, calling controller.startUserShutdown() does not restart or shutdown the browser.  It just flips a boolean so that it becomes safe to do so.  If the browser isn't getting restarted, that's a different issue.

Comment 16

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

Attachment: testcase for failed shutdown

It doesn't matter which way you are using to shutdown the browser it will always fail. Take this example and apply it to the mozilla1.9.2 branch of the mozmill-tests repository. Run the testExtensionInstallUninstall.js and you will notice that the browser gets closed but on the command line we freeze.

Comment 17

[Andrew Halberstadt [:ahal]]

Hmm, both the test case I provided and the modified testExtensionInstallUninstall.js in the mozilla-1.9.2 branch work perfectly fine (7 passed, 0 failed) for me in Ubuntu 10.04.

When you kill the python process for the extension install/uninstall test was it the same stack trace as the test I provided?

Comment 18

[Andrew Halberstadt [:ahal]]

Comment on attachment 460157 [details] [diff] [review]
testcase for failed shutdown

Looking at this patch, it seems that controller.waitThenClick() is not restarting the browser regardless of whether or not we first call controller.startUserShutdown().

I think another thing that might be causing problems (although for some reason works fine in Ubuntu) is that if no restart happens, the test ends before the startUserShutdown timeout expires.  Try adding a controller.sleep(3000) to the end of the file and I don't think the python process will freeze (I know this shouldn't happen, and I don't know why it is happening for Mac).

Comment 19

[Andrew Halberstadt [:ahal]]

Attachment: Remove requirement to sleep after restarting

This one was actually more complicated than I first thought.  Without going into too much detail...

The Issue:

When the browser gets restarted the JS thread doesn't die. What was happening was that those alerts in my tests were running after the browser was shutdown causing exceptions. These exceptions were then caught and ignored which is expected behavior (because userShutdownEnabled was set to true). This gave the appearance of everything working fine.  When I ran the tests without the alerts, the currentShutdownMode was not getting set back to 'default' which caused the behavior described by Henrik.

The Solution:

When the test ends before the timeout gets to expire we run into a problem. Did the browser get shutdown as expected?  Or did the test function return without shutting down the browser?  As it stood, this was impossible to know, as the JS thread continued to execute regardless of whether the browser was actually restarted or not. To get around this I use the nsIObserverService and listen for the 'application-quit' notification.

I've tested the above patch on multiple restart tests on both Ubuntu and Windows (don't have a Mac available). Also, there is no need to sleep after restarting the browser. Henrik note: the 'testExtensionInstallUninstall.js' test still doesn't click the 'restart' button correctly.  At least now with this patch Mozmill will notify you of that.

Comment 20

[cmtalbert]

Comment on attachment 460409 [details] [diff] [review]
Remove requirement to sleep after restarting

Nice patch, this looks good. r=ctalbert

Comment 21

[Andrew Halberstadt [:ahal]]

Resolving again.

Note: (1) The last three test cases I provide should fail.
      (2) The 'waitThenClick()' method on the restart button in the extension install/uninstall doesn't work, so this test still reports a failure (although should not hang anymore).

Comment 22

[Andrew Halberstadt [:ahal]]

And the commit:
http://github.com/mozautomation/mozmill/commit/1ed51ce30ac0c653281bf049b8f170e7f975267c

Comment 23

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

As talked yesterday on IRC this doesn't work reliable for me and by running the included tests for Mozmill I also get failed tests but what's more important, Firefox completely hangs in-between the restarts. As following a small and easy test shows that you cannot close Firefox by a command shortcut like Cmd+Q. I haven't tested other paths yet but will do.

var testRestartBeforeTimeout = function() {
  controller = mozmill.getBrowserController();
  controller.startUserShutdown(5000, true);

  // This key combination is for OS X (Cmd+Q) - modify as needed
  controller.keypress(null, "q", {accelKey: true});
}

The hang is still the same as the I have mentioned in comment 13. But now in line 197 instead of 198.

(In reply to comment #6)
> This will work for both mozmill and mozmill-restart (although note that you
> shouldn't use the restart option with mozmill tests)

Can you please enlighten me, why we need the restart option? It only has to be used for restart tests? If yes, can't we handle that depending on which CLI instance we are in? The user shouldn't take care of it. Or do I miss something? The documentation isn't really clear for me.

Comment 24

[Andrew Halberstadt [:ahal]]

Henrik... like I told you on IRC, this hang is because you have the restart parameter set to true instead of false.

mozmill-restart normally starts the browser back up for us, but if we are restarting it ourselves, we need to tell mozmill not to do it for us (otherwise we get an instance already exists popup error). In other words you must set that parameter to FALSE if you are shutting down the browser instead of restarting.

The test case I provided in the test/restart/test_user_shutdown tests the exact case you provided plus much much more and works perfectly fine for me on Linux (note that the test case I provided is supposed to fail the last three tests).

Comment 25

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

(In reply to comment #24)
> Henrik... like I told you on IRC, this hang is because you have the restart
> parameter set to true instead of false.

It doesn't matter which value I set on OS X. True and false cause a hang with different behavior. Setting it to true I get the "Another instance is already running" modal dialog instead and Firefox cannot be reopened for the next test in the folder.

> mozmill-restart normally starts the browser back up for us, but if we are
> restarting it ourselves, we need to tell mozmill not to do it for us (otherwise
> we get an instance already exists popup error). In other words you must set
> that parameter to FALSE if you are shutting down the browser instead of
> restarting.

That's exactly the other way around I see here on OS X. 

> The test case I provided in the test/restart/test_user_shutdown tests the exact
> case you provided plus much much more and works perfectly fine for me on Linux
> (note that the test case I provided is supposed to fail the last three tests).

Looks like it's a case for controller.keypress. I will have to check other instances too, if it is reproducible.

Comment 26

[Andrew Halberstadt [:ahal]]

After talking with Henrik on irc and testing on OSX it seems that there were two problems.

Issue 1:  When synthesizing the keyboard shortcut "Ctrl-Q" (or Cmd-Q for mac) Firefox wasn't sending out the 'application-quit' notification. When doing this manually with my own hands, the notification got sent correctly. 

This means we are not synthesizing our keypress event correctly (we might be disabling bubbling or something like that) and we should probably file a separate bug for this. I'm inclined to say it's not a problem with user-restart as that is independent to the method that Firefox gets shutdown, but rather an event system bug.

Issue 2: I only managed to reproduce this once (Henrik could reproduce more reliably), but sometimes on mac the tests were failing when they shouldn't. I believe the cause is as follows:

If the test has code which shuts down the browser right before the end of the test, sometimes mozmill's end_runner function (which results in a hard process kill) is called before firefox can quit cleanly.

I found that this only happened occasionally and is easily fixed by adding a small sleep to the end of the test file. A better solution should be investigated for the future, but if we are satisfied with our other test cases we can probably safely ignore this one for 1.4.2.

Comment 27

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

(In reply to comment #26)
> Issue 1:  When synthesizing the keyboard shortcut "Ctrl-Q" (or Cmd-Q for mac)
> Firefox wasn't sending out the 'application-quit' notification. When doing this
> manually with my own hands, the notification got sent correctly. 
> 
> This means we are not synthesizing our keypress event correctly (we might be
> disabling bubbling or something like that) and we should probably file a
> separate bug for this. I'm inclined to say it's not a problem with user-restart
> as that is independent to the method that Firefox gets shutdown, but rather an
> event system bug.

The question here is if it is our fault or a bug in EventUtils. But yeah, we should handle it separately.

> Issue 2: I only managed to reproduce this once (Henrik could reproduce more
> reliably), but sometimes on mac the tests were failing when they shouldn't. I
> believe the cause is as follows:

This happens when you are using the default way how we handle menu clicks. Use the following line to make this issue visible. For me it fails all the time:

controller.click(new elementslib.Elem(controller.menus["file-menu"].menu_FileQuitItem))
 
> If the test has code which shuts down the browser right before the end of the
> test, sometimes mozmill's end_runner function (which results in a hard process
> kill) is called before firefox can quit cleanly.
> 
> I found that this only happened occasionally and is easily fixed by adding a
> small sleep to the end of the test file. A better solution should be
> investigated for the future, but if we are satisfied with our other test cases
> we can probably safely ignore this one for 1.4.2.

The workaround is fine for me.

For upcoming tests we have to write the following steps have to reliable close the browser:

1. Session Store
* Start with a fresh profile
* Open multiple tabs and close the browser
=> The save session dialog which pops-up should reliable restart Firefox

2. Add-ons Manager
* Goto addons.mozilla.org and install any of the compatible extensions
=> Minefield: A notification appears at the top to restart Firefox
=> Namoroka: A notification appears in the addons manager to restart Firefox

We don't have code for those specific situations, and the restart button in the add-ons manager for older builds is broken due to a wrong lookup string. I don't have time right now to fix this specific issue.

Aaron, could you please assist?

Comment 28

[Andrew Halberstadt [:ahal]]

Attachment: Prevents race condition between mozmill end_runner and normal FFx shutdown

Henrik, can you apply this patch and let me know if it fixes the race condition when clicking the quit item in the menu?

I wasn't able to reliably reproduce this before, so I'm not sure if this fix worked or if I'm just having trouble reproducing the issue.

Thanks

Comment 29

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

(In reply to comment #28)
> Created attachment 462853 [details] [diff] [review]
> Prevents race condition between mozmill end_runner and normal FFx shutdown
> 
> Henrik, can you apply this patch and let me know if it fixes the race condition
> when clicking the quit item in the menu?

No, it doesn't fix it. We still hang with this stack:

  File "mozmill/mozmill/__init__.py", line 554, in start_runner
    self.create_network()
  File "mozmill/mozmill/__init__.py", line 213, in create_network
    self.jsbridge_port)
  File "mozmill/jsbridge/__init__.py", line 69, in wait_and_create_network
    sleep(.25)

Comment 30

[Andrew Halberstadt [:ahal]]

Attachment: Updated race condition fix

I made a dumb mistake.  I put the sleep on the python side, but of course that won't work because a failure will still get thrown if the end of test is reached before the browser shuts itself down.

I was able to reproduce again on OSX (with the other patch applied) but have not been able to reproduce again with this one. This should be the fix.

Sorry for wasting your time with the last one.  If you can still reproduce the failure, could you try increasing the sleep?

Thanks

Comment 31

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

Comment on attachment 462891 [details] [diff] [review]
Updated race condition fix

With this patch the test isn't marked as failed anymore. But, we still suffer from the hang on the Python side. Whether I use true or false as second parameter I still see the same behavior as reported before. :(

This is the last check I can do today. I will try to check my mail later tomorrow.

Comment 32

[Andrew Halberstadt [:ahal]]

But you don't get the hang if you put a sleep at the end of your test?  If that's the case, could you try changing the sleep I added in that patch to something bigger, like 1000?

Comment 33

[Andrew Halberstadt [:ahal]]

After a discussion with Clint, I will check this patch in (and increase the sleep time) because it seems to fix the problem for me.

Henrik, if you can still reproduce this issue, just use the work around of putting a sleep at the end of your test for now and we'll open a new bug post 1.4.2.

master: http://github.com/mozautomation/mozmill/commit/404be5953f953d9dd95dbf5a0c24a4ae3019da59

1.4.2: http://github.com/mozautomation/mozmill/commit/0968d0b70a3f47cdfc439b6a818988d775a81a5a

From now on, any other issues relating to user-restart should be filed in a separate bug, as this one is trying to keep track of too many things as it is.

Comment 34

[Aaron Train [:aaronmt]]

Testing this on 1.4.2b3 to mark as verified, I ran a couple of the test_user_restart tests and am receiving a fail[1] while testing on a minefield nightly[2]:

[1] TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed
[2] Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b4pre) Gecko/20100809 Minefield/4.0b4pre

Comment 35

[Andrew Halberstadt [:ahal]]

Aaron, which specific test file is that for?  Those tests aren't really typical 'mozmill' tests in that they are meant to test mozmill itself and not the browser.  That means some of those tests are meant to fail (the expected tests behaviour is documented in the test files themselves.

test1.js -> should pass
test2.js -> should pass
test3.js -> should throw failure
test4.js -> should throw failure
test5.js -> should have Disconnect Error

Comment 36

[Aaron Train [:aaronmt]]

test1.js:

TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed
INFO Passed: 2
INFO Failed: 0

test2.js:

TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed
INFO Passed: 2
INFO Failed: 0

test3.js:

TEST-UNEXPECTED-FAIL | /restart/test_user_restart/test3.js | testNoExpectedRestartByTimeout
INFO Passed: 1
INFO Failed: 1

test4.js: 

TEST-UNEXPECTED-FAIL | /restart/test_user_restart/test4.js | testNoExpectedRestartByEndTest
INFO Passed: 1
INFO Failed: 1

test5.js:

TEST-UNEXPECTED-FAIL | /restart/test_user_restart/test5.js | testRestartAfterTimeout
TEST-UNEXPECTED-FAIL | Disconnect Error: Application unexpectedly closed
INFO Passed: 1
INFO Failed: 1

Comment 37

[Aaron Train [:aaronmt]]

If those are the expected results than I will mark this as verified.

Comment 38

[Andrew Halberstadt [:ahal]]

I think you are running those as mozmill tests. You should use mozmill-restart for those tests.

There's another test called test/test_user_shutdown.js which can be run to test regular mozmill tests (it should pass).

Comment 39

[Aaron Train [:aaronmt]]

I was running test3.js, test4.js and test5.js as mozmill-restart and test1.js/test2.js as mozmill tests. test1.js and test2.js both pass as mozmill-restart, alongside test_user_shutdown.

Verified fixed on 1.4.2b3

Comment 40

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

Looks like everything works fine now. Once a new issue comes up I will file a new bug.

(In reply to comment #12)
> Here is the link:
> https://developer.mozilla.org/en/Mozmill/Mozmill_Controller_Object#startUserShutdown.28.29

I have updated the docs so we have a working example for all platforms.