Bug 37387 - run_webkit_tests.py shouldn't have platform-specific logic
Summary: run_webkit_tests.py shouldn't have platform-specific logic
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: Nobody
URL:
Keywords:
Depends on:
Blocks: 37388
  Show dependency treegraph
 
Reported: 2010-04-10 08:08 PDT by Adam Barth
Modified: 2010-04-10 15:17 PDT (History)
4 users (show)

See Also:


Attachments
Patch (6.16 KB, patch)
2010-04-10 08:10 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (6.25 KB, patch)
2010-04-10 08:13 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-04-10 08:08:45 PDT
run_webkit_tests.py shouldn't have platform-specific logic
Comment 1 Adam Barth 2010-04-10 08:10:54 PDT
Created attachment 53040 [details]
Patch
Comment 2 Adam Barth 2010-04-10 08:13:50 PDT
Created attachment 53041 [details]
Patch
Comment 3 Dirk Pranke 2010-04-10 11:11:25 PDT
Comment on attachment 53041 [details]
Patch

Looks basically good to me, although I'm really not that familiar with what the Executive package does; I'll have to look into that more now that this code is rather dependent on it.
Comment 4 Adam Barth 2010-04-10 12:43:14 PDT
Thanks.  Executive.cpu_count shouldn't exist.  It only exits because the multiprocessing package is in Python 2.6 and later.  We shouldn't have a bunch of different pieces of code that work around the lack of the multiprocessing.  In principle, we could just implement our own version of multiprocessing, like we do for os.relpath, but we decide to put it on executive so it could be non-static (which is better for testing).

The main point of this patch is that it makes sense for a WebKit port to recommend a default number of DRT instances.  It doesn't make a lot of sense for a WebKit port to tell you how many CPUs your machine has.  Some lower-level piece of code should do that.  The webkitpy.common.system package houses all the non-WebKit specific code for interacting with the operating system (that isn't supplied by the standard library), so the CPU counting code should live there.  Now, what to do with the CPU count, that's a question for the layout_test package.
Comment 5 Eric Seidel (no email) 2010-04-10 14:12:17 PDT
Comment on attachment 53041 [details]
Patch

I wish executive had more testing.

OK.

We need to kill the 4-limit hack.  I have never reproduced the errors Dirk saw, but I would like to... so we can fix them.
Comment 6 Adam Barth 2010-04-10 15:17:37 PDT
Comment on attachment 53041 [details]
Patch

Clearing flags on attachment: 53041

Committed r57422: <http://trac.webkit.org/changeset/57422>
Comment 7 Adam Barth 2010-04-10 15:17:50 PDT
All reviewed patches have been landed.  Closing bug.