Log Message
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):
Modified Paths
- trunk/Tools/ChangeLog
- trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py
- trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_mock.py
- trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py
- trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py
- trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py
- trunk/Tools/Scripts/webkitpy/tool/commands/queries.py
Diff
Modified: trunk/Tools/ChangeLog (104753 => 104754)
--- trunk/Tools/ChangeLog 2012-01-11 23:42:27 UTC (rev 104753)
+++ trunk/Tools/ChangeLog 2012-01-11 23:46:53 UTC (rev 104754)
@@ -1,3 +1,35 @@
+2012-01-11 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Unreviewed, fix build bustage on win32 introduced by r104725.
Modified: trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py (104753 => 104754)
--- trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py 2012-01-11 23:42:27 UTC (rev 104753)
+++ trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot.py 2012-01-11 23:46:53 UTC (rev 104754)
@@ -436,20 +436,51 @@
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)
builders = [builder for builder in self.builders() if compiled_builder_name_regex.search(builder.name())]
@@ -458,13 +489,21 @@
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
Modified: trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_mock.py (104753 => 104754)
--- trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_mock.py 2012-01-11 23:42:27 UTC (rev 104753)
+++ trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_mock.py 2012-01-11 23:46:53 UTC (rev 104754)
@@ -91,7 +91,7 @@
]
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
Modified: trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py (104753 => 104754)
--- trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py 2012-01-11 23:42:27 UTC (rev 104753)
+++ trunk/Tools/Scripts/webkitpy/common/net/buildbot/buildbot_unittest.py 2012-01-11 23:46:53 UTC (rev 104754)
@@ -358,6 +358,11 @@
<td class="success">failure</td>
</tr>
<tr class="">
+ <td>Jan 10 11:58</td>
+ <td><span class="revision" title="Revision ??"><a href=""
+ <td class="retry">retry</td>
+ </tr>
+ <tr class="">
<td>Jan 10 14:51</td>
<td><span class="revision" title="Revision 104633"><a href=""
<td class="failure">failure</td>
@@ -365,17 +370,70 @@
</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,21 +443,22 @@
# 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:
return "correct build"
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py (104753 => 104754)
--- trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py 2012-01-11 23:42:27 UTC (rev 104753)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/irc_command.py 2012-01-11 23:46:53 UTC (rev 104754)
@@ -57,7 +57,9 @@
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):
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py (104753 => 104754)
--- trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py 2012-01-11 23:42:27 UTC (rev 104753)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/sheriffircbot_unittest.py 2012-01-11 23:46:53 UTC (rev 104754)
@@ -86,8 +86,8 @@
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"
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queries.py (104753 => 104754)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queries.py 2012-01-11 23:42:27 UTC (rev 104753)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queries.py 2012-01-11 23:46:53 UTC (rev 104754)
@@ -130,6 +130,7 @@
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])
_______________________________________________ webkit-changes mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
