Skip to site navigation (Press enter)

[webkit-changes] [97198] trunk/Tools

abarth Tue, 11 Oct 2011 16:49:34 -0700

Title: [97198] trunk/Tools

Revision: 97198
Author: [email protected]
Date: 2011-10-11 16:50:33 -0700 (Tue, 11 Oct 2011)

Log Message

sheriffbot takes too long to acknowledge rollout commands
https://bugs.webkit.org/show_bug.cgi?id=69871


Reviewed by Eric Seidel.

We used to update the working copy before acknowledging the command
because we wanted to ping all the relevant IRC nicks.  That's caused a
bunch of frustration because folks don't know whether the bot has heard
their commands.

This patch makes the bot reply immediately before updating the working
copy.  All the relevenat folks are still pinged when the bot finishes
preparing the rollout.

* Scripts/webkitpy/tool/bot/irc_command.py:
* Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py:

Modified Paths

trunk/Tools/ChangeLog
trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py
trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py

Diff

Modified: trunk/Tools/ChangeLog (97197 => 97198)


--- trunk/Tools/ChangeLog	2011-10-11 23:50:26 UTC (rev 97197)
+++ trunk/Tools/ChangeLog	2011-10-11 23:50:33 UTC (rev 97198)
@@ -1,5 +1,24 @@
 2011-10-11  Adam Barth  <[email protected]>
 
+        sheriffbot takes too long to acknowledge rollout commands
+        https://bugs.webkit.org/show_bug.cgi?id=69871
+
+        Reviewed by Eric Seidel.
+
+        We used to update the working copy before acknowledging the command
+        because we wanted to ping all the relevant IRC nicks.  That's caused a
+        bunch of frustration because folks don't know whether the bot has heard
+        their commands.
+
+        This patch makes the bot reply immediately before updating the working
+        copy.  All the relevenat folks are still pinged when the bot finishes
+        preparing the rollout.
+
+        * Scripts/webkitpy/tool/bot/irc_command.py:
+        * Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py:
+
+2011-10-11  Adam Barth  <[email protected]>
+
         test-webkitpy fails on Lion
         https://bugs.webkit.org/show_bug.cgi?id=69873

Modified: trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py (97197 => 97198)


--- trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py	2011-10-11 23:50:26 UTC (rev 97197)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py	2011-10-11 23:50:33 UTC (rev 97198)
@@ -127,15 +127,15 @@
             # return is equivalent to an irc().post(), but makes for easier unit testing.
             return "%s: Usage: rollout SVN_REVISION [SVN_REVISIONS] REASON" % nick
 
+        revision_urls_string = join_with_separators([urls.view_revision_url(revision) for revision in svn_revision_list])
+        tool.irc().post("%s: Preparing rollout for %s..." % (nick, revision_urls_string))
+
         self._update_working_copy(tool)
 
         # FIXME: IRCCommand should bind to a tool and have a self._tool like Command objects do.
         # Likewise we should probably have a self._sheriff.
         nicks_string = self._nicks_string(tool, sheriff, nick, svn_revision_list)
 
-        revision_urls_string = join_with_separators([urls.view_revision_url(revision) for revision in svn_revision_list])
-        tool.irc().post("%s: Preparing rollout for %s..." % (nicks_string, revision_urls_string))
-
         try:
             complete_reason = "%s (Requested by %s on %s)." % (
                 rollout_reason, nick, config_irc.channel)

Modified: trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py (97197 => 97198)


--- trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py	2011-10-11 23:50:26 UTC (rev 97197)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py	2011-10-11 23:50:33 UTC (rev 97198)
@@ -94,11 +94,11 @@
         OutputCapture().assert_outputs(self, run, args=["restart"], expected_stderr=expected_stderr, expected_exception=TerminateQueue)
 
     def test_rollout(self):
-        expected_stderr = "MOCK: irc.post: mock_nick, abarth, darin, eseidel: Preparing rollout for http://trac.webkit.org/changeset/21654...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
+        expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
         OutputCapture().assert_outputs(self, run, args=["rollout 21654 This patch broke the world"], expected_stderr=expected_stderr)
 
     def test_revert(self):
-        expected_stderr = "MOCK: irc.post: mock_nick, abarth, darin, eseidel: Preparing rollout for http://trac.webkit.org/changeset/21654...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
+        expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
         OutputCapture().assert_outputs(self, run, args=["revert 21654 This patch broke the world"], expected_stderr=expected_stderr)
 
     def test_roll_chromium_deps(self):
@@ -110,15 +110,15 @@
         OutputCapture().assert_outputs(self, run, args=["roll-chromium-deps"], expected_stderr=expected_stderr)
 
     def test_multi_rollout(self):
-        expected_stderr = "MOCK: irc.post: mock_nick, abarth, darin, eseidel: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
+        expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
         OutputCapture().assert_outputs(self, run, args=["rollout 21654 21655 21656 This 21654 patch broke the world"], expected_stderr=expected_stderr)
 
     def test_rollout_with_r_in_svn_revision(self):
-        expected_stderr = "MOCK: irc.post: mock_nick, abarth, darin, eseidel: Preparing rollout for http://trac.webkit.org/changeset/21654...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
+        expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
         OutputCapture().assert_outputs(self, run, args=["rollout r21654 This patch broke the world"], expected_stderr=expected_stderr)
 
     def test_multi_rollout_with_r_in_svn_revision(self):
-        expected_stderr = "MOCK: irc.post: mock_nick, abarth, darin, eseidel: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
+        expected_stderr = "MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656...\nMOCK: irc.post: mock_nick, abarth, darin, eseidel: Created rollout: http://example.com/36936\n"
         OutputCapture().assert_outputs(self, run, args=["rollout r21654 21655 r21656 This r21654 patch broke the world"], expected_stderr=expected_stderr)
 
     def test_rollout_bananas(self):
@@ -134,7 +134,7 @@
 
     def test_rollout_invalidate_reason(self):
         # FIXME: I'm slightly confused as to why this doesn't return the USAGE message.
-        expected_stderr = """MOCK: irc.post: mock_nick, abarth, darin, eseidel: Preparing rollout for http://trac.webkit.org/changeset/21654...
+        expected_stderr = """MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654...
 MOCK: irc.post: mock_nick, abarth, darin, eseidel: Failed to create rollout patch:
 MOCK: irc.post: The rollout reason may not begin with - (\"-bad (Requested by mock_nick on #webkit).\").
 """
@@ -143,7 +143,7 @@
                                        expected_stderr=expected_stderr)
 
     def test_multi_rollout_invalidate_reason(self):
-        expected_stderr = """MOCK: irc.post: mock_nick, abarth, darin, eseidel: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656...
+        expected_stderr = """MOCK: irc.post: mock_nick: Preparing rollout for http://trac.webkit.org/changeset/21654, http://trac.webkit.org/changeset/21655, and http://trac.webkit.org/changeset/21656...
 MOCK: irc.post: mock_nick, abarth, darin, eseidel: Failed to create rollout patch:
 MOCK: irc.post: The rollout reason may not begin with - (\"-bad (Requested by mock_nick on #webkit).\").
 """

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes