Bug 69484 - watchlist: If the style check fails, then the watchlist will not be run.
Summary: watchlist: If the style check fails, then the watchlist will not be run.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: David Levin
URL:
Keywords:
Depends on:
Blocks: 68822
  Show dependency treegraph
 
Reported: 2011-10-05 17:26 PDT by David Levin
Modified: 2011-10-14 15:11 PDT (History)
1 user (show)

See Also:


Attachments
Patch (6.76 KB, patch)
2011-10-14 14:29 PDT, David Levin
abarth: review+
abarth: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Levin 2011-10-05 17:26:20 PDT
Right now, we run the watchlist after the stylebot (see http://trac.webkit.org/changeset/96541/trunk/Tools/Scripts/webkitpy/tool/commands/queues.py) but if the style bot fails, it will throw and the watchlist will not run.

The ordering seems fine but we should consider doing something about not running watchlist.
Comment 1 Adam Barth 2011-10-05 20:29:50 PDT
We can just catch the exception and re-throw it.
Comment 2 David Levin 2011-10-14 14:29:01 PDT
Created attachment 111078 [details]
Patch
Comment 3 Adam Barth 2011-10-14 14:52:16 PDT
Comment on attachment 111078 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=111078&action=review

I like executive_throws_when_run.  :)

> Tools/Scripts/webkitpy/tool/commands/queues.py:440
> +        # Raise any exceptions that occurred while running check-style
> +        if exception:
> +            raise exception

There's a slightly snazzier way of doing using the "raise" keyword with no arguments, but this is more than fine.

> Tools/Scripts/webkitpy/tool/mocktool.py:726
> -    def __init__(self, should_log=False, should_throw=False):
> +    def __init__(self, should_log=False, should_throw=False, should_throw_when_run=set()):

Eric will tell you to use Non for the default paramater value.  The problem is that the default paramater value is a static and we modify it by accident, all future callers will get the modified value!

> Tools/Scripts/webkitpy/tool/mocktool.py:729
> +        self._should_throw_when_run = should_throw_when_run

If you use None, then this line becomes:

self._should_throw_when_run = should_throw_when_run or set()
Comment 4 David Levin 2011-10-14 15:11:06 PDT
Committed as http://trac.webkit.org/changeset/97515