Closed Bug 1440577 Opened 4 years ago Closed 3 years ago
Marionette has to use "e
Force Quit" for in _app shutdown and restart requests
To get started please follow the steps as listed here: https://firefox-source-docs.mozilla.org/testing/marionette/marionette/NewContributors.html In case of questions don't hesitate to ask, here or in irc.mozilla.org/#ateam.
59 bytes, text/x-review-board-request
Currently Marionette uses `eAttemptQuit` for an in_app shutdown request: https://dxr.mozilla.org/mozilla-central/rev/3f474e48db7f7b0f1c2c090f812290a8d9a21650/testing/marionette/client/marionette_driver/marionette.py#1040 This should be changed to `eForceQuit`, which then will force all the windows to close. Beside the code change also the Python docstring of the method needs to be updated. After changing it make sure that all Marionette tests are still passing.
Hi, i'd like to solve this.
Alvaro, you are welcome to work on this bug. I will assign it to you once a patch got uploaded. In case of questions don't hesitate to ask.
Here's is the patch: https://reviewboard.mozilla.org/r/222952 Hope I did this right.
Alvaro, thank you for the patch! I have seen you uploaded it already to mozreview. Can I assume that it is ready for review? If yes, please open the reviewboard link, and set `whimboo` as reviewer for the patch.
Assignee: nobody → arb952
Status: NEW → ASSIGNED
Comment on attachment 8953712 [details] Bug 1440577 - Marionette has to use "eForceQuit" for in_app shutdown and restart requests https://reviewboard.mozilla.org/r/222952/#review229182 Thank you for the patch. Code-wise it looks fine, but please update the docstring as mentioned inline. In the meantime I will trigger a try build so we can see if there are any regressions as caused by this change. ::: testing/marionette/client/marionette_driver/marionette.py:1038 (Diff revision 1) > # removed from here in Firefox 55 at the earliest. > > # remove duplicates > flags = set(shutdown_flags) > > - # add eAttemptQuit if there are no *Quits > + # add eForceQuit if there are no *Quits The docstring I was referring to is above and related to the method itself. Check for the `shutdown_flags` parameter.
Attachment #8953712 - Flags: review?(hskupin) → review-
Comment on attachment 8966643 [details] Bug 1440577 - Changed 'eAttemptQuit' occurrences in docstring to 'eForceQuit' https://reviewboard.mozilla.org/r/235356/#review241276 ::: testing/marionette/client/marionette_driver/marionette.py:1014 (Diff revision 1) > def _request_in_app_shutdown(self, *shutdown_flags): > """Attempt to quit the currently running instance from inside the > application. > > Duplicate entries in `shutdown_flags` are removed, and > - `"eAttemptQuit"` is added if no other `*Quit` flags are given. > + `"eForceQuit"` is added if no other `*Quit` flags are given. Please squash those changes into the other commit. You can use `hg histedit` in combination with `r` for the second commit. Also make sure that the commit message has a proper message. It's not necessary to call out the docstring changes.
Comment on attachment 8953712 [details] Bug 1440577 - Marionette has to use "eForceQuit" for in_app shutdown and restart requests https://reviewboard.mozilla.org/r/222952/#review241278
Attachment #8953712 - Flags: review+
Alvaro, would you mind finishing this patch? It would be great to have it in tree.
Attachment #8966643 - Attachment is obsolete: true
Sorry for being late here. I somehow missed your last update on that bug and to check the results from try. It all looks fine now and I'm going to push your patch. Thanks a lot!
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/d2e9671be0df Marionette has to use "eForceQuit" for in_app shutdown and restart requests r=whimboo
Alvaro, thank you for working on this bug! In case you are interested to contribute more please let me know. As best via email, and we can figure out a good next bug for you.
You need to log in before you can comment on or make changes to this bug.