Closed
Bug 1203155
Opened 9 years ago
Closed 9 years ago
Update firefox-ui-tests config to use strict version package dependencies and optional packages
Categories
(Release Engineering :: Applications: MozharnessCore, defect)
Release Engineering
Applications: MozharnessCore
Tracking
(firefox43 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
whimboo
:
review+
|
Details | Diff | Splinter Review |
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.
Flags: needinfo?(armenzg)
Assignee | ||
Comment 1•9 years ago
|
||
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.
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(armenzg)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8658932 -
Flags: review?(armenzg)
Comment 3•9 years ago
|
||
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?
Attachment #8658932 -
Flags: review?(armenzg) → review+
Assignee | ||
Comment 4•9 years ago
|
||
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•9 years ago
|
||
(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.
Assignee | ||
Comment 6•9 years ago
|
||
Last patch was missing the commit message, so here an updated one. Taking over r+.
Attachment #8658932 -
Attachment is obsolete: true
Attachment #8659333 -
Flags: review+
Assignee | ||
Comment 7•9 years ago
|
||
Inbound is closed so I cannot land it myself now.
Keywords: checkin-needed
Comment 10•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•