last-green-revision should report the revision that succeeded on all bots
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jan 2012 23:46:53 +0000 (23:46 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Jan 2012 23:46:53 +0000 (23:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=76109

Reviewed by Adam Barth.

Add an algorithm to find the last known good revision among the specified bots. For a revision to be
considered green by this algorithm, all matching builders must have a successful run at the revision
or two consecutive successful runs before and after the revision.

Also fixed a bug in irc_command that the result wasn't posted on IRC properly and a bug in queries.py
that resulted in an exception when the user didn't pass BUILDER_NAME argument.

* Scripts/webkitpy/common/net/buildbot/buildbot.py:
(BuildBot._revisions_for_builder):
(BuildBot):
(BuildBot._find_green_revision):
(BuildBot.last_green_revision):
* Scripts/webkitpy/common/net/buildbot/buildbot_mock.py:
(MockBuildBot.last_green_revision):
* Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py:
(test_revisions_for_builder):
(test_find_green_revision):
(test_last_green_revision):
* Scripts/webkitpy/tool/bot/irc_command.py:
(LastGreenRevision.execute):
* Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py:
(SheriffIRCBotTest.test_lgr):
* Scripts/webkitpy/tool/commands/queries.py:
(LastGreenRevision):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@104754 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py
Tools/Scripts/webkitpy/common/net/buildbot/buildbot_mock.py
Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py
Tools/Scripts/webkitpy/tool/bot/irc_command.py
Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py
Tools/Scripts/webkitpy/tool/commands/queries.py

index 11bd29cd46acc64bbd60b35c26b0d444ba1355de..c9fd332915b58b91a78a5ae5a4f719e207bb92a5 100644 (file)
@@ -1,3 +1,35 @@
+2012-01-11  Ryosuke Niwa  <rniwa@webkit.org>
+
+        last-green-revision should report the revision that succeeded on all bots
+        https://bugs.webkit.org/show_bug.cgi?id=76109
+
+        Reviewed by Adam Barth.
+
+        Add an algorithm to find the last known good revision among the specified bots. For a revision to be
+        considered green by this algorithm, all matching builders must have a successful run at the revision
+        or two consecutive successful runs before and after the revision.
+
+        Also fixed a bug in irc_command that the result wasn't posted on IRC properly and a bug in queries.py
+        that resulted in an exception when the user didn't pass BUILDER_NAME argument.
+
+        * Scripts/webkitpy/common/net/buildbot/buildbot.py:
+        (BuildBot._revisions_for_builder):
+        (BuildBot):
+        (BuildBot._find_green_revision):
+        (BuildBot.last_green_revision):
+        * Scripts/webkitpy/common/net/buildbot/buildbot_mock.py:
+        (MockBuildBot.last_green_revision):
+        * Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py:
+        (test_revisions_for_builder):
+        (test_find_green_revision):
+        (test_last_green_revision):
+        * Scripts/webkitpy/tool/bot/irc_command.py:
+        (LastGreenRevision.execute):
+        * Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py:
+        (SheriffIRCBotTest.test_lgr):
+        * Scripts/webkitpy/tool/commands/queries.py:
+        (LastGreenRevision):
+
 2012-01-11  Dirk Pranke  <dpranke@chromium.org>
 
         Unreviewed, fix build bustage on win32 introduced by r104725.
index 8fed4b1b31a2e179bec240a1912658e5b9b10812..e84fcbffdbe7ea478cd8258894509637d269063f 100644 (file)
@@ -436,19 +436,50 @@ class BuildBot(object):
         builder_page_url = "%s/builders/%s?numbuilds=100" % (self.buildbot_url, urllib2.quote(builder.name()))
         return urllib2.urlopen(builder_page_url)
 
-    def _green_revision_for_builder(self, builder):
+    def _revisions_for_builder(self, builder):
         soup = BeautifulSoup(self._fetch_builder_page(builder))
-        revision = None
-        number_of_revisions = 0
+        revisions = []
         for status_row in soup.find('table').findAll('tr'):
             revision_anchor = status_row.find('a')
             table_cells = status_row.findAll('td')
-            if not revision_anchor or not table_cells or len(table_cells) < 3 or not table_cells[2].string:
+            if not table_cells or len(table_cells) < 3 or not table_cells[2].string:
                 continue
-            number_of_revisions += 1
-            if revision == None and 'success' in table_cells[2].string:
-                revision = int(revision_anchor.string)
-        return revision, number_of_revisions
+            if revision_anchor and revision_anchor.string and re.match(r'^\d+$', revision_anchor.string):
+                revisions.append((int(revision_anchor.string), 'success' in table_cells[2].string))
+        return revisions
+
+    def _find_green_revision(self, builder_revisions):
+        revision_statuses = {}
+        for builder in builder_revisions:
+            for revision, succeeded in builder_revisions[builder]:
+                revision_statuses.setdefault(revision, set())
+                if succeeded and revision_statuses[revision] != None:
+                    revision_statuses[revision].add(builder)
+                else:
+                    revision_statuses[revision] = None
+
+        # In descending order, look for a revision X with successful builds
+        # Once we found X, check if remaining builders succeeded in the neighborhood of X.
+        revisions_in_order = sorted(revision_statuses.keys(), reverse=True)
+        for i, revision in enumerate(revisions_in_order):
+            if not revision_statuses[revision]:
+                continue
+
+            builders_succeeded_in_future = set()
+            for future_revision in sorted(revisions_in_order[:i + 1]):
+                if not revision_statuses[future_revision]:
+                    break
+                builders_succeeded_in_future = builders_succeeded_in_future.union(revision_statuses[future_revision])
+
+            builders_succeeded_in_past = set()
+            for past_revision in revisions_in_order[i:]:
+                if not revision_statuses[past_revision]:
+                    break
+                builders_succeeded_in_past = builders_succeeded_in_past.union(revision_statuses[past_revision])
+
+            if len(builders_succeeded_in_future) == len(builder_revisions) and len(builders_succeeded_in_past) == len(builder_revisions):
+                return revision
+        return None
 
     def last_green_revision(self, builder_name_regex):
         compiled_builder_name_regex = re.compile(builder_name_regex, flags=re.IGNORECASE)
@@ -458,13 +489,21 @@ class BuildBot(object):
         elif not len(builders):
             return '"%s" doesn\'t match any bot' % builder_name_regex
 
+        builder_revisions = {}
+        for builder in builders:
+            builder_revisions[builder] = self._revisions_for_builder(builder)
+
         result = ''
+        revision_with_all_builders = self._find_green_revision(builder_revisions)
+        if revision_with_all_builders:
+            result += 'The last known green revision is %d\n' % revision_with_all_builders
+
         for builder in builders:
-            revision, number_of_revisions = self._green_revision_for_builder(builder)
-            if revision == None:
-                result += '%s has had no green revision in the last %d runs' % (builder.name(), number_of_revisions)
+            succeeded_revisions = [revision for revision, succeeded in builder_revisions[builder] if succeeded]
+            if not succeeded_revisions:
+                result += '%s has had no green revision in the last %d runs' % (builder.name(), len(builder_revisions[builder]))
             else:
-                result += '%s: %d' % (builder.name(), revision)
+                result += '%s: %d' % (builder.name(), max(succeeded_revisions))
             result += "\n"
 
         return result
index b487728749ffd3fb53c59ba4b6560a50e87403cc..34f7149854fca84d01c890ee18ca03fd2267e5c7 100644 (file)
@@ -91,7 +91,7 @@ class MockBuildBot(object):
         ]
 
     def last_green_revision(self, builder_name):
-        return builder_name + ': ' + str(9479)
+        return builder_name + ' 1: ' + str(9479) + '\n' + builder_name + ' 2: ' + str(9400)
 
     def light_tree_on_fire(self):
         self._mock_builder2_status["is_green"] = False
index 7ce257770ebbd7a0398ab118c1d3d1fd1cbb665f..7e22cf5ffa1e4f61636d4b4e68fbafae25ebbf54 100644 (file)
@@ -357,6 +357,11 @@ class BuildBotTest(unittest.TestCase):
         <td><span class="revision" title="Revision 104635"><a href="http://trac.webkit.org/changeset/104635">104635</a></span></td>
         <td class="success">failure</td>
       </tr>
+      <tr class="">
+          <td>Jan 10 11:58</td>
+          <td><span class="revision" title="Revision ??"><a href="http://trac.webkit.org/changeset/%3F%3F">??</a></span></td>
+          <td class="retry">retry</td>
+        </tr>
       <tr class="">
         <td>Jan 10 14:51</td>
         <td><span class="revision" title="Revision 104633"><a href="http://trac.webkit.org/changeset/104633">104633</a></span></td>
@@ -365,16 +370,69 @@ class BuildBotTest(unittest.TestCase):
     </table>
     </body>'''
 
-    def test_green_revision_for_builder(self):
+    def test_revisions_for_builder(self):
         buildbot = BuildBot()
         buildbot._fetch_builder_page = lambda builder: builder.page
         builder_with_success = Builder('Some builder', None)
         builder_with_success.page = self._fake_builder_page
-        self.assertEqual(buildbot._green_revision_for_builder(builder_with_success), (104635, 4))
+        self.assertEqual(buildbot._revisions_for_builder(builder_with_success), [(104643, False), (104636, False), (104635, True), (104633, False)])
 
         builder_without_success = Builder('Some builder', None)
         builder_without_success.page = self._fake_builder_page_without_success
-        self.assertEqual(buildbot._green_revision_for_builder(builder_without_success), (None, 4))
+        self.assertEqual(buildbot._revisions_for_builder(builder_without_success), [(104643, False), (104636, False), (104635, False), (104633, False)])
+
+    def test_find_green_revision(self):
+        buildbot = BuildBot()
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, True), (3, True)],
+            'Builder 2': [(1, True), (3, False)],
+            'Builder 3': [(1, True), (3, True)],
+        }), 1)
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, False), (3, True)],
+            'Builder 2': [(1, True), (3, True)],
+            'Builder 3': [(1, True), (3, True)],
+        }), 3)
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, True), (2, True)],
+            'Builder 2': [(1, False), (2, True), (3, True)],
+            'Builder 3': [(1, True), (3, True)],
+        }), None)
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, True), (2, True)],
+            'Builder 2': [(1, True), (2, True), (3, True)],
+            'Builder 3': [(1, True), (3, True)],
+        }), 2)
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, False), (2, True)],
+            'Builder 2': [(1, True), (3, True)],
+            'Builder 3': [(1, True), (3, True)],
+        }), None)
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, True), (3, True)],
+            'Builder 2': [(1, False), (2, True), (3, True), (4, True)],
+            'Builder 3': [(2, True), (4, True)],
+        }), 3)
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, True), (3, True)],
+            'Builder 2': [(1, False), (2, True), (3, True), (4, False)],
+            'Builder 3': [(2, True), (4, True)],
+        }), None)
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, True), (3, True)],
+            'Builder 2': [(1, False), (2, True), (3, True), (4, False)],
+            'Builder 3': [(2, True), (3, True), (4, True)],
+        }), 3)
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, True), (2, True)],
+            'Builder 2': [],
+            'Builder 3': [(1, True), (2, True)],
+        }), None)
+        self.assertEqual(buildbot._find_green_revision({
+            'Builder 1': [(1, True), (3, False), (5, True), (10, True), (12, False)],
+            'Builder 2': [(1, True), (3, False), (7, True), (9, True), (12, False)],
+            'Builder 3': [(1, True), (3, True), (7, True), (11, False), (12, True)],
+        }), 7)
 
     def test_last_green_revision(self):
         buildbot = BuildBot()
@@ -385,20 +443,21 @@ class BuildBotTest(unittest.TestCase):
         # Revision, is_green
         # Ordered from newest (highest number) to oldest.
         fake_builder1 = Builder("Fake Builder 1", None)
-        fake_builder1.revision = 1234
-        fake_builder1.revision_count = 3
+        fake_builder1.revisions = [(1, True), (3, False), (5, True), (10, True), (12, False)]
         fake_builder2 = Builder("Fake Builder 2", None)
-        fake_builder2.revision = 1230
-        fake_builder2.revision_count = 4
+        fake_builder2.revisions = [(1, True), (3, False), (7, True), (9, True), (12, False)]
         some_builder = Builder("Some Builder", None)
-        some_builder.revision = None
-        some_builder.revision_count = 3
+        some_builder.revisions = [(1, True), (3, True), (7, True), (11, False), (12, True)]
 
         buildbot.builders = lambda: [fake_builder1, fake_builder2, some_builder]
-        buildbot._green_revision_for_builder = lambda builder: (builder.revision, builder.revision_count)
+        buildbot._revisions_for_builder = lambda builder: builder.revisions
         buildbot._latest_builds_from_builders = mock_builds_from_builders
         self.assertEqual(buildbot.last_green_revision(''),
-            "Fake Builder 1: 1234\nFake Builder 2: 1230\nSome Builder has had no green revision in the last 3 runs\n")
+            "The last known green revision is 7\nFake Builder 1: 10\nFake Builder 2: 9\nSome Builder: 12\n")
+
+        some_builder.revisions = [(1, False), (3, False)]
+        self.assertEqual(buildbot.last_green_revision(''),
+            "Fake Builder 1: 10\nFake Builder 2: 9\nSome Builder has had no green revision in the last 2 runs\n")
 
     def _fetch_build(self, build_number):
         if build_number == 5:
index 2c7f832fb696de3ddc14f0c3805bcd0806b758e6..0ae60428a750d450953381bdd9b27aabb5a62cd9 100644 (file)
@@ -57,7 +57,9 @@ class LastGreenRevision(IRCCommand):
     def execute(self, nick, args, tool, sheriff):
         if not args:
             return "%s: Usage: last-green-revision BUILDER_NAME" % nick
-        return "%s: %s" % (nick, tool.buildbot.last_green_revision(args[0]))
+        result = tool.buildbot.last_green_revision(' '.join(args))
+        for line in result.split('\n'):
+            tool.irc().post("%s: %s" % (nick, line))
 
 
 class Restart(IRCCommand):
index e1501f6027c53ed304fbdfb1c5dcbca8919ce26c..2de64eda685a4bc27b574a2769de936efe0787a5 100644 (file)
@@ -86,8 +86,8 @@ class SheriffIRCBotTest(unittest.TestCase):
         OutputCapture().assert_outputs(self, run, args=["help"], expected_stderr=expected_stderr)
 
     def test_lgr(self):
-        expected_stderr = "MOCK: irc.post: mock_nick: Builder: 9479\n"
-        OutputCapture().assert_outputs(self, run, args=["last-green-revision Builder"], expected_stderr=expected_stderr)
+        expected_stderr = "MOCK: irc.post: mock_nick: Some Builder 1: 9479\nMOCK: irc.post: mock_nick: Some Builder 2: 9400\n"
+        OutputCapture().assert_outputs(self, run, args=["last-green-revision Some Builder"], expected_stderr=expected_stderr)
 
     def test_restart(self):
         expected_stderr = "MOCK: irc.post: Restarting...\n"
index 5a14afde3d34357db7d14c1c2c00ac298ecf5ba8..cf7eb9daaca8f0a844e5dc5a6e35d680d5e506dc 100644 (file)
@@ -130,6 +130,7 @@ class PatchesToReview(AbstractDeclarativeCommand):
 class LastGreenRevision(AbstractDeclarativeCommand):
     name = "last-green-revision"
     help_text = "Prints the last known good revision"
+    argument_names = "BUILDER_NAME"
 
     def execute(self, options, args, tool):
         print self._tool.buildbot.last_green_revision(args[0])