Closed
Bug 421611
Opened 16 years ago
Closed 15 years ago
Need to be able to run tests on arbitrary build
Categories
(Firefox Build System :: General, defect, P3)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: shaver, Assigned: ted)
References
Details
(Keywords: fixed1.9.1)
Attachments
(4 files, 8 obsolete files)
3.94 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
6.34 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
1.86 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
21.04 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
Case 1: We have builds that take a long time, like our PGO ones, and we want to test them fully. This means that we should be doing those builds once, and testing them for correctness in parallel and on other machines, as we do for perf. Case 2: We want to use the trunk test suite to help track down regressions on branch, or compare to other builds. Being able to do just a test-export pass on a source tree pulled with the same timestamp would let the unit boxes set up their bits and then drive a centrally-produced build through its paces. This probably has some deps for specific enable-tests cases decouplings, if they affect perf in some way we're not happy with for our perf testing. Hope not, though!
Flags: blocking1.9?
Comment 1•16 years ago
|
||
I'm not sure if I'm the right person to set blocking on this, and for now will refrain from potentially overstepping my bounds, but I think this is pretty important in terms of being able to properly test the builds that we're shipping to users. I was a little surprised to learn that we don't actually do that with the RCs we spin up for various shipping milestones. I would really hope we could get this for Fx3b5, and almost insist that we have it for Fx3rc1.
Priority: -- → P2
Comment 2•16 years ago
|
||
This came up from discussing why we don't do unittests and perf tests for releases.
Comment 3•16 years ago
|
||
What? We're not testing builds we're shipping to users? (Comment #1). Please explain. :-/
Comment 4•16 years ago
|
||
At minimum, we should send release builds through Talos, and trigger unittest builders from the release tag. These can be done in parallel to the existing QA activities.
Updated•16 years ago
|
OS: Windows Vista → All
Hardware: PC → All
Comment 5•16 years ago
|
||
We have five kinds of testcases: * mochitest/mochichrome: we should definitely be able to run these on an arbitrary build; ted had an extension to do this, IIRC * reftest: we already have the capability to run these on a packaged build, but should automate the capability * crashtests: I don't know much about these... Jesse, are these something we can run on a packaged build? * xpcshell tests: this is a mixed bag. You can't run these on a packaged build right now because the packages don't contain xpcshell. With that fixed, you might be able to get some useful results... more of a long-pole, though * random custom 'make check' tests: most of these are run on test-binaries that aren't in a packaged build or rely on data which you would have to rebuild to be useful... not worth trying to create a new type of package to test these, IMO
Comment 6•16 years ago
|
||
Crashtests use the same code as reftests, so if reftests work, crashtests should work.
Assignee | ||
Comment 7•16 years ago
|
||
I have a reftest extension, that's easy enough (and that gets you crashtest too). Mochitest/mochichrome/browser tests all are easy to run on an arbitrary build. The xpcshell tests are in fact the hard part, but just packaging xpcshell would probably fix that.
Comment 8•16 years ago
|
||
Yea - if we can get this done it will save us a ton of hassle and time as the unit test machines will not have to be kept up to date with build config changes on the build machines. Not to mention cycle time benefits. I think the talos arrangement has generally worked great. So b+..
Flags: blocking1.9? → blocking1.9+
Comment 9•16 years ago
|
||
Schrep, we are not going to be able to do this for the arbitrary "make check" tests, nor IMO should we try. So I think we're still going to need build+makecheck machines somewhere. Perhaps we can have the current build+debug+leakcheck machines do that work?
Comment 10•16 years ago
|
||
We should consider getting the unit test machines to check out the same mozconfig's as the nightly/dep builders do. There's really no reason for them to get out of sync.
Comment 11•16 years ago
|
||
yeah, that's a no-brainer, though again make check causes problems there with the addition of --enable-tests. I'll sync up with ted sometime this week and get his extension for reftests/crashtests and see if we can setup some unittest machines that run these and the mochitests+variants on generated builds. I'm reluctant to out-right replace the current unittest machines with these as it's nice to have some "quick" (I realize this is a relative term) feedback on checkins.
Status: NEW → ASSIGNED
Updated•16 years ago
|
Assignee: nobody → rcampbell
Status: ASSIGNED → NEW
Updated•16 years ago
|
Status: NEW → ASSIGNED
Updated•16 years ago
|
Assignee: rcampbell → nobody
Status: ASSIGNED → NEW
Component: Testing → Release Engineering: Projects
Flags: blocking1.9+
Product: Core → mozilla.org
QA Contact: testing → release
Version: unspecified → other
Comment 12•16 years ago
|
||
Damon, schrep: for trunk/1.9, - we're not doing PGO linux or mac builds - for PGO win32 builds, we have bug#420073 to setup unittest run on PGO - for PGO win32 builds, we already have Talos builds running with PGO builds. (from discussion in triage meeting... Is there anything else left to do here for 1.9/trunk? Or is this bug a more general bug about anyone being able send any build to test machines... in which case maybe rename to "integrate try server with unittest & talos" and remove blocking1.9 flag?)
Updated•16 years ago
|
Priority: P2 → P3
Assignee | ||
Comment 14•16 years ago
|
||
I've been talking about this with various people lately, so I thought I'd just commit to a comment what's been floating around in my head. I think there are a few possible approaches here: 1) Do a normal build, upload it as usual. On a separate test machine, download that build, checkout the source, build just the test harnesses, and run the tests on the build. This would not allow us to run the TUnit tests, as some of those are standalone C++ programs, and the rest rely on xpcshell, which we don't package. 2) Do a normal build with --enable-tests, upload the build as usual, but also package and upload the test harnesses/test files. On a separate test machine, download the build and the test package, run the tests on the packaged build. We could feasibly package the standalone tests and the xpcshell binary, so we could run the TUnit tests. It might be difficult to package all the test files currently, as the xpcshell tests often rely on files from the srcdir, and reftest leaves all its tests in the srcdir. 3) Some sort of hybrid of the above. Do a normal build with --enable-tests, upload the build, package and upload just the test harnesses + necessary binaries (standalone tests, xpcshell). On a separate test machine, download the build, test package, and also checkout the same source the build was made from. Run the tests using the test harnesses from the test package, with the actual test files from the srcdir. This would probably be easier if we modified the xpcshell and Mochitest harnesses to run the tests from the srcdir instead of copying to the objdir.
Reporter | ||
Comment 15•16 years ago
|
||
#2, but package the source files as well (or a subset of them, if we can use the metadata to identify the sets of srcdir things we need)?
Assignee | ||
Comment 16•16 years ago
|
||
Why package the source at all? We know exactly what changeset we're building from these days, so it's trivial to pull the matching code from hg.
Assignee | ||
Comment 17•16 years ago
|
||
Just as a data point, I did some cursory investigation of how long we spend building/testing on the unittest machines, and for Windows and Linux unittest machines, the total cycle took almost an hour for each (59 and 52 minutes, respectively), compilation took 12 and 3.5 minutes, respectively, and running TUnit took 3.5 and 4 minutes, respectively. I'm going to gather some more data, but it sounds like making TUnit portable to another machine isn't going to be worth the effort. One possibility here would be flipping our build machines to --enable-tests, then also have them run TUnit locally. The unit test machines could then run Mochitest and Reftest against the packaged builds.
Comment 18•16 years ago
|
||
Is this an issue with cross-compiled builds? I'm under the impression that an ARMEL build will not run on an x86 scratchbox machine, and compiling on the N8x0's seems like the wrong solution. (Please correct me if I'm wrong.) Ideally we'd be able to grab a build and a test suite and go.
Assignee | ||
Comment 19•16 years ago
|
||
I had a chat with Joel, he and Aki are both working on running tests on mobile devices. Joel has some great wiki pages documenting his process: https://wiki.mozilla.org/Mobile/Fennec_Mochitest https://wiki.mozilla.org/Mobile/Fennec_Reftests Mochitest sounds like the lowest-hanging fruit here. Right now, it would entail packaging xpcshell and ssltunnel (either with the browser or separately) and the _tests/testing/mochitest dir from the objdir, which contains the entire test harness + tests. The one caveat is that per bug 445611 comment 27, we can't use --enable-logrefcnt on our build machines, as it will slow down reference counting, which is undesirable for builds we're going to ship. This means that Mochitest leak tracking will not work. (Waldo will probably kill someone if we break this.) xpcshell tests will be a little trickier. Technically the test files all wind up in _tests/xpcshell-simple, but tests can reference arbitrary files from the srcdir, so that makes life exciting. Reftest is trickier still, as all the tests are run out of the srcdir. We can easily package the reftest chrome bits. Per the links above, Joel has written a script (http://people.mozilla.com/~jmaher/fennec/reftest_extract.py) that parses the reftest.list manifests and figures out what directories to package. This seems suboptimal. Either we should provide an easier way to find and package necessary reftest files, or we should just require a source directory pull that matches the build you're testing. This might be tough on mobile, where space is limited. Currently Joel says that the test files alone take up ~30Mb. A plain mozilla-central source tree (without hg repo, generated via `hg archive`) takes up ~260Mb on my Windows machine, and a .tar.bz2 of it is not quite 40Mb. I think that covers everything I currently know. I will start working on Mochitest first. I welcome any suggestions for how to overcome issues mentioned above.
Assignee: nobody → ted.mielczarek
Assignee | ||
Comment 20•16 years ago
|
||
Oh, there's one other tricky bit with reftest, in that it currently won't build in a Mac OSX universal build, because it copies autoconf.mk to autoconf.js, which winds up with differences between the ppc and x86 builds, so the build fails in unify. We could fix this with bug 420084 or something similar.
Comment 21•16 years ago
|
||
As long as we're building with tracerefcnt on at least some machines that run Mochitests (any debug machine would have it, all the current opt ones on tinderbox have it enabled), that should catch most leaks. Mochitests primarily test correctness, and the leak checking is just a bonus; if release builds get the correctness tests and basically can't get leak checking, that seems good enough. Refcounting is impossible to do without a perf hit, and we can't get around that. There's also a small number of compiled-code tests that execute in |make check| that would need to be run on the build machine. Note that xpcshell has do_get_file to make most file access just be from the source directory. A number of tests do mess with `pwd`, tho, but we {c,sh}ould probably fix that by adding something like do_get_tmpdir() and making them use that, possibly even cd to it to be absolutely safe. Reftest's manifest format could probably be extended to record file dependencies without too much trouble.
Assignee | ||
Updated•16 years ago
|
Component: Release Engineering: Future → Build Config
Product: mozilla.org → Core
QA Contact: release → build-config
Version: other → unspecified
Assignee | ||
Comment 24•16 years ago
|
||
Here's a rough draft of what this might look like for Mochitest. This adds a make target so that you can do "make -C objdir/testing/mochitest package", and you will wind up with $(DIST)/test-package.tar.bz2, which contains a harness/ directory, containing all of objdir/_tests/testing/mochitest, and a bin/ directory containing xpcshell, ssltunnel, certutil. Of course, you can't do much with it yet, since runtests.py expects to have a working objdir at the moment, and xpcshell doesn't want to run from anywhere but dist/bin right now. It's a start, though.
Assignee | ||
Comment 25•16 years ago
|
||
Using this patch, along with the patches from dependent bugs here, I'm able to build with tests, package the build (make -C $OBJDIR package), package mochitest (make -C $OBJDIR/testing/mochitest package), then unpack both of those into a new directory (c:\unittest), copy bin/* to firefox/, copy bin/components/test_necko.xpt to firefox/components (until bug 470971 has a patch), and then run mochitest as follows: python harness/runtests.py --app="c:\\unittest\\firefox\\firefox.exe" --utility-path="c:\\unittest\\firefox" --certificate-path="c:\\unittest\\certs" ...and it works!
Attachment #353228 -
Attachment is obsolete: true
Assignee | ||
Comment 26•15 years ago
|
||
Oops, that was completely the wrong patch.
Attachment #354328 -
Attachment is obsolete: true
Comment 27•15 years ago
|
||
(In reply to comment #25) > Created an attachment (id=354328) [details] > rough draft, take two > > Using this patch, along with the patches from dependent bugs here, I'm able to > build with tests, package the build (make -C $OBJDIR package), package > mochitest (make -C $OBJDIR/testing/mochitest package), then unpack both of > those into a new directory (c:\unittest), copy bin/* to firefox/, copy > bin/components/test_necko.xpt to firefox/components (until bug 470971 has a > patch), and then run mochitest as follows: > python harness/runtests.py --app="c:\\unittest\\firefox\\firefox.exe" > --utility-path="c:\\unittest\\firefox" --certificate-path="c:\\unittest\\certs" > > ...and it works! heh, nice! Curious, you mention mochitest here; do you expect the same approach to work for the other test suites within "UnitTest" ? I know its early work-in-progress still, so maybe this is just the first suite you started on? I ask because in order to speed up unittest elapsed time, we're thinking of running each UnitTest suites concurrently, all on the same identical pre-existing build - *without* rebuilding. Is that a scenario this bugfix will work for also?
Assignee | ||
Comment 28•15 years ago
|
||
(In reply to comment #27) > Curious, you mention mochitest here; do you expect the same approach to work > for the other test suites within "UnitTest" ? I know its early work-in-progress > still, so maybe this is just the first suite you started on? I ask because in > order to speed up unittest elapsed time, we're thinking of running each > UnitTest suites concurrently, all on the same identical pre-existing build - > *without* rebuilding. Is that a scenario this bugfix will work for also? The ultimate goal of this bug is to be able to run all the unit tests (except probably compiled C++ tests) against an existing build on a different machine without recompiling. I started with Mochitest because it seemed like the lowest-hanging fruit to me. There are really only three unit test harnesses that need work done: Mochitest, Reftest, and TUnit. Mochitest encompasses the normal mochitest suite, mochitest-chrome, the browser chrome tests, and the a11y tests. Reftest includes Crashtest. Since they're each separate harnesses, they each have their own issues. I would estimate that Reftest and TUnit will take more work than Mochitest, simply because of the way the harnesses work.
Comment 29•15 years ago
|
||
I ran this in fennec (maemo) and came close to success out of the box. two main issues: Makefile has nsinstall instead of $(INSTALL) runtests.py had LD_LIBRARY_PATH=self._appPath instead of self._utilityPath Here are the instructions: 1) install 3 patches (421611, 460515, 470914) 2) make -C client.mk build 3) make -C ($fennec_objdir) package 4) sed -i "s/nsinstall/\$\(INSTALL\)/g" $(xr_objdir)/testing/mochitest/Makefile 5) make -C ($xr_objdir)/testing/mochitest package 6) bunzip $(fennec_objdir)/dist/fennec*.bz2 7) scp $(fennec_objdir)/dist/fennec*.tar <device>:~/ 8) bunzip $(xr_objdir)/dist/test-package*.bz2 9) scp $(xr_objdir)/dist/test-package*.tar <device>:~/ 10)ssh <device> 11)tar -xvf *.tar 12)cp bin/* fennec/xulrunner/ 13)cp bin/components/test_necko.xpt fennec/components 14)sed -i 's/= self._appDir/= self._utilityPath/g' harness/runtests.py 15)python harness/runtests.py --appname=/root/fennec/fennec --utility-path=/root/fennec/xulrunner --certificate-path=/root/certs --test-path=MochiKit_Unit_Tests --autorun I wanted to include all the steps I take to ensure that we all understand what it takes to run on maemo. The one quirky issue is where we copy the bin/* to and the test_necko.xpt to. It would be nice if this step was not there as it is very confusing.
Assignee | ||
Comment 30•15 years ago
|
||
(In reply to comment #29) > The one quirky issue is where we copy the bin/* to and the test_necko.xpt to. > It would be nice if this step was not there as it is very confusing. That should be fixed by bug 470971 (and a little harness tweaking to coincide). Thanks for testing this!
Assignee | ||
Comment 31•15 years ago
|
||
Oops, replace nsinstall with $(NSINSTALL).
Attachment #354411 -
Attachment is obsolete: true
Assignee | ||
Comment 32•15 years ago
|
||
Josh's test plugin in bug 386676 will need to be handled somehow in this work, as it doesn't ship with the packaged build.
Comment 33•15 years ago
|
||
I tested this latest patch (along with the updated dependent patches), and from my previous list in comment #29, steps 4 and 14 are not necessary anymore. Keep in mind that you need to add a --xre-path to the cli such as: python harness/runtests.py --appname=/root/fennec/fennec --utility-path=/root/fennec/xulrunner --certificate-path=/root/certs --test-path=MochiKit_Unit_Tests --xre-path=/root/fennec/xulrunner --autorun
Comment 34•15 years ago
|
||
I have also tested this with the --chrome flag and it appears to work. I have yet to do a full end to end test with it, but a few smaller tests have been successful. We should look at browser-chrome and a11y tests as well.
Assignee | ||
Comment 35•15 years ago
|
||
I've got a cleaned up patch for this, but I need to give it a once-over on Mac/Linux to make sure I didn't screw things up. Should have it up today.
Assignee | ||
Comment 36•15 years ago
|
||
Ok, this changes the way things work a little bit. Now, the make target to invoke the packaging is "make test-package" in the root of the objdir. The test package will wind up as dist/$packagename.tests.tar.bz2, so on windows, like: dist/firefox-3.2a1pre.en-US.win32.tests.tar.bz2 Unpacking this somewhere along with the packaged build from the same objdir, you can run Mochitest like: python mochitest/runtests.py --appname=/path/to/firefox/firefox --xre-path=/path/to/firefox --utility-path=`pwd`/bin/ --certificate-path=`pwd`/certs/ If you want to run the chrome Mochitests, you'll also need to add: --extra-profile-file=`pwd`/plugins to get the testing plugin copied to the testing profile.
Attachment #357144 -
Attachment is obsolete: true
Attachment #359076 -
Flags: review?(benjamin)
Assignee | ||
Comment 37•15 years ago
|
||
There are test builds/test packages produced with this patch (+ all dependencies) available here for Windows and OS X: http://mavra.perilith.com/~luser/latest-teds-builds/
Comment 38•15 years ago
|
||
Comment on attachment 359076 [details] [diff] [review] add package target and packaging bits for mochitest [Checkin: Comment 45 & 64] >diff --git a/testing/mochitest/Makefile.in b/testing/mochitest/Makefile.in >+# We need the test plugin as some tests rely on it >+ifeq (Darwin,$(OS_TARGET)) >+TEST_HARNESS_PLUGINS := \ >+ Test.plugin/ >+else >+TEST_HARNESS_PLUGINS := \ >+ $(DLL_PREFIX)nptest$(DLL_SUFFIX) >+endif Is that a stray slash at the end of Test.plugin, or is that a directory? >+test-package: stage-mochitest >+ @(cd $(PKG_STAGE) && tar $(TAR_CREATE_FLAGS) - *) | bzip2 -f > $(DIST)/$(PKG_PATH)$(TEST_PACKAGE) Can I bikeshed this and have you call it "package-tests" (verb-object)
Attachment #359076 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 39•15 years ago
|
||
(In reply to comment #38) > Is that a stray slash at the end of Test.plugin, or is that a directory? It's a bundle, so yeah, it's a directory. Just figured I'd make that explicit. > Can I bikeshed this and have you call it "package-tests" (verb-object) That's fine with me, I was thinking of "test-package" as a noun, in parallel with "make package" and "make installer".
Comment 40•15 years ago
|
||
I retested this end to end on a maemo device. I did this in comment #29 but have updated the process here. Here are the instructions: 1) install 4 patches (421611, 460515, 470971, 475383) 2) make -C client.mk build 3) make -C ($fennec_objdir) package 4) make -C ($xr_objdir) test-package 5) bunzip $(fennec_objdir)/dist/fennec*.bz2 6) scp $(fennec_objdir)/dist/fennec*.tar <device>:~/ 7) bunzip $(xr_objdir)/dist/xulrunner*.bz2 8) scp $(xr_objdir)/dist/xulrunner*.tar <device>:~/ 9)<device>:tar -xvf *.tar 10)<device>:python mochitest/runtests.py --appname=/root/fennec/fennec --utility-path=/root/bin --certificate-path=/root/bin --xre-path=/root/fennec/xulrunner --test-path=MochiKit_Unit_Tests --autorun This is awesome. We removed a lot of the hacky steps. Next up reftests?
Comment 41•15 years ago
|
||
(In reply to comment #28) > (In reply to comment #27) > > Curious, you mention mochitest here; do you expect the same approach to work > > for the other test suites within "UnitTest" ? I know its early work-in-progress > > still, so maybe this is just the first suite you started on? I ask because in > > order to speed up unittest elapsed time, we're thinking of running each > > UnitTest suites concurrently, all on the same identical pre-existing build - > > *without* rebuilding. Is that a scenario this bugfix will work for also? > > The ultimate goal of this bug is to be able to run all the unit tests (except > probably compiled C++ tests) against an existing build on a different machine > without recompiling. What should we do about these C++ tests, once all of the rest of this bug is completed? We talked about a few ideas yesterday, but who would know if we still even need them, and if so, whats best way to run them? > I started with Mochitest because it seemed like the > lowest-hanging fruit to me. Cool. > There are really only three unit test harnesses > that need work done: Mochitest, Reftest, and TUnit. Mochitest encompasses the > normal mochitest suite, mochitest-chrome, the browser chrome tests, and the > a11y tests. Reftest includes Crashtest. Since they're each separate harnesses, > they each have their own issues. I would estimate that Reftest and TUnit will > take more work than Mochitest, simply because of the way the harnesses work. Ted, thinking of rollout: Once you get Mochitest working like this, we can start working on running Mochitest on existing builds in production, and disabling Mochitest as part of the current build+unittest step. We'll have to figure out how to tie the two sets of results together, but at least it would get us some improvement on turnaround time, while you are still working on the next suites. Seem like a reasonable approach to rollout?
Comment 42•15 years ago
|
||
Most of the C++ tests cover very low-level behaviors, and are very quick. I think we should continue to run them from the build machines for the forseeable future, and not worry about that we can't run them on mobile or the other situations where we want to run tests on arbitrary builds.
Assignee | ||
Comment 43•15 years ago
|
||
(In reply to comment #41) > Ted, thinking of rollout: Once you get Mochitest working like this, we can > start working on running Mochitest on existing builds in production, and > disabling Mochitest as part of the current build+unittest step. We'll have to > figure out how to tie the two sets of results together, but at least it would > get us some improvement on turnaround time, while you are still working on the > next suites. > > Seem like a reasonable approach to rollout? Even with this (and dependencies) landed, we'll still have at least one blocker standing in our way of actually using this in production. Notably, we'll have to make the build machines enable tests, and I would not do that without having fixed bug 463605, since otherwise we'd start shipping gobs of test junk in our Mac nightlies. If we fix that, then we should be able to make the build machines enable tests and upload this test package.
Reporter | ||
Comment 44•15 years ago
|
||
Let's do it for non-Mac systems first, then, while bug 463605 is being fixed.
Assignee | ||
Comment 45•15 years ago
|
||
Comment on attachment 359076 [details] [diff] [review] add package target and packaging bits for mochitest [Checkin: Comment 45 & 64] Pushed: http://hg.mozilla.org/mozilla-central/rev/078ad879e689
Attachment #359076 -
Attachment description: with some cleanup → add package target and packaging bits for mochitest [checked in]
Comment 46•15 years ago
|
||
Verified this with a fresh hg pull and build for fennec on both desktop and maemo device. No patches and using above steps from comment #40. Great work!
Assignee | ||
Comment 47•15 years ago
|
||
This adds a "stage-package" target to layout/tools/reftest, and calls it from the "package-tests" target. The implementation is a little crazy, but I think it's the best way to go. I load reftest.js in xpcshell, then use ReadTopManifest() to parse reftest.list and get the full list of tests. Then the script just prints out a list of directories containing tests (as well as the manifest files themselves), and I feed that to tar (via xargs). The nice thing about this approach is that we don't have to maintain a separate reftest manifest parsing script. This doesn't quite work on Windows yet, due to path issues (of course).
Comment 48•15 years ago
|
||
what about assuming reftest manifests are called reftest.list and using something like "find -name reftest.list | sed 's@/reftest.list@@'" instead? That's relying on some conventions, but I don't see a good reason people would use another name for manifests (except by error, there's one manifest named reftests.list that should be renamed).
Assignee | ||
Comment 49•15 years ago
|
||
Yeah, that's possible. The nice thing about this patch is that it only has to parse the actual manifests, starting from the main manifest, as opposed to crawling the entire source directory looking for them, which is slow. It's not terribly complicated anyway, since it reuses the reftest parsing code.
Comment 50•15 years ago
|
||
yeah, fair enough. And it could be slower if the objdir is inside the srcdir and gets crawled.
Assignee | ||
Comment 51•15 years ago
|
||
Ok, so this approach isn't going to work. Joel tested my patch and didn't get any tests packaged. I realize now that we can't actually run xpcshell in a cross-compile. Oops. Guess I'll have to duplicate the manifest parser in Python. :-/
Comment 52•15 years ago
|
||
correct. Look at my code that I use to extract and run reftest as this is written in python and might be of some use for your solution. there are links to the code here: https://wiki.mozilla.org/Mobile/Fennec_Reftests
Assignee | ||
Comment 53•15 years ago
|
||
Yeah, I started with that, and then realized that reftest.js already had all this parsing code, and failed to consider the cross-compile case. :-/
Comment 54•15 years ago
|
||
I have made this point before, perhaps in other bugs, but it seems to be assumed that the xpcshell that is used to run tests in a build needs to come from that build. At least, that assumption seems to be implied in the comments in this bug. It would probably be more reliable if one could use an xpcshell that was previously built or downloaded separate from the build that is being tested.
Comment 55•15 years ago
|
||
It makes no sense to use an xpcshell from another build... xpcshell is tied pretty tightly to the JS and mozilla versions, and you shouldn't feel free to mix and match.
Assignee | ||
Comment 56•15 years ago
|
||
Ok, similar in concept to the above, except I wrote a Python reftest manifest parser. It doesn't have to know quite as much as the real parser, since it just has to be able to get the test filenames and process includes. I'm not super happy about having to go that way, but I also don't think it's that bad.
Attachment #364366 -
Attachment is obsolete: true
Attachment #365725 -
Flags: review?(benjamin)
Comment 57•15 years ago
|
||
looks like you attached the wrong patch (it's the same as the previous version).
Assignee | ||
Comment 58•15 years ago
|
||
Comment on attachment 365725 [details] [diff] [review] add packaging bits for reftest, take two Apparently so. Not sure how that happened. Thanks! Waldo pointed out on IRC that this isn't quite sufficient anyway, as the test manifest can specify things like HTTP(..), which means that the reftest httpd will make ../ available, so we need to package that directory as well.
Attachment #365725 -
Attachment is obsolete: true
Attachment #365725 -
Flags: review?(benjamin)
I think the only current user of HTTP(..) is reaching layout/reftests/fonts/ from other subdirectories of layout/reftests/, although we might at some point want to reach it from something not inside layout... in which case this approach wouldn't work too well anymore. Maybe we should just have a directive in the reftest.list for what directories need to be packaged? (Then we could just package layout/reftests/ as a whole, and the other directories as needed, and likewise for crashtests, although that's a tad more involved.)
Assignee | ||
Comment 60•15 years ago
|
||
I'm certainly open to suggestions, and ways we can change the reftest manifest to make this easier, but I'm also aiming for a quick solution at the moment, so I think I'll just handle HTTP(..) by packaging .., and we can follow up with a cleaner solution in another bug.
Assignee | ||
Comment 61•15 years ago
|
||
Ok yeah, this is the right patch, and I've added handling for HTTP(path).
Attachment #365903 -
Flags: review?(benjamin)
Updated•15 years ago
|
Attachment #365903 -
Flags: review?(benjamin) → review+
Comment 62•15 years ago
|
||
Comment on attachment 365903 [details] [diff] [review] add packaging bits for reftest, take three [checked in] >+commentRE = re.compile("\s+#") >+conditionsRE = re.compile("^(fails|random|skip|asserts)") >+httpRE = re.compile("HTTP\((\.\.(\/\.\.)*)\)") These all need to be r''... I don't think commentRE or httpRE do what you want at all at the moment.
Assignee | ||
Comment 63•15 years ago
|
||
Despite my blind copying of regexes from JS, Python legitimately doesn't care in these cases.
Comment 64•15 years ago
|
||
Comment on attachment 359076 [details] [diff] [review] add package target and packaging bits for mochitest [Checkin: Comment 45 & 64] http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e1bdbbc8211c This is need for the diff context of bug 476163...
Attachment #359076 -
Attachment description: add package target and packaging bits for mochitest [checked in] → add package target and packaging bits for mochitest
[Checkin: Comment 45 & 64]
Updated•15 years ago
|
Keywords: fixed1.9.1
Whiteboard: [fixed1.9.1b4]
Target Milestone: --- → mozilla1.9.2a1
Version: unspecified → Trunk
Updated•15 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 65•15 years ago
|
||
I'll probably land it on branch eventually, but it's not a big deal right now. I'll merge the other patch to branch myself.
Comment 66•15 years ago
|
||
One thing that is problematic is the requirement for python2.5. In scratchbox (what we use to cross compile fennec to the maemo platform) there is not support for 2.5 (only 2.3). We can continue to use the scripts I have written for extracting the reftests from the source tree. I would like to see a uniform approach if possible. My few attempts at installing python2.5 into scratchbox were not successful.
Comment 67•15 years ago
|
||
Joel: I was able to install python2.5 on our scratchboxen. https://wiki.mozilla.org/ReferencePlatforms/Linux-CentOS-5.0#Update_scratchbox_mercurial_and_python There's probably a lot in there you don't have to do. Updating the apt sources and fixing scratchbox dns are probably the big ones. Ping me if you still have issues.
Assignee | ||
Comment 68•15 years ago
|
||
Just to clarify, we only require 2.4, mostly for the subprocess module. I have a patch to make that explicit in configure, it was blocked on getting the tinderbox scratchbox Python updated (which Aki did, as mentioned in the previous comment).
Assignee | ||
Comment 69•15 years ago
|
||
(In reply to comment #64) > (From update of attachment 359076 [details] [diff] [review]) > > http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e1bdbbc8211c > > This is need for the diff context of bug 476163... I guess I didn't read well enough. Serge, in the future I'd prefer if you didn't land my patches to branch without asking first. Also, the fixed1.9.1 keyword is misleading here, as this is going to encompass more than one patch.
Keywords: fixed1.9.1
Whiteboard: [fixed1.9.1b4]
Comment 70•15 years ago
|
||
This morning I did a fresh clone of m-c and m-b to build fennec. I verified the makefile is in the source tree with the target package-tests. I do a build and everything comes out just fine, but the problem is when I cd $(xr_objdir);make package-tests, I get a no target found. This was working on Saturday. I don't know if something changed with the makefiles, or if there is a problem with my build.
Comment 71•15 years ago
|
||
my bad, this works just fine if you have --enable-tests in your mozconfig file.
Comment 72•15 years ago
|
||
In running this on maemo, I ran into an issue for the reftests as I did for the mochitests, we are assuming the LD_LIBRARY_PATH is the same as the directory where fennec is. For mochitest, we added the --xre-path to resolve this and I have verified that changing the runreftest.py to use this path works.
Assignee | ||
Comment 73•15 years ago
|
||
Thanks for catching that Joel! I think I considered adding that to runreftests.py but couldn't remember why it was there (aside from being necessary to run xpcshell). I'll update the patch here to include it.
Assignee | ||
Comment 74•15 years ago
|
||
Er, I forgot that I already checked in runreftest.py. Joel, can you try out this patch along with the packaging one and see if it works for you? It adds --xre-path to runreftest.py just like runtests.py.
Assignee | ||
Comment 75•15 years ago
|
||
Comment on attachment 365903 [details] [diff] [review] add packaging bits for reftest, take three [checked in] Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/5ff0bc2194db
Attachment #365903 -
Attachment description: add packaging bits for reftest, take three → add packaging bits for reftest, take three [checked in]
Comment 76•15 years ago
|
||
Comment on attachment 365903 [details] [diff] [review] add packaging bits for reftest, take three [checked in] >diff --git a/testing/testsuite-targets.mk b/testing/testsuite-targets.mk >+stage-reftest: make-stage-dir >+ $(MAKE) -C $(DEPTH)/layout/tools/reftest stage-package >+ > .PHONY: mochitest mochitest-plain mochitest-chrome mochitest-a11y \ > package-tests make-stage-dir stage-mochitest Looks like 'stage-reftest' should be added to '.PHONY'.
Assignee | ||
Updated•15 years ago
|
Attachment #366808 -
Flags: review?(benjamin)
Assignee | ||
Comment 77•15 years ago
|
||
(In reply to comment #76) > Looks like 'stage-reftest' should be added to '.PHONY'. Feel free to add this if you're ever in this file in another patch. If not, I'll try to remember to put it there when I do xpcshell packaging.
Assignee | ||
Comment 78•15 years ago
|
||
I have a patch that packages xpcshell tests. I'm currently failing one of the necko tests when run from the package, but most others seem to run fine. I have realized that once packaged, the test directory has no obvious structure and no manifest, so the harness doesn't know what directories to run. I'm going to have to fix this (I think I'll write out an ad-hoc manifest into _tests/xpcshell during the build process, then teach runxpcshelltests.py how to read it.)
Comment 79•15 years ago
|
||
Is that maybe another vote in favor of making test-file dependencies in xpcshell explicit?
Comment 80•15 years ago
|
||
After thinking about this more, I would like to see a --log-file added to the new harness rewrites. The main reason here is on mobile devices we run the tests in much smaller chunks. This is done because we need to conserve in total memory usage. So we have wrappers that run one test/directory at a time. The advantage of a --log-file is we can output the results of a single test chunk into a known file and not have to redirect the output of the master script. This also saves us from creating a huge file which we might not have the luxury of enough space. The smaller files can be ftp'd off the device if we are limited in space between chunks. this has already proven very useful in the mochitests when running on Fennec.
Assignee | ||
Comment 81•15 years ago
|
||
(In reply to comment #79) > Is that maybe another vote in favor of making test-file dependencies in > xpcshell explicit? Yeah, we need a bug on that if we don't already have one. Like I said, I think I'll do something ad-hoc here just to make it work, then we can hash out a better system. (In reply to comment #80) > After thinking about this more, I would like to see a --log-file added to the > new harness rewrites. Can you file a new bug on this? I can see how it could be useful, but it doesn't need to block this bug particularly.
Assignee | ||
Comment 82•15 years ago
|
||
This works, but I'm failing a bunch of tests. I'm going to split that out into another bug. Could be bugs in tests, just assumptions, or files that the tests wanted that I am failing to package.
Assignee | ||
Comment 83•15 years ago
|
||
Fixed some issues I encountered. I still need to add a way for runxpcshelltests.py to read the ad-hoc manifest it's writing. For my testing I've just been using cat | xargs. I'm currently failing the bits of the test that were added in bug 435687, not sure what's up with that, but I'll worry about that separately.
Attachment #367205 -
Attachment is obsolete: true
Assignee | ||
Comment 84•15 years ago
|
||
With this patch + the patch from bug 482085 I'm running tests like so: 1) do a build, then "make package package-tests", unpack the build + the tests into some other dir 2) In the new dir, copy some files into the app dir: (workaround for bug 483202) cp bin/xpcshell firefox/ cp bin/components/* firefox/components/* cp bin/plugins/* firefox/plugins/* 3) cat xpcshell/tests/all-test-dirs.list | sed "s|^|./xpcshell/tests/|" | xargs python -u xpcshell/runxpcshelltests.py ./firefox/xpcshell
Assignee | ||
Comment 85•15 years ago
|
||
Also, a clobber seems to have fixed the failure I was seeing in comment 83 there, so I'm passing all tests now.
Assignee | ||
Comment 86•15 years ago
|
||
Ok, good enough. I added a --manifest=/path/to/manifest option to runxpcshelltests.py, so you can now run from a test package like: python -u xpcshell/runxpcshelltests.py --manifest=./xpcshell/tests/all-test-dirs.list ./firefox/xpcshell Also I updated the patch to merge a few test changes.
Attachment #367864 -
Attachment is obsolete: true
Attachment #368007 -
Flags: review?(benjamin)
Updated•15 years ago
|
Attachment #366808 -
Flags: review?(benjamin) → review+
Comment 87•15 years ago
|
||
Comment on attachment 368007 [details] [diff] [review] xpcshell packaging bits, rev 3 [checked in] >diff --git a/config/rules.mk b/config/rules.mk > define _INSTALL_TESTS > $(TEST_INSTALLER) $(wildcard $(srcdir)/$(dir)/*) $(testxpcobjdir)/$(MODULE)/$(dir) >+@echo "$(MODULE)/$(dir)" >> $(testxpcobjdir)/all-test-dirs.list > > endef # do not remove the blank line! I'd prefer build-list.pl here, ugly as it may be. >--- a/testing/mochitest/Makefile.in > stage-package: >- $(NSINSTALL) -D $(PKG_STAGE)/mochitest && $(NSINSTALL) -D $(PKG_STAGE)/plugins >+ $(NSINSTALL) -D $(PKG_STAGE)/mochitest && $(NSINSTALL) -D $(PKG_STAGE)/bin/plugins While you're here, cut out the extraneous invocation and just use -D dir1 dir2
Attachment #368007 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 88•15 years ago
|
||
I'm taking bug 460282 and bug 463605 off of the dep list here, and moving them to block bug 383136, which is the RelEng side of this. They don't block the ability to use this code, they just block the ability to use it in in our hourly/nightly builds on tinderbox.
Assignee | ||
Comment 89•15 years ago
|
||
Comment on attachment 366808 [details] [diff] [review] add --xre-path to runreftest.py [checked in] Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/a22f6eddb038
Attachment #366808 -
Attachment description: add --xre-path to runreftest.py → add --xre-path to runreftest.py [checked in]
Assignee | ||
Comment 90•15 years ago
|
||
Comment on attachment 368007 [details] [diff] [review] xpcshell packaging bits, rev 3 [checked in] Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/99930ad81298
Attachment #368007 -
Attachment description: xpcshell packaging bits, rev 3 → xpcshell packaging bits, rev 3 [checked in]
Assignee | ||
Comment 91•15 years ago
|
||
That's a wrap. I'm not going to actually block on bug 483202, since it's possible to work around it, but I'd like to get that in since it would make things easier.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 92•15 years ago
|
||
Comment on attachment 365903 [details] [diff] [review] add packaging bits for reftest, take three [checked in] Pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/fd94b9aed9b4
Assignee | ||
Comment 93•15 years ago
|
||
Comment on attachment 366808 [details] [diff] [review] add --xre-path to runreftest.py [checked in] Pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/917aa449b42e
Assignee | ||
Comment 94•15 years ago
|
||
Comment on attachment 368007 [details] [diff] [review] xpcshell packaging bits, rev 3 [checked in] Pushed to 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/3bd4730954eb
Assignee | ||
Updated•15 years ago
|
Keywords: fixed1.9.1
Updated•6 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•