Closed Bug 917999 Opened 11 years ago Closed 9 years ago

Split test package into per-suite packages

Categories

(Testing :: General, defect)

Product:
Testing
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox39 fixed, firefox40 fixed, firefox41 fixed, firefox42 fixed, firefox-esr38 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla42
Tracking Flags:
Tracking Status
firefox39 --- fixed
firefox40 --- fixed
firefox41 --- fixed
firefox42 --- fixed
firefox-esr38 --- fixed

People

(Reporter: ted, Assigned: chmanchester)

References

(Blocks 4 open bugs)

Details

Attachments

(8 files, 4 obsolete files)

Follow up for b2g desktop script.
1.96 KB, patch
jlund
: review+
Details | Diff | Splinter Review
Add aliases for test package requirements for plaforms/scripts referring to more suite_categories than are known on desktop
3.42 KB, patch
jlund
: review+
Details | Diff | Splinter Review
MozReview Request: Bug 917999 - Part 3 - Split tests into harness specific zips.
39 bytes, text/x-review-board-request
ted
: review+
Details
MozReview Request: Bug 917999 - Part 2 - Make mozharness read an uploaded manifest if available to determine which test archives to download before running tests.;r=jlund
39 bytes, text/x-review-board-request
jlund
: review+
Details
MozReview Request: Bug 917999 - Part 3.1 - Fixes for taskcluster.
40 bytes, text/x-review-board-request
jlal
: review+
Details
Follow up to set test_packages_url for gaia tests and specify a suite category in wpt.
2.96 KB, patch
jlund
: review+
Details | Diff | Splinter Review
Accommodate directory rename for cppunittests -> cppunittest
9.39 KB, patch
jlund
: review+
Details | Diff | Splinter Review
Follow up to specify a suite category for android x86
1.49 KB, patch
gbrown
: review+
Details | Diff | Splinter Review
The test package is pretty ridiculously big. From latest-trunk today: [ ] firefox-26.0a1.en-US.linux-i686.tests.zip 17-Sep-2013 12:53 119M [ ] firefox-26.0a1.en-US.mac.tests.zip 17-Sep-2013 12:04 124M [ ] firefox-26.0a1.en-US.win32.tests.zip 17-Sep-2013 13:36 83M I think we should split the test package into per-suite packages, along with perhaps a common package (for mozbase and whatever else is needed by all suites). This should help out a little bit with test setup time, since we won't have to download quite as much data. (Mostly I'm implementing a new harness and I wish I didn't have to put my handful of tests in the giant test package.)
Does this finally give us the ability to move away from zip? IIRC the main reason we like it is because we can easily extract file subsets without significant penalty. But the compression ratio is crap. We should just give mozharness a generic uncompress() API that can recognize archives by file extension or by sniffing file headers and we can upload whatever archive format is current in style.
Split up by dir, the compressed sizes are: cppunittests: 181 381 318 mochitest: 28 116 450 reftest: 21 073 335 xpcshell: 15 885 186 jsreftest: 4 846 268 jit-test: 1 846 856 bin: 1 672 486 jetpack: 911 380 marionette: 414 355 mozbase: 348 612 tps: 165 769 peptest: 107 338 modules: 71 641 certs: 59 298 steeplechase: 47 185 config: 822 so it's probably worth splitting cppunittests out even before splitting it entirely.
Are the cppunittests not stripped? I thought we fixed that.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3) > Are the cppunittests not stripped? I thought we fixed that. They were stripped everywhere in bug 889078, but this behaviour was changed in bug 920055 to support ASAN.
Another benefit of splitting is we can move away from zip. IIRC we use zip to facilitate extraction of specific directories. If the entire archive is relevant, we redundantly compress some contents among multiple archives, but we should get better overall compression since zip doesn't reuse compression contexts across files (like other formats do). Take mochitests for an example: zip: 27 842 706 gzip: 21 460 639 bzip: 19 514 083 Archives with a larger proportion of binary (not very compressible) content will see a lesser size reduction.
(In reply to Gregory Szorc [:gps] from comment #6) > Another benefit of splitting is we can move away from zip. IIRC we use zip > to facilitate extraction of specific directories. If the entire archive is > relevant, we redundantly compress some contents among multiple archives, but > we should get better overall compression since zip doesn't reuse compression > contexts across files (like other formats do). > > Take mochitests for an example: > > zip: 27 842 706 > gzip: 21 460 639 > bzip: 19 514 083 bzip is slow to compress. That would add to build times. The difference between gzip and zip is interesting, since zip uses the same compression. Did you gzip -9? If you did, then a fair comparison would be with zip -9.
> IIRC we use zip to facilitate extraction of specific directories. Note that sounds like a bad reason, because it's possible with tar, too (albeit less efficient, since there is no index).
(and since it's the entire archive that's compressed, instead of individual files)
gzip w/ 9 produces 21 368 356 for mochitests. I used -9 with zip. gzip is probably the right balance between size and speed. Also, the way we create test archives is bat shit insane. We stage files as part of the build to objdir/_tests. Then, we stage them again (often using tar | tar) as part of |make package-tests|. Then, we zip that "unified" directory and then delete it. Clownshoes. If we got our act together, we could probably create the archive straight from files in the source directory using data within manifests, mozpack, and Python's stdlib routines for creating archives: no excessive file copying necessary. This is tracked in another bug somewhere.
xz is faster than bzip2 generally, and gets better compression than gz, I'd be open to looking at it as a possibility. IIRC we chose .zip because some windows platform ages ago couldn't unpack .tar.gz reliably.
> Also, the way we create test archives is bat shit insane. We stage files as > part of the build to objdir/_tests. Then, we stage them again (often using > tar | tar) as part of |make package-tests|. Then, we zip that "unified" > directory and then delete it. Clownshoes. If we got our act together, we > could probably create the archive straight from files in the source > directory using data within manifests, mozpack, and Python's stdlib routines > for creating archives: no excessive file copying necessary. This is tracked > in another bug somewhere. These extra directories/files are also left on disk afterwards, bloating the space required for each build on the machine.
So, splitting should be relatively easy: we just replace our |make package-tests| logic. If we want to go a step further and minimize I/O during packaging, we can start by replacing the package-tests logic with Python. If we want to stop the staging to objdir/_tests, that requires a bit more effort, especially when you consider that some test files are preprocessed. Not sure if we'll ever get there. But if we wanted to try, xpcshell would be a good place to start!
I had thought about putting the test package contents definitions into moz.build, which would make it much easier for us to make the packaging less stupid.
Note that objdir/_tests is used to run tests locally. Getting rid of that would need more smarts from the test harnesses.
(In reply to Mike Hommey [:glandium] from comment #15) > Note that objdir/_tests is used to run tests locally. Getting rid of that > would need more smarts from the test harnesses. Indeed. xpcshell is *almost* there. Preprocessed and otherwise build-system-installed files are the big hurdle here. Unless we want to invent some VPATH like solution, we may just have to always stage to a common location. Boo.
Depends on: 1014659
See Also: → 1059943
I filed bug 1059943 for an alternate idea that catlee and I have been kicking about: uploading all the files from the test package to s3 keyed by a hash.
catlee and I decided bug 1059943 was unworkable, but we talked about a variant of this bug that would be useful. Here's what I propose for this bug: * Make each test suite package its files into a separate archive (don't care if it's zip or what) * Put common files into a common archive * Each test suite should produce a JSON manifest that describes which archive files it needs and what subdirectory they should be unpacked into. Then mozharness instead of downloading the entire tests.zip would download the per-suite manifest and download the archives it references, which is nicer than what we do now. There's a follow-on (that I will file shortly) that we talked about which is even cooler--once we start checking out the source tree on the test machines we can stop packaging files that come verbatim from the source (most of our tests), and the manifest file would list them as "from the source tree" so mozharness would simply copy them from the source tree into the unpacked test archive directory.
Blocks: 1145385
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #18) > catlee and I decided bug 1059943 was unworkable, but we talked about a > variant of this bug that would be useful. Here's what I propose for this bug: > * Make each test suite package its files into a separate archive (don't care > if it's zip or what) > * Put common files into a common archive > * Each test suite should produce a JSON manifest that describes which > archive files it needs and what subdirectory they should be unpacked into. Funny, this manifest sounds like just what I would want to help set up an environment to help people run tests downloaded from automation. > > Then mozharness instead of downloading the entire tests.zip would download > the per-suite manifest and download the archives it references, which is > nicer than what we do now. > > There's a follow-on (that I will file shortly) that we talked about which is > even cooler--once we start checking out the source tree on the test machines > we can stop packaging files that come verbatim from the source (most of our > tests), and the manifest file would list them as "from the source tree" so > mozharness would simply copy them from the source tree into the unpacked > test archive directory.
Relatedly, is anyone planning to take this bug soon?
I'd like to but I'm not sure I have time. Here's what I'd do if you're interested: Phase 1: Write and upload manifests that describe the current state of affairs. Everything requires the full tests.zip, js tests also require the jsshell zip. Phase 2: Revamp mozharness to use the manifests for downloading instead of just fetching the tests zip. Phase 3: Split the tests zip up, make the manifests refer to the proper split zip, mozharness should do the right thing.
I'll take a stab at this.
Assignee: nobody → cmanchester
I have this working on android and desktop as far as splitting out the cpp unittest binaries. I'm a bit concerned about various tools that might live out of tree expecting to find a monolithic tests.zip to consume.
Eh, if it's not in mozharness it'll have to adapt.
I agree but we should make a post to dev.platform explaining the change before we actually push the big red button.
done: most B2G jobs, univeral binaries todo: taskcluster
Taskcluster encodes the idea that a test job needs a build and a tests.zip in a lot of places that will need replacing, but makes this even a bit harder by re-naming things as it uploads them (all its tests.zips are called "target.tests.zip" rather than $(PKG_BASENAME).tests.zip).
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #24) > Eh, if it's not in mozharness it'll have to adapt. I guess it's a good thing I moved TB's mozmill tests to mozharness a few weeks ago? I guess this also means I should look at moving TB builds to mozharness-based, since I'm not really able to maintain buildbot code at all... Oh well, that was already on my "consider doing this" list.
Depends on: 1158276
We currently trigger test jobs in buildbot by passing testsUrl as a property; mozci follows this model. Once we are close to stop passing the URL and instead use the uploaded manifests, I will need to know what will be expected so I can adjust how mozci triggers jobs on buildbot. Thanks for the dev.platform post!
(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #29) > We currently trigger test jobs in buildbot by passing testsUrl as a > property; mozci follows this model. > > Once we are close to stop passing the URL and instead use the uploaded > manifests, I will need to know what will be expected so I can adjust how > mozci triggers jobs on buildbot. > > Thanks for the dev.platform post! This will mean something like passing a testPackagesUrl that is a JSON mapping from harness names to file names in the upload directory that are the required packages. Getting mozharness to do the right thing is part of this bug (Ted's phase 2 from comment 21), once that's done we can make that change in mozCI and things should go ok after the actual split. I'll file a bug for mozCI so I can keep track.
Adding some TaskCluster people to CC...
/r/8051 - Bug 917999 - Part 1 - Write out and upload a manifest of uploaded test archives and the harnesses that depend on them. Pull down this commit: hg pull -r ecfbe966840d5e7ffc7d72d04dea89c26b17e58d https://reviewboard-hg.mozilla.org/gecko/
Attachment #8600417 - Flags: review?(ted)
https://reviewboard.mozilla.org/r/8051/#review6891 ::: build/gen_test_packages_manifest.py:57 (Diff revision 1) > + return harness_requirements Can we write out this file as part of backend generation? That's where we produce all-tests.json.
https://reviewboard.mozilla.org/r/8051/#review7207 > Can we write out this file as part of backend generation? That's where we produce all-tests.json. Possibly, can I get the package name stuff from the python side?
https://reviewboard.mozilla.org/r/8051/#review7423 For some reason I had envisioned writing a separate per-suite manifest, but I guess that doesn't really make anything better or easier. The manifest files might get a little bloaty if we go all-in and list files to fetch from the source repo, but it's probably not going to be that bad.
https://reviewboard.mozilla.org/r/8051/#review7421 > Possibly, can I get the package name stuff from the python side? Unfortunately I don't think there's any straightforward way to do that right now. File a followup and we'll sort it out, it would be nice to do this in backend generation. We could probably eviscerate most of package-name.mk, move it into Python, and just generate the bits we need into a makefile for inclusion there: https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/package-name.mk
Blocks: 992983
/r/8855 - Bug 917999 - Part 2 - Make mozharness read an uploaded manifest if available to determine which test archives to download before running tests.;r=jlund Pull down this commit: hg pull -r 3e45e2b27885f9ec4bbef09bd0315f276dab5419 https://reviewboard-hg.mozilla.org/build-mozharness
Attachment #8606439 - Flags: review?(jlund)
Comment on attachment 8600417 [details] MozReview Request: bz://917999/chmanchester Reviewboard didn't move my r+ here for some reason.
Attachment #8600417 - Flags: review?(ted) → review+
https://reviewboard.mozilla.org/r/8855/#review7849 overall, this looks fantastic. a few in line comments/questions. bear with me, we are essentially going for a no-op perf change as the test_packages.json manifest will just be pointing to the same test zip for every suite for now right? ::: mozharness/mozilla/testing/testbase.py:13 (Diff revision 1) > -import getpass > +import json <3 for getting rid of rogue imports :) ::: mozharness/mozilla/testing/testbase.py:153 (Diff revision 1) > - last_slash = self.installer_url.rfind('/') > + self.fatal("Can't figure out build directory urls without an installer_url!") I suppose we can derive the base url from the test_package_url too so this fatal msg may be misleading ::: mozharness/mozilla/testing/testbase.py:397 (Diff revision 1) > + if "jsshell-" in file_name: it's too bad this is still a special snowflake ::: mozharness/mozilla/testing/testbase.py:208 (Diff revision 1) > + raise AssertionError("You must use --test-url or --test-packages-url with developer_config.py") if we specify both, --test-packages-url will be ignored correct? should we bother checking for that? ::: scripts/android_emulator_unittest.py:670 (Diff revision 1) > + suite_categories = ['mochitest' if s == 'mochitest-gl' else s for s in suite_categories] rather than hardcoding this in the script, what do you think about having the manifest support this? As in, have a 'mochitest-gl' key that points to the same thing as 'mochitest' ::: scripts/android_panda.py:216 (Diff revision 1) > + target_categories = [cat for cat in SUITE_CATEGORIES I've noticed that you have interchangably used tuples and lists for suite_categories type. I suppose if we are just iterating over the items, any container will do but thought I'd sanity check if there was a particular reason for you doing so. ::: mozharness/mozilla/testing/testbase.py:363 (Diff revision 1) > - def _download_unzip(self, url, parent_dir): > + def _read_packages_manifest(self): do you think there is any use in logging the manifest contents here?
https://reviewboard.mozilla.org/r/8855/#review8123 Exactly, this a no op until we actually split the test package (part 3 of this bug). > if we specify both, --test-packages-url will be ignored correct? should we bother checking for that? I guess we should print a warning in this case, I'll update. > do you think there is any use in logging the manifest contents here? I think it has potential value for debugging, I'll add it. > it's too bad this is still a special snowflake Yeah, I don't think going as far as having an install destination in the manifest is quite worth it. > rather than hardcoding this in the script, what do you think about having the manifest support this? As in, have a 'mochitest-gl' key that points to the same thing as 'mochitest' I don't have a lot of background for this, but the presence of this suite category just for emulator tests seems like a inconsistency we shouldn't spread around (on desktop web gl tests have "mochitest" as a suite_category and "mochitest-gl" as a suite).
https://reviewboard.mozilla.org/r/8855/#review8129 > I've noticed that you have interchangably used tuples and lists for suite_categories type. I suppose if we are just iterating over the items, any container will do but thought I'd sanity check if there was a particular reason for you doing so. I guess I got in the habit of writing down a tuple when I know beforehand what the elements are, but it doesn't really matter in this case and consistency is probably better so I changed them all to be lists.
Comment on attachment 8606439 [details] MozReview Request: bz://917999/chmanchester /r/8855 - Bug 917999 - Part 2 - Make mozharness read an uploaded manifest if available to determine which test archives to download before running tests.;r=jlund Pull down this commit: hg pull -r 618469840a094715273c8755a70a364149f5b74f https://reviewboard-hg.mozilla.org/build-mozharness
https://reviewboard.mozilla.org/r/8855/#review8343 ::: mozharness/mozilla/building/buildbase.py:1395 (Diff revisions 1 - 2) > - # packageUrl must be last! > + ('packageUrl', lambda m: m.endswith(packageName)), who would have thought we would add a json file while you were on PTO ;) ::: scripts/android_emulator_unittest.py:573 (Diff revisions 1 - 2) > + suite_category = self.test_suite_definitions[self.test_suite]['category'] does this mean we would only support doing one test suite per script run? Originally I desktop_unittest.py was able to do more than one test suite and run them sequentially but it seems like sometimes we define self.test_suite and other times self.test_suites depending on the script: http://mxr.mozilla.org/build/search?string=self.test_suite&find=mozharness&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=build or am I groking this wrong?
https://reviewboard.mozilla.org/r/8855/#review8351 > does this mean we would only support doing one test suite per script run? > > Originally I desktop_unittest.py was able to do more than one test suite and run them sequentially but it seems like sometimes we define self.test_suite and other times self.test_suites depending on the script: http://mxr.mozilla.org/build/search?string=self.test_suite&find=mozharness&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=build > > or am I groking this wrong? No, that's correct. It looks like the multiple test suite invocation feature was dropped for the android emulator tests in a recent patch in https://bugzilla.mozilla.org/show_bug.cgi?id=1164866 I don't think we use the feature very much in automation, if at all.
Comment on attachment 8606439 [details] MozReview Request: bz://917999/chmanchester not sure how to make this an r+ via reviewboard, I thought 'ship it' button did that
Attachment #8606439 - Flags: review?(jlund) → review+
I'm trying to figure out the best way to accommodate comment 27, I'm thinking for now we can just write out two package manifests and only upload one based on whether we're running in taskcluster.
Short follow up for b2g desktop (I think I missed this because mulet reftests use the script, but not mulet mochitests!). Not using rb for this because it gives a diff from the last patch that's fairly confusing.
Attachment #8614778 - Flags: review?(jlund)
Comment on attachment 8614778 [details] [diff] [review] Follow up for b2g desktop script. Review of attachment 8614778 [details] [diff] [review]: ----------------------------------------------------------------- so many scripts/mixins. I'm surprised we only missed one!
Attachment #8614778 - Flags: review?(jlund) → review+
Sorry about the churn on this. I'm sort of lost in a mess of per-platform configuration differences.
Attachment #8615492 - Flags: review?(jlund)
Comment on attachment 8615492 [details] [diff] [review] Add aliases for test package requirements for plaforms/scripts referring to more suite_categories than are known on desktop Sorry, didn't mean to request review on this yet.
Attachment #8615492 - Flags: review?(jlund)
Comment on attachment 8615492 [details] [diff] [review] Add aliases for test package requirements for plaforms/scripts referring to more suite_categories than are known on desktop Ok, confirmed reasonably effective in https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6d9c4061da7
Attachment #8615492 - Flags: review?(jlund)
Comment on attachment 8615492 [details] [diff] [review] Add aliases for test package requirements for plaforms/scripts referring to more suite_categories than are known on desktop Review of attachment 8615492 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozharness/mozilla/testing/testbase.py @@ +382,5 @@ > def _download_test_packages(self, suite_categories, target_unzip_dirs): > + # Some platforms define more suite categories/names than others. > + # This is a difference in the convention of the configs more than > + # to how these tests are run, so we pave over these differences here. > + aliases = { especially now that this is n>1 for these aliases, my gut tells me these should be in a config or the newly created in-tree manifest you did. but I don't want to block on a nit :)
Attachment #8615492 - Flags: review?(jlund) → review+
NI myself in order to get back to chmanchester.
Flags: needinfo?(armenzg)
(In reply to Armen Zambrano G. (:armenzg - Toronto) from comment #64) > NI myself in order to get back to chmanchester. We sorted this out on GH.
Flags: needinfo?(armenzg)
Another set of straggler scripts.
Attachment #8617647 - Flags: review?(jlund)
I'd like to start with cppunittests, reftest, xpcshell, and mochitest. Those 4 account for 70-80% of the zip. Here it is on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ddbc32104dd1
cppunittests are the single biggest offender in the zip, so if you can get those out that'd be fantastic. This also unblocks fixing bug 992983, which would get a big set of tests out of `make check`, which would also be great.
From your try push: https://hg.mozilla.org/try/rev/e9de8b55d5df#l2.21 2.21 +# For now, "cppunitests" vs. "cppunitest" is the only difference between a 2.22 +# subdir and a package name. 2.23 +DIR_FOR_PKG = $(subst cppunittest,cppunittests,$(1)) Just rename the directory to make it match, I don't think it should hurt anything (as long as you fix all the references to it).
From the test package in your try push, here's a summary of bytes by top-level directory for what's left: mach: 5886 luciddream: 21314 config: 41975 tools: 127468 certs: 208790 steeplechase: 276936 modules: 317798 tps: 621237 mozbase: 1431768 jetpack: 4340294 marionette: 10321362 bin: 10470539 jit-test: 18671062 jsreftest: 19500557 web-platform: 36540813 Working your way up from the bottom of that list would seem to be a reasonable thing to do. :) Also: I wondered why bin/ was so big, turns out we're putting a bunch of binaries in there without stripping them... I'll file a bug to fix that.
Filed bug 1173323 on the unstripped bin/ dir.
Attachment #8600417 - Attachment is obsolete: true
Attachment #8606439 - Attachment is obsolete: true
Attachment #8618043 - Flags: review+
Attachment #8618044 - Flags: review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #70) > From the test package in your try push, here's a summary of bytes by > top-level directory for what's left: > mach: 5886 > luciddream: 21314 > config: 41975 > tools: 127468 > certs: 208790 > steeplechase: 276936 > modules: 317798 > tps: 621237 > mozbase: 1431768 > jetpack: 4340294 > marionette: 10321362 > bin: 10470539 > jit-test: 18671062 > jsreftest: 19500557 > web-platform: 36540813 > > Working your way up from the bottom of that list would seem to be a > reasonable thing to do. :) > > Also: I wondered why bin/ was so big, turns out we're putting a bunch of > binaries in there without stripping them... I'll file a bug to fix that. web-platform should be no problem... some of the others might require non-trivial mozharness changes, I might leave them to a follow up.
Comment on attachment 8617647 [details] [diff] [review] Set test_packages_url for gaia tests. Review of attachment 8617647 [details] [diff] [review]: ----------------------------------------------------------------- I can tell this isn't going to be the last mozharness patch, so let's hold off on review until I know what we're up against.
Attachment #8617647 - Flags: review?(jlund)
Try splitting another zip out and implementing some of Ted's feedback: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dfeaa184de1c I'll request the rest of the review when that comes back green.
Comment on attachment 8618043 [details] MozReview Request: Bug 917999 - Part 3 - Split tests into harness specific zips. Bug 917999 - Part 3 - Split tests into harness specific zips.
Attachment #8618043 - Attachment description: MozReview Request: Bug 917999 - Part 1 - Write out and upload a manifest of uploaded test archives and the harnesses that depend on them. → MozReview Request: Bug 917999 - Part 3 - Split tests into harness specific zips.
Attachment #8618043 - Flags: review+ → review?(ted)
Attachment #8617647 - Attachment is obsolete: true
Attachment #8621105 - Flags: review?(jlal) → review+
Comment on attachment 8621105 [details] MozReview Request: Bug 917999 - Part 3.1 - Fixes for taskcluster. https://reviewboard.mozilla.org/r/10877/#review9559 Ship It!
Comment on attachment 8621106 [details] [diff] [review] Follow up to set test_packages_url for gaia tests and specify a suite category in wpt. Review of attachment 8621106 [details] [diff] [review]: ----------------------------------------------------------------- lgtm :)
Attachment #8621106 - Flags: review?(jlund) → review+
Attachment #8621107 - Flags: review?(jlund) → review+
(In reply to Chris Manchester [:chmanchester] from comment #82) > Created attachment 8621107 [details] [diff] [review] > Accommodate directory rename for cppunittests -> cppunittest This needs to land and then get enabled with a mozharness.json bump when comment 79 does.
(In reply to Jordan Lund (:jlund) from comment #84) > Comment on attachment 8621106 [details] [diff] [review] > Follow up to set test_packages_url for gaia tests and specify a suite > category in wpt. > > Review of attachment 8621106 [details] [diff] [review]: > ----------------------------------------------------------------- > > lgtm :) https://hg.mozilla.org/build/mozharness/rev/38b99a593e4b
Attachment #8618043 - Flags: review?(ted)
Comment on attachment 8618043 [details] MozReview Request: Bug 917999 - Part 3 - Split tests into harness specific zips. https://reviewboard.mozilla.org/r/8051/#review10137 ::: build/gen_test_packages_manifest.py:20 (Diff revision 2) > + 'web-platform', Sorry, I didn't mean that you *had* to split web-platform out in this patch series, I was just looking at what the best things to do next would be! ::: testing/testsuite-targets.mk:420 (Diff revision 2) > +TEST_PKGS = \ nit: use := for Makefile vars with static content ::: testing/testsuite-targets.mk:439 (Diff revision 2) > - @rm -f '$(DIST)/$(PKG_PATH)$(TEST_PACKAGE)' > + rm -f '$(DIST)/$(PKG_PATH)$(PKG_BASENAME).common.tests.zip' You changed the value of $(TEST_PACKAGE) in package-name.mk, so I don't think you need this change (or the one below). ::: testing/testsuite-targets.mk:447 (Diff revision 2) > - * -x \*/.mkdir.done \*.pyc > + * -x \*/.mkdir.done \*.pyc $(foreach name,$(TEST_PKGS),$(name)\*) && \ This is a little gross, can you file a followup on making this less gross? Maybe we can just stage things that aren't going into the common test package into different dirs for now. Eventually I'd like to use the packager code to copy files directly into the archives and avoid the staging completely, but that's more work. ::: toolkit/mozapps/installer/package-name.mk:135 (Diff revision 2) > +MOCHI_TEST_PACKAGE = $(PKG_BASENAME).mochitest.tests.zip The underscores in these names are weird. Can you just take them out for the things where we commonly call them by one word, like _Mochitest_?
Comment on attachment 8618043 [details] MozReview Request: Bug 917999 - Part 3 - Split tests into harness specific zips. https://reviewboard.mozilla.org/r/8051/#review10139 Ship It!
Attachment #8618043 - Flags: review+
https://reviewboard.mozilla.org/r/8051/#review10143 > Sorry, I didn't mean that you *had* to split web-platform out in this patch series, I was just looking at what the best things to do next would be! No problem, was trivial in this case :) > This is a little gross, can you file a followup on making this less gross? Maybe we can just stage things that aren't going into the common test package into different dirs for now. Eventually I'd like to use the packager code to copy files directly into the archives and avoid the staging completely, but that's more work. I had an earlier version that staged each suite in a separate dir, but that required changes to every makefile, and some ugly stuff to deal with unified builds correctly for the same effect (https://hg.mozilla.org/try/rev/7bc008456cec for reference). At any rate, we can discuss in bug 1176036 .
This one's quick.
Attachment #8627378 - Flags: review?(jlund)
Attachment #8627378 - Flags: review?(gbrown)
Comment on attachment 8627378 [details] [diff] [review] Follow up to specify a suite category for android x86 Review of attachment 8627378 [details] [diff] [review]: ----------------------------------------------------------------- Other tests - mochitest, reftests - are run on Android x86 on the ash repo only. Those are kind-of broken, with no current plans to fix them, so I'm not very concerned about breaking them further. Of course, if it wouldn't be too hard, I would prefer a more generic solution, originating in the config. Is the issue that we download_and_extract for a set, while the configs are per suite?
Attachment #8627378 - Flags: review?(gbrown) → review+
(In reply to Geoff Brown [:gbrown] from comment #98) > Comment on attachment 8627378 [details] [diff] [review] > Follow up to specify a suite category for android x86 > > Review of attachment 8627378 [details] [diff] [review]: > ----------------------------------------------------------------- > > Other tests - mochitest, reftests - are run on Android x86 on the ash repo > only. Those are kind-of broken, with no current plans to fix them, so I'm > not very concerned about breaking them further. Of course, if it wouldn't be > too hard, I would prefer a more generic solution, originating in the config. > Is the issue that we download_and_extract for a set, while the configs are > per suite? Ok, I think I see a better way to do this!
Attachment #8627378 - Attachment is obsolete: true
Comment on attachment 8627432 [details] [diff] [review] Follow up to specify a suite category for android x86 Review of attachment 8627432 [details] [diff] [review]: ----------------------------------------------------------------- That's great!
Attachment #8627432 - Flags: review?(gbrown) → review+
hey Chris, in the effort to put mozharness into the gecko tree, it looks like using the latest production version of mozharness is busted because of our efforts in this bug. Will it just be a matter of uplifting the above m-c patches across trees? context: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=c06de6062d0e
Flags: needinfo?(cmanchester)
(In reply to Jordan Lund (:jlund) from comment #105) > hey Chris, > > in the effort to put mozharness into the gecko tree, it looks like using the > latest production version of mozharness is busted because of our efforts in > this bug. Will it just be a matter of uplifting the above m-c patches across > trees? > > context: > https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&revision=c06de6062d0e Yep, that should do it! I also need to backport the fixes from bug 1179945, but that just needs to happen before the next merge.
Depends on: 1179945, 1181732
Flags: needinfo?(cmanchester)
Blocks: 1181793
Blocks: 1198179
(In reply to Gregory Szorc [:gps] from comment #1) > Does this finally give us the ability to move away from zip? IIRC the main > reason we like it is because we can easily extract file subsets without > significant penalty. But the compression ratio is crap. We should just give > mozharness a generic uncompress() API that can recognize archives by file > extension or by sniffing file headers and we can upload whatever archive > format is current in style. That's what I'm actually working on in bug 1237706. For the beginning it will support zip, gzip, bz2.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: