Diff
Modified: trunk/Tools/ChangeLog (95499 => 95500)
--- trunk/Tools/ChangeLog 2011-09-20 00:38:09 UTC (rev 95499)
+++ trunk/Tools/ChangeLog 2011-09-20 00:56:48 UTC (rev 95500)
@@ -1,5 +1,17 @@
2011-09-19 David Levin <[email protected]>
+ Sheriffbot rollout should be more intuitive.
+ https://bugs.webkit.org/show_bug.cgi?id=68415
+
+ Reviewed by Adam Barth.
+
+ * Scripts/webkitpy/tool/bot/irc_command.py: Add support for revert and comma separated args.
+ * Scripts/webkitpy/tool/bot/irc_command_unittest.py: Add parsing tests for comma separated args
+ and a few others cases.
+ * Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py: Verify that revert works.
+
+2011-09-19 David Levin <[email protected]>
+
check-webkit-style generates bogus warning for StructuredExceptionHandlerSupressor.h
https://bugs.webkit.org/show_bug.cgi?id=68391
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py (95499 => 95500)
--- trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py 2011-09-20 00:38:09 UTC (rev 95499)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py 2011-09-20 00:56:48 UTC (rev 95500)
@@ -66,26 +66,42 @@
class Rollout(IRCCommand):
+ def _extract_revisions(self, arg):
+
+ revision_list = []
+ possible_revisions = arg.split(",")
+ for revision in possible_revisions:
+ revision = revision.strip()
+ if not revision:
+ continue
+ revision = revision.lstrip("r")
+ # If one part of the arg isn't in the correct format,
+ # then none of the arg should be considered a revision.
+ if not revision.isdigit():
+ return None
+ revision_list.append(int(revision))
+ return revision_list
+
def _parse_args(self, args):
if not args:
return (None, None)
- # the first argument must be a revision number
- first_revision = args[0].lstrip("r")
- if not first_revision.isdigit():
+ svn_revision_list = []
+ remaining_args = args[:]
+ # First process all revisions.
+ while remaining_args:
+ new_revisions = self._extract_revisions(remaining_args[0])
+ if not new_revisions:
+ break
+ svn_revision_list += new_revisions
+ remaining_args = remaining_args[1:]
+
+ # Was there a revision number?
+ if not len(svn_revision_list):
return (None, None)
- parsing_revision = True
- svn_revision_list = [int(first_revision)]
- rollout_reason = []
- for arg in args[1:]:
- if arg.lstrip("r").isdigit() and parsing_revision:
- svn_revision_list.append(int(arg.lstrip("r")))
- else:
- parsing_revision = False
- rollout_reason.append(arg)
-
- rollout_reason = " ".join(rollout_reason)
+ # Everything left is the reason.
+ rollout_reason = " ".join(remaining_args)
return svn_revision_list, rollout_reason
def _responsible_nicknames_from_revisions(self, tool, sheriff, svn_revision_list):
@@ -161,7 +177,7 @@
class Help(IRCCommand):
def execute(self, nick, args, tool, sheriff):
- return "%s: Available commands: %s" % (nick, ", ".join(sorted(commands.keys())))
+ return "%s: Available commands: %s" % (nick, ", ".join(sorted(visible_commands.keys())))
class Hi(IRCCommand):
@@ -235,7 +251,7 @@
# FIXME: Lame. We should have an auto-registering CommandCenter.
-commands = {
+visible_commands = {
"help": Help,
"hi": Hi,
"last-green-revision": LastGreenRevision,
@@ -245,3 +261,10 @@
"create-bug": CreateBug,
"roll-chromium-deps": RollChromiumDEPS,
}
+
+# Add revert as an "easter egg" command. Why?
+# revert is the same as rollout and it would be confusing to list both when
+# they do the same thing. However, this command is a very natural thing for
+# people to use and it seems silly to have them hunt around for "rollout" instead.
+commands = visible_commands.copy()
+commands["revert"] = Rollout
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/irc_command_unittest.py (95499 => 95500)
--- trunk/Tools/Scripts/webkitpy/tool/bot/irc_command_unittest.py 2011-09-20 00:38:09 UTC (rev 95499)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/irc_command_unittest.py 2011-09-20 00:56:48 UTC (rev 95500)
@@ -98,6 +98,18 @@
self.assertEquals(([1234], "testing foo"),
rollout._parse_args(["1234", "testing", "foo"]))
+ self.assertEquals(([554], "testing foo"),
+ rollout._parse_args(["r554", "testing", "foo"]))
+
+ self.assertEquals(([556, 792], "testing foo"),
+ rollout._parse_args(["r556", "792", "testing", "foo"]))
+
+ self.assertEquals(([128, 256], "testing foo"),
+ rollout._parse_args(["r128,r256", "testing", "foo"]))
+
+ self.assertEquals(([512, 1024, 2048], "testing foo"),
+ rollout._parse_args(["512,", "1024,2048", "testing", "foo"]))
+
# Test invalid argument parsing:
self.assertEquals((None, None), rollout._parse_args([]))
self.assertEquals((None, None), rollout._parse_args(["--bar", "1234"]))
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py (95499 => 95500)
--- trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py 2011-09-20 00:38:09 UTC (rev 95499)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py 2011-09-20 00:56:48 UTC (rev 95500)
@@ -97,6 +97,10 @@
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"
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"
+ OutputCapture().assert_outputs(self, run, args=["revert 21654 This patch broke the world"], expected_stderr=expected_stderr)
+
def test_roll_chromium_deps(self):
expected_stderr = "MOCK: irc.post: mock_nick: Rolling Chromium DEPS to r21654\nMOCK: irc.post: mock_nick: Created DEPS roll: http://example.com/36936\n"
OutputCapture().assert_outputs(self, run, args=["roll-chromium-deps 21654"], expected_stderr=expected_stderr)
