From c440d56e9295c4337d40c1a1f69bc9192ab2a5d4 Mon Sep 17 00:00:00 2001 From: "phoglund@webrtc.org" Date: Tue, 17 Apr 2012 08:49:10 +0000 Subject: [PATCH] Rewired the oath2 symlink and updated tgrid_parser to the new Build Bot version's tgrid syntax. Added back the gviz_api to the tools deps - will remove it from the main project DEPS in a different patch. Rewired those symlinks too. Made the build status loader algorithm more scalable. It read in the whole database on each load which is probably unsustainable in the long run. Also, it will now forget bots that have offline for more than ~5 revisions. Fixed a bug in the coverage tracker. BUG= TEST= Review URL: https://webrtc-codereview.appspot.com/486002 git-svn-id: http://webrtc.googlecode.com/svn/trunk@2039 4adac7df-926f-26a2-2b94-8c16560cd09d --- tools/.gitignore | 1 + tools/DEPS | 4 + .../dashboard/add_coverage_data.py | 2 +- tools/quality_tracking/dashboard/gviz_api.py | 2 +- .../dashboard/load_build_status.py | 22 +- tools/quality_tracking/oauth2 | 2 +- tools/quality_tracking/tgrid_parser.py | 19 +- tools/quality_tracking/tgrid_parser_test.py | 563 +++++++++++++++--- 8 files changed, 503 insertions(+), 112 deletions(-) diff --git a/tools/.gitignore b/tools/.gitignore index 491476a820..0bd04f58ed 100644 --- a/tools/.gitignore +++ b/tools/.gitignore @@ -13,6 +13,7 @@ /continuous_build/depot_tools /third_party/gaeunit /third_party/oauth2 +/third_party/google-visualization-python # Buildbot creates these files: .manhole diff --git a/tools/DEPS b/tools/DEPS index 68dd43abfa..a1b57b53fd 100644 --- a/tools/DEPS +++ b/tools/DEPS @@ -22,6 +22,10 @@ deps = { # Chromium buildbot scripts needs their own depot_tools. "tools/continuous_build/depot_tools": Var("chromium_trunk") + "/tools/depot_tools", + + # Used by tools/quality_tracking/dashboard and tools/python_charts. + "tools/third_party/google-visualization-python": + "http://google-visualization-python.googlecode.com/svn/trunk/@15", } hooks = [ diff --git a/tools/quality_tracking/dashboard/add_coverage_data.py b/tools/quality_tracking/dashboard/add_coverage_data.py index 7f3a1a54cc..ade5a6a829 100644 --- a/tools/quality_tracking/dashboard/add_coverage_data.py +++ b/tools/quality_tracking/dashboard/add_coverage_data.py @@ -77,7 +77,7 @@ class AddCoverageData(oauth_post_request_handler.OAuthPostRequestHandler): def _parse_percentage(self, key): """Parses out a percentage value from the request.""" - value = float(self.request.get(key)) + percentage = float(self.request.get(key)) if percentage < 0.0 or percentage > 100.0: raise ValueError('%s is not a valid percentage.' % string_value) return percentage diff --git a/tools/quality_tracking/dashboard/gviz_api.py b/tools/quality_tracking/dashboard/gviz_api.py index f3a22fccb7..c9dca90fa4 120000 --- a/tools/quality_tracking/dashboard/gviz_api.py +++ b/tools/quality_tracking/dashboard/gviz_api.py @@ -1 +1 @@ -../../../third_party/google-visualization-python/gviz_api.py \ No newline at end of file +../../third_party/google-visualization-python/gviz_api.py \ No newline at end of file diff --git a/tools/quality_tracking/dashboard/load_build_status.py b/tools/quality_tracking/dashboard/load_build_status.py index ab991e26ba..d4265f92fd 100644 --- a/tools/quality_tracking/dashboard/load_build_status.py +++ b/tools/quality_tracking/dashboard/load_build_status.py @@ -38,10 +38,18 @@ class BuildStatusLoader: The statuses OK, failed and warnings are considered to be conclusive. - The two most recent revisions are considered. The set of bots returned - will therefore be the bots that were reported the two most recent - revisions. This script will therefore adapt automatically to any changes - in the set of available bots. + The algorithm looks at the 100 most recent status entries, which should + give data on roughly the last five revisions if the number of bots stay + around 20 (The number 100 should be increased if the number of bots + increases significantly). This should give us enough data to get a + conclusive build status for all active bots. + + With this limit, the algorithm will adapt automatically if a bot is + decommissioned - it will eventually disappear. The limit should not be + too high either since we will perhaps remember offline bots too long, + which could be confusing. The algorithm also adapts automatically to new + bots - these show up immediately if they get a build status for a recent + revision. Returns: A list of BuildStatusData entities with one entity per bot. @@ -49,7 +57,8 @@ class BuildStatusLoader: build_status_entries = db.GqlQuery('SELECT * ' 'FROM BuildStatusData ' - 'ORDER BY revision DESC ') + 'ORDER BY revision DESC ' + 'LIMIT 100') bots_to_latest_conclusive_entry = dict() for entry in build_status_entries: @@ -85,7 +94,8 @@ class BuildStatusLoader: Implementation note: The data store fetches stuff as we go, so we won't read in the whole status table unless the LKGR is right at the end or - we don't have a LKGR. + we don't have a LKGR. Bots that are offline do not affect the LKGR + computation (e.g. they are not considered to be failed). """ build_status_entries = db.GqlQuery('SELECT * ' 'FROM BuildStatusData ' diff --git a/tools/quality_tracking/oauth2 b/tools/quality_tracking/oauth2 index a4d1dde741..63ab40baf9 120000 --- a/tools/quality_tracking/oauth2 +++ b/tools/quality_tracking/oauth2 @@ -1 +1 @@ -../../third_party/oauth2/oauth2/ \ No newline at end of file +../third_party/oauth2/oauth2 \ No newline at end of file diff --git a/tools/quality_tracking/tgrid_parser.py b/tools/quality_tracking/tgrid_parser.py index 0889e0c462..319c64ec4a 100644 --- a/tools/quality_tracking/tgrid_parser.py +++ b/tools/quality_tracking/tgrid_parser.py @@ -8,19 +8,27 @@ # in the file PATENTS. All contributing project authors may # be found in the AUTHORS file in the root of the source tree. -"""Contains functions for parsing the build master's transposed grid page.""" +"""Contains functions for parsing the build master's transposed grid page. + + Compatible with build bot 0.8.4 P1. +""" __author__ = 'phoglund@webrtc.org (Patrik Höglund)' import re +# This is here to work around a buggy build bot status message which makes no +# sense, but which means the build failed when the slave was lost. +BB_084_P1_BUGGY_STATUS = 'build
successful
exception
slave
lost' + + class FailedToParseBuildStatus(Exception): pass def _map_status(status): - if status == 'exception': + if status == 'exception' or status == BB_084_P1_BUGGY_STATUS: return 'failed' return status @@ -38,8 +46,9 @@ def _parse_builds(revision, html): result = {} for match in re.finditer('.*?' - '(OK|failed|building|warnings|exception)' - '.*?', + '(OK|failed|building|warnings|exception|' + + BB_084_P1_BUGGY_STATUS + ')' + '.*?.*?', html, re.DOTALL): revision_and_bot_name = revision + "--" + match.group(1) build_number_and_status = match.group(2) + "--" + _map_status( @@ -72,7 +81,7 @@ def parse_tgrid_page(html): """ result = {} - for match in re.finditer('(\d+)(.*?)', + for match in re.finditer('(\d+) (.*?)', html, re.DOTALL): revision = match.group(1) builds_for_revision_html = match.group(2) diff --git a/tools/quality_tracking/tgrid_parser_test.py b/tools/quality_tracking/tgrid_parser_test.py index d427945be8..288376ce9a 100755 --- a/tools/quality_tracking/tgrid_parser_test.py +++ b/tools/quality_tracking/tgrid_parser_test.py @@ -8,7 +8,10 @@ # in the file PATENTS. All contributing project authors may # be found in the AUTHORS file in the root of the source tree. -"""Contains functions for parsing the build master's transposed grid page.""" +"""Test the tgrid parser. + + Compatible with build bot 0.8.4 P1. +""" __author__ = 'phoglund@webrtc.org (Patrik Höglund)' @@ -18,89 +21,427 @@ import tgrid_parser SAMPLE_FILE = """ - - - - Buildbot - - + + + + + Buildbot + + + + + +
+ +
+

Transposed Grid View

- + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
1570warnings -
-make chrome
- OK - OK - OK - OK - OK - OK - OK - OK - OK - OK - OK - OK
1571warnings -
-make chrome
- OK - OK - OK - OK - OK - OK - OK - OK - failed
-voe_auto_test
- OK - building - OK
WebRTC + + + + Android + AndroidNDK + Chrome + ChromeOS + Linux32DBG + Linux32Release + Linux64DBG + Linux64DBG-GCC4.6 + Linux64Release + LinuxClang + LinuxValgrind + LinuxVideoTest + MacOS32DBG + MacOS32Release
(building)
+ Win32Debug + Win32Release
(building)
2006 + OK + + OK + + warnings + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK +   + OK +   + OK +  
2007 + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + failed
voe_auto_test
+
+ OK + + OK + + OK + + OK +
2008 + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK +
2010 + OK + + OK +   + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK +
2011 + OK + + OK +   + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + OK + + building + + OK + + building +
latest + building + + OK +
- + +
+ """ MINIMAL_OK = """ -1570 +1570 OK @@ -108,16 +449,16 @@ MINIMAL_OK = """ MINIMAL_FAIL = """ -1573 +1573 -failed
-voe_auto_test + failed
voe_auto_test
+ """ MINIMAL_BUILDING = """ -1576 +1576 building voe_auto_test @@ -126,7 +467,7 @@ voe_auto_test MINIMAL_WARNED = """ -1576 +1576 warnings
make chrome @@ -135,13 +476,22 @@ make chrome MINIMAL_EXCEPTION = """ -1576 +1576 exception
Sync """ +MINIMAL_EXCEPTION_SLAVE_LOST = """ + +1576 + + build
successful
exception
slave
lost
+ + +""" + class TGridParserTest(unittest.TestCase): def test_parser_throws_exception_on_empty_html(self): self.assertRaises(tgrid_parser.FailedToParseBuildStatus, @@ -163,7 +513,7 @@ class TGridParserTest(unittest.TestCase): first_mapping = result.items()[0] self.assertEqual('1573--LinuxVideoTest', first_mapping[0]) - self.assertEqual('347--failed', first_mapping[1]) + self.assertEqual('731--failed', first_mapping[1]) def test_parser_finds_building_bot(self): result = tgrid_parser.parse_tgrid_page(MINIMAL_BUILDING) @@ -192,34 +542,51 @@ class TGridParserTest(unittest.TestCase): self.assertEqual('1576--Chrome', first_mapping[0]) self.assertEqual('109--failed', first_mapping[1]) + def test_parser_finds_exception_slave_lost_and_maps_to_failed(self): + # This is to work around a bug in build bot 0.8.4p1 where it may say that + # the build was successful AND the slave was lost. In this case the build + # is not actually successful, so treat it as such. + result = tgrid_parser.parse_tgrid_page(MINIMAL_EXCEPTION_SLAVE_LOST) - def test_parser_finds_all_bots_and_revisions(self): + self.assertEqual(1, len(result), 'There is only one bot in the sample.') + first_mapping = result.items()[0] + + self.assertEqual('1576--LinuxValgrind', first_mapping[0]) + self.assertEqual('324--failed', first_mapping[1]) + + def test_parser_finds_all_bots_and_revisions_except_forced_builds(self): result = tgrid_parser.parse_tgrid_page(SAMPLE_FILE) - # 2 * 13 = 26 bots in sample - self.assertEqual(26, len(result)) + # 5*16 = 80 bots in sample. There's also five empty results because some + # bots did not run for some revisions, so 80 - 5 = 75 results. There are + # two additional statuses under an explicit 'latest' revision, which should + # be ignored since that means the build was forced. + self.assertEqual(75, len(result)) # Make some samples - self.assertTrue(result.has_key('1570--ChromeOS')) - self.assertEquals('578--OK', result['1570--ChromeOS']) + self.assertTrue(result.has_key('2006--ChromeOS')) + self.assertEquals('933--OK', result['2006--ChromeOS']) - self.assertTrue(result.has_key('1570--Chrome')) - self.assertEquals('109--warnings', result['1570--Chrome']) + self.assertTrue(result.has_key('2006--Chrome')) + self.assertEquals('243--warnings', result['2006--Chrome']) - self.assertTrue(result.has_key('1570--LinuxCLANG')) - self.assertEquals('259--OK', result['1570--LinuxCLANG']) + self.assertTrue(result.has_key('2006--LinuxClang')) + self.assertEquals('610--OK', result['2006--LinuxClang']) - self.assertTrue(result.has_key('1570--Win32Release')) - self.assertEquals('440--OK', result['1570--Win32Release']) + # This one happened to not get reported in revision 2006, but it should be + # there in the next revision: + self.assertFalse(result.has_key('2006--Win32Release')) + self.assertTrue(result.has_key('2007--Win32Release')) + self.assertEquals('809--OK', result['2007--Win32Release']) - self.assertTrue(result.has_key('1571--ChromeOS')) - self.assertEquals('579--OK', result['1571--ChromeOS']) + self.assertTrue(result.has_key('2007--ChromeOS')) + self.assertEquals('934--OK', result['2007--ChromeOS']) - self.assertTrue(result.has_key('1571--LinuxVideoTest')) - self.assertEquals('346--failed', result['1571--LinuxVideoTest']) + self.assertTrue(result.has_key('2007--LinuxVideoTest')) + self.assertEquals('731--failed', result['2007--LinuxVideoTest']) - self.assertTrue(result.has_key('1571--Win32Debug')) - self.assertEquals('441--building', result['1571--Win32Debug']) + self.assertTrue(result.has_key('2011--Win32Release')) + self.assertEquals('813--building', result['2011--Win32Release']) if __name__ == '__main__':