Changeset 57115 in webkit


Ignore:
Timestamp:
Apr 5, 2010 8:02:13 PM (14 years ago)
Author:
abarth@webkit.org
Message:

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

Reviewed by Eric Seidel.

SheriffBot should include blamelist when posting to bugs
https://bugs.webkit.org/show_bug.cgi?id=37113

When posting on bugs, we should include the full list of SVN revisions
that caused the regression to folks have a better sense of whether they
are to blame.

  • Scripts/webkitpy/tool/bot/sheriff.py:
  • Scripts/webkitpy/tool/bot/sheriff_unittest.py:
  • Scripts/webkitpy/tool/commands/sheriffbot.py:
  • Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:
  • Scripts/webkitpy/tool/commands/upload_unittest.py:
  • Scripts/webkitpy/tool/mocktool.py:
Location:
trunk/WebKitTools
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebKitTools/ChangeLog

    r57114 r57115  
     12010-04-05  Adam Barth  <abarth@webkit.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        SheriffBot should include blamelist when posting to bugs
     6        https://bugs.webkit.org/show_bug.cgi?id=37113
     7
     8        When posting on bugs, we should include the full list of SVN revisions
     9        that caused the regression to folks have a better sense of whether they
     10        are to blame.
     11
     12        * Scripts/webkitpy/tool/bot/sheriff.py:
     13        * Scripts/webkitpy/tool/bot/sheriff_unittest.py:
     14        * Scripts/webkitpy/tool/commands/sheriffbot.py:
     15        * Scripts/webkitpy/tool/commands/sheriffbot_unittest.py:
     16        * Scripts/webkitpy/tool/commands/upload_unittest.py:
     17        * Scripts/webkitpy/tool/mocktool.py:
     18
    1192010-04-05  Chris Jerdonek  <cjerdonek@webkit.org>
    220
  • trunk/WebKitTools/Scripts/webkitpy/tool/bot/sheriff.py

    r57040 r57115  
    9696            log("Failed to create-rollout.")
    9797
    98     def post_blame_comment_on_bug(self, commit_info, builders):
     98    def post_blame_comment_on_bug(self, commit_info, builders, blame_list):
    9999        if not commit_info.bug_id():
    100100            return
     
    102102            view_source_url(commit_info.revision()),
    103103            join_with_separators([builder.name() for builder in builders]))
     104        if len(blame_list) > 1:
     105            comment += "\nThe following changes are on the blame list:\n"
     106            comment += "\n".join(map(view_source_url, blame_list))
    104107        self._tool.bugs.post_comment_to_bug(commit_info.bug_id(),
    105108                                            comment,
  • trunk/WebKitTools/Scripts/webkitpy/tool/bot/sheriff_unittest.py

    r57040 r57115  
    3131
    3232from webkitpy.common.net.buildbot import Builder
     33from webkitpy.common.system.outputcapture import OutputCapture
    3334from webkitpy.thirdparty.mock import Mock
    3435from webkitpy.tool.bot.sheriff import Sheriff
     
    5657
    5758    def test_post_blame_comment_on_bug(self):
    58         sheriff = Sheriff(MockTool(), MockSheriffBot())
    59         builders = [
    60             Builder("Foo", None),
    61             Builder("Bar", None),
    62         ]
    63         commit_info = Mock()
    64         commit_info.bug_id = lambda: None
    65         commit_info.revision = lambda: 4321
    66         # Should do nothing with no bug_id
    67         sheriff.post_blame_comment_on_bug(commit_info, builders)
    68         # Should try to post a comment to the bug, but MockTool.bugs does nothing.
    69         commit_info.bug_id = lambda: 1234
    70         sheriff.post_blame_comment_on_bug(commit_info, builders)
     59        def run():
     60            sheriff = Sheriff(MockTool(), MockSheriffBot())
     61            builders = [
     62                Builder("Foo", None),
     63                Builder("Bar", None),
     64            ]
     65            commit_info = Mock()
     66            commit_info.bug_id = lambda: None
     67            commit_info.revision = lambda: 4321
     68            # Should do nothing with no bug_id
     69            sheriff.post_blame_comment_on_bug(commit_info, builders, [])
     70            sheriff.post_blame_comment_on_bug(commit_info, builders, [2468, 5646])
     71            # Should try to post a comment to the bug, but MockTool.bugs does nothing.
     72            commit_info.bug_id = lambda: 1234
     73            sheriff.post_blame_comment_on_bug(commit_info, builders, [])
     74            sheriff.post_blame_comment_on_bug(commit_info, builders, [3432])
     75            sheriff.post_blame_comment_on_bug(commit_info, builders, [841, 5646])
     76
     77        expected_stderr = u"MOCK bug comment: bug_id=1234, cc=['watcher@example.com']\n--- Begin comment ---\\http://trac.webkit.org/changeset/4321 might have broken Foo and Bar\n--- End comment ---\n\nMOCK bug comment: bug_id=1234, cc=['watcher@example.com']\n--- Begin comment ---\\http://trac.webkit.org/changeset/4321 might have broken Foo and Bar\n--- End comment ---\n\nMOCK bug comment: bug_id=1234, cc=['watcher@example.com']\n--- Begin comment ---\\http://trac.webkit.org/changeset/4321 might have broken Foo and Bar\nThe following changes are on the blame list:\nhttp://trac.webkit.org/changeset/841\nhttp://trac.webkit.org/changeset/5646\n--- End comment ---\n\n"
     78        OutputCapture().assert_outputs(self, run, expected_stderr=expected_stderr)
  • trunk/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot.py

    r57040 r57115  
    5555        self.tool.ensure_irc_connected(self._irc_bot.irc_delegate())
    5656
    57     def work_item_log_path(self, failure_info):
    58         return os.path.join("%s-logs" % self.name, "%s.log" % failure_info["svn_revision"])
     57    def work_item_log_path(self, new_failures):
     58        return os.path.join("%s-logs" % self.name, "%s.log" % new_failures.keys()[0])
    5959
    6060    def next_work_item(self):
    6161        self._irc_bot.process_pending_messages()
    6262        self._update()
     63        new_failures = {}
    6364        for svn_revision, builders in self.tool.buildbot.revisions_causing_failures().items():
    6465            if self.tool.status_server.svn_revision(svn_revision):
     
    6667                # https://bugs.webkit.org/show_bug.cgi?id=36581
    6768                continue
    68             return {
    69                 "svn_revision": svn_revision,
    70                 "builders": builders,
    71                 # FIXME: Sheriff._rollout_reason needs Build objects which we could pass here.
    72             }
    73         return None
     69            new_failures[svn_revision] = builders
     70        return new_failures
    7471
    75     def should_proceed_with_work_item(self, failure_info):
     72    def should_proceed_with_work_item(self, new_failures):
    7673        # Currently, we don't have any reasons not to proceed with work items.
    7774        return True
    7875
    79     def process_work_item(self, failure_info):
    80         svn_revision = failure_info["svn_revision"]
    81         builders = failure_info["builders"]
     76    def process_work_item(self, new_failures):
     77        blame_list = new_failures.keys()
     78        for svn_revision, builders in new_failures.items():
     79            commit_info = self.tool.checkout().commit_info_for_revision(svn_revision)
     80            self._sheriff.post_irc_warning(commit_info, builders)
     81            self._sheriff.post_blame_comment_on_bug(commit_info,
     82                                                    builders,
     83                                                    blame_list)
     84            self._sheriff.post_automatic_rollout_patch(commit_info, builders)
    8285
    83         self._update()
    84         commit_info = self.tool.checkout().commit_info_for_revision(svn_revision)
    85         self._sheriff.post_irc_warning(commit_info, builders)
    86         self._sheriff.post_blame_comment_on_bug(commit_info, builders)
    87         self._sheriff.post_automatic_rollout_patch(commit_info, builders)
    88 
    89         for builder in builders:
    90             self.tool.status_server.update_svn_revision(svn_revision, builder.name())
     86            for builder in builders:
     87                self.tool.status_server.update_svn_revision(svn_revision,
     88                                                            builder.name())
    9189        return True
    9290
    93     def handle_unexpected_error(self, failure_info, message):
     91    def handle_unexpected_error(self, new_failures, message):
    9492        log(message)
    9593
  • trunk/WebKitTools/Scripts/webkitpy/tool/commands/sheriffbot_unittest.py

    r56947 r57115  
    3737    def test_sheriff_bot(self):
    3838        mock_work_item = {
    39             "svn_revision": 29837,
    40             "builders": [mock_builder]
     39            29837: [mock_builder],
    4140        }
    4241        expected_stderr = {
    4342            "begin_work_queue": "CAUTION: sheriff-bot will discard all local changes in \"%s\"\nRunning WebKit sheriff-bot.\n" % os.getcwd(),
    4443            "next_work_item": "",
    45             "process_work_item": "MOCK: irc.post: abarth, darin, eseidel: http://trac.webkit.org/changeset/29837 might have broken Mock builder name (Tests)\n",
     44            "process_work_item": "MOCK: irc.post: abarth, darin, eseidel: http://trac.webkit.org/changeset/29837 might have broken Mock builder name (Tests)\nMOCK bug comment: bug_id=42, cc=['webkit-bot-watchers@googlegroups.com', 'abarth@webkit.org', 'eric@webkit.org']\n--- Begin comment ---\\http://trac.webkit.org/changeset/29837 might have broken Mock builder name (Tests)\n--- End comment ---\n\n",
    4645            "handle_unexpected_error": "Mock error message\n"
    4746        }
  • trunk/WebKitTools/Scripts/webkitpy/tool/commands/upload_unittest.py

    r56592 r57115  
    7878        options = Mock()
    7979        options.bug_id = 42
    80         expected_stderr = """Bug: <http://example.com/42> Bug with two r+'d and cq+'d patches, one of which has an invalid commit-queue setter.
    81 Revision: 9876
    82 MOCK: user.open_url: http://example.com/42
    83 Adding comment to Bug 42.
    84 """
     80        options.comment = "MOCK comment"
     81        expected_stderr = "Bug: <http://example.com/42> Bug with two r+'d and cq+'d patches, one of which has an invalid commit-queue setter.\nRevision: 9876\nMOCK: user.open_url: http://example.com/42\nAdding comment to Bug 42.\nMOCK bug comment: bug_id=42, cc=None\n--- Begin comment ---\\MOCK comment\n\nCommitted r9876: <http://trac.webkit.org/changeset/9876>\n--- End comment ---\n\n"
    8582        self.assert_execute_outputs(MarkBugFixed(), [], expected_stderr=expected_stderr, tool=tool, options=options)
    8683
  • trunk/WebKitTools/Scripts/webkitpy/tool/mocktool.py

    r57035 r57115  
    292292        return "%s/%s%s" % (self.bug_server_url, attachment_id, action_param)
    293293
     294    def post_comment_to_bug(self, bug_id, comment_text, cc=None):
     295        log("MOCK bug comment: bug_id=%s, cc=%s\n--- Begin comment ---\%s\n--- End comment ---\n" % (
     296            bug_id, cc, comment_text))
     297
    294298
    295299class MockBuildBot(object):
Note: See TracChangeset for help on using the changeset viewer.