Commit cc86e6e5 authored by Stephen Martinis's avatar Stephen Martinis Committed by Commit Bot

rebaseline_cl.py: Set step_name when retrieving results

Layout test results are now uploaded into a directory indicating which
step name they were run as. This is to allow for without patch runs to
be recorded for analytics. This CL changes rebaseline_cl to pass the
step name when retrieving resuls.

Bug: 882946
Change-Id: Iad18ca54cc8804fff76f059ceab49d8c69fadd3e
Reviewed-on: https://chromium-review.googlesource.com/1244598
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Reviewed-by: default avatarRobert Ma <robertma@chromium.org>
Reviewed-by: default avatarQuinten Yearsley <qyearsley@chromium.org>
Reviewed-by: default avatarDirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595949}
parent 763f8972
...@@ -39,6 +39,8 @@ _log = logging.getLogger(__name__) ...@@ -39,6 +39,8 @@ _log = logging.getLogger(__name__)
RESULTS_URL_BASE = 'https://test-results.appspot.com/data/layout_results' RESULTS_URL_BASE = 'https://test-results.appspot.com/data/layout_results'
DEFAULT_STEP_NAME = 'webkit_layout_tests (with patch)'
class Build(collections.namedtuple('Build', ('builder_name', 'build_number'))): class Build(collections.namedtuple('Build', ('builder_name', 'build_number'))):
"""Represents a combination of builder and build number. """Represents a combination of builder and build number.
...@@ -58,7 +60,8 @@ class BuildBot(object): ...@@ -58,7 +60,8 @@ class BuildBot(object):
https://www.chromium.org/developers/the-json-test-results-format https://www.chromium.org/developers/the-json-test-results-format
""" """
def results_url(self, builder_name, build_number=None): def results_url(self, builder_name, build_number=None,
step_name=DEFAULT_STEP_NAME):
"""Returns a URL for one set of archived layout test results. """Returns a URL for one set of archived layout test results.
If a build number is given, this will be results for a particular run; If a build number is given, this will be results for a particular run;
...@@ -66,9 +69,11 @@ class BuildBot(object): ...@@ -66,9 +69,11 @@ class BuildBot(object):
the latest results. the latest results.
""" """
if build_number: if build_number:
assert str(build_number).isdigit(), 'expected numeric build number, got %s' % build_number assert str(build_number).isdigit(), (
'expected numeric build number, got %s' % build_number)
url_base = self.builder_results_url_base(builder_name) url_base = self.builder_results_url_base(builder_name)
return '%s/%s/layout-test-results' % (url_base, build_number) return '%s/%s/%s/layout-test-results' % (
url_base, build_number, step_name)
return self.accumulated_results_url_base(builder_name) return self.accumulated_results_url_base(builder_name)
def builder_results_url_base(self, builder_name): def builder_results_url_base(self, builder_name):
...@@ -81,7 +86,7 @@ class BuildBot(object): ...@@ -81,7 +86,7 @@ class BuildBot(object):
return '%s/%s' % (RESULTS_URL_BASE, re.sub('[ .()]', '_', builder_name)) return '%s/%s' % (RESULTS_URL_BASE, re.sub('[ .()]', '_', builder_name))
@memoized @memoized
def fetch_retry_summary_json(self, build): def fetch_retry_summary_json(self, build, step_name=DEFAULT_STEP_NAME):
"""Fetches and returns the text of the archived retry_summary file. """Fetches and returns the text of the archived retry_summary file.
This file is expected to contain the results of retrying layout tests This file is expected to contain the results of retrying layout tests
...@@ -89,7 +94,9 @@ class BuildBot(object): ...@@ -89,7 +94,9 @@ class BuildBot(object):
that failed only with the patch ("failures"), and tests that failed that failed only with the patch ("failures"), and tests that failed
both with and without ("ignored"). both with and without ("ignored").
""" """
url_base = '%s/%s' % (self.builder_results_url_base(build.builder_name), build.build_number) url_base = '%s/%s/%s' % (
self.builder_results_url_base(build.builder_name),
build.build_number, step_name)
return NetworkTransaction(return_none_on_404=True).run( return NetworkTransaction(return_none_on_404=True).run(
lambda: self.fetch_file(url_base, 'retry_summary.json')) lambda: self.fetch_file(url_base, 'retry_summary.json'))
......
...@@ -46,7 +46,8 @@ class BuilderTest(LoggingTestCase): ...@@ -46,7 +46,8 @@ class BuilderTest(LoggingTestCase):
def test_results_url_with_build_number(self): def test_results_url_with_build_number(self):
self.assertEqual( self.assertEqual(
BuildBot().results_url('Test Builder', 10), BuildBot().results_url('Test Builder', 10),
'https://test-results.appspot.com/data/layout_results/Test_Builder/10/layout-test-results') 'https://test-results.appspot.com/data/layout_results/Test_Builder/'
'10/webkit_layout_tests (with patch)/layout-test-results')
def test_results_url_with_non_numeric_build_number(self): def test_results_url_with_non_numeric_build_number(self):
with self.assertRaisesRegexp(AssertionError, 'expected numeric build number'): with self.assertRaisesRegexp(AssertionError, 'expected numeric build number'):
......
...@@ -60,6 +60,10 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -60,6 +60,10 @@ class RebaselineCL(AbstractParallelRebaselineCommand):
'--builders', default=None, action='append', '--builders', default=None, action='append',
help=('Comma-separated-list of builders to pull new baselines ' help=('Comma-separated-list of builders to pull new baselines '
'from (can also be provided multiple times).')), 'from (can also be provided multiple times).')),
optparse.make_option(
'--step-name', default='webkit_layout_tests (with patch)',
help=('Layout test step name to fetch results from. Defaults '
' to \'webkit_layout_tests (with patch)\'.')),
optparse.make_option( optparse.make_option(
'--patchset', default=None, '--patchset', default=None,
help='Patchset number to fetch new baselines from.'), help='Patchset number to fetch new baselines from.'),
...@@ -106,7 +110,7 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -106,7 +110,7 @@ class RebaselineCL(AbstractParallelRebaselineCommand):
self.trigger_try_jobs(builders_with_no_jobs) self.trigger_try_jobs(builders_with_no_jobs)
return 1 return 1
jobs_to_results = self._fetch_results(jobs) jobs_to_results = self._fetch_results(jobs, options.step_name)
builders_with_results = {b.builder_name for b in jobs_to_results} builders_with_results = {b.builder_name for b in jobs_to_results}
builders_without_results = set(self.selected_try_bots) - builders_with_results builders_without_results = set(self.selected_try_bots) - builders_with_results
...@@ -210,7 +214,7 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -210,7 +214,7 @@ class RebaselineCL(AbstractParallelRebaselineCommand):
for builder in sorted(builders): for builder in sorted(builders):
_log.info(' %s', builder) _log.info(' %s', builder)
def _fetch_results(self, jobs): def _fetch_results(self, jobs, step_name):
"""Fetches results for all of the given builds. """Fetches results for all of the given builds.
There should be a one-to-one correspondence between Builds, supported There should be a one-to-one correspondence between Builds, supported
...@@ -221,6 +225,7 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -221,6 +225,7 @@ class RebaselineCL(AbstractParallelRebaselineCommand):
Args: Args:
jobs: A dict mapping Build objects to TryJobStatus objects. jobs: A dict mapping Build objects to TryJobStatus objects.
step_name: The step name to fetch when retrieving test results.
Returns: Returns:
A dict mapping Build to LayoutTestResults for all completed jobs. A dict mapping Build to LayoutTestResults for all completed jobs.
...@@ -237,7 +242,8 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -237,7 +242,8 @@ class RebaselineCL(AbstractParallelRebaselineCommand):
# Only completed failed builds will contain actual failed # Only completed failed builds will contain actual failed
# layout tests to download baselines for. # layout tests to download baselines for.
continue continue
results_url = buildbot.results_url(build.builder_name, build.build_number) results_url = buildbot.results_url(
build.builder_name, build.build_number, step_name)
layout_test_results = buildbot.fetch_results(build) layout_test_results = buildbot.fetch_results(build)
if layout_test_results is None: if layout_test_results is None:
_log.info('Failed to fetch results for "%s".', build.builder_name) _log.info('Failed to fetch results for "%s".', build.builder_name)
......
...@@ -119,6 +119,7 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase): ...@@ -119,6 +119,7 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase):
'verbose': False, 'verbose': False,
'builders': [], 'builders': [],
'patchset': None, 'patchset': None,
'step_name': 'webkit_layout_tests (with patch)',
} }
options.update(kwargs) options.update(kwargs)
return optparse.Values(dict(**options)) return optparse.Values(dict(**options))
...@@ -411,7 +412,7 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase): ...@@ -411,7 +412,7 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase):
'INFO: Finished try jobs found for all try bots.\n', 'INFO: Finished try jobs found for all try bots.\n',
'INFO: Failed to fetch results for "MOCK Try Win".\n', 'INFO: Failed to fetch results for "MOCK Try Win".\n',
('INFO: Results URL: https://test-results.appspot.com/data/layout_results' ('INFO: Results URL: https://test-results.appspot.com/data/layout_results'
'/MOCK_Try_Win/5000/layout-test-results/results.html\n'), '/MOCK_Try_Win/5000/webkit_layout_tests (with patch)/layout-test-results/results.html\n'),
'INFO: There are some builders with no results:\n', 'INFO: There are some builders with no results:\n',
'INFO: MOCK Try Win\n', 'INFO: MOCK Try Win\n',
'INFO: Would you like to continue?\n', 'INFO: Would you like to continue?\n',
...@@ -428,7 +429,7 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase): ...@@ -428,7 +429,7 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase):
'INFO: Finished try jobs found for all try bots.\n', 'INFO: Finished try jobs found for all try bots.\n',
'INFO: Failed to fetch results for "MOCK Try Win".\n', 'INFO: Failed to fetch results for "MOCK Try Win".\n',
('INFO: Results URL: https://test-results.appspot.com/data/layout_results' ('INFO: Results URL: https://test-results.appspot.com/data/layout_results'
'/MOCK_Try_Win/5000/layout-test-results/results.html\n'), '/MOCK_Try_Win/5000/webkit_layout_tests (with patch)/layout-test-results/results.html\n'),
'INFO: There are some builders with no results:\n', 'INFO: There are some builders with no results:\n',
'INFO: MOCK Try Win\n', 'INFO: MOCK Try Win\n',
'INFO: For one/flaky-fail.html:\n', 'INFO: For one/flaky-fail.html:\n',
......
...@@ -134,7 +134,7 @@ class TryFlagTest(unittest.TestCase): ...@@ -134,7 +134,7 @@ class TryFlagTest(unittest.TestCase):
TryFlag(cmd, host, MockGitCL(host, self.mock_try_results)).run() TryFlag(cmd, host, MockGitCL(host, self.mock_try_results)).run()
def results_url(build): def results_url(build):
return '%s/%s/%s/layout-test-results/results.html' % ( return '%s/%s/%s/webkit_layout_tests (with patch)/layout-test-results/results.html' % (
'https://test-results.appspot.com/data/layout_results', 'https://test-results.appspot.com/data/layout_results',
build.builder_name, build.builder_name,
build.build_number build.build_number
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment