Changeset 57035 in webkit

Timestamp:

Apr 2, 2010 5:14:45 PM (14 years ago)

Author:

abarth@webkit.org

Message:

2010-04-02 Adam Barth <​abarth@webkit.org>

Reviewed by Darin Adler.

commit-queue should ignore builders when landing rollouts
​https://bugs.webkit.org/show_bug.cgi?id=37051

When we moved the "builders are red" check into the master process, we
forgot about rollouts. I thought we had a test covering this case, but
looking at the test, it was a bit too loose. I added a new test and
introduced some new logging technology into MockTool to make the test
tighter.

• Scripts/webkitpy/tool/commands/queues.py:
• Scripts/webkitpy/tool/commands/queues_unittest.py:
• Scripts/webkitpy/tool/mocktool.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/tool/commands/queues.py (modified) (2 diffs)
• Scripts/webkitpy/tool/commands/queues_unittest.py (modified) (3 diffs)
• Scripts/webkitpy/tool/mocktool.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-04-02  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        commit-queue should ignore builders when landing rollouts
+        https://bugs.webkit.org/show_bug.cgi?id=37051
+
+        When we moved the "builders are red" check into the master process, we
+        forgot about rollouts.  I thought we had a test covering this case, but
+        looking at the test, it was a bit too loose.  I added a new test and
+        introduced some new logging technology into MockTool to make the test
+        tighter.
+
+        * Scripts/webkitpy/tool/commands/queues.py:
+        * Scripts/webkitpy/tool/commands/queues_unittest.py:
+        * Scripts/webkitpy/tool/mocktool.py:
+
 2010-04-02  David Levin  <levin@chromium.org>
 

trunk/WebKitTools/Scripts/webkitpy/tool/commands/queues.py

@@ -206 +206 @@
     def _land(self, patch, first_run=False):
         try:
-            if not self._builders_are_green():
+            # We need to check the builders, unless we're trying to land a
+            # rollout (in which case the builders are probably red.)
+            if not patch.is_rollout() and not self._builders_are_green():
                 # We return true here because we want to return to the main
                 # QueueEngine loop as quickly as possible.
@@ -232 +234 @@
                 # built and tested.
                 args.append("--no-update")
-            if patch.is_rollout():
-                # We need to ignore the builders when landing a rollout
-                # because they're probably red.
-                args.append("--ignore-builders")
             self.run_webkit_patch(args)
             self._did_pass(patch)

trunk/WebKitTools/Scripts/webkitpy/tool/commands/queues_unittest.py

@@ -44 +44 @@
 class TestReviewQueue(AbstractReviewQueue):
     name = "test-review-queue"
+
+
+class MockPatch(object):
+    def is_rollout(self):
+        return True
+
+    def bug_id(self):
+        return 12345
+
+    def id(self):
+        return 76543
 
 
@@ -113 +124 @@
 
     def test_rollout(self):
-        tool = MockTool()
+        tool = MockTool(log_executive=True)
         tool.buildbot.light_tree_on_fire()
         expected_stderr = {
@@ -125 +136 @@
         self.assert_queue_outputs(CommitQueue(), tool=tool, expected_stderr=expected_stderr)
 
+    def test_rollout_lands(self):
+        tool = MockTool(log_executive=True)
+        tool.buildbot.light_tree_on_fire()
+        rollout_patch = MockPatch()
+        expected_stderr = {
+            "begin_work_queue": "CAUTION: commit-queue will discard all local changes in \"%s\"\nRunning WebKit commit-queue.\n" % MockSCM.fake_checkout_root,
+            # FIXME: The commit-queue warns about bad committers twice.  This is due to the fact that we access Attachment.reviewer() twice and it logs each time.
+            "next_work_item": """Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com)
+Warning, attachment 128 on bug 42 has invalid committer (non-committer@example.com)
+1 patch in commit-queue [106]
+""",
+            "process_work_item": "MOCK run_and_throw_if_fail: ['echo', '--status-host=example.com', 'land-attachment', '--force-clean', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', '76543']\n",
+        }
+        self.assert_queue_outputs(CommitQueue(), tool=tool, work_item=rollout_patch, expected_stderr=expected_stderr)
+
 
 class StyleQueueTest(QueuesTest):

trunk/WebKitTools/Scripts/webkitpy/tool/mocktool.py

@@ -446 +446 @@
 class MockTool():
 
-    def __init__(self):
+    def __init__(self, log_executive=False):
         self.wakeup_event = threading.Event()
         self.bugs = MockBugzilla()
         self.buildbot = MockBuildBot()
         self.executive = Mock()
+        if log_executive:
+            self.executive.run_and_throw_if_fail = lambda args: log("MOCK run_and_throw_if_fail: %s" % args)
         self._irc = None
         self.user = MockUser()