Closed Bug 1196489 Opened 9 years ago Closed 9 years ago

If gaiatest upgrades to marionette-client 0.19, it needs to rename BaseMarionetteOptions and HTMLReportingOptionsMixin

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
FxOS-S9 (16Oct)

People

(Reporter: martijn.martijn, Assigned: pyang)

References

Details

(Keywords: qablocker)

Attachments

(2 files)

This was mentioned on the tools-marionette mailing list: https://groups.google.com/forum/#!forum/mozilla.tools.marionette " Hey all, Bug 1163801 tracks an incoming upgrade to argparse for marionette-client. This is a relatively straightforward change, but it will cause backwards incompatibilities. I'll follow up with another post once this is landed and pushed to pypi. The following classes have been renamed: runner.base.BaseMarionetteOptions -> BaseMarionetteArguments runner.mixins.endurance.EnduranceOptionsMixin -> EnduranceArguments runner.mixins.browsermob.BrowserMobProxyOptionsMixin -> BrowserMobProxyArguments runner.mixins.reporting.HTMLReportingOptionsMixin -> HTMLReportingArguments Furthermore, if you extended BaseMarionetteOptions, you'll need to follow the general python upgrade path from optparse to argparse: https://docs.python.org/2.7/library/argparse.html#upgrading-optparse-code Finally, BaseMarionetteOptions had a system by which you could use these so called "OptionsMixin" classes. It turns out this doesn't work so well with argparse. So the mixin system has been replaced with a registration system. In the off chance you were actually making use of "OptionsMixins", instead of inheriting from the class, you call parser.register_argument_container(instance). After this lands, look at runner.mixins.endurance for an example. Please let me know if you have any questions or concerns. Cheers, Andrew " Gaia UI test uses some of these older classes: http://mxr.mozilla.org/gaia/search?string=BaseMarionetteOptions&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=gaia http://mxr.mozilla.org/gaia/search?string=HTMLReportingOptionsMixin&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=gaia From Andrew: Gaiatest pins marionette-client to version 0.16, so you shouldn't run into any problems until you try to upgrade it to version 0.19 or later. But when/if you do decide to upgrade, you'll need to deal with this. If you want to upgrade and need some pointers, let me know and i can help. So if we decide to upgrade marionette-client, we need to rename these classes.
Note: I noticed that the emulator Gip tests that run hidden in treeherder break right away due to this (because they use the in-tree copy of marionette). So either we should fix this bug at the same time as bug 1163801, or else just turn the treeherder Gip tests off completely. Talking to :mwargers and :jlorenzo on irc, it sounds like maybe we should just turn them off if no one is looking at them anyway.
Keywords: qablocker
It seems that this is behind the cause of the current jenkins automation failures, such as one below: http://jenkins1.qa.scl3.mozilla.com/view/Graphics/job/flame-kk-319.mozilla-central.nightly.ui.graphics.reference_nonsmoke.bitbar/19/console ImportError: cannot import name BaseMarionetteOptions
Blocks: 1185501
I think we need to pin down the marionette_client version at: http://mxr.mozilla.org/gaia/source/tests/python/gaia-ui-tests/requirements.txt#3
Comment on attachment 8652968 [details] [gaia] mwargers:1196489_pin_marionette_client > mozilla-b2g:master This would fix the issue for now then.
Attachment #8652968 - Flags: review?(npark)
Attachment #8652968 - Flags: review?(npark) → review+
Also decided to pin this: marionette_driver==0.13 We need to wait until the tree reopens, before we can check this in.
I got the same error on TPE lab. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 06:28:43 Cleaning up... 06:28:43 + gaiatest --address=localhost:2840 --testvars=/var/lib/jenkins/B2G-resource/res/testvars_flamekk.json --xml-output=results/result.xml --html-output=results/index.html --restart --timeout=30000 --type=b2g+smoketest-dsds gaiatest/tests/functional/manifest.ini 06:28:43 Traceback (most recent call last): 06:28:43 File "/var/lib/jenkins/workspace/B2G.master.PVT.FlameKK.gaiatest.smoketest/tests/python/gaia-ui-tests/.env/bin/gaiatest", line 9, in <module> 06:28:43 load_entry_point('gaiatest==0.33', 'console_scripts', 'gaiatest')() 06:28:43 File "/var/lib/jenkins/workspace/B2G.master.PVT.FlameKK.gaiatest.smoketest/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/distribute-0.6.24-py2.7.egg/pkg_resources.py", line 337, in load_entry_point 06:28:43 return get_distribution(dist).load_entry_point(group, name) 06:28:43 File "/var/lib/jenkins/workspace/B2G.master.PVT.FlameKK.gaiatest.smoketest/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/distribute-0.6.24-py2.7.egg/pkg_resources.py", line 2279, in load_entry_point 06:28:43 return ep.load() 06:28:43 File "/var/lib/jenkins/workspace/B2G.master.PVT.FlameKK.gaiatest.smoketest/tests/python/gaia-ui-tests/.env/local/lib/python2.7/site-packages/distribute-0.6.24-py2.7.egg/pkg_resources.py", line 1989, in load 06:28:43 entry = __import__(self.module_name, globals(),globals(), ['__name__']) 06:28:43 File "/var/lib/jenkins/workspace/B2G.master.PVT.FlameKK.gaiatest.smoketest/tests/python/gaia-ui-tests/gaiatest/runtests.py", line 8, in <module> 06:28:43 from marionette import (BaseMarionetteOptions, 06:28:43 ImportError: cannot import name BaseMarionetteOptions
Comment on attachment 8652968 [details] [gaia] mwargers:1196489_pin_marionette_client > mozilla-b2g:master Works for me locally too.
Attachment #8652968 - Flags: review+
Comment on attachment 8652968 [details] [gaia] mwargers:1196489_pin_marionette_client > mozilla-b2g:master Merged this: https://github.com/mozilla-b2g/gaia/commit/f2bcb83a52fd1f06583df486636323efa6a4bed3
As marionette announced stable version 0.19, gaiatest should catch it at least for master.
Assignee: nobody → pyang
Comment on attachment 8653845 [details] [gaia] zapion:bug1196489 > mozilla-b2g:master Look like some error in treeherder, ask for feedback if anything we can see earlier.
Attachment #8653845 - Flags: feedback?(npark)
Attachment #8653845 - Flags: feedback?(martijn.martijn)
Comment on attachment 8653845 [details] [gaia] zapion:bug1196489 > mozilla-b2g:master Thanks for fixing this! --- a/tests/python/gaia-ui-tests/requirements.txt +++ b/tests/python/gaia-ui-tests/requirements.txt @@ -1,5 +1,5 @@ boto>=2.32.1 -marionette_driver==0.13 -marionette_client==0.17 +marionette_driver>=0.13 +marionette_client>=0.19 I'm not sure we want to loosen the package requirements again. Often, this causes failures in the Jenkins labs at the worst times, when we're already battling multiple issues. Otoh, this is the only way to keep track and stay updated on incompatible changes to those packages. Johan, what do you think?
Attachment #8653845 - Flags: feedback?(jlorenzo)
Comment on attachment 8653845 [details] [gaia] zapion:bug1196489 > mozilla-b2g:master The pull request itself works well. I tested with a new virtualenv and I didn't see any problems. I don't know much about the code, but it seems good to me.
Attachment #8653845 - Flags: feedback?(martijn.martijn) → feedback+
Pin this "marionette_driver==0.13" on local CI. It works for me.
Comment on attachment 8653845 [details] [gaia] zapion:bug1196489 > mozilla-b2g:master code looks good, and as martijn said, we need to decide how are we going to deal with marionette upgrades. are we going to pick latests, and fix issues when the test is failing, or are we lock the version and periodically check for the availability of new marionette and upgrade them if we have time?
Attachment #8653845 - Flags: feedback?(npark) → feedback+
Comment on attachment 8653845 [details] [gaia] zapion:bug1196489 > mozilla-b2g:master Thanks for the fix, Paul! I agree with Martijn at this point, let's pin the dependencies. At the moment, our infrastructure has too many issues to deal with. Reducing the number of moving parts will help.
Attachment #8653845 - Flags: feedback?(jlorenzo) → feedback+
I guess we should catch mozilla-central. I understand it is hard so a early branch for gaiatest is considerable; such like creating a gaiatest-v2.5 package on pypi before gaia branched, and we pin the requirement on this package. The reason to stick with latest marionette was it is highly coupled between client and server. If b2g fetch latest gecko and there's change in marionette-server, we still need to update whole dependencies or we'll be blocked. Dave, what do you recommend the strategy for branching gaiatest?
Flags: needinfo?(dave.hunt)
I'd prefer it too if we keep up with mozilla-central. With bug 1198950 sort of fixed (not really, but it's not disruptive anymore), we have Jenkins running again. The thing is, we're so busy with fixing all of the issues related to Jenkins, fixing test failures, creating new tests, that it's far easier to just pin the versioning down than trying to keep up changes in those packages.
(In reply to Paul Yang [:pyang] from comment #18) > I guess we should catch mozilla-central. I understand it is hard so a early > branch for gaiatest is considerable; such like creating a gaiatest-v2.5 > package on pypi before gaia branched, and we pin the requirement on this > package. > The reason to stick with latest marionette was it is highly coupled between > client and server. If b2g fetch latest gecko and there's change in > marionette-server, we still need to update whole dependencies or we'll be > blocked. > Dave, what do you recommend the strategy for branching gaiatest? I'm no longer working on the project, so I don't really have an opinion here. Generally I recommend pinning, and previously we've branched marionette and gaiatest whenever gecko branched for B2G. This was largely due to breaking changes landing in Marionette server, which may be less of an issue now.
Flags: needinfo?(dave.hunt)
Paul, your pull request still needs to be merged, right?
Flags: needinfo?(pyang)
Yes. I can rebase and push to treeherder to see if result's ok now.
Flags: needinfo?(pyang)
Btw, there is a new feature in Marionette client to be able to work in the shadow dom, this was fixed in bug 1194224. I would love to be able to used that. But I guess when a new marionette-client version has been released, we need to update the requirements.txt file then.
Attachment #8653845 - Flags: review?(npark)
Attachment #8653845 - Flags: review?(martijn.martijn)
Comment on attachment 8653845 [details] [gaia] zapion:bug1196489 > mozilla-b2g:master I just tested it, it works fine. +marionette_driver>=0.13 -marionette_client==0.17 +marionette_client>=0.19 I guess that's what we've decided upon? To not pin these?
Attachment #8653845 - Flags: review?(martijn.martijn) → review+
Comment on attachment 8653845 [details] [gaia] zapion:bug1196489 > mozilla-b2g:master LGTM. Imagecompare runs fine as well.
Attachment #8653845 - Flags: review?(npark) → review+
(In reply to Martijn Wargers [:mwargers] (QA) from comment #24) > Comment on attachment 8653845 [details] > [gaia] zapion:bug1196489 > mozilla-b2g:master > > I just tested it, it works fine. > +marionette_driver>=0.13 > -marionette_client==0.17 > +marionette_client>=0.19 > > I guess that's what we've decided upon? To not pin these? from c18 and c19 I think so. but dhunt gave different pov and I am not sure we have consensus though.
We really want the changes from bug 1194224 in the new marionette-client (when it's there), so perhaps it's better to unpin for now. I guess we can deal with this outage just fine and repin again when it turns out there is an incompatibility.
Looks like we should have a version meta to collect these changes and check-in in one time. However we currently don't suppose to merge them somewhere and test as well.
adding change for pinning marionette client by 0.20 -marionette_client==0.17 +marionette_client==0.20
I guess we can start pinning on 0.20 and see we need to move on latest version of marionette. anything considerable about checkin this?
Flags: needinfo?(martijn.martijn)
Keywords: checkin-needed
Blocks: 1211325
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → FxOS-S9 (16Oct)
I'm fine with you checking this in whenever you feel it can be done.
Flags: needinfo?(martijn.martijn)
Paul, the marionette changed its version number to 1.0.0, we might need to update our code. https://bugzilla.mozilla.org/show_bug.cgi?id=1209698
Flags: needinfo?(pyang)
sent a review request to update
Flags: needinfo?(pyang)
Depends on: 1212623
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: