Commit f7bdf58e authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Rebaseline-cl should not change TestExpectations

This is because we only rebaseline-cl for unexpected failures.
We can't remove lines from TestExpectations because the lines contains
expectations other than Failure.

Bug: 1012792
Change-Id: Iaa710b002af7f4f75eb15bdefe4d7a14bb9da675
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1883624Reviewed-by: default avatarRobert Ma <robertma@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710485}
parent f0a1aafc
...@@ -28,7 +28,6 @@ class RebaselineTest(AbstractRebaseliningCommand): ...@@ -28,7 +28,6 @@ class RebaselineTest(AbstractRebaseliningCommand):
def execute(self, options, args, tool): def execute(self, options, args, tool):
self._tool = tool self._tool = tool
self._rebaseline_test_and_update_expectations(options) self._rebaseline_test_and_update_expectations(options)
self._print_expectation_line_changes()
def _rebaseline_test_and_update_expectations(self, options): def _rebaseline_test_and_update_expectations(self, options):
self._baseline_suffix_list = options.suffixes.split(',') self._baseline_suffix_list = options.suffixes.split(',')
...@@ -40,15 +39,10 @@ class RebaselineTest(AbstractRebaseliningCommand): ...@@ -40,15 +39,10 @@ class RebaselineTest(AbstractRebaseliningCommand):
options.builder, build_number=options.build_number, options.builder, build_number=options.build_number,
step_name=options.step_name) step_name=options.step_name)
succeeded = True
port_name = options.port_name or self._tool.builders.port_name_for_builder_name(options.builder) port_name = options.port_name or self._tool.builders.port_name_for_builder_name(options.builder)
test_name = options.test test_name = options.test
for suffix in self._baseline_suffix_list: for suffix in self._baseline_suffix_list:
if not self._rebaseline_test(port_name, test_name, suffix, results_url): self._rebaseline_test(port_name, test_name, suffix, results_url)
succeeded = False
if succeeded:
self.expectation_line_changes.remove_line(test=test_name, port_name=port_name)
def _rebaseline_test(self, port_name, test_name, suffix, results_url): def _rebaseline_test(self, port_name, test_name, suffix, results_url):
"""Downloads a baseline file and saves it to the filesystem. """Downloads a baseline file and saves it to the filesystem.
...@@ -60,9 +54,6 @@ class RebaselineTest(AbstractRebaseliningCommand): ...@@ -60,9 +54,6 @@ class RebaselineTest(AbstractRebaseliningCommand):
suffix: The baseline file extension (e.g. png); together with the suffix: The baseline file extension (e.g. png); together with the
test name and results_url this determines what file to download. test name and results_url this determines what file to download.
results_url: Base URL to download the actual result from. results_url: Base URL to download the actual result from.
Returns:
True if the rebaseline is successful.
""" """
port = self._tool.port_factory.get(port_name) port = self._tool.port_factory.get(port_name)
...@@ -71,10 +62,8 @@ class RebaselineTest(AbstractRebaseliningCommand): ...@@ -71,10 +62,8 @@ class RebaselineTest(AbstractRebaseliningCommand):
source_baseline = '%s/%s' % (results_url, self._file_name_for_actual_result(test_name, suffix)) source_baseline = '%s/%s' % (results_url, self._file_name_for_actual_result(test_name, suffix))
target_baseline = self._tool.filesystem.join(baseline_directory, self._file_name_for_expected_result(test_name, suffix)) target_baseline = self._tool.filesystem.join(baseline_directory, self._file_name_for_expected_result(test_name, suffix))
succeeded = True
if suffix == 'png' and port.reference_files(test_name): if suffix == 'png' and port.reference_files(test_name):
_log.warning('Cannot rebaseline image result for reftest: %s', test_name) _log.warning('Cannot rebaseline image result for reftest: %s', test_name)
succeeded = False
data = '' data = ''
# Still continue in case we can remove extra -expected.png. # Still continue in case we can remove extra -expected.png.
else: else:
...@@ -91,7 +80,3 @@ class RebaselineTest(AbstractRebaseliningCommand): ...@@ -91,7 +80,3 @@ class RebaselineTest(AbstractRebaseliningCommand):
filesystem = self._tool.filesystem filesystem = self._tool.filesystem
filesystem.maybe_make_directory(filesystem.dirname(target_baseline)) filesystem.maybe_make_directory(filesystem.dirname(target_baseline))
filesystem.write_binary_file(target_baseline, data) filesystem.write_binary_file(target_baseline, data)
return succeeded
def _print_expectation_line_changes(self):
print json.dumps(self.expectation_line_changes.to_dict())
...@@ -59,8 +59,8 @@ class TestRebaselineTest(BaseTestCase): ...@@ -59,8 +59,8 @@ class TestRebaselineTest(BaseTestCase):
'new win10 result') 'new win10 result')
self.assertFalse(self.tool.filesystem.exists(self.tool.filesystem.join( self.assertFalse(self.tool.filesystem.exists(self.tool.filesystem.join(
port.web_tests_dir(), 'platform/test-win-win7/failures/expected/image-expected.txt'))) port.web_tests_dir(), 'platform/test-win-win7/failures/expected/image-expected.txt')))
self.assertMultiLineEqual( # We should not change TestExpectations for unexpected failures.
out, '{"remove-lines": [{"test": "failures/expected/image.html", "port_name": "test-win-win10"}]}\n') self.assertMultiLineEqual(out, '')
def test_baseline_directory(self): def test_baseline_directory(self):
self.assertMultiLineEqual( self.assertMultiLineEqual(
......
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