1203155 - Update firefox-ui-tests config to use strict version package dependencies and optional packages

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

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

So far we install the latest version of all the mozbase packages for our tests. As we have seen lately with mozlog==3.0 it can cause major conflicts and breakage. To make our tests more stable we should pin specific versions before actually installing the firefox-ui-tests dependencies. 

Armen, where would you usually do this? Is it a general requirements.txt file under config/firefox-ui-tests/? The optional packages (like mozdownload) could then go into opt_requirements.txt and would be loaded when e.g. --with-optional-packages is specified on the CLI.

Comment 1

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

Ok, so I can use the virtualenv_modules section of a config file (/config/firefox-ui-tests/config.py) to specify the list of required packages. That way we can make it strict or loosely handled for testing newer mozbase packages.

I don't want to go with an optional CLI argument for our Jenkins configs but create a different config file for it.

Comment 2

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

Attachment: patch v1

Comment 3

[Armen [:armenzg]]

Comment on attachment 8658932 [details] [diff] [review]
patch v1

Review of attachment 8658932 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm!

::: testing/mozharness/mozharness/base/python.py
@@ +159,5 @@
>              packages[package] = version
>  
>          if log_output:
>              self.info("Current package versions:")
> +            for package in sorted(packages):

Sweet!

::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py
@@ +147,5 @@
> +        if self.config.get('virtualenv_modules'):
> +            for module in self.config['virtualenv_modules']:
> +                self.register_virtualenv_module(module)
> +
> +        self.register_virtualenv_module('firefox-ui-tests', url=dirs['fx_ui_dir'])

What would this extra step add that is not added by the requirements.txt? The generation of the binaries through console_scripts? Could you please add a note if so?

Comment 4

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

Comment on attachment 8658932 [details] [diff] [review]
patch v1

Review of attachment 8658932 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py
@@ +147,5 @@
> +        if self.config.get('virtualenv_modules'):
> +            for module in self.config['virtualenv_modules']:
> +                self.register_virtualenv_module(module)
> +
> +        self.register_virtualenv_module('firefox-ui-tests', url=dirs['fx_ui_dir'])

Sorry, that was just an indentation fix to have the call in a single line. So nothing changes here. If you don't like it I can certainly revert it. 

I will wait with the landing until the patch on bug 1203267 has been backported to all older branches.

Comment 5

[Armen [:armenzg]]

(In reply to Henrik Skupin (:whimboo) from comment #4)
> Comment on attachment 8658932 [details] [diff] [review]
> patch v1
> 
> Review of attachment 8658932 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/mozharness/mozharness/mozilla/testing/firefox_ui_tests.py
> @@ +147,5 @@
> > +        if self.config.get('virtualenv_modules'):
> > +            for module in self.config['virtualenv_modules']:
> > +                self.register_virtualenv_module(module)
> > +
> > +        self.register_virtualenv_module('firefox-ui-tests', url=dirs['fx_ui_dir'])
> 
> Sorry, that was just an indentation fix to have the call in a single line.
> So nothing changes here. If you don't like it I can certainly revert it. 
> 
Not a problem and yes, please.

> I will wait with the landing until the patch on bug 1203267 has been
> backported to all older branches.

Sure.

Comment 6

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

Attachment: patch v1.1

Last patch was missing the commit message, so here an updated one. Taking over r+.

Comment 7

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

Inbound is closed so I cannot land it myself now.

Comment 8

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

Inbound is open and I will land it myself.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b5e4fcb51b17

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b5e4fcb51b17