ExceptionNotifier: a new MailNotifier which treats exceptions as failures.

ExceptionNotifier: a new MailNotifier which treats exceptions as failures.

This is the first CL to fix the bug. Another CL needs to land in
build_internal which uses the new notifier instead of MailNotifier.

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

[Sergey Berezin, 2014-11-15 02:34:08]

sergeyberezin@chromium.org changed reviewers:
+ agable@chromium.org

[Sergey Berezin, 2014-11-15 02:34:58]

PTAL

[agable, 2014-11-17 19:26:42]

https://codereview.chromium.org/720273003/diff/1/scripts/master/exception_not...
File scripts/master/exception_notifier.py (right):

https://codereview.chromium.org/720273003/diff/1/scripts/master/exception_not...
scripts/master/exception_notifier.py:1: # Copyright (c) 2012 The Chromium
Authors. All rights reserved.
2014

Honestly, would consider putting this in the same file as MailNotifier. No
biggie either way, though.

https://codereview.chromium.org/720273003/diff/1/scripts/master/exception_not...
scripts/master/exception_notifier.py:1: # Copyright (c) 2012 The Chromium
Authors. All rights reserved.
2014

There are already a bunch of MailNotifier extensions in scripts/master. Consider
putting this in the same file as those.

https://codereview.chromium.org/720273003/diff/1/scripts/master/exception_not...
scripts/master/exception_notifier.py:11: """Same as MailNotifier, only treats
EXCEPTION as failure."""
"only" is not the word you want here :)

https://codereview.chromium.org/720273003/diff/1/scripts/master/unittests/exc...
File scripts/master/unittests/exception_notifier_test.py (right):

https://codereview.chromium.org/720273003/diff/1/scripts/master/unittests/exc...
scripts/master/unittests/exception_notifier_test.py:2: # Copyright (c) 2011 The
Chromium Authors. All rights reserved.
2014

[Sergey Berezin, 2015-01-06 21:39:48]

Finally getting back to this CL. PTAL.

https://codereview.chromium.org/720273003/diff/1/scripts/master/exception_not...
File scripts/master/exception_notifier.py (right):

https://codereview.chromium.org/720273003/diff/1/scripts/master/exception_not...
scripts/master/exception_notifier.py:1: # Copyright (c) 2012 The Chromium
Authors. All rights reserved.
On 2014/11/17 19:26:42, agable wrote:
> 2014

Done.

> Honestly, would consider putting this in the same file as MailNotifier. No
> biggie either way, though.

It seems the common pattern here is that every MailNotifier extension is in its
own separate file. I think I'd rather stick to the same pattern, to make it
easier to find the class.

https://codereview.chromium.org/720273003/diff/1/scripts/master/exception_not...
scripts/master/exception_notifier.py:11: """Same as MailNotifier, only treats
EXCEPTION as failure."""
On 2014/11/17 19:26:42, agable wrote:
> "only" is not the word you want here :)

Done.

https://codereview.chromium.org/720273003/diff/1/scripts/master/unittests/exc...
File scripts/master/unittests/exception_notifier_test.py (right):

https://codereview.chromium.org/720273003/diff/1/scripts/master/unittests/exc...
scripts/master/unittests/exception_notifier_test.py:2: # Copyright (c) 2011 The
Chromium Authors. All rights reserved.
On 2014/11/17 19:26:42, agable wrote:
> 2014

Done.

[agable, 2015-01-07 00:26:46]

You made sure the new tests gets picked up during presubmit? LGTM.

[Sergey Berezin, 2015-01-09 01:35:52]

On 2015/01/07 00:26:46, agable wrote:
> You made sure the new tests gets picked up during presubmit? LGTM.

Yes, a deliberately bad test in exception_notifier_test.py failed 'git cl
presubmit --force' as it should (just checked). Thanks!

[Sergey Berezin, 2015-01-09 01:36:08]

The CQ bit was checked by sergeyberezin@chromium.org

[commit-bot: I haz the power, 2015-01-09 01:37:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/720273003/20001

[commit-bot: I haz the power, 2015-01-09 01:39:44]

Committed patchset #2 (id:20001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=293576