adds a flag to exit if a test fails

adds a flag to exit if a test fails

Change-Id: Ifd23ca19abaef24e30e0bbd9e751f8a0ea8cf667

Merge branch 'master' of ssh://gitrw.chromium.org:9222/crosutils into fail-on-exit

Change-Id: I4e6270cc401a1450a6f2de6bc156e2dabc6d4334

BUG=none
TEST=tested both success and failure cases with and without the flag

[petkov, 2010-08-25 20:57:58]

Any downside in not providing an option for this?

http://codereview.chromium.org/3150036/diff/4001/5001
File generate_test_report.py (right):

http://codereview.chromium.org/3150036/diff/4001/5001#newcode271
generate_test_report.py:271: parser.add_option('--exit-on-fail',
dest='exitOnFail', action='store_true',
I'd make this the default and not add an option at all. Then you don't need any
changes to run_remote_test.

If you decide to keep an option, change variable name to exit_on_fail per Python
style.

http://codereview.chromium.org/3150036/diff/4001/5001#newcode285
generate_test_report.py:285: if v["status"] != 'PASS':
'status'

[rtc, 2010-08-25 21:03:42]

On Wed, Aug 25, 2010 at 1:57 PM, <petkov@chromium.org> wrote:

> Any downside in not providing an option for this?
>
>
>
> http://codereview.chromium.org/3150036/diff/4001/5001
> File generate_test_report.py (right):
>
> http://codereview.chromium.org/3150036/diff/4001/5001#newcode271
> generate_test_report.py:271: parser.add_option('--exit-on-fail',
>
> dest='exitOnFail', action='store_true',
> I'd make this the default and not add an option at all. Then you don't
> need any changes to run_remote_test.
>
>
+Nick

 I'd actually prefer to make this behavior the default, but I'm not sure who
will break as a result. Ken mentioned that there are others (factory
testing) who depend on run_remote_tests behaving as it does today. Ken/Nick,
any opinions?

-Ryan



> If you decide to keep an option, change variable name to exit_on_fail
> per Python style.
>
> http://codereview.chromium.org/3150036/diff/4001/5001#newcode285
> generate_test_report.py:285: if v["status"] != 'PASS':
> 'status'
>
>
> http://codereview.chromium.org/3150036/show
>

[rtc, 2010-08-25 21:13:29]

PTAL

[petkov, 2010-08-25 21:21:15]

LGTM w/ a couple of style nits.

http://codereview.chromium.org/3150036/diff/10001/4002
File generate_test_report.py (right):

http://codereview.chromium.org/3150036/diff/10001/4002#newcode280
generate_test_report.py:280: for v in generator._results.itervalues():
theoretically, _ signifies a private member. care to move this inside Run?

http://codereview.chromium.org/3150036/diff/10001/4002#newcode281
generate_test_report.py:281: if v["status"] != 'PASS':
change to 'status' for consistency.

[rtc, 2010-08-25 21:25:40]

On Wed, Aug 25, 2010 at 2:21 PM, <petkov@chromium.org> wrote:

> LGTM w/ a couple of style nits.
>
>
>
> http://codereview.chromium.org/3150036/diff/10001/4002
>
> File generate_test_report.py (right):
>
> http://codereview.chromium.org/3150036/diff/10001/4002#newcode280
> generate_test_report.py:280: for v in generator._results.itervalues():
> theoretically, _ signifies a private member. care to move this inside
> Run?
>
> http://codereview.chromium.org/3150036/diff/10001/4002#newcode281
> generate_test_report.py:281: if v["status"] != 'PASS':
> change to 'status' for consistency.


Done and done. I'll retest then submit.

Thanks for the quick review.

>
>
> http://codereview.chromium.org/3150036/show
>

[Nick Sanders, 2010-08-26 07:12:04]

On Wed, Aug 25, 2010 at 2:03 PM, Ryan Cairns <rtc@chromium.org> wrote:

> +Nick
>
>  I'd actually prefer to make this behavior the default, but I'm not sure
> who will break as a result. Ken mentioned that there are others (factory
> testing) who depend on run_remote_tests behaving as it does today. Ken/Nick,
> any opinions?
>
> -Ryan
>

The factory tests all run locally w/o network connection, so autoserv,
run_remote_tests, etc. are not used at all.

Thanks,
  -Nick


>
>
>
>> If you decide to keep an option, change variable name to exit_on_fail
>> per Python style.
>>
>> http://codereview.chromium.org/3150036/diff/4001/5001#newcode285
>> generate_test_report.py:285: if v["status"] != 'PASS':
>> 'status'
>>
>>
>> http://codereview.chromium.org/3150036/show
>>
>
>

[kmixter1, 2010-08-27 16:38:52]

This is what I was thinking of:

http://codereview.chromium.org/1513033/show

Looks like the source was forked, so no worries about changing this file.

On Thu, Aug 26, 2010 at 12:11 AM, Nick Sanders <nsanders@chromium.org> wrote:
>
>
> On Wed, Aug 25, 2010 at 2:03 PM, Ryan Cairns <rtc@chromium.org> wrote:
>>
>> +Nick
>>  I'd actually prefer to make this behavior the default, but I'm not sure
>> who will break as a result. Ken mentioned that there are others (factory
>> testing) who depend on run_remote_tests behaving as it does today. Ken/Nick,
>> any opinions?
>> -Ryan
>
> The factory tests all run locally w/o network connection, so autoserv,
> run_remote_tests, etc. are not used at all.
> Thanks,
>   -Nick
>
>>
>>
>>>
>>> If you decide to keep an option, change variable name to exit_on_fail
>>> per Python style.
>>>
>>> http://codereview.chromium.org/3150036/diff/4001/5001#newcode285
>>> generate_test_report.py:285: if v["status"] != 'PASS':
>>> 'status'
>>>
>>> http://codereview.chromium.org/3150036/show
>>
>
>