1189847 - [mozversion] mozversion requires mozdevice, it shoudn't

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Julien Pagès (:parkouss)]

Currently mozversion dependencies are:

dependencies = ['mozdevice >= 0.44',
                'mozfile >= 1.0',
                'mozlog >= 3.0']

It does not make sense to require mozdevice here. And I am fighting pypi nightmare dependencies because of that when installing talos locally (and talos do not require mozdevice at all now!).

Comment 1

[Julien Pagès (:parkouss)]

:davehunt, do you mind if I fix this ? How is mozversion used around ?

Comment 2

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

mozversion uses mozdevice to determine the version information on Firefox OS devices: http://hg.mozilla.org/mozilla-central/file/32712cd01159/testing/mozbase/mozversion/mozversion/mozversion.py#l185

Comment 3

[Julien Pagès (:parkouss)]

Well, I am not asking to remove any code, just make the dependency optional (sorry, that was not clear enough).

We can use pip extra dependencies to still be able to require it explicitly from a pip install command, something like:

> pip install mozversion[remote]

See https://pythonhosted.org/setuptools/setuptools.html#declaring-extras-optional-features-with-their-own-dependencies.

Comment 4

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

I see, that makes sense. I wonder if the feature should be 'device' rather than 'remote' though? We should also update the documentation, and notify as many consumers of mozversion for Firefox OS that we're aware of. I've CC'd a few that come to mind.

Comment 5

[Julien Pagès (:parkouss)]

Attachment: 1189847.patch

Thanks Dave!

I followed your recommendation, using "device" as a feature name. I also added some documentation, and the generation is OK locally.

I am not sure if you should be reviewer here, feel free to redirect the r? if it is not the case. :)

Comment 6

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

Comment on attachment 8642353 [details] [diff] [review]
1189847.patch

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

With the nit addressed this looks good to me.

::: testing/mozbase/docs/mozversion.rst
@@ +5,5 @@
>  provides version information such as the application name and the changesets
>  that it has been built from. This is commonly used in reporting or for
>  conditional logic based on the application under test.
>  
> +Note that mozversion can report the version of remote devices (e.g. FirefoxOS)

Nit: Firefox OS

Comment 7

[Julien Pagès (:parkouss)]

Attachment: 1189847.patch

Thanks! Fixed. :)

I will push that to try later. I'm not sure what try syntax I should use, I was thinking about one desktop and one b2g device build with one mochitest and one reftest test (in case they use that). If you have a better idea, please tell me.

Comment 8

[Julien Pagès (:parkouss)]

Pushed to try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=74bf23f931a2

Comment 9

[Julien Pagès (:parkouss)]

try is green.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/95e44c9c8457

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/95e44c9c8457