[firefox-ui-tests] mozharness scripts have to install and uninstall the application

RESOLVED FIXED

Status

Release Engineering
Mozharness
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

unspecified

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
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

a year ago
Created attachment 8691585 [details] [diff] [review]
Patch v1
Attachment #8691585 - Flags: review?(armenzg)
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

a year 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)
(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

a year 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 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

a year 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

a year ago
Keywords: checkin-needed

Comment 8

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f7e0c71f6cb
Keywords: checkin-needed

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9f7e0c71f6cb
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.