Closed Bug 471676 Opened 11 years ago Closed 10 years ago

reftest on maemo require smaller chunks to run successfully

Categories

(Testing :: Reftest, defect)

ARM
Maemo
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: jmaher)

References

Details

Attachments

(1 file, 2 obsolete files)

currently with the maemo platform we cannot run all ~2500 reftests at once.  The work around is to parse the reftest.list and run each include file as a single instance.  This works very well (compared to the same approach for mochitest) and reliable.

There are a few exceptions.  The big one being layout/reftests/bugs/reftest.list where there are over 930 tests that are run.  In doing 3 consecutive runs overnight (keep in mind this is 4+ hours/run on maemo) the bugs directory would not run a couple hundred at the end, oddly enough always after this test:
!= data:application/xml,<foo/> data:text/plain, #bug 404419

It turns out that splitting the bugs/reftest.list into smaller .list files (100 tests each) allows me to run all the tests with no problem.

Another test that is problematic is the libpr0n tests.  These run just fine in firefox and desktop fennec, but not on maemo.  There are 166 tests here and sometimes they all run and other times they run 124 (oddly always the same).  Running this in smaller chunks always works.  

As a workaround, I am using a reftest_chunked.py script (as found at the bottom of this page: https://wiki.mozilla.org/Mobile/Fennec_Reftests) to run them one reftest.list file at a time.  I will continue to tweak this script to make smaller chunks out of problematic directories.

For a more permanent solution, one idea that ted mentioned in IRC is:
[10:25am] ted: like -reftest reftest.list -reftest-test-start 20 -reftest test-count 50
[10:25am] ted: to run 50 tests, starting at #20
[10:25am] ted: iirc, the reftest code reads all the manifests in in one shot, then runs the tests, so it would be pretty easy to fix


There might be a better way to approach this and I am all ears.
my workaround is to have maemkit create a set of manifest files that have 100 tests each for a given list of problematic directories.  This method had been working well:
http://hg.mozilla.org/qa/maemkit/file/527dc910d556/refdriver.py#71

If we decide that this technique or another technique should be rolled into the main test harness, we can remove this from maemkit.
My comments in comment 0 could be implemented fairly easily in combination with runreftest.py. I imagine something like runreftest.py --chunk-by=50 ... would instruct the harness to run tests batches of 50 at a time. It could break that down by running reftest like firefox -reftest reftest.list -reftest-test-start N -reftest-test-count 50, and then reftest would read all the manifests (as usual), but only execute tests N to N+50. We'd need to make reftest exit with a different return code when it runs out of tests, so the harness knows when to quit.
ted, I like this idea.  Would there be any issues running between manifests?  Right now the original reftest.list is a collection of "include xyz.list" statements.

I would like to be able to create log files for the smaller chunks.  Since I will be running this on multiple devices to speed up the total time, it would be nice to have this type of functionality:

> python runreftest.py --print-number-tests <path>/reftest.list
2138 tests found   <- use this data to calculate how many tests to run
> python runreftest.py --chunk-by=50 --max-tests=1069 --start-test=0 <path>/reftest.list > reftest_1.log
> python runreftest.py --chunk-by=50 --max-tests=1069 --start-test=1069 <path>/reftest.list > reftest_2.log

It sounds like we could have this working without much trouble.
Reftest parses the full manifest including all includes on startup, and then has an array in memory of all the tests. I forgot that you wanted to split it up by test, but yeah, we could probably do what you're proposing.
Blocks: 475778
We should just implement the same commandline params in the reftest harness that bug 494165 implemented for Mochitest.
I have added the ability to run reftests the same way as mochitests in smaller chunks via the --total-chunks and --this-chunk method.  I do this by setting a pref in the profile (user.js), and read that pref inside the reftest.js extension.

This is needed for testing on devices that have limited memory.
Assignee: nobody → jmaher
Attachment #432851 - Flags: review?(ted.mielczarek)
Comment on attachment 432851 [details] [diff] [review]
--total-chunks, --this-chunk added to reftest harness

:dbaron, can you review this as well.  This touches reftest.js
Attachment #432851 - Flags: review?(dbaron)
Comment on attachment 432851 [details] [diff] [review]
--total-chunks, --this-chunk added to reftest harness

I'd like to see the info line also list (a) the chunk number ("chunk " + gThisChunk + " out of " + gTotalChunks + " chunks") and (b) indices of the whole tests (i.e., "0-" + gURLs.length)

It would probably also be good to document in the add_option calls that "which chunk to run" is in the range [1 .. totalChunks]

r=dbaron with that
Attachment #432851 - Flags: review?(dbaron) → review+
updated with dbaron's review comments.  Passed on try server.  Ted, if you could take a quick look at this I would appreciate it.
Attachment #432851 - Attachment is obsolete: true
Attachment #433052 - Flags: review?(ted.mielczarek)
Attachment #432851 - Flags: review?(ted.mielczarek)
Blocks: 554026
ted: can I get your review on this patch before checking it in?
Comment on attachment 433052 [details] [diff] [review]
--total-chunks, --this-chunk added to reftest harness (2)

This looks good, except I think you should also have a condition like runtests.py has:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py.in#232

to error if someone specifies --total-chunks without --this-chunk.
Attachment #433052 - Flags: review?(ted.mielczarek) → review+
adding cli checking for total-chunks and this-chunk as per Ted's feedback.  Tested locally and ready to checkin.
Attachment #433052 - Attachment is obsolete: true
Attachment #435642 - Flags: review+
Checked in as 70698587d13f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.