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)

defect

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?
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
This came up from discussing why we don't do unittests and perf tests for releases. 
What?  We're not testing builds we're shipping to users? (Comment #1).  Please
explain.  :-/
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.
OS: Windows Vista → All
Hardware: PC → All
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
Crashtests use the same code as reftests, so if reftests work, crashtests should work.
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.
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+
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?
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.
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
Assignee: nobody → rcampbell
Status: ASSIGNED → NEW
Depends on: 422398
Status: NEW → ASSIGNED
Depends on: 417633
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
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?)
Blocks: 457753
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.
Depends on: 460282
#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)?
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.
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.
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.
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
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.
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.
Component: Release Engineering: Future → Build Config
Product: mozilla.org → Core
QA Contact: release → build-config
Version: other → unspecified
Attached patch rough draft (obsolete) — Splinter Review
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.
Depends on: 460515
No longer depends on: 422398
Attached patch rough draft, take two (obsolete) — Splinter Review
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
Attached patch rough draft, take two (obsolete) — Splinter Review
Oops, that was completely the wrong patch.
Attachment #354328 - Attachment is obsolete: true
(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?
No longer blocks: 462889, 463262
(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.
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.
(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!
Attached patch fix nsinstall usage (obsolete) — Splinter Review
Oops, replace nsinstall with $(NSINSTALL).
Attachment #354411 - Attachment is obsolete: true
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.
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
Depends on: 463605
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.
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.
Depends on: 475383
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)
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 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+
(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".
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?
Depends on: 374458
(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?
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.
(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.
Let's do it for non-Mac systems first, then, while bug 463605 is being fixed.
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]
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!
Attached patch add packaging bits for reftest (obsolete) — Splinter Review
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).
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).
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.
yeah, fair enough. And it could be slower if the objdir is inside the srcdir and gets crawled.
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. :-/
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
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. :-/
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.
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.
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)
looks like you attached the wrong patch (it's the same as the previous version).
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.)
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.
Ok yeah, this is the right patch, and I've added handling for HTTP(path).
Attachment #365903 - Flags: review?(benjamin)
Attachment #365903 - Flags: review?(benjamin) → review+
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.
Despite my blind copying of regexes from JS, Python legitimately doesn't care in these cases.
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]
Keywords: fixed1.9.1
Whiteboard: [fixed1.9.1b4]
Target Milestone: --- → mozilla1.9.2a1
Version: unspecified → Trunk
Status: NEW → ASSIGNED
Depends on: 482084
Depends on: 482085
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.
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.
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.
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).
(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]
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.
my bad, this works just fine if you have --enable-tests in your mozconfig file.
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.
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.
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.
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 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'.
Attachment #366808 - Flags: review?(benjamin)
(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.
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.)
Is that maybe another vote in favor of making test-file dependencies in xpcshell explicit?
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.
(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.
Attached patch xpcshell packaging bits (obsolete) — Splinter Review
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.
Attached patch xpcshell packaging bits, rev 2 (obsolete) — Splinter Review
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
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
Also, a clobber seems to have fixed the failure I was seeing in comment 83 there, so I'm passing all tests now.
Depends on: 483202
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)
Attachment #366808 - Flags: review?(benjamin) → review+
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+
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.
No longer depends on: 460282, 463605
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]
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]
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
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
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
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
Keywords: fixed1.9.1
Depends on: 484747
Depends on: 394875
No longer depends on: 394875
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.