Recognize crashed tests as failures in sharding_supervisor

Recognize crashed tests as failures in sharding_supervisor

Added some regexes and shard output processing

BUG=92653
TEST=crashed tests are shown in the summary


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

[clee, 2011-08-15 15:35:54]

I've been having my own doubts about copying over parts of gtest_command.py too,
so I decided to just expand on the regexes I've been using all along to find
failed tests instead. There's still some duplication of functionality, but
really not that much, and it's MUCH simpler IMHO.

[M-A Ruel, 2011-08-15 15:46:08]

lgtm with nit

http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/shard...
File tools/sharding_supervisor/sharding_supervisor.py (right):

http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/shard...
tools/sharding_supervisor/sharding_supervisor.py:213: ansi_regex +
"\[\s+RUN\s+\] " + ansi_regex + test_name_regex)
Use r"".

[Paweł Hajdan Jr., 2011-08-15 17:33:17]

http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/shard...
File tools/sharding_supervisor/sharding_supervisor.py (right):

http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/shard...
tools/sharding_supervisor/sharding_supervisor.py:119: results =
self.test_timeout.search(line)
Don't rely on that line to be present. It's fragile.

[clee, 2011-08-15 18:07:09]

Please tell me what you think of Paweł's comment and my response. I'd like to
hear both of your opinions before committing/changing.

http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/shard...
File tools/sharding_supervisor/sharding_supervisor.py (right):

http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/shard...
tools/sharding_supervisor/sharding_supervisor.py:119: results =
self.test_timeout.search(line)
On 2011/08/15 17:33:17, Paweł Hajdan Jr. wrote:
> Don't rely on that line to be present. It's fragile.

It's what gtest_command uses to detect timeouts. There isn't a '[  FAILED  ]'
line for timeouts, so this seems to be the only way to detect one. Or can
timeouts and crashes be grouped together (as 'incomplete' perhaps?), in which
case the current check for 'crashed' would suffice? Based on past experience,
would there be an advantage in having timeouts and crashes be two separate
categories? wdyt?

http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/shard...
tools/sharding_supervisor/sharding_supervisor.py:213: ansi_regex +
"\[\s+RUN\s+\] " + ansi_regex + test_name_regex)
On 2011/08/15 15:46:08, Marc-Antoine Ruel wrote:
> Use r"".

Done.

[charleslee, 2011-08-16 13:32:11]

PING

wdyt? Is it worth putting timeout and crashed tests in the same category to
get rid of the fragile check?

Charles


On Mon, Aug 15, 2011 at 2:07 PM, <charleslee@chromium.org> wrote:

> Please tell me what you think of Paweł's comment and my response. I'd like
> to
> hear both of your opinions before committing/changing.
>
>
>
>
> http://codereview.chromium.**org/7650012/diff/1/tools/**
>
sharding_supervisor/sharding_**supervisor.py<http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/sharding_supervisor.py>
> File tools/sharding_supervisor/**sharding_supervisor.py (right):
>
> http://codereview.chromium.**org/7650012/diff/1/tools/**
>
sharding_supervisor/sharding_**supervisor.py#newcode119<http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/sharding_supervisor.py#newcode119>
> tools/sharding_supervisor/**sharding_supervisor.py:119: results =
> self.test_timeout.search(line)
> On 2011/08/15 17:33:17, Paweł Hajdan Jr. wrote:
>
>> Don't rely on that line to be present. It's fragile.
>>
>
> It's what gtest_command uses to detect timeouts. There isn't a '[
> FAILED  ]' line for timeouts, so this seems to be the only way to detect
> one. Or can timeouts and crashes be grouped together (as 'incomplete'
> perhaps?), in which case the current check for 'crashed' would suffice?
> Based on past experience, would there be an advantage in having timeouts
> and crashes be two separate categories? wdyt?
>
>
> http://codereview.chromium.**org/7650012/diff/1/tools/**
>
sharding_supervisor/sharding_**supervisor.py#newcode213<http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/sharding_supervisor.py#newcode213>
> tools/sharding_supervisor/**sharding_supervisor.py:213: ansi_regex +
> "\[\s+RUN\s+\] " + ansi_regex + test_name_regex)
> On 2011/08/15 15:46:08, Marc-Antoine Ruel wrote:
>
>> Use r"".
>>
>
> Done.
>
>
>
http://codereview.chromium.**org/7650012/<http://codereview.chromium.org/7650...
>

[M-A Ruel, 2011-08-16 13:38:56]

Hereby I declare to have no opinion,

Undersigned,

M-A

[Nicolas Sylvain, 2011-08-16 16:59:04]

On Tue, Aug 16, 2011 at 6:38 AM, <maruel@chromium.org> wrote:

> Hereby I declare to have no opinion,
>

+1. Do what you think is right.

When I look at a log, it's usually hard to tell the difference between a
timeout and a crash, so I usually just put them both in the same mental
bucket.

Nicolas


>
> Undersigned,
>
> M-A
>
>
>
http://codereview.chromium.**org/7650012/<http://codereview.chromium.org/7650...
>

[Paweł Hajdan Jr., 2011-08-16 17:36:07]

http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/shard...
File tools/sharding_supervisor/sharding_supervisor.py (right):

http://codereview.chromium.org/7650012/diff/1/tools/sharding_supervisor/shard...
tools/sharding_supervisor/sharding_supervisor.py:119: results =
self.test_timeout.search(line)
On 2011/08/15 18:07:09, clee wrote:
> On 2011/08/15 17:33:17, Paweł Hajdan Jr. wrote:
> > Don't rely on that line to be present. It's fragile.
> 
> It's what gtest_command uses to detect timeouts.

And guess what? It's broken.

> There isn't a '[  FAILED  ]'
> line for timeouts, so this seems to be the only way to detect one. Or can
> timeouts and crashes be grouped together (as 'incomplete' perhaps?), in which
> case the current check for 'crashed' would suffice? Based on past experience,
> would there be an advantage in having timeouts and crashes be two separate
> categories? wdyt? 

It's not really useful to trying to distinguish between crashes and hangs IMO.

I think that a more reliable solution would be to detect that you see one test
starting, but you never see it finishing: you either encounter EOF or another
test starting. That is a pretty good sign something bad has happened.

[commit-bot: I haz the power, 2011-08-16 19:04:29]

Change committed as 96999