Bug 56760 - new-run-webkit-tests: run tests in ascending alphabetical order per dir
Summary: new-run-webkit-tests: run tests in ascending alphabetical order per dir
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on: 56801
Blocks: 56047
  Show dependency treegraph
 
Reported: 2011-03-21 11:38 PDT by Dirk Pranke
Modified: 2011-03-23 15:35 PDT (History)
7 users (show)

See Also:


Attachments
Patch (4.19 KB, patch)
2011-03-21 11:56 PDT, Dirk Pranke
no flags Details | Formatted Diff | Diff
Patch (6.09 KB, patch)
2011-03-23 14:58 PDT, Dirk Pranke
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Pranke 2011-03-21 11:38:34 PDT
new-run-webkit-tests: run tests in ascending alphabetical order per dir
Comment 1 Dirk Pranke 2011-03-21 11:56:41 PDT
Created attachment 86345 [details]
Patch
Comment 2 Dirk Pranke 2011-03-21 12:10:13 PDT
Committed r81597: <http://trac.webkit.org/changeset/81597>
Comment 3 Mihai Parparita 2011-03-21 22:21:55 PDT
Pavel is rolling this out because on the Chromium bots the http/ tests are being run backwards.  From http://build.chromium.org/p/chromium.webkit/builders/Webkit%20Linux/builds/4600/steps/webkit_tests/logs/stdio:

worker-0 http/tests/loading/preload-img-test.html passed
worker-0 http/tests/loading/onload-vs-immediate-refresh.pl
worker-0 http/tests/loading/location-hash-reload-cycle.html
worker-0 http/tests/loading/load-javascript-after-many-xhrs.html passed
Comment 4 Dirk Pranke 2011-03-21 22:29:13 PDT
Fascinating. I wonder how that is possible.

If that is the case, it probably would've been clearer to  say "tests are being run backwards" in the other bug rather than "tests are unexpectedly flaky".
Comment 5 Dirk Pranke 2011-03-22 19:57:53 PDT
Reopening, because it was rolled out in http://trac.webkit.org/changeset/81675.

So, this code has been broken for quite a while, at least since http://trac.webkit.org/changeset/68903 and http://trac.webkit.org/changeset/69074.

There are four paths through the "sharding" code, depending on whether we're running through the single-threaded path (which triggers if you're running with --worker-model=inline or --child-processes=1, or, less commonly, if you're running with --experimental-fully-parallel), or through the multi-threaded path.

If you run through the single-threaded path, then test are either put into the http_lock bucket (in order) or to their own bucket. Then the tests in the http_lock bucket are reversed. The net result is that http tests are in a single batch backwards but all the other tests are individual batches of one file each.

The --worker-model={old-threads,old-inline} would then get the list of reversed tests and *pop* them one at a time and run them, hence effectively reversing the http tests again and having no effect on the other batches. The net result would be that everything would run in order.

In the multithreaded case, we would use real batches for the non-http tests, but we would reverse each batch, so that again, popping them would work and they'd be run in order.

The new worker model code, however, doesn't pop items from the batches, it iterates over them in order. The net result is that in the single-threaded case, we run the http tests in reverse and the other tests in order, and in the multi-threaded case, we run everything in reverse per directory (which is what the bots are currently doing).

One solution could be to change the sharding logic to leave the reverses in for the old-* worker models, and take the reverses out for the new worker models. 

Another would be to take the reverses out, and change the old-* implementations to pop from the front of the batch rather than pop from the end. 

A third would be to change the new-* implementations to pop the tests rather than iterate over them.

Since my long term plan is to delete the old implementations, I suggest that we take the reverses out and change the old-* implementation to pop from the front of the batch.

Anyone disagree?

Note that the real implication of all of this is that currently Chromium is running the tests backwards by default, and the test_expectations are set to deal with that. When we "fix" this to run them forwards, we will get a bunch of diffs and have to adjust for them. This will probably require some coordinating with the gardeners.
Comment 6 Ojan Vafai 2011-03-22 20:16:16 PDT
(In reply to comment #5)
> Since my long term plan is to delete the old implementations, I suggest that we take the reverses out and change the old-* implementation to pop from the front of the batch.
> 
> Anyone disagree?

Sounds good to me.

> Note that the real implication of all of this is that currently Chromium is running the tests backwards by default, and the test_expectations are set to deal with that. When we "fix" this to run them forwards, we will get a bunch of diffs and have to adjust for them. This will probably require some coordinating with the gardeners.

Sigh. This is just a pain we'll have to deal with. I don't understand though. Looking at the bot stdio, they seem to be running them in alphabetical order per worker.
Comment 7 Dirk Pranke 2011-03-22 20:30:02 PDT
(In reply to comment #6)
> Sigh. This is just a pain we'll have to deal with. I don't understand though. Looking at the bot stdio, they seem to be running them in alphabetical order per worker.

Which bot?

Mac 10.5 is running them in the correct order because it is single threaded (we hard-code to --worker-model=old-threads and --child-processes=1 because we don't have multiprocessing module on 10.5)

Mac 10.6 is running them in reverse order, because it is running --worker-model=processes and multiple children.

The Win bots are still running old-threads and hence running them in order, because the last time I tried to switch them over to processes we had a bunch of issues. I need to fix a bug or two and flip the switch again this week.

The Linux Hardy bots are still running with --old-threads, presumably because Hardy also only has Python 2.5; the 64-bit bots are running with processes and hence, in reverse order. 

Fun, huh?
Comment 8 Ojan Vafai 2011-03-22 21:12:58 PDT
I see. I had looked at a Windows bot.
Comment 9 Dirk Pranke 2011-03-23 14:58:47 PDT
Created attachment 86697 [details]
Patch
Comment 10 Dirk Pranke 2011-03-23 15:35:08 PDT
Comment on attachment 86697 [details]
Patch

Clearing flags on attachment: 86697

Committed r81815: <http://trac.webkit.org/changeset/81815>
Comment 11 Dirk Pranke 2011-03-23 15:35:12 PDT
All reviewed patches have been landed.  Closing bug.