943008 - Enable pinning of marionette_client package in setup.py

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Malini Das [:mdas] - Away, not checking bugmail]

We currently do not pin the version of marionette_client used in gaiatest because we want to enable testing of changes to the mozilla-central in-tree marionette_client on TBPL. By not pinning a version, TBPL uses the intree one and runs the Gu tests. The problem with this is if we don't pin our version of marionette_client, if a significant change happens in the client, then even if a package pins its version of gaiatest, it will still see the marionette_client changes. If the client changes causes an older version of gaiatest to break in some way, it breaks the expectation that no change will be seen if packages that depend on gaiatest pin the gaiatest version.

We can fix this by adding a tbpl_setup.py which our mozharness scripts can use. This file will have an unpinned marionette_client version and will enable us to test the in-tree changes. We can then pin a version of marionette in the normal setup.py file.

Comment 1

[Malini Das [:mdas] - Away, not checking bugmail]

Attachment: mozharness change

We can pin the version of marionette_client easily if we use requirements.txt files instead of defining dependencies in the setup.py file. This way, there's the default file used by gaiatest is the requirements.txt file with the pinned marionette version, and the file used by tbpl will be tbpl_requirements.

Comment 2

[Malini Das [:mdas] - Away, not checking bugmail]

Attachment: gaia-ui-tests changes

you can test the gaia change locally by doing the following in the gaia-ui-tests directory:

pip install -r tbpl_requirements.txt .

which is similar to what mozharness is doing. It will use the tbpl_requirements file even if the install_requires dependencies in setup.py point use those in  requirements.txt

Comment 3

[Aki Sasaki (not active)]

Comment on attachment 8339525 [details] [diff] [review]
mozharness change

>             self.register_virtualenv_module('gaia-ui-tests',
>-                url=self.query_abs_dirs()['abs_gaiatest_dir'], method='pip')
>+                url=os.path.join(self.query_abs_dirs()['abs_gaiatest_dir']), method='pip', requirements=[requirements])

self.query_abs_dirs() returns a dict of directory nickname keys to absolute path values, so self.query_abs_dirs()['abs_gaiatest_dir'] is a single absolute path value.
I think the os.path.join() on a single string that's already in directory path format is redundant, though harmless; r=me without it.

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8339533 [details] [review]
gaia-ui-tests changes

If there was a way to maintain a single list of dependencies and only call out the marionette client version as different for TBPL then that would be preferable from a maintenance perspective. We are going to need to remember that any new dependencies or changes to the dependencies must be made to both requirements files. I'm not sure I have any suggestions for how to do this though.. does TBPL perhaps have an environment variable we could use to know we're running on it?

Otherwise, r=me.

Comment 5

[Malini Das [:mdas] - Away, not checking bugmail]

Attachment: mozharness change

Thanks, I've updated the patch and tested it locally. Moving r+ forward.

Comment 6

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Dave Hunt (:davehunt) from comment #4)
> Comment on attachment 8339533 [details] [review]
> gaia-ui-tests changes
> 
> If there was a way to maintain a single list of dependencies and only call
> out the marionette client version as different for TBPL then that would be
> preferable from a maintenance perspective. We are going to need to remember
> that any new dependencies or changes to the dependencies must be made to
> both requirements files. I'm not sure I have any suggestions for how to do
> this though.. does TBPL perhaps have an environment variable we could use to
> know we're running on it?
> 
> Otherwise, r=me.

The environment variable way seems a bit roundabout, but I just tested it with a minimal tbpl_requirements.txt with just marionette_client listed and it works this way. You can now specify the other deps just once in the requirements.txt file. I've updated the PR.

Comment 7

[Malini Das [:mdas] - Away, not checking bugmail]

Comment on attachment 8339533 [details] [review]
gaia-ui-tests changes

What do think of this approach?

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8339533 [details] [review]
gaia-ui-tests changes

I didn't realise it would be that simple! Looks great!

Comment 9

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to Dave Hunt (:davehunt) from comment #8)
> Comment on attachment 8339533 [details] [review]
> gaia-ui-tests changes
> 
> I didn't realise it would be that simple! Looks great!

Dave, mind merging this into gaia?

