Modified: trunk/Tools/ChangeLog (97514 => 97515)
--- trunk/Tools/ChangeLog 2011-10-14 22:11:12 UTC (rev 97514)
+++ trunk/Tools/ChangeLog 2011-10-14 22:11:30 UTC (rev 97515)
@@ -1,3 +1,16 @@
+2011-10-14 David Levin <[email protected]>
+
+ watchlist: If the style check fails, then the watchlist will not be run.
+ https://bugs.webkit.org/show_bug.cgi?id=69484
+
+ Reviewed by Adam Barth.
+
+ * Scripts/webkitpy/tool/commands/queues.py: Run the watch list even
+ if the style part fails and don't allow watch list failures turn the
+ bot run red.
+ * Scripts/webkitpy/tool/commands/queues_unittest.py: Appropriate unit tests.
+ * Scripts/webkitpy/tool/mocktool.py: Add support to make an executive command throw.
+
2011-10-14 Dimitri Glazkov <[email protected]>
Plumb style-checker filter up to command options and make land-cowboy use it.
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues.py (97514 => 97515)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2011-10-14 22:11:12 UTC (rev 97514)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues.py 2011-10-14 22:11:30 UTC (rev 97515)
@@ -419,8 +419,17 @@
return True
def review_patch(self, patch):
- self.run_webkit_patch(["check-style", "--force-clean", "--non-interactive", "--parent-command=style-queue", patch.id()])
- self.run_webkit_patch(["apply-watchlist-local", patch.bug_id()])
+ try:
+ # Run the style checks.
+ self.run_webkit_patch(["check-style", "--force-clean", "--non-interactive", "--parent-command=style-queue", patch.id()])
+ finally:
+ # Apply the watch list.
+ try:
+ self.run_webkit_patch(["apply-watchlist-local", patch.bug_id()])
+ except ScriptError, e:
+ # Don't turn the style bot block red due to watchlist errors.
+ pass
+
return True
@classmethod
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py (97514 => 97515)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2011-10-14 22:11:12 UTC (rev 97514)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2011-10-14 22:11:30 UTC (rev 97515)
@@ -446,13 +446,32 @@
class StyleQueueTest(QueuesTest):
- def test_style_queue(self):
+ def test_style_queue_with_style_exception(self):
expected_stderr = {
"begin_work_queue": self._default_begin_work_queue_stderr("style-queue"),
"next_work_item": "",
"should_proceed_with_work_item": "MOCK: update_status: style-queue Checking style\n",
"process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'check-style', '--force-clean', '--non-interactive', '--parent-command=style-queue', 10000], cwd=/mock-checkout
MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-watchlist-local', 50000], cwd=/mock-checkout
+MOCK: update_status: style-queue Fail
+MOCK: release_work_item: style-queue 10000\n""",
+ "handle_unexpected_error": "Mock error message\n",
+ "handle_script_error": "MOCK: update_status: style-queue ScriptError error message\nMOCK bug comment: bug_id=50000, cc=[]\n--- Begin comment ---\nAttachment 10000 did not pass style-queue:\n\nScriptError error message\n\nIf any of these errors are false positives, please file a bug against check-webkit-style.\n--- End comment ---\n\n",
+ }
+ expected_exceptions = {
+ "process_work_item": ScriptError,
+ "handle_script_error": SystemExit,
+ }
+ tool = MockTool(log_executive=True, executive_throws_when_run=set(['check-style']))
+ self.assert_queue_outputs(StyleQueue(), expected_stderr=expected_stderr, expected_exceptions=expected_exceptions, tool=tool)
+
+ def test_style_queue_with_watch_list_exception(self):
+ expected_stderr = {
+ "begin_work_queue": self._default_begin_work_queue_stderr("style-queue"),
+ "next_work_item": "",
+ "should_proceed_with_work_item": "MOCK: update_status: style-queue Checking style\n",
+ "process_work_item": """MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'check-style', '--force-clean', '--non-interactive', '--parent-command=style-queue', 10000], cwd=/mock-checkout
+MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'apply-watchlist-local', 50000], cwd=/mock-checkout
MOCK: update_status: style-queue Pass
MOCK: release_work_item: style-queue 10000\n""",
"handle_unexpected_error": "Mock error message\n",
@@ -461,5 +480,5 @@
expected_exceptions = {
"handle_script_error": SystemExit,
}
- tool = MockTool(log_executive=True)
+ tool = MockTool(log_executive=True, executive_throws_when_run=set(['apply-watchlist-local']))
self.assert_queue_outputs(StyleQueue(), expected_stderr=expected_stderr, expected_exceptions=expected_exceptions, tool=tool)
Modified: trunk/Tools/Scripts/webkitpy/tool/mocktool.py (97514 => 97515)
--- trunk/Tools/Scripts/webkitpy/tool/mocktool.py 2011-10-14 22:11:12 UTC (rev 97514)
+++ trunk/Tools/Scripts/webkitpy/tool/mocktool.py 2011-10-14 22:11:30 UTC (rev 97515)
@@ -723,9 +723,10 @@
# FIXME: Unify with common.system.executive_mock.MockExecutive.
class MockExecutive(object):
- def __init__(self, should_log=False, should_throw=False):
+ def __init__(self, should_log=False, should_throw=False, should_throw_when_run=None):
self._should_log = should_log
self._should_throw = should_throw
+ self._should_throw_when_run = should_throw_when_run or set()
# FIXME: Once executive wraps os.getpid() we can just use a static pid for "this" process.
self._running_pids = [os.getpid()]
@@ -735,6 +736,8 @@
def run_and_throw_if_fail(self, args, quiet=False, cwd=None):
if self._should_log:
log("MOCK run_and_throw_if_fail: %s, cwd=%s" % (args, cwd))
+ if self._should_throw_when_run.intersection(args):
+ raise ScriptError("Exception for %s" % args)
return "MOCK output of child process"
def run_command(self,
@@ -840,11 +843,11 @@
class MockTool(object):
- def __init__(self, log_executive=False):
+ def __init__(self, log_executive=False, executive_throws_when_run=None):
self.wakeup_event = threading.Event()
self.bugs = MockBugzilla()
self.buildbot = MockBuildBot()
- self.executive = MockExecutive(should_log=log_executive)
+ self.executive = MockExecutive(should_log=log_executive, should_throw_when_run=executive_throws_when_run)
self.web = MockWeb()
self.workspace = MockWorkspace()
self._irc = None
