Closed
Bug 618924
Opened 14 years ago
Closed 12 years ago
[synchronization] Mozmill tests on mozilla-central should be updated with manifests
Categories
(Testing Graveyard :: Mozmill, defect)
Testing Graveyard
Mozmill
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: k0scist, Unassigned)
References
Details
Attachments
(3 files, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•14 years ago
|
||
mozmill should be updated on m-c before it makes sense to land this (though landing won't do any harm)
Reporter | ||
Comment 2•14 years ago
|
||
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
Whiteboard: [mozmill-1.5.2?]
(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.
Hit enter too soon, sorry. But we should take these manifest changes for 1.5.2, obviously.
Whiteboard: [mozmill-1.5.2?] → [mozmill-1.5.2+]
Reporter | ||
Updated•14 years ago
|
Summary: Mozmill tests on mozilla-central should be updated with manifests → [synchronization] Mozmill tests on mozilla-central should be updated with manifests
Comment 5•14 years ago
|
||
Could I get a firm ETA for rolling 1.5.2 out ?
Reporter | ||
Comment 6•14 years ago
|
||
I will get a patch up for m-c tomorrow. It will need to be tested, but should be doable soon after that
Reporter | ||
Comment 7•14 years ago
|
||
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
Reporter | ||
Comment 8•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
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.
Attachment #506926 -
Flags: feedback?(ctalbert) → feedback-
Reporter | ||
Comment 11•14 years ago
|
||
> 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
Reporter | ||
Comment 12•14 years ago
|
||
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.
Reporter | ||
Comment 13•14 years ago
|
||
Attachment #506926 -
Attachment is obsolete: true
Attachment #507888 -
Flags: review?(ctalbert)
Reporter | ||
Comment 14•14 years ago
|
||
> 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.
Reporter | ||
Comment 15•14 years ago
|
||
(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
Reporter | ||
Comment 16•14 years ago
|
||
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
Attachment #507888 -
Flags: review?(ctalbert)
Reporter | ||
Comment 17•14 years ago
|
||
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
Attachment #508486 -
Flags: review?(ctalbert)
Reporter | ||
Comment 18•14 years ago
|
||
version 0.2.2 (current head)
Attachment #507888 -
Attachment is obsolete: true
Attachment #508849 -
Flags: review?(ctalbert)
Comment 19•14 years ago
|
||
That's not a Mozmill 1.5.2 blocker but depends on the release.
Depends on: 629107
Whiteboard: [mozmill-1.5.2+]
Comment 20•13 years ago
|
||
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.
Attachment #508486 -
Flags: review?(ctalbert)
Attachment #508849 -
Flags: review?(ctalbert)
Reporter | ||
Comment 21•13 years ago
|
||
(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
Reporter | ||
Updated•13 years ago
|
Assignee: jhammel → nobody
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 22•13 years ago
|
||
oops, i clicked wrong :( sorry
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Comment 23•12 years ago
|
||
(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.
Status: REOPENED → RESOLVED
Closed: 13 years ago → 12 years ago
Resolution: --- → WONTFIX
Assignee | ||
Updated•8 years ago
|
Product: Testing → Testing Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•