Closed Bug 827531 Opened 7 years ago Closed 7 years ago

Update mozharness Marionette scripts to pull packages from puppetagain

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jgriffin, Assigned: jgriffin)

References

Details

(Whiteboard: [mozharness])

Attachments

(3 files, 5 obsolete files)

Courtesy of bug 827508, we now have all the packages that Marionette needs at puppetagain.  We should update the mozharness Marionette scripts to use these, rather than installing mozbase packages from tests.zip.
This is running successfully on ash-mozharness.
Attachment #699436 - Flags: review?(aki)
Attachment #699436 - Flags: review?(aki) → review+
I'm going to wait to push this until first thing tomorrow, so I can watch the trees more closely in case of any unexpected problems.
Comment on attachment 699436 [details] [diff] [review]
Install mozbase packages from puppetagain,

http://hg.mozilla.org/build/mozharness/rev/bffbc0bf551e
Attachment #699436 - Flags: checked-in+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
This seems to have caused a perma-orange on xpcshell tests on the mozilla-b2g18 branch:

15:59:48     INFO -  Traceback (most recent call last):
15:59:48     INFO -    File "runtestsb2g.py", line 217, in <module>
15:59:48     INFO -      main()
15:59:48     INFO -    File "runtestsb2g.py", line 196, in main
15:59:48     INFO -      xpcsh = B2GXPCShellRemote(dm, options, args)
15:59:48     INFO -    File "/home/cltbld/talos-slave/test/build/tests/xpcshell/remotexpcshelltests.py", line 50, in __init__
15:59:48     INFO -      self.setupTestDir()
15:59:48     INFO -    File "runtestsb2g.py", line 37, in setupTestDir
15:59:48     INFO -      if self.device.useZip:
15:59:48     INFO -  AttributeError: DeviceManagerADB instance has no attribute 'useZip'

full log:  https://tbpl.mozilla.org/php/getParsedLog.php?id=18649462&tree=Mozilla-B2g18&full=1

I've backed out only the changes to the unittest file:

http://hg.mozilla.org/build/mozharness/rev/6ccba4314c12
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
You can just s/useZip/_useZip in runtestsb2g.py and it should fix it
I think what we want to do is to add a pip requirements.txt file to the tree (in testing/xpcshell), which specifies the versions of packages we need, and then let them be installed from puppetagain.  So, this will take the place of a setup.py, which our unittest testrunners don't have.

I'm not sure if mozharness supports using a requirements.txt file, but if not, it shouldn't be hard to add.

We can't pin the mozdevice version directly in the mozharness script, since we need different versions of it in different trees.  And fixing this by patching runtestsb2g.py will leave us open to future breakage when the next API change to devicemanager gets uploaded to puppetagain.
We can call VirtualenvMixin.create_virtualenv() to create the virtualenv (possibly with a subset of python modules, or not), then run whatever command(s) afterwards to install further python modules from the requirements.txt.