Comment 10

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Malini Das [:mdas] from comment #9)
> (In reply to Dave Hunt (:davehunt) from comment #8)
> > Comment on attachment 8339533 [details] [review]
> > gaia-ui-tests changes
> > 
> > I didn't realise it would be that simple! Looks great!
> 
> Dave, mind merging this into gaia?

Landed in:
https://github.com/mozilla-b2g/gaia/commit/fc51878943c362f21629967c05bbc457d51d7ebe (master)
https://github.com/mozilla-b2g/gaia/commit/ab6c92069cc9a32c0563141447ee54da44a2d4be (v1.2)

Comment 11

[Malini Das [:mdas] - Away, not checking bugmail]

Landed mozharness change in  https://hg.mozilla.org/build/mozharness/rev/735c70159fa1

So we need to wait for this to migrate to production. In any case, tip marionette-client will be used as long as we don't change the tip marionette client's version number

Comment 12

[[e-mail] (:bhearsum)]

I merged this to production just now.

Comment 13

[[e-mail] (:bhearsum)]

I think this broke all gaia-ui-tests: https://tbpl.mozilla.org/php/getParsedLog.php?id=31266528&tree=Mozilla-Inbound&full=1

09:25:29     INFO - Installing gaia-ui-tests into virtualenv /builds/slave/test/build/venv
09:25:29     INFO - Running command: ['/builds/slave/test/build/venv/bin/pip', 'install', '--download-cache', '/builds/slave/test/build/venv/cache', '-r', '/builds/slave/test/gaia/tests/python/gaia-ui-tests/tbpl_requirements.txt', '--no-index', '--find-links', 'http://pypi.pvt.build.mozilla.org/pub', '--find-links', 'http://pypi.pub.build.mozilla.org/pub', '/builds/slave/test/gaia/tests/python/gaia-ui-tests'] in /builds/slave/test/build
09:25:29     INFO - Copy/paste: /builds/slave/test/build/venv/bin/pip install --download-cache /builds/slave/test/build/venv/cache -r /builds/slave/test/gaia/tests/python/gaia-ui-tests/tbpl_requirements.txt --no-index --find-links http://pypi.pvt.build.mozilla.org/pub --find-links http://pypi.pub.build.mozilla.org/pub /builds/slave/test/gaia/tests/python/gaia-ui-tests
09:25:29     INFO -  Ignoring indexes: http://pypi.python.org/simple/
09:25:29     INFO -  Could not open requirements file: [Errno 2] No such file or directory: '/builds/slave/test/gaia/tests/python/gaia-ui-tests/tbpl_requirements.txt'
09:25:29     INFO -  Storing complete log in /home/cltbld/.pip/pip.log
09:25:29    ERROR - Return code: 1
09:25:29    FATAL - Unable to install /builds/slave/test/gaia/tests/python/gaia-ui-tests!
09:25:29    FATAL - Running post_fatal callback...
09:25:29    FATAL - Exiting -1

I backed it out.

Comment 14

[Dave Hunt [:davehunt] [he/him] ⌚BST]

(In reply to Ben Hearsum [:bhearsum] from comment #13)
> I backed it out.

Which patch was backed out? The mozharness one or the gaia one?

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/90d84177ff3d

Comment 16

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I don't think this is resolved due to the backout, presumably of the mozharness patch.

Comment 17

[[e-mail] (:bhearsum)]

(In reply to Dave Hunt (:davehunt) from comment #16)
> I don't think this is resolved due to the backout, presumably of the
> mozharness patch.

Yeah, sorry for the confusion. It was the mozharness one backed out.

Comment 18

[Malini Das [:mdas] - Away, not checking bugmail]

oh, a new version of gaiatest has to be released for the tbpl_requirements.txt file to be available. Let me coordinate with davehunt

Comment 19

[Malini Das [:mdas] - Away, not checking bugmail]

Ah, davehunt pointed out that it's using tip gaia to install gaiatest, so I jumped the gun. I now think it's because the changes to gaiatest didn't make it into the gaia blob we pull in. I'm testing it on ash to see if that's the case now.

Comment 20

[Malini Das [:mdas] - Away, not checking bugmail]

Great, it seems the problem was that the change didn't migrate in time for mozharness to get it. The try attempt passed: https://tbpl.mozilla.org/php/getParsedLog.php?id=31325385&tree=Ash

Bhearsum, do you think you can re-land this please?

Comment 21

[Malini Das [:mdas] - Away, not checking bugmail]

pushed to mozharness as https://hg.mozilla.org/build/mozharness/rev/b8168d737be3 and will be moved to production soon.

Comment 22

[Malini Das [:mdas] - Away, not checking bugmail]

Seems to run fine in production!