Bug 34739 – update run-chromium-webkit-tests to work w/ chromium-linux, chromium-win

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Dirk Pranke]

There are various bugs preventing run-chromiu-webkit-tests from actually working on either the Linux or Win ports of chromium.

[Dirk Pranke]

Created attachment 48384 [details]
Patch

[Adam Barth]

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?

[Dirk Pranke]

Created attachment 48438 [details]
Patch

[Dirk Pranke]

(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?

[Eric Seidel (no email)]

Comment on attachment 48438 [details]
Patch

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

OK though.

[WebKit Commit Bot]

Comment on attachment 48438 [details]
Patch

Clearing flags on attachment: 48438

Committed r54635: <http://trac.webkit.org/changeset/54635>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

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

Tests for the tests!  Where does the madness end?