Bug 34739

Summary: update run-chromium-webkit-tests to work w/ chromium-linux, chromium-win
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: Tools / TestsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, commit-queue, eric, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch none

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?