Increase Android test robustness.

Increase Android test robustness.

Use of the emulator is probably a lost cause, but maybe this can hold
us for now.

BUG=None
TEST=

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

[John Grabowski, 2012-01-12 05:51:24]

Green bot or bust.

[Peter Beverloo, 2012-01-12 11:19:18]

I'm not really enthusiastically about the "fast_and_loose" name, maybe
"lossy_run" or "quick_run"? Furthermore, since the code explicitly mentions not
to use this on bots, how is this going to get the bot green?

http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py
File build/android/emulator.py (right):

http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:99: fast_and_loose: go fast and loose.  Intended for
quick testing.
Instead of "go fast and loose", could you copy the description written in
run_tests.py for additional clarity?

http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py#newco...
build/android/emulator.py:140: # ALWAYS wipe the data.  We've seen cases where
an emulator
This "ALWAYS" is not accurate anymore now that it's depending on the
fast_and_loose mode.

http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py
File build/android/run_tests.py (right):

http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newc...
build/android/run_tests.py:136: fast_and_loose=False):
This would fit on line 135.

http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newc...
build/android/run_tests.py:256: 
nit: newline (since you fixed it elsewhere)

http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newc...
build/android/run_tests.py:275: option_parser.add_option('-f', '--gtest_filter',
dest='gtest_filter',
This seems unrelated.

[John Grabowski, 2012-01-12 18:27:09]

> I'm not really enthusiastically about the "fast_and_loose" name, maybe
> "lossy_run" or "quick_run"? Furthermore, since the code explicitly
> mentions not
> to use this on bots, how is this going to get the bot green?
>

It's not really "lossy".  I picked the name because a --fast option seems
too tempting to use always; "loose" as in "loosening up the rules".

Happy to choose a new name if you want something specific, but I don't want
to call it --quick or --fast.

This will get the bot green because, on pexpect timeout, we try 2 more
times (by default) in "fast and loose" mode.

jrg


>
>
> http://codereview.chromium.**org/9185043/diff/1/build/**
>
android/emulator.py<http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py>
> File build/android/emulator.py (right):
>
> http://codereview.chromium.**org/9185043/diff/1/build/**
>
android/emulator.py#newcode99<http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py#newcode99>
> build/android/emulator.py:99: fast_and_loose: go fast and loose.
> Intended for quick testing.
> Instead of "go fast and loose", could you copy the description written
> in run_tests.py for additional clarity?
>
> http://codereview.chromium.**org/9185043/diff/1/build/**
>
android/emulator.py#newcode140<http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py#newcode140>
> build/android/emulator.py:140: # ALWAYS wipe the data.  We've seen cases
> where an emulator
> This "ALWAYS" is not accurate anymore now that it's depending on the
> fast_and_loose mode.
>
> http://codereview.chromium.**org/9185043/diff/1/build/**
>
android/run_tests.py<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py>
> File build/android/run_tests.py (right):
>
> http://codereview.chromium.**org/9185043/diff/1/build/**
>
android/run_tests.py#**newcode136<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newcode136>
> build/android/run_tests.py:**136: fast_and_loose=False):
> This would fit on line 135.
>
> http://codereview.chromium.**org/9185043/diff/1/build/**
>
android/run_tests.py#**newcode256<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newcode256>
> build/android/run_tests.py:**256:
> nit: newline (since you fixed it elsewhere)
>
> http://codereview.chromium.**org/9185043/diff/1/build/**
>
android/run_tests.py#**newcode275<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newcode275>
> build/android/run_tests.py:**275: option_parser.add_option('-f',
>
> '--gtest_filter', dest='gtest_filter',
> This seems unrelated.
>
>
http://codereview.chromium.**org/9185043/<http://codereview.chromium.org/9185...
>

[Peter Beverloo, 2012-01-12 18:30:39]

On 2012/01/12 18:27:09, John Grabowski wrote:
> > I'm not really enthusiastically about the "fast_and_loose" name, maybe
> > "lossy_run" or "quick_run"? Furthermore, since the code explicitly
> > mentions not
> > to use this on bots, how is this going to get the bot green?
> >
> 
> It's not really "lossy".  I picked the name because a --fast option seems
> too tempting to use always; "loose" as in "loosening up the rules".
> 
> Happy to choose a new name if you want something specific, but I don't want
> to call it --quick or --fast.
> 

Ok, thank you.

> This will get the bot green because, on pexpect timeout, we try 2 more
> times (by default) in "fast and loose" mode.
> 

My concern is that run_tests.py:301 explicitly mentions to not use this on bots,
yet we are going to use this on our bot?

> jrg
> 
> 
> >
> >
> > http://codereview.chromium.**org/9185043/diff/1/build/**
> >
>
android/emulator.py<http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py>
> > File build/android/emulator.py (right):
> >
> > http://codereview.chromium.**org/9185043/diff/1/build/**
> >
>
android/emulator.py#newcode99<http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py#newcode99>
> > build/android/emulator.py:99: fast_and_loose: go fast and loose.
> > Intended for quick testing.
> > Instead of "go fast and loose", could you copy the description written
> > in run_tests.py for additional clarity?
> >
> > http://codereview.chromium.**org/9185043/diff/1/build/**
> >
>
android/emulator.py#newcode140<http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py#newcode140>
> > build/android/emulator.py:140: # ALWAYS wipe the data.  We've seen cases
> > where an emulator
> > This "ALWAYS" is not accurate anymore now that it's depending on the
> > fast_and_loose mode.
> >
> > http://codereview.chromium.**org/9185043/diff/1/build/**
> >
>
android/run_tests.py<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py>
> > File build/android/run_tests.py (right):
> >
> > http://codereview.chromium.**org/9185043/diff/1/build/**
> >
>
android/run_tests.py#**newcode136<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newcode136>
> > build/android/run_tests.py:**136: fast_and_loose=False):
> > This would fit on line 135.
> >
> > http://codereview.chromium.**org/9185043/diff/1/build/**
> >
>
android/run_tests.py#**newcode256<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newcode256>
> > build/android/run_tests.py:**256:
> > nit: newline (since you fixed it elsewhere)
> >
> > http://codereview.chromium.**org/9185043/diff/1/build/**
> >
>
android/run_tests.py#**newcode275<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newcode275>
> > build/android/run_tests.py:**275: option_parser.add_option('-f',
> >
> > '--gtest_filter', dest='gtest_filter',
> > This seems unrelated.
> >
> >
>
http://codereview.chromium.**org/9185043/%3Chttp://codereview.chromium.org/91...>
> >

[John Grabowski, 2012-01-12 18:33:50]

>
>
>
> My concern is that run_tests.py:301 explicitly mentions to not use this on
> bots,
> yet we are going to use this on our bot?
>

OK, clarified "don't use on bots by default"


>
>  jrg
>>
>
>
>  >
>> >
>> > http://codereview.chromium.****org/9185043/diff/1/build/**
>> >
>>
>
> android/emulator.py<http://**codereview.chromium.org/**
>
9185043/diff/1/build/android/**emulator.py<http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py>
> >
>
>> > File build/android/emulator.py (right):
>> >
>> > http://codereview.chromium.****org/9185043/diff/1/build/**
>> >
>>
>
> android/emulator.py#newcode99<**http://codereview.chromium.**
>
org/9185043/diff/1/build/**android/emulator.py#newcode99<http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py#newcode99>
> >
>
>> > build/android/emulator.py:99: fast_and_loose: go fast and loose.
>> > Intended for quick testing.
>> > Instead of "go fast and loose", could you copy the description written
>> > in run_tests.py for additional clarity?
>> >
>> > http://codereview.chromium.****org/9185043/diff/1/build/**
>> >
>>
>
> android/emulator.py#**newcode140<http://codereview.**
>
chromium.org/9185043/diff/1/**build/android/emulator.py#**newcode140<http://codereview.chromium.org/9185043/diff/1/build/android/emulator.py#newcode140>
> >
>
>> > build/android/emulator.py:140: # ALWAYS wipe the data.  We've seen cases
>> > where an emulator
>> > This "ALWAYS" is not accurate anymore now that it's depending on the
>> > fast_and_loose mode.
>> >
>> > http://codereview.chromium.****org/9185043/diff/1/build/**
>> >
>>
>
> android/run_tests.py<http://**codereview.chromium.org/**
>
9185043/diff/1/build/android/**run_tests.py<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py>
> >
>
>> > File build/android/run_tests.py (right):
>> >
>> > http://codereview.chromium.****org/9185043/diff/1/build/**
>> >
>>
>
> android/run_tests.py#****newcode136<http://codereview.**
>
chromium.org/9185043/diff/1/**build/android/run_tests.py#**newcode136<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newcode136>
> >
>
>> > build/android/run_tests.py:****136: fast_and_loose=False):
>>
>> > This would fit on line 135.
>> >
>> > http://codereview.chromium.****org/9185043/diff/1/build/**
>> >
>>
>
> android/run_tests.py#****newcode256<http://codereview.**
>
chromium.org/9185043/diff/1/**build/android/run_tests.py#**newcode256<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newcode256>
> >
>
>> > build/android/run_tests.py:****256:
>>
>> > nit: newline (since you fixed it elsewhere)
>> >
>> > http://codereview.chromium.****org/9185043/diff/1/build/**
>> >
>>
>
> android/run_tests.py#****newcode275<http://codereview.**
>
chromium.org/9185043/diff/1/**build/android/run_tests.py#**newcode275<http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newcode275>
> >
>
>> > build/android/run_tests.py:****275: option_parser.add_option('-f',
>>
>> >
>> > '--gtest_filter', dest='gtest_filter',
>> > This seems unrelated.
>> >
>> >
>>
>
> http://codereview.chromium.****org/9185043/%3Chttp://coderevi**
> ew.chromium.org/9185043/ <http://codereview.chromium.org/9185043/>>
>
>> >
>>
>
>
>
>
http://codereview.chromium.**org/9185043/<http://codereview.chromium.org/9185...
>

[John Grabowski, 2012-01-12 18:34:17]

Feedback applied except for choosing a new name (need to settle on that)

http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py
File build/android/run_tests.py (right):

http://codereview.chromium.org/9185043/diff/1/build/android/run_tests.py#newc...
build/android/run_tests.py:275: option_parser.add_option('-f', '--gtest_filter',
dest='gtest_filter',
On 2012/01/12 11:19:18, Peter Beverloo wrote:
> This seems unrelated.

Well, it makes use of "fast and loose" mode a lot more convenient.  E.g. find a
hanging test, try run just that one test again (in fast and loose mode, with a
filter) to see if we should disable it or if it is emulaor flakiness).

[Peter Beverloo, 2012-01-12 18:41:34]

LGTM, thank you!

http://codereview.chromium.org/9185043/diff/2003/build/android/emulator.py
File build/android/emulator.py (right):

http://codereview.chromium.org/9185043/diff/2003/build/android/emulator.py#ne...
build/android/emulator.py:140: # ALWAYS wipe the data.  We've seen cases where
an emulator
nit: this still says "ALWAYS". See comment on PS1.

[John Grabowski, 2012-01-12 18:48:01]

ALWAYS removed on final diffs.  Thx!

jrg

On Thu, Jan 12, 2012 at 10:41 AM, <peter@chromium.org> wrote:

> LGTM, thank you!
>
>
> http://codereview.chromium.**org/9185043/diff/2003/build/**
>
android/emulator.py<http://codereview.chromium.org/9185043/diff/2003/build/android/emulator.py>
> File build/android/emulator.py (right):
>
> http://codereview.chromium.**org/9185043/diff/2003/build/**
>
android/emulator.py#newcode140<http://codereview.chromium.org/9185043/diff/2003/build/android/emulator.py#newcode140>
>
> build/android/emulator.py:140: # ALWAYS wipe the data.  We've seen cases
> where an emulator
> nit: this still says "ALWAYS". See comment on PS1.
>
>
http://codereview.chromium.**org/9185043/<http://codereview.chromium.org/9185...
>