Closed
Bug 1227571
Opened 9 years ago
Closed 9 years ago
[firefox-ui-tests] mozharness scripts have to install and uninstall the application
Categories
(Release Engineering :: Applications: MozharnessCore, defect)
Release Engineering
Applications: MozharnessCore
Tracking
(firefox45 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
References
Details
Attachments
(1 file)
7.90 KB,
patch
|
armenzg
:
review+
|
Details | Diff | Splinter Review |
Via bug 1227570 we want to get rid of the --installer option. That means the consumer has to take care of the installation and uninstallation. In this case the firefox-ui-tests scripts have to do it themselves.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8691585 -
Flags: review?(armenzg)
Comment 2•9 years ago
|
||
Comment on attachment 8691585 [details] [diff] [review] Patch v1 Review of attachment 8691585 [details] [diff] [review]: ----------------------------------------------------------------- Please address the actions comment. Overall it looks good. ::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py @@ +103,2 @@ > 'run-tests', > + 'uninstall', This works because update_release.py sets its own actions. Would it make sense to setting those actions in update.py? This class is looking less like a base class but instead and implementation of update.py
Attachment #8691585 -
Flags: review?(armenzg) → review-
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Armen Zambrano Gasparnian [:armenzg] from comment #2) > ::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py > @@ +103,2 @@ > > 'run-tests', > > + 'uninstall', > > This works because update_release.py sets its own actions. > > Would it make sense to setting those actions in update.py? You mean setting the "Steps" in update_release? Otherwise I don't understand. The release update tests are totally different and you cannot have hard-coded steps like those defined. Keep in mind that you run tests for x-amount of different installers. To not rely on the steps there are the methods install_app() and uninstall_app() which > This class is looking less like a base class but instead and implementation > of update.py Hm, sorry but I do not understand.
Flags: needinfo?(armenzg)
Comment 4•9 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #3) > (In reply to Armen Zambrano Gasparnian [:armenzg] from comment #2) > > ::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py > > @@ +103,2 @@ > > > 'run-tests', > > > + 'uninstall', > > > > This works because update_release.py sets its own actions. > > > > Would it make sense to setting those actions in update.py? > > You mean setting the "Steps" in update_release? Otherwise I don't > understand. The release update tests are totally different and you cannot > have hard-coded steps like those defined. Keep in mind that you run tests > for x-amount of different installers. To not rely on the steps there are the > methods install_app() and uninstall_app() which > I'm meaning that a base set of actions can be put in FirefoxUIUpdateTests while the one specific to your script can overwrite them like this: https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/firefox_ui_tests/update_release.py#67 > > This class is looking less like a base class but instead and implementation > > of update.py > > Hm, sorry but I do not understand. I'm intending for FirefoxUIUpdateTests to stay as a base class and let subclassing scripts to overwrite actions. With this patch we're making "install" and "uninstall" defacto actions.
Flags: needinfo?(armenzg)
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Armen Zambrano Gasparnian [:armenzg] from comment #4) > I'm meaning that a base set of actions can be put in FirefoxUIUpdateTests > while the one specific to your script can overwrite them like this: > https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/ > firefox_ui_tests/update_release.py#67 The base set of actions for update tests are identical to the ones in the FirefoxUITests base class. Why should we replicate the same steps for FirefoxUIUpdateTests? > I'm intending for FirefoxUIUpdateTests to stay as a base class and let > subclassing scripts to overwrite actions. > With this patch we're making "install" and "uninstall" defacto actions. Those actions are REQUIRED. Otherwise you wont be able to run our tests anymore given that the --installer option is going away. If a subclass of FirefoxUIUpdateTests need something specific it can always overwrite the actions. And exactly that is happening for the update release class.
Comment 6•9 years ago
|
||
Comment on attachment 8691585 [details] [diff] [review] Patch v1 Review of attachment 8691585 [details] [diff] [review]: ----------------------------------------------------------------- r+ in that case. Thanks for clarifying it. ::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py @@ +103,2 @@ > 'run-tests', > + 'uninstall', This works because update_release.py sets its own actions. Would it make sense to setting those actions in update.py? This class is looking less like a base class but instead and implementation of update.py
Attachment #8691585 -
Flags: review- → review+
Assignee | ||
Comment 7•9 years ago
|
||
Thanks Armen, and sorry that it was not that clearly described initially. I should do better in the future especially wit the larger changes coming up now by moving our tests into m-c.
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 9•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9f7e0c71f6cb
You need to log in
before you can comment on or make changes to this bug.
Description
•