Find out the Webkit layout_tests names by reading their stdout

Find out the Webkit layout_tests names by reading their stdout
TEST=`./tools/valgrind/chrome_tests.sh --test webkit` with suppressions removed
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=109805

[Timur Iskhodzhanov, 2011-11-11 14:46:52]

Hi Lei,

Can you please carefully review this?

Thanks,
Timur

http://codereview.chromium.org/8537013/diff/1/tools/valgrind/valgrind_test.py
File tools/valgrind/valgrind_test.py (left):

http://codereview.chromium.org/8537013/diff/1/tools/valgrind/valgrind_test.py...
tools/valgrind/valgrind_test.py:407: # separate log files for different UI tests
at the analyze stage.
the comment was in the wrong place.

http://codereview.chromium.org/8537013/diff/1/tools/valgrind/valgrind_test.py
File tools/valgrind/valgrind_test.py (right):

http://codereview.chromium.org/8537013/diff/1/tools/valgrind/valgrind_test.py...
tools/valgrind/valgrind_test.py:419: # Webkit layout_tests print out the test
URL as the first line of stdout.
Do you know if this assumption holds?

http://codereview.chromium.org/8537013/diff/1/tools/valgrind/valgrind_test.py...
tools/valgrind/valgrind_test.py:421: '    EXITCODE=$PIPESTATUS\n'  # $? holds
the tee's exit code
please pay special attention to the PIPE status thing, I'm not 100% sure here.

[Timur Iskhodzhanov, 2011-11-11 15:22:24]

http://codereview.chromium.org/8537013/diff/5002/tools/valgrind/valgrind_test.py
File tools/valgrind/valgrind_test.py (right):

http://codereview.chromium.org/8537013/diff/5002/tools/valgrind/valgrind_test...
tools/valgrind/valgrind_test.py:420: f.write('    %s "$@" | tee
$DIR/test.$$.stdout\n'
pardon: accidentally removed the initial patchset.

Pay special attention to this part of the code

[Lei Zhang, 2011-11-12 00:03:37]

LGTM

http://codereview.chromium.org/8537013/diff/5002/tools/valgrind/valgrind_test.py
File tools/valgrind/valgrind_test.py (right):

http://codereview.chromium.org/8537013/diff/5002/tools/valgrind/valgrind_test...
tools/valgrind/valgrind_test.py:409: 'for arg in $@; do\n'
FYI, this doesn't handle arguments with spaces, but maybe we don't care since we
are looking for '--test-name=NoSpaces.Here'

http://codereview.chromium.org/8537013/diff/5002/tools/valgrind/valgrind_test...
tools/valgrind/valgrind_test.py:416: f.write('if [ "$FOUND_TESTNAME" == "1" ];
then\n'
nit: == is valid in bash, but can you use = so people used to portable shell
won't get alarmed?

[Timur Iskhodzhanov, 2011-11-12 19:42:11]

Thanks for the review!

http://codereview.chromium.org/8537013/diff/5002/tools/valgrind/valgrind_test.py
File tools/valgrind/valgrind_test.py (right):

http://codereview.chromium.org/8537013/diff/5002/tools/valgrind/valgrind_test...
tools/valgrind/valgrind_test.py:409: 'for arg in $@; do\n'
True; added a TODO just in case

On 2011/11/12 00:03:37, Lei Zhang wrote:
> FYI, this doesn't handle arguments with spaces, but maybe we don't care since
we
> are looking for '--test-name=NoSpaces.Here'

http://codereview.chromium.org/8537013/diff/5002/tools/valgrind/valgrind_test...
tools/valgrind/valgrind_test.py:416: f.write('if [ "$FOUND_TESTNAME" == "1" ];
then\n'
Ok, the C++ part of me is too defensive :)

On 2011/11/12 00:03:37, Lei Zhang wrote:
> nit: == is valid in bash, but can you use = so people used to portable shell
> won't get alarmed?

http://codereview.chromium.org/8537013/diff/9002/tools/valgrind/valgrind_test.py
File tools/valgrind/valgrind_test.py (right):

http://codereview.chromium.org/8537013/diff/9002/tools/valgrind/valgrind_test...
tools/valgrind/valgrind_test.py:424: '      grep URL |\n'
also: extra check won't hurt.

[Lei Zhang, 2011-11-14 22:57:16]

Still LGTM