Bug 34739 - update run-chromium-webkit-tests to work w/ chromium-linux, chromium-win
Summary: update run-chromium-webkit-tests to work w/ chromium-linux, chromium-win
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Dirk Pranke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-08 19:12 PST by Dirk Pranke
Modified: 2010-02-10 17:56 PST (History)
4 users (show)

See Also:


Attachments
Patch (14.76 KB, patch)
2010-02-08 19:18 PST, Dirk Pranke
no flags Details | Formatted Diff | Diff
Patch (15.08 KB, patch)
2010-02-09 12:44 PST, 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 2010-02-08 19:12:34 PST
There are various bugs preventing run-chromiu-webkit-tests from actually working on either the Linux or Win ports of chromium.
Comment 1 Dirk Pranke 2010-02-08 19:18:13 PST
Created attachment 48384 [details]
Patch
Comment 2 Adam Barth 2010-02-09 12:28:19 PST
Comment on attachment 48384 [details]
Patch

+ port_to_use

There are some whitespace errors around the above.

+ Working around a race in Python 2.4's implementation of Popen().

Huh?  This doesn't look right.  We either need a more detailed comment or to call the API in another way that avoids the race.  This just looks like it will be buggy.  Maybe we need to call wait() at some point?  We do something similar in webkit-patch somewhere.

The rest of the patch LGTM.  Maybe land the rest and re-visit the Popen issue later?
Comment 3 Dirk Pranke 2010-02-09 12:44:15 PST
Created attachment 48438 [details]
Patch
Comment 4 Dirk Pranke 2010-02-09 12:46:52 PST
(In reply to comment #2)
> (From update of attachment 48384 [details])
> + port_to_use
> 
> There are some whitespace errors around the above.
>

Fixed.
 
> + Working around a race in Python 2.4's implementation of Popen().
> 
> Huh?  This doesn't look right.  We either need a more detailed comment or to
> call the API in another way that avoids the race.  This just looks like it will
> be buggy.  Maybe we need to call wait() at some point?  We do something similar
> in webkit-patch somewhere.
>

I added a comment with a pointer to the python bug ( http://bugs.python.org/issue1199282 ). Note that the bug is fixed in Python 2.5.

You can't work around it, and the windows port won't work reliably without this. I've needed to use this workaround several times in the chromium.org codebase .
 
> The rest of the patch LGTM.  Maybe land the rest and re-visit the Popen issue
> later?
Comment 5 Eric Seidel (no email) 2010-02-10 13:35:55 PST
Comment on attachment 48438 [details]
Patch

Clearly we need unit testing.  Most of these could have been caught by unit tests.

OK though.
Comment 6 WebKit Commit Bot 2010-02-10 17:50:51 PST
Comment on attachment 48438 [details]
Patch

Clearing flags on attachment: 48438

Committed r54635: <http://trac.webkit.org/changeset/54635>
Comment 7 WebKit Commit Bot 2010-02-10 17:51:02 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Adam Barth 2010-02-10 17:56:18 PST
> Clearly we need unit testing.  Most of these could have been caught by unit
> tests.

Tests for the tests!  Where does the madness end?