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

Revert "rebaseline_cl.py: Set step_name when retrieving results"

This reverts commit cc86e6e5.

Reason for revert: https://crbug.com/891729, didn't account for strange step names.

Original change's description:
> 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: Robert Ma <robertma@chromium.org>
> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#595949}

TBR=qyearsley@chromium.org,dpranke@chromium.org,martiniss@chromium.org,robertma@chromium.org

Change-Id: Ib5a73d70e234f73312bb32db7189ea9cdd43c185
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 882946, 891729
Reviewed-on: https://chromium-review.googlesource.com/1259282Reviewed-by: default avatarStephen Martinis <martiniss@chromium.org>
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596267}
parent 1e3ba8f5
...@@ -39,8 +39,6 @@ _log = logging.getLogger(__name__) ...@@ -39,8 +39,6 @@ _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.
...@@ -60,8 +58,7 @@ class BuildBot(object): ...@@ -60,8 +58,7 @@ 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;
...@@ -69,11 +66,9 @@ class BuildBot(object): ...@@ -69,11 +66,9 @@ class BuildBot(object):
the latest results. the latest results.
""" """
if build_number: if build_number:
assert str(build_number).isdigit(), ( assert str(build_number).isdigit(), 'expected numeric build number, got %s' % build_number
'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/%s/layout-test-results' % ( return '%s/%s/layout-test-results' % (url_base, build_number)
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):
...@@ -86,7 +81,7 @@ class BuildBot(object): ...@@ -86,7 +81,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, step_name=DEFAULT_STEP_NAME): def fetch_retry_summary_json(self, build):
"""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
...@@ -94,9 +89,7 @@ class BuildBot(object): ...@@ -94,9 +89,7 @@ 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/%s' % ( url_base = '%s/%s' % (self.builder_results_url_base(build.builder_name), build.build_number)
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,8 +46,7 @@ class BuilderTest(LoggingTestCase): ...@@ -46,8 +46,7 @@ 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/' 'https://test-results.appspot.com/data/layout_results/Test_Builder/10/layout-test-results')
'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,10 +60,6 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -60,10 +60,6 @@ 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.'),
...@@ -110,7 +106,7 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -110,7 +106,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, options.step_name) jobs_to_results = self._fetch_results(jobs)
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
...@@ -214,7 +210,7 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -214,7 +210,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, step_name): def _fetch_results(self, jobs):
"""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
...@@ -225,7 +221,6 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -225,7 +221,6 @@ 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.
...@@ -242,8 +237,7 @@ class RebaselineCL(AbstractParallelRebaselineCommand): ...@@ -242,8 +237,7 @@ 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( results_url = buildbot.results_url(build.builder_name, build.build_number)
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,7 +119,6 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase): ...@@ -119,7 +119,6 @@ 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))
...@@ -412,7 +411,7 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase): ...@@ -412,7 +411,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/webkit_layout_tests (with patch)/layout-test-results/results.html\n'), '/MOCK_Try_Win/5000/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',
...@@ -429,7 +428,7 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase): ...@@ -429,7 +428,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/webkit_layout_tests (with patch)/layout-test-results/results.html\n'), '/MOCK_Try_Win/5000/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/webkit_layout_tests (with patch)/layout-test-results/results.html' % ( return '%s/%s/%s/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