Bug 1726460 Comment 2 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to :Gijs (he/him) from comment #1)
> Could we add test-only code into the silent restart code that opens a window after waiting for 2 seconds or something along those lines? We can detect the test-only case by using `Cu.isInAutomation`

We probably could, but at that point I'm not sure what we are testing for. If a window gets opened at startup (even 2 seconds later), the entire work of Bug 1726239 has kind of been defeated, so I'm not really sure what meaningful tests of that patch could be conducted at that point.

I'm not sure that using `Cu.isInAutomation` will actually work here. I believe that this limitation is why both checks are needed [here](https://searchfox.org/mozilla-central/rev/9dceacf3d761eb91237108ec438d64099a56f442/toolkit/mozapps/update/UpdateService.jsm#3645). It might be reasonable to read the `-marionette` command line flag though. I believe that will be available at this point.

This could potentially enable some basic tests for Bug 1720742 so that we could check that we restart when windows were closed with an update ready (this is assuming that marionette is ok with having all the windows close - I haven't tried this yet). But I think that there are also other ways of testing this, like intercepting the `quit()` call.

> Alternatively, did you consider fixing marionette so it detects the silent restart case and doesn't wait for the parent window to open ( ie drop reliance on `toplevel-window-ready` - https://searchfox.org/mozilla-central/rev/9dceacf3d761eb91237108ec438d64099a56f442/remote/components/Marionette.jsm#149 and other places that gets mentioned in that file) -- but this may be more involved, you'd want to talk to someone, maybe jgraham or whimboo, about it...

I actually spent a lot of time trying to make this work. I detailed those efforts in this bug's description. To make a long story short, I ended up determining that expecting a window at startup seems to be baked deeply enough into the design of the marionette server that it would delay Bug 1720742's timeline to block on changing marionette to accommodate this situation.
(In reply to :Gijs (he/him) from comment #1)
> Could we add test-only code into the silent restart code that opens a window after waiting for 2 seconds or something along those lines? We can detect the test-only case by using `Cu.isInAutomation`

We probably could, but at that point I'm not sure what we are testing for. If a window gets opened at startup (even 2 seconds later), the entire work of Bug 1726239 has kind of been defeated, so I'm not really sure what meaningful tests of that patch could be conducted at that point.

I'm not sure that using `Cu.isInAutomation` will actually work here. I believe that this limitation is why both checks are needed [here](https://searchfox.org/mozilla-central/rev/9dceacf3d761eb91237108ec438d64099a56f442/toolkit/mozapps/update/UpdateService.jsm#3645). It might be reasonable to read the `-marionette` command line flag though. I believe that will be available at this point.

This could potentially enable some basic tests for Bug 1720742 so that we could check that we restart when windows were closed with an update ready (this is assuming that marionette is ok with having all the windows close - I haven't tried this yet). But I think that there are also other ways of testing this, like intercepting the `quit()` call.

> Alternatively, did you consider fixing marionette so it detects the silent restart case and doesn't wait for the parent window to open ( ie drop reliance on `toplevel-window-ready` - https://searchfox.org/mozilla-central/rev/9dceacf3d761eb91237108ec438d64099a56f442/remote/components/Marionette.jsm#149 and other places that gets mentioned in that file) -- but this may be more involved, you'd want to talk to someone, maybe jgraham or whimboo, about it...

I actually spent a lot of time trying to make this work. I summarized those efforts in this bug's description. To make a long story short, I ended up determining that expecting a window at startup seems to be baked deeply enough into the design of the marionette server that it would delay Bug 1720742's timeline to block on changing marionette to accommodate this situation.

Back to Bug 1726460 Comment 2