618924 - [synchronization] Mozmill tests on mozilla-central should be updated with manifests

Status: RESOLVED WONTFIX

(Testing Graveyard :: Mozmill, defect)

Description

[Jeff Hammel]

When mozmill 1.5.2 is pushed to mozilla-central, manifests should be
pushed to mozilla-central as well since they have landed for bug 585106

Comment 1

[Jeff Hammel]

Attachment: manifests for files currently on m-c

mozmill should be updated on m-c before it makes sense to land this (though landing won't do any harm)

Comment 2

[Jeff Hammel]

This should be rolled out with the 1.5.2 release now that it supports manifests.  The tree could (should) be updated with the entire set of mozmill-tests so that what is run can just be a change in a manifest versus having to move testfiles around

Comment 3

[cmtalbert]

(In reply to comment #2)
> This should be rolled out with the 1.5.2 release now that it supports
> manifests.  The tree could (should) be updated with the entire set of
> mozmill-tests so that what is run can just be a change in a manifest versus
> having to move testfiles around

I think it'd be better to simply use the manifests to help us move files into m-c.  I'm really against putting test files in m-c that may never ever be run in m-c.

Comment 4

[cmtalbert]

Hit enter too soon, sorry.  But we should take these manifest changes for 1.5.2, obviously.

Comment 5

[Nick Thomas [:nthomas] (UTC+12)]

Could I get a firm ETA for rolling 1.5.2 out ?

Comment 6

[Jeff Hammel]

I will get a patch up for m-c tomorrow.  It will need to be tested, but should be doable soon after that

Comment 7

[Jeff Hammel]

There are several auxilliary tasks here too:

- mozmill on m-c needs to be updated to 1.5.2(pre?)

- `make mozmill` etc should be updated to use the manifests; this
  however will break test paths so.....good idea?

- ManifestDestiny should be downloaded to
  http://hg.mozilla.org/mozilla-central/file/tip/testing/mozmill

- ManifestDestiny should be added to the packages that the Makefile uses:
  http://mxr.mozilla.org/mozilla-central/source/testing/mozmill/Makefile.in#60

- the tests should be updated from the manifests

- buildbotcustom should be updated:
  https://bugzilla.mozilla.org/show_bug.cgi?id=618958

- all of this information should be communicated to the developers and
  tree maintainers such that it is know what to do when a mozmill test
  fails

Comment 8

[Jeff Hammel]

Attachment: update with manifests, tests, and ManifestDestiny

This does about half of what needs to be done here. This patch
- adds the manifests to m-c
- adds ManifestParser to m-c
- updates the relevent tests from current http://hg.mozilla.org/qa/mozmill-tests

It does not (and the final patch should):
- update mozmill (and mozrunner and jsbridge) from 1.5.2(pre?)
- change the `make` invocation appropriately (maybe?)
- verified that all of this works appropriately

This final work can be done as soon as whatever version of mozmill (with manifests) is landed

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

All the shared modules have been changed and renamed. Those changes are not part of the patch. Further I think we should split up the patch into several pieces.

Comment 10

[cmtalbert]

Comment on attachment 506926 [details] [diff] [review]
update with manifests, tests, and ManifestDestiny

First, comments on the manifest system:
* Manifest destiny should not live inside mozmill.  It should probably be in /build/manifest or some such which is where a bunch of our other test automation stuff lives that is shared among test harnesses.
** I'm not certain that syncing the entire tree of the manifest system into m-c is really necessary.  We aren't going to run the manifest's tests from m-c, so having them in m-c is just unnecessary overhead.
** We need to coordinate with jmaher about where to put this so that it is in a location he expects when he gets his xpcshell work ready to land.

As far as separate patches go:
* Patch 1: manifest necessary source into m-c. (and tag the manifest's hg repo when that patch lands).
* Patch 2: changes to mozmill/tests (i.e. the sync of the hg.m.o/qa/mozmill-tests/ to m-c)
* Patch 3: changes to mozmill source to update to 1.5.2 (syncing git repo to m-c) - this happens after QA of mozmill is complete, approx 1 week from now.
* Patch 4: Changes to mozmill make files to accommodate manifests, any other changes in how mozmill is now called.
* Patch 5: Any necessary changes to mozmill's buildbot custom unit test steps.

I don't really care about order, but I think that this division breaks this up into nice reviewable & manageable chunks.

Comment 11

[Jeff Hammel]

> First, comments on the manifest system:
> * Manifest destiny should not live inside mozmill.  It should probably be in
> /build/manifest or some such which is where a bunch of our other test
> automation stuff lives that is shared among test harnesses.

You mean in the testing/mozmill directory, I assume? mozmill will need to have access to ManifestDestiny in order to parse the manifests.  Currently we manage this with virtualenv, which is probably the python-preferred way of doing things.  If we want to depart from this, that's probably a larger conversation than this bug.  

The alternatives include:
- using config/pythonpath.py for everything.  kill virtualenv entirely and fix up mozmill + co on m-c to exclusively use this file.  Not surprisingly, I think this is a horrible idea, ultimately a pain to maintain (I know for sure it makes me want to have large unreadable files versus sensibly packaged modules so that i don't have to futz around with PYTHONPATH and the associated front-end script everytime I add a file)
- convert m-c to use a single sensibly designed virtualenv for all of its python modules.  Not surprisingly, I'm for this.  On the other hand, this is a monumental effort and probably has a ton of cleanup to do before we even start.  So I'm not for it for this bug.
- we only need one file from ManifestDestiny, manifests.py.  We can sync this into mozmill and sync that into m-c.  Again not surprisingly, I'm against this.  We don't really have a syncing story.  I'd prefer to A) do things as much as possible that don't require syncing and B) for the cases where this isn't possible, have a real story there that answers the questions "where does this ultimately come from?" and "how do I update it?" 

I'll defer to better judgement here.  I put ManifestDestiny in testing/mozmill because it was easy and because it is identical to how the other mozmill dependencies (simplejson, mozrunner, jsbridge) are already dealt with.  

> ** I'm not certain that syncing the entire tree of the manifest system into m-c
> is really necessary.  We aren't going to run the manifest's tests from m-c, so
> having them in m-c is just unnecessary overhead.

No, we don't need the suite of tests.  I thought about leaving them out and can do so in subsequent patches.  technically, we can get this down to a two file package: setup.py and manifests.py.  copy.py and convert.py could conceivably go in the already bloated manifests.py file.  Worth turning attention to?


> As far as separate patches go:
> * Patch 1: manifest necessary source into m-c. (and tag the manifest's hg repo
> when that patch lands).
> * Patch 2: changes to mozmill/tests (i.e. the sync of the
> hg.m.o/qa/mozmill-tests/ to m-c)
> * Patch 3: changes to mozmill source to update to 1.5.2 (syncing git repo to
> m-c) - this happens after QA of mozmill is complete, approx 1 week from now.
> * Patch 4: Changes to mozmill make files to accommodate manifests, any other
> changes in how mozmill is now called.
> * Patch 5: Any necessary changes to mozmill's buildbot custom unit test steps.

I'll clean this up.  Note that Patch 5 is separately ticketed since this bug is a hard blocker to that: bug 618958

Comment 12

[Jeff Hammel]

I've refactored and cleaned up ManifestDestiny quite a bit to be a single file:

http://hg.mozilla.org/automation/ManifestDestiny/file/tip/manifestparser.py

The repo also contains a README and tests, but manifestparser.py contains all functionality and is a setup-able package, e.g.: ``python manifestparser.py setup install``. This will it allow it to work with Mozmill and other python packages without having to do anything funny with the import path.  I'll next look towards doing this from the Makefiles and writing the various patches.

I'm guessing inclusion of the manifests itself should be part of Patch 4?

I've also introduced at blocker, as the manifests import is different now: Bug 629575 . Its a short patch for 1.5.2 and 2.0 that I'll do tomorrow.

Comment 13

[Jeff Hammel]

Attachment: add manifestparser/manifestdestiny to the build

Comment 14

[Jeff Hammel]

> I'm guessing inclusion of the manifests itself should be part of Patch 4?

On second thought, maybe itd be more appropriate for Patch 2.

Comment 15

[Jeff Hammel]

(In reply to comment #14)
> > I'm guessing inclusion of the manifests itself should be part of Patch 4?
> 
> On second thought, maybe itd be more appropriate for Patch 2.

Reason: Having a manifest makes updating the tests much easier

Comment 16

[Jeff Hammel]

Comment on attachment 507888 [details] [diff] [review]
add manifestparser/manifestdestiny to the build

Removing review flag until we figure out bug 629721; then i'll put up a fresh copy for review

Comment 17

[Jeff Hammel]

Attachment: add manifests + update tests

This patch:
 - adds manifests
 - updates the tests from momzill-tests
 - updates shared-modules

It doesn't look like there was any changes from test-files

If this should be broken up additionally, let me know

Comment 18

[Jeff Hammel]

Attachment: add manifestparser/manifestdestiny to the build

version 0.2.2 (current head)

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

That's not a Mozmill 1.5.2 blocker but depends on the release.

Comment 20

[cmtalbert]

Comment on attachment 508486 [details] [diff] [review]
add manifests + update tests

unsetting review because we're not going to move this into m-c at this time.

Comment 21

[Jeff Hammel]

(In reply to Clint Talbert ( :ctalbert ) from comment #20)
> Comment on attachment 508486 [details] [diff] [review]
> add manifests + update tests
> 
> unsetting review because we're not going to move this into m-c at this time.

See also https://bugzilla.mozilla.org/show_bug.cgi?id=676078 ; we should probably close this bug when that lands. Mozmill will likely live on m-c once again but we won't need this separate bug

Comment 22

[Jeff Hammel]

oops, i clicked wrong :( sorry

Comment 23

[Jeff Hammel]

(In reply to Jeff Hammel [:jhammel] from comment #21)
> (In reply to Clint Talbert ( :ctalbert ) from comment #20)
> > Comment on attachment 508486 [details] [diff] [review]
> > add manifests + update tests
> > 
> > unsetting review because we're not going to move this into m-c at this time.
> 
> See also https://bugzilla.mozilla.org/show_bug.cgi?id=676078 ; we should
> probably close this bug when that lands. Mozmill will likely live on m-c
> once again but we won't need this separate bug

And doing so.