Improve waterfall.sh - fetch only reports and failed test names rather then t...

Improve waterfall.sh - fetch only reports and failed test names rather then the whole stdio
TEST=`./tools/valgrind/waterfall.sh fetch` then `./tools/valgrind/waterfall.sh match`

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71653

[Timur Iskhodzhanov, 2010-12-29 16:07:25]

Hi Alexander, Jochen

Can you please review this patch?
This speeds up waterfall.sh quite a lot for me, especially the "match" step.
Also, it should decrease the load on the master when doing "fetch".

Thank you,
Timur Iskhodzhanov

[jochen (gone - plz use gerrit), 2011-01-05 12:25:37]

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh
File tools/valgrind/waterfall.sh (right):

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
tools/valgrind/waterfall.sh:82: rm $REPORT_FILE 2>/dev/null || true  # make sure
it doesn't exist
use rm -f instead 2>/dev/null || true

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
tools/valgrind/waterfall.sh:85: "$TMPFILE" || true)  # `true` is to succeed on
empty output
why does this need to succeed?

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
tools/valgrind/waterfall.sh:98: cat -v "$TMPFILE" | sed "s/\^M$//" >>
"$REPORT_FILE"
if you want to convert CRLF to unix, you should pipe through tr -d '\r'

[Timur Iskhodzhanov, 2011-01-17 15:28:26]

I've updated the changelist since you've fixed issue 68233

Please take another look

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh
File tools/valgrind/waterfall.sh (right):

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
tools/valgrind/waterfall.sh:82: rm $REPORT_FILE 2>/dev/null || true  # make sure
it doesn't exist
On 2011/01/05 12:25:37, jochen wrote:
> use rm -f instead 2>/dev/null || true

Done.

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
tools/valgrind/waterfall.sh:85: "$TMPFILE" || true)  # `true` is to succeed on
empty output
There's "set -e" and grep returns failure if no lines has matched the regexp

On 2011/01/05 12:25:37, jochen wrote:
> why does this need to succeed?

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
tools/valgrind/waterfall.sh:98: cat -v "$TMPFILE" | sed "s/\^M$//" >>
"$REPORT_FILE"
On 2011/01/05 12:25:37, jochen wrote:
> if you want to convert CRLF to unix, you should pipe through tr -d '\r'

Done.

[Alexander Potapenko, 2011-01-17 15:30:46]

Looks good to me

On Mon, Jan 17, 2011 at 6:28 PM,  <timurrrr@chromium.org> wrote:
> I've updated the changelist since you've fixed issue 68233
>
> Please take another look
>
>
> http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh
> File tools/valgrind/waterfall.sh (right):
>
>
http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
> tools/valgrind/waterfall.sh:82: rm $REPORT_FILE 2>/dev/null || true  #
> make sure it doesn't exist
> On 2011/01/05 12:25:37, jochen wrote:
>>
>> use rm -f instead 2>/dev/null || true
>
> Done.
>
>
http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
> tools/valgrind/waterfall.sh:85: "$TMPFILE" || true)  # `true` is to
> succeed on empty output
> There's "set -e" and grep returns failure if no lines has matched the
> regexp
>
> On 2011/01/05 12:25:37, jochen wrote:
>>
>> why does this need to succeed?
>
>
http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
> tools/valgrind/waterfall.sh:98: cat -v "$TMPFILE" | sed "s/\^M$//" >>
> "$REPORT_FILE"
> On 2011/01/05 12:25:37, jochen wrote:
>>
>> if you want to convert CRLF to unix, you should pipe through tr -d
>
> '\r'
>
> Done.
>
> http://codereview.chromium.org/6057006/
>



-- 
Alexander Potapenko
Software Engineer
Google Moscow

[jochen (gone - plz use gerrit), 2011-01-17 19:54:00]

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh
File tools/valgrind/waterfall.sh (right):

http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
tools/valgrind/waterfall.sh:85: "$TMPFILE" || true)  # `true` is to succeed on
empty output
On 2011/01/17 15:28:26, Timur Iskhodzhanov wrote:
> There's "set -e" and grep returns failure if no lines has matched the regexp
> 
> On 2011/01/05 12:25:37, jochen wrote:
> > why does this need to succeed?
> 

But you don't use the return value, so it doesn't matter whether it succeeds or
not?

[Timur Iskhodzhanov, 2011-01-17 19:56:21]

I don't.
But bash exits if the grep return value is nonzero (set -e) 

On 2011/01/17 19:54:00, jochen wrote:
> http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh
> File tools/valgrind/waterfall.sh (right):
> 
>
http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
> tools/valgrind/waterfall.sh:85: "$TMPFILE" || true)  # `true` is to succeed on
> empty output
> On 2011/01/17 15:28:26, Timur Iskhodzhanov wrote:
> > There's "set -e" and grep returns failure if no lines has matched the regexp
> > 
> > On 2011/01/05 12:25:37, jochen wrote:
> > > why does this need to succeed?
> > 
> 
> But you don't use the return value, so it doesn't matter whether it succeeds
or
> not?

[jochen (gone - plz use gerrit), 2011-01-17 19:59:48]

On 2011/01/17 19:56:21, Timur Iskhodzhanov wrote:
> I don't.
> But bash exits if the grep return value is nonzero (set -e) 

gnah, I should have read your answer more carefully.

lgtm then


> 
> On 2011/01/17 19:54:00, jochen wrote:
> > http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh
> > File tools/valgrind/waterfall.sh (right):
> > 
> >
>
http://codereview.chromium.org/6057006/diff/3001/tools/valgrind/waterfall.sh#...
> > tools/valgrind/waterfall.sh:85: "$TMPFILE" || true)  # `true` is to succeed
on
> > empty output
> > On 2011/01/17 15:28:26, Timur Iskhodzhanov wrote:
> > > There's "set -e" and grep returns failure if no lines has matched the
regexp
> > > 
> > > On 2011/01/05 12:25:37, jochen wrote:
> > > > why does this need to succeed?
> > > 
> > 
> > But you don't use the return value, so it doesn't matter whether it succeeds
> or
> > not?