Commit 6189334c authored by Robert Ma's avatar Robert Ma Committed by Commit Bot

[blinkpy] Make BuildBot.results_url get step names by default

https://crrev.com/c/1272435 made it so that rebaseline-test-internal
would receive the step name from the parent rebaseline process, which
fixed rebaseline-cl.

However, there are still two users of BuildBot.results_url that do not
provide step_name: a log message in rebaseline_cl, and try_flag. This
change makes BuildBot.results_url get the step name if no step name is
provided, which will fix both the log message and try_flag.
https://crrev.com/c/1272435 is still good to have, because it saves lots
of network requests in the children rebaseline-test-internal processes.

Also include some drive-by cleanups.

Bug: 882946
Change-Id: I889885f42c001937123e733c6cf8124e2a9f2b3a
Reviewed-on: https://chromium-review.googlesource.com/c/1274665Reviewed-by: default avatarStephen Martinis <martiniss@chromium.org>
Commit-Queue: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598777}
parent 380406f9
...@@ -72,6 +72,8 @@ class BuildBot(object): ...@@ -72,6 +72,8 @@ class BuildBot(object):
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)
if step_name is None:
step_name = self.get_layout_test_step_name(Build(builder_name, build_number))
if step_name: if step_name:
return '%s/%s/%s/layout-test-results' % ( return '%s/%s/%s/layout-test-results' % (
url_base, build_number, urllib.quote(step_name)) url_base, build_number, urllib.quote(step_name))
...@@ -120,7 +122,7 @@ class BuildBot(object): ...@@ -120,7 +122,7 @@ class BuildBot(object):
'buildnumber': build.build_number, 'buildnumber': build.build_number,
'name': 'full_results.json', 'name': 'full_results.json',
# This forces the server to gives us JSON rather than an HTML page. # This forces the server to gives us JSON rather than an HTML page.
'callback': 'ADD_RESULTS', 'callback': json_results_generator.JSON_CALLBACK,
})) }))
data = NetworkTransaction(return_none_on_404=True).run( data = NetworkTransaction(return_none_on_404=True).run(
lambda: self.fetch_file(url)) lambda: self.fetch_file(url))
......
...@@ -76,6 +76,10 @@ class AbstractRebaseliningCommand(Command): ...@@ -76,6 +76,10 @@ class AbstractRebaseliningCommand(Command):
build_number_option = optparse.make_option( build_number_option = optparse.make_option(
'--build-number', default=None, type='int', '--build-number', default=None, type='int',
help='Optional build number; if not given, the latest build is used.') help='Optional build number; if not given, the latest build is used.')
step_name_option = optparse.make_option(
'--step-name',
help=('Name of the step which ran the actual tests, and which '
'should be used to retrieve results from.'))
def __init__(self, options=None): def __init__(self, options=None):
super(AbstractRebaseliningCommand, self).__init__(options=options) super(AbstractRebaseliningCommand, self).__init__(options=options)
...@@ -296,7 +300,6 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand): ...@@ -296,7 +300,6 @@ class AbstractParallelRebaselineCommand(AbstractRebaseliningCommand):
'--port-name', port_name, '--port-name', port_name,
]) ])
copy_command = [self._tool.executable, path_to_blink_tool, 'copy-existing-baselines-internal'] + args copy_command = [self._tool.executable, path_to_blink_tool, 'copy-existing-baselines-internal'] + args
copy_baseline_commands.append(tuple([copy_command, cwd])) copy_baseline_commands.append(tuple([copy_command, cwd]))
......
...@@ -412,8 +412,8 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase): ...@@ -412,8 +412,8 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase):
self.assertLog([ self.assertLog([
'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%20%28with%20patch%29/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,8 +429,8 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase): ...@@ -429,8 +429,8 @@ class RebaselineCLTest(BaseTestCase, LoggingTestCase):
self.assertLog([ self.assertLog([
'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%20%28with%20patch%29/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',
......
...@@ -4,7 +4,6 @@ ...@@ -4,7 +4,6 @@
import json import json
import logging import logging
import optparse
from blinkpy.tool.commands.rebaseline import AbstractRebaseliningCommand from blinkpy.tool.commands.rebaseline import AbstractRebaseliningCommand
...@@ -22,11 +21,8 @@ class RebaselineTest(AbstractRebaseliningCommand): ...@@ -22,11 +21,8 @@ class RebaselineTest(AbstractRebaseliningCommand):
self.port_name_option, self.port_name_option,
self.builder_option, self.builder_option,
self.build_number_option, self.build_number_option,
self.step_name_option,
self.results_directory_option, self.results_directory_option,
optparse.make_option(
'--step-name',
help=('Name of the step which ran the actual tests, and which '
'should be used to retrieve results from.'))
]) ])
def execute(self, options, args, tool): def execute(self, options, args, tool):
......
...@@ -31,7 +31,8 @@ import logging ...@@ -31,7 +31,8 @@ import logging
_log = logging.getLogger(__name__) _log = logging.getLogger(__name__)
_JSON_PREFIX = "ADD_RESULTS(" JSON_CALLBACK = "ADD_RESULTS"
_JSON_PREFIX = JSON_CALLBACK + "("
_JSON_SUFFIX = ");" _JSON_SUFFIX = ");"
......
...@@ -134,10 +134,11 @@ class TryFlagTest(unittest.TestCase): ...@@ -134,10 +134,11 @@ 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/%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,
'webkit_layout_tests%20%28with%20patch%29'
) )
self.assertEqual(host.stdout.getvalue(), '\n'.join([ self.assertEqual(host.stdout.getvalue(), '\n'.join([
'Fetching results...', 'Fetching results...',
......
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