Alternately, we can add requirements.txt support directly to VirtualenvMixin.
This adds support for pip requirements files to mozharness.  There are many different ways this could be done, so if you'd prefer something different, let me know.  I'll try this out on ash (works locally).
Attachment #702045 - Flags: review?(aki)
The corresponding gecko change.  This is for an m-c version (and thus ash); we'd need potentially different versions on different trees.  We only need specify the versions required directly by the xpcshell testrunner; Marionette's deps are already specified correctly in its setup.py.
Attachment #702048 - Flags: review?(aki)
(In reply to Jonathan Griffin (:jgriffin) from comment #10)
> Created attachment 702048 [details] [diff] [review]
> Add a pip requirements.txt file for running B2G xpcshell tests
> 
> The corresponding gecko change.  This is for an m-c version (and thus ash);
> we'd need potentially different versions on different trees.  We only need
> specify the versions required directly by the xpcshell testrunner;
> Marionette's deps are already specified correctly in its setup.py.

I just realized this same requirements.txt file will need to be used by b2g mochitests and reftests as well, so will need a couple of other entries.
Make a requirements.txt file for all b2g unittests, r=aki
Attachment #702075 - Flags: review?(aki)
Attachment #702048 - Attachment is obsolete: true
Attachment #702048 - Flags: review?(aki)
Change the location of the in-tree requirements file to reflect that it isn't harness-specific.
Attachment #702076 - Flags: review?(aki)
Attachment #702045 - Attachment is obsolete: true
Attachment #702045 - Flags: review?(aki)
There's nothing outwardly wrong with this, except if we ever have a package called 'requirements', but I would tend to separate out requirements files handling from modules.  At the very least I would tend to keep requirements files as their own entity in config files (e.g. virtualenv_requirements) as opposed to a magical name of 'requirements'.  This also opens up the possibility of using different requirements files for different modules, which while I hope we never need I have seen sticky situations in the past where this was the only way to get the packages one really wanted (well, the sanest of insane ways, anyway).

While its probably pointless to delve into how pip deals with requirements files vs package order and dependencies internally (I'm guessing its somewhat complicated, though I'd also guess it doesn't matter how they're interleaved on the CLI), it is common practice to specify requirements files and modules on the same line: 

(tmp)│pip install src/foo/ -r requirements.txt 
Unpacking ./src/foo
  Running setup.py egg_info for package from file:///home/jhammel/tmp/src/foo
    
Downloading/unpacking mozdevice==0.18 (from -r requirements.txt (line 1))
  Downloading mozdevice-0.18.tar.gz (46Kb): 46Kb downloaded
  Running setup.py egg_info for package mozdevice
    
Requirement already satisfied (use --upgrade to upgrade): MakeItSo in ./src/MakeItSo (from foo==0.0)
Requirement already satisfied (use --upgrade to upgrade): webob in ./lib/python2.7/site-packages/WebOb-1.2.3-py2.7.egg (from foo==0.0)
Downloading/unpacking mozprocess==0.8 (from mozdevice==0.18->-r requirements.txt (line 1))
  Downloading mozprocess-0.8.tar.gz
  Running setup.py egg_info for package mozprocess
    
Requirement already satisfied (use --upgrade to upgrade): tempita>=0.5.1 in ./lib/python2.7/site-packages/Tempita-0.5.1-py2.7.egg (from MakeItSo->foo==0.0)
Downloading/unpacking mozinfo (from mozprocess==0.8->mozdevice==0.18->-r requirements.txt (line 1))
  Downloading mozinfo-0.4.tar.gz
  Running setup.py egg_info for package mozinfo
    
Installing collected packages: mozdevice, foo, mozprocess, mozinfo
  Running setup.py install for mozdevice
    
    Installing dm script to /home/jhammel/tmp/bin
  Running setup.py install for foo
    
    Installing foo script to /home/jhammel/tmp/bin
    Installing foo-template script to /home/jhammel/tmp/bin
  Running setup.py install for mozprocess
    
  Running setup.py install for mozinfo
    
    Installing mozinfo script to /home/jhammel/tmp/bin
Successfully installed mozdevice foo mozprocess mozinfo
Cleaning up...

The above is for the 'foo' package that depends on mozdevice (no version ranges specified).  If a package that satisfies the requirements (either requirements file or `install_requires` in setup.py) is already installed it won't be rerun.  So, for the time being, we could run all of the requirements files before the installation of modules here: http://hg.mozilla.org/build/mozharness/file/9555adc60ae6/mozharness/base/python.py#l252 . HOWEVER... requirements files are for more than just for installing things!  If we want to use requirements files in the future for, e.g. specifying specific locations of packages, etc, then we may want to run them along the side of each module.  For more reading, see: http://www.pip-installer.org/en/latest/requirements.html . In fact, since you can specify the pypi url and findlinks urls here, except for pywin32 and other cursed non-pip-able packages, we could replace our stuff with a in situ requirements file.

$0.02
Thanks Jeff, I'll update this patch accordingly.
Attachment #702076 - Flags: review?(aki) → review+
Comment on attachment 702075 [details] [diff] [review]
Add a pip requirements.txt file for running B2G xpcshell tests

Hm, I'm not entirely familiar with the makefiles here.

Does testing/xpcshell/* get copied to $(topsrcdir/b2g/test/)?
Because this patch creates a testing/xpcshell/b2g-requirements.txt but nsinstalls a $(topsrcdir)/b2g/test/b2g-unittest-requuiirements.txt , so it's changing directories and names in a way that's not entirely obvious to me.

I'd guess it should be 
$(NSINSTALL) $(topsrcdir)/testing/b2g-requirements.txt $(PKG_STAGE)/b2g or something similar.

r=me with this fixed, or if I'm wrong and the patch should be as written.
Attachment #702075 - Flags: review?(aki) → review+
(In reply to Jonathan Griffin (:jgriffin) from comment #15)
> Thanks Jeff, I'll update this patch accordingly.

Awesome.  Let me know if you need a hand or further clarifications
Just to confirm, if we have:

virtualenv_requirements = [
   os.path.join('tests', 'b2g', 'b2g-unittest-requirements.txt')
]

virtualenv_modules = [
    'mozinstall',
    {'marionette': os.path.join('tests', 'marionette')}
]

what you propose is that we execute:

pip install -r b2g-unittest-requirements.txt mozinstall
pip install -r b2g-unittest-requirements.txt tests/marionette

Is that correct?
Yeah, I think that seems the best easy thing to do.  More flexible would be designating being to specify a set of requirements file per module, but that may be YAGNI.
yes, you're right, the last patch contained the wrong file; this one has the right one!
Attachment #702075 - Attachment is obsolete: true
Attachment #702893 - Flags: review?(aki)
This implements the method described in comment #18.
Attachment #702076 - Attachment is obsolete: true
Attachment #702894 - Flags: review?(jhammel)
Attachment #702894 - Flags: feedback?(aki)
Comment on attachment 702893 [details] [diff] [review]
Add requirements.txt file for running b2g unit tests

That looks better :)
Attachment #702893 - Flags: review?(aki) → review+
Comment on attachment 702894 [details] [diff] [review]
Add support for installing pip requirements files

We might want to add install_module() nosetests soonish.
Maybe we can do that by overriding self.run_command() to just echo or something.
Attachment #702894 - Flags: feedback?(aki) → feedback+
Comment on attachment 702894 [details] [diff] [review]
Add support for installing pip requirements files

LGTM! Just a few opinionated nits:

+    def install_module(self, module=None, module_url=None, install_method=None,
+                       requirements=None):

When possible (as it should be here), I generally use an empty tuple for default arguments that are "arrays".  This could avoid this check too:

+        if not requirements:
+            requirements = []
Attachment #702894 - Flags: review?(jhammel) → review+
This try run looks good, at least from the point of view of this change.  ;)  I'm going to land the gecko change and let it propagate everywhere, then land the mozharness change.
(In reply to Jeff Hammel [:jhammel] from comment #25)
> Comment on attachment 702894 [details] [diff] [review]
> Add support for installing pip requirements files
> 
> LGTM! Just a few opinionated nits:
> 
> +    def install_module(self, module=None, module_url=None,
> install_method=None,
> +                       requirements=None):
> 
> When possible (as it should be here), I generally use an empty tuple for
> default arguments that are "arrays".  This could avoid this check too:
> 
> +        if not requirements:
> +            requirements = []

Excellent suggestion, thanks.
Update per review comments
Attachment #702894 - Attachment is obsolete: true
Comment on attachment 703038 [details] [diff] [review]
Add support for installation of pip requirements files,

Carry r+ forward.
Attachment #703038 - Flags: review+
I landed a different version on mozilla-b2g18, to reflect the different mozdevice version needed by that tree: https://hg.mozilla.org/releases/mozilla-b2g18/rev/67abf06a9f5a
(In reply to Ed Morley [:edmorley UTC+0] from comment #31)
> https://hg.mozilla.org/mozilla-central/rev/62123f318edd

Waiting until this gets merged into fx-team before I can the related mozharness change.
Comment on attachment 703038 [details] [diff] [review]
Add support for installation of pip requirements files,

http://hg.mozilla.org/build/mozharness/rev/7a2f56ea8272
Attachment #703038 - Flags: checked-in+
Merged to mozharness production.
This looks ok; I've seen runs on both mozilla-central and mozilla-b2g18 that were green after this was merged into production.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Whiteboard: [leave open]
Whiteboard: [mozharness]
Product: mozilla.org → Release Engineering
Component: General Automation → Mozharness
You need to log in before you can comment on or make changes to this bug.