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

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

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

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.

Comment 1

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

Attachment: Patch v1

Comment 2

[Armen [: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

Comment 3

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

(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.

Comment 4

[Armen [: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.

Comment 5

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

(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

[Armen [:armenzg]]

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

Comment 7

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

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.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9f7e0c71f6cb

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9f7e0c71f6cb