Commit 49895755 authored by qyearsley's avatar qyearsley Committed by Commit bot

Always use html_diff to generate -pretty-diff.html files.

The idea of this patch is to start using html_diff for -pretty-diff.html
files; once these files are always available then the wdiff-related code
can be removed and there will still be some kind of HTML diff on Windows.

In order to keep this change minimal, the files are still called
-pretty-diff.html and "has_pretty_patch" is still set in the results JSON,
but after this, the "has_pretty_patch" and "has_wdiff" keys in the results
JSON should be removed.

BUG=672651

Review-Url: https://codereview.chromium.org/2580343003
Cr-Commit-Position: refs/heads/master@{#440765}
parent cc618746
...@@ -29,6 +29,7 @@ ...@@ -29,6 +29,7 @@
import logging import logging
from webkitpy.common.html_diff import html_diff
from webkitpy.layout_tests.controllers import repaint_overlay from webkitpy.layout_tests.controllers import repaint_overlay
from webkitpy.layout_tests.models import test_failures from webkitpy.layout_tests.models import test_failures
...@@ -113,7 +114,7 @@ class TestResultWriter(object): ...@@ -113,7 +114,7 @@ class TestResultWriter(object):
FILENAME_SUFFIX_SAMPLE = "-sample" FILENAME_SUFFIX_SAMPLE = "-sample"
FILENAME_SUFFIX_LEAK_LOG = "-leak-log" FILENAME_SUFFIX_LEAK_LOG = "-leak-log"
FILENAME_SUFFIX_WDIFF = "-wdiff.html" FILENAME_SUFFIX_WDIFF = "-wdiff.html"
FILENAME_SUFFIX_PRETTY_PATCH = "-pretty-diff.html" FILENAME_SUFFIX_HTML_DIFF = "-pretty-diff.html"
FILENAME_SUFFIX_IMAGE_DIFF = "-diff.png" FILENAME_SUFFIX_IMAGE_DIFF = "-diff.png"
FILENAME_SUFFIX_IMAGE_DIFFS_HTML = "-diffs.html" FILENAME_SUFFIX_IMAGE_DIFFS_HTML = "-diffs.html"
FILENAME_SUFFIX_OVERLAY = "-overlay.html" FILENAME_SUFFIX_OVERLAY = "-overlay.html"
...@@ -199,11 +200,10 @@ class TestResultWriter(object): ...@@ -199,11 +200,10 @@ class TestResultWriter(object):
if not actual_text or not expected_text: if not actual_text or not expected_text:
return return
# Output a plain-text diff file.
file_type = '.txt' file_type = '.txt'
actual_filename = self.output_filename(self.FILENAME_SUFFIX_ACTUAL + file_type) actual_filename = self.output_filename(self.FILENAME_SUFFIX_ACTUAL + file_type)
expected_filename = self.output_filename(self.FILENAME_SUFFIX_EXPECTED + file_type) expected_filename = self.output_filename(self.FILENAME_SUFFIX_EXPECTED + file_type)
# We treat diff output as binary. Diff output may contain multiple files
# in conflicting encodings.
diff = self._port.diff_text(expected_text, actual_text, expected_filename, actual_filename) diff = self._port.diff_text(expected_text, actual_text, expected_filename, actual_filename)
diff_filename = self.output_filename(self.FILENAME_SUFFIX_DIFF + file_type) diff_filename = self.output_filename(self.FILENAME_SUFFIX_DIFF + file_type)
self._write_file(diff_filename, diff) self._write_file(diff_filename, diff)
...@@ -214,11 +214,10 @@ class TestResultWriter(object): ...@@ -214,11 +214,10 @@ class TestResultWriter(object):
wdiff_filename = self.output_filename(self.FILENAME_SUFFIX_WDIFF) wdiff_filename = self.output_filename(self.FILENAME_SUFFIX_WDIFF)
self._write_file(wdiff_filename, wdiff) self._write_file(wdiff_filename, wdiff)
# Use WebKit's PrettyPatch.rb to get an HTML diff. # Output a HTML diff file.
if self._port.pretty_patch_available(): html_diff_filename = self.output_filename(self.FILENAME_SUFFIX_HTML_DIFF)
pretty_patch = self._port.pretty_patch_text(diff_filename) html_diff_contents = html_diff(expected_text, actual_text)
pretty_patch_filename = self.output_filename(self.FILENAME_SUFFIX_PRETTY_PATCH) self._write_file(html_diff_filename, html_diff_contents)
self._write_file(pretty_patch_filename, pretty_patch)
def create_repaint_overlay_result(self, actual_text, expected_text): def create_repaint_overlay_result(self, actual_text, expected_text):
html = repaint_overlay.generate_repaint_overlay_html(self._test_name, actual_text, expected_text) html = repaint_overlay.generate_repaint_overlay_html(self._test_name, actual_text, expected_text)
......
...@@ -329,7 +329,7 @@ def summarize_results(port_obj, expectations, initial_results, ...@@ -329,7 +329,7 @@ def summarize_results(port_obj, expectations, initial_results,
results['interrupted'] = initial_results.interrupted results['interrupted'] = initial_results.interrupted
results['layout_tests_dir'] = port_obj.layout_tests_dir() results['layout_tests_dir'] = port_obj.layout_tests_dir()
results['has_wdiff'] = port_obj.wdiff_available() results['has_wdiff'] = port_obj.wdiff_available()
results['has_pretty_patch'] = port_obj.pretty_patch_available() results['has_pretty_patch'] = True
results['pixel_tests_enabled'] = port_obj.get_option('pixel_tests') results['pixel_tests_enabled'] = port_obj.get_option('pixel_tests')
results['seconds_since_epoch'] = int(time.time()) results['seconds_since_epoch'] = int(time.time())
results['build_number'] = port_obj.get_option('build_number') results['build_number'] = port_obj.get_option('build_number')
...@@ -337,6 +337,9 @@ def summarize_results(port_obj, expectations, initial_results, ...@@ -337,6 +337,9 @@ def summarize_results(port_obj, expectations, initial_results,
if port_obj.get_option('order') == 'random': if port_obj.get_option('order') == 'random':
results['random_order_seed'] = port_obj.get_option('seed') results['random_order_seed'] = port_obj.get_option('seed')
results['path_delimiter'] = '/' results['path_delimiter'] = '/'
# The pretty-diff.html files should always be available.
# TODO(qyearsley): Change this key since PrettyPatch.rb has been removed.
results['has_pretty_patch'] = True
# Don't do this by default since it takes >100ms. # Don't do this by default since it takes >100ms.
# It's only used for rebaselining and uploading data to the flakiness dashboard. # It's only used for rebaselining and uploading data to the flakiness dashboard.
......
...@@ -967,19 +967,19 @@ class RunTest(unittest.TestCase, StreamTestingMixin): ...@@ -967,19 +967,19 @@ class RunTest(unittest.TestCase, StreamTestingMixin):
self.assertNotIn('platform/test-win-win7/http/test.html', tests_run) self.assertNotIn('platform/test-win-win7/http/test.html', tests_run)
def test_output_diffs(self): def test_output_diffs(self):
# Test to ensure that we don't generate -wdiff.html or -pretty.html if wdiff and PrettyPatch # Test to ensure that we don't generate -wdiff.html if wdiff isn't available,
# aren't available. # but we always generate -diff.txt an -pretty-diff.html.
host = MockHost() host = MockHost()
logging_run(['--pixel-tests', 'failures/unexpected/text-image-checksum.html'], tests_included=True, host=host) logging_run(['--pixel-tests', 'failures/unexpected/text-image-checksum.html'], tests_included=True, host=host)
written_files = host.filesystem.written_files written_files = host.filesystem.written_files
self.assertTrue(any(path.endswith('-diff.txt') for path in written_files.keys())) self.assertTrue(any(path.endswith('-diff.txt') for path in written_files))
self.assertFalse(any(path.endswith('-wdiff.html') for path in written_files.keys())) self.assertTrue(any(path.endswith('-pretty-diff.html') for path in written_files))
self.assertFalse(any(path.endswith('-pretty-diff.html') for path in written_files.keys())) self.assertFalse(any(path.endswith('-wdiff.html') for path in written_files))
full_results_text = host.filesystem.read_text_file('/tmp/layout-test-results/full_results.json') full_results_text = host.filesystem.read_text_file('/tmp/layout-test-results/full_results.json')
full_results = json.loads(full_results_text.replace("ADD_RESULTS(", "").replace(");", "")) full_results = json.loads(full_results_text.replace("ADD_RESULTS(", "").replace(");", ""))
self.assertEqual(full_results['has_wdiff'], False) self.assertEqual(full_results['has_wdiff'], False)
self.assertEqual(full_results['has_pretty_patch'], False) self.assertEqual(full_results['has_pretty_patch'], True)
def test_unsupported_platform(self): def test_unsupported_platform(self):
stdout = StringIO.StringIO() stdout = StringIO.StringIO()
......
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