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
+
+
+
+
+
+"""
+
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__':