⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 284785 in webkit


Ignore:
Timestamp:
Oct 25, 2021, 8:26:54 AM (5 years ago)
Author:
Simon Fraser
Message:

webkitpy: have diff_image() return a ImageDiffResult
​https://bugs.webkit.org/show_bug.cgi?id=232222

Reviewed by Jonathan Bedard.

diff_image() returned a list which resulted in hard to read code. In addition, the
presence of a diff_image is used to indicate failure; future patches will change this,
so having diff_image() return a class makes that easier.

  • Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:

(SingleTestRunner._compare_image):
(SingleTestRunner._compare_output_with_reference):

  • Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py:

(TestResultWriterTest.test_reftest_diff_image.ImageDiffTestPort.diff_image):
(TestResultWriterTest):

  • Scripts/webkitpy/layout_tests/models/test_failures.py:

(FailureReftestMismatch.write_failure):

  • Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:

(RunTest.test_tolerance.ImageDiffTestPort.diff_image):

  • Scripts/webkitpy/port/base.py:

(Port.diff_image):

  • Scripts/webkitpy/port/image_diff.py:

(ImageDiffResult):
(ImageDiffResult.init):
(ImageDiffResult.eq):
(ImageDiffResult.ne):
(ImageDiffResult.repr):
(ImageDiffer._read):

  • Scripts/webkitpy/port/port_testcase.py:

(PortTestCase.integration_test_image_diff):
(PortTestCase.test_diff_imagemissing_both):
(PortTestCase.test_diff_imagemissing_actual):
(PortTestCase.test_diff_imagemissing_expected):
(PortTestCase.test_diff_image):
(PortTestCase.test_diff_image_passed):
(PortTestCase.test_diff_image_failed):
(PortTestCase.test_diff_image_crashed):

  • Scripts/webkitpy/port/test.py:
Location:
trunk/Tools
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Tools/ChangeLog

    r284784 r284785  
     12021-10-25  Simon Fraser  <simon.fraser@apple.com>
     2
     3        webkitpy: have diff_image() return a ImageDiffResult
     4        https://bugs.webkit.org/show_bug.cgi?id=232222
     5
     6        Reviewed by Jonathan Bedard.
     7
     8        diff_image() returned a list which resulted in hard to read code. In addition, the
     9        presence of a diff_image is used to indicate failure; future patches will change this,
     10        so having diff_image() return a class makes that easier.
     11
     12        * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py:
     13        (SingleTestRunner._compare_image):
     14        (SingleTestRunner._compare_output_with_reference):
     15        * Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py:
     16        (TestResultWriterTest.test_reftest_diff_image.ImageDiffTestPort.diff_image):
     17        (TestResultWriterTest):
     18        * Scripts/webkitpy/layout_tests/models/test_failures.py:
     19        (FailureReftestMismatch.write_failure):
     20        * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:
     21        (RunTest.test_tolerance.ImageDiffTestPort.diff_image):
     22        * Scripts/webkitpy/port/base.py:
     23        (Port.diff_image):
     24        * Scripts/webkitpy/port/image_diff.py:
     25        (ImageDiffResult):
     26        (ImageDiffResult.__init__):
     27        (ImageDiffResult.__eq__):
     28        (ImageDiffResult.__ne__):
     29        (ImageDiffResult.__repr__):
     30        (ImageDiffer._read):
     31        * Scripts/webkitpy/port/port_testcase.py:
     32        (PortTestCase.integration_test_image_diff):
     33        (PortTestCase.test_diff_image__missing_both):
     34        (PortTestCase.test_diff_image__missing_actual):
     35        (PortTestCase.test_diff_image__missing_expected):
     36        (PortTestCase.test_diff_image):
     37        (PortTestCase.test_diff_image_passed):
     38        (PortTestCase.test_diff_image_failed):
     39        (PortTestCase.test_diff_image_crashed):
     40        * Scripts/webkitpy/port/test.py:
     41
    1422021-10-25  Carlos Alberto Lopez Perez  <clopez@igalia.com>
    243

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py

    r281297 r284785  
    295295        elif driver_output.image_hash != expected_driver_output.image_hash:
    296296            diff_result = self._port.diff_image(expected_driver_output.image, driver_output.image)
    297             error_string = diff_result[2]
    298             if error_string:
    299                 _log.warning('  %s : %s' % (self._test_name, error_string))
     297            if diff_result.error_string:
     298                _log.warning('  %s : %s' % (self._test_name, diff_result.error_string))
    300299                failures.append(test_failures.FailureImageHashMismatch())
    301                 driver_output.error = (driver_output.error or '') + error_string
     300                driver_output.error = (driver_output.error or '') + diff_result.error_string
    302301            else:
    303                 driver_output.image_diff = diff_result[0]
     302                driver_output.image_diff = diff_result.diff_image
    304303                if driver_output.image_diff:
    305                     failures.append(test_failures.FailureImageHashMismatch(diff_result[1]))
     304                    failures.append(test_failures.FailureImageHashMismatch(diff_result.diff_percent))
    306305                else:
    307306                    # See https://bugs.webkit.org/show_bug.cgi?id=69444 for why this isn't a full failure.
    … …  
    359358            # ImageDiff has a hard coded color distance threshold even though tolerance=0 is specified.
    360359            diff_result = self._port.diff_image(reference_driver_output.image, actual_driver_output.image, tolerance=0)
    361             error_string = diff_result[2]
    362             if error_string:
    363                 _log.warning('  %s : %s' % (self._test_name, error_string))
     360            if diff_result.error_string:
     361                _log.warning('  %s : %s' % (self._test_name, diff_result.error_string))
    364362                failures.append(test_failures.FailureReftestMismatch(reference_filename))
    365                 actual_driver_output.error = (actual_driver_output.error or '') + error_string
    366             elif diff_result[0]:
     363                actual_driver_output.error = (actual_driver_output.error or '') + diff_result.error_string
     364            elif diff_result.diff_image:
    367365                failures.append(test_failures.FailureReftestMismatch(reference_filename))
    368366

  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer_unittest.py

    r174136 r284785  
    3131from webkitpy.port.driver import DriverOutput
    3232from webkitpy.port.test import TestPort
     33from webkitpy.port.image_diff import ImageDiffResult
    3334
    3435
    … …  
    4243            def diff_image(self, expected_contents, actual_contents, tolerance=None):
    4344                used_tolerance_values.append(tolerance)
    44                 return (True, 1, None)
     45                return ImageDiffResult(True, 1, None)
    4546
    4647        host = MockHost()

  • trunk/Tools/Scripts/webkitpy/layout_tests/models/test_failures.py

    r276081 r284785  
    233233        # FIXME: We should always have 2 images here.
    234234        if driver_output.image and expected_driver_output.image:
    235             diff_image, diff_percent, err_str = port.diff_image(expected_driver_output.image, driver_output.image, tolerance=0)
    236             if diff_image:
    237                 writer.write_image_diff_files(diff_image)
    238                 self.diff_percent = diff_percent
     235            diff_result = port.diff_image(expected_driver_output.image, driver_output.image, tolerance=0)
     236            if diff_result.diff_image:
     237                writer.write_image_diff_files(diff_result.diff_image)
     238                self.diff_percent = diff_result.diff_percent
    239239            else:
    240240                _log.warn('ref test mismatch did not produce an image diff.')

  • trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py

    r284775 r284785  
    4343from webkitpy.layout_tests.models.test_run_results import INTERRUPTED_EXIT_STATUS
    4444from webkitpy.port import test
     45from webkitpy.port.image_diff import ImageDiffResult
    4546from webkitpy.xcode.device_type import DeviceType
    4647
    … …  
    752753            def diff_image(self, expected_contents, actual_contents, tolerance=None):
    753754                self.tolerance_used_for_diff_image = self._options.tolerance
    754                 return (True, 1, None)
     755                return ImageDiffResult(True, 1, None)
    755756
    756757        def get_port_for_run(args):

  • trunk/Tools/Scripts/webkitpy/port/base.py

    r283132 r284785  
    5757from webkitpy.port import server_process
    5858from webkitpy.port.factory import PortFactory
     59from webkitpy.port.image_diff import ImageDiffResult
    5960from webkitpy.layout_tests.servers import apache_http_server, http_server, http_server_base
    6061from webkitpy.layout_tests.servers import web_platform_test_server
    … …  
    307308        """
    308309        if not actual_contents and not expected_contents:
    309             return (None, 0, None)
     310            return ImageDiffResult(None, 0, None)
     311
    310312        if not actual_contents or not expected_contents:
    311             return (True, 0, None)
     313            return ImageDiffResult(True, 0, None)
     314
    312315        if not self._image_differ:
    313316            self._image_differ = image_diff.ImageDiffer(self)
     317
    314318        self.set_option_default('tolerance', 0.1)
    315319        if tolerance is None:

  • trunk/Tools/Scripts/webkitpy/port/image_diff.py

    r277853 r284785  
    3737from webkitcorepy import BytesIO, string_utils
    3838
    39 
    4039_log = logging.getLogger(__name__)
    4140
     41
     42class ImageDiffResult(object):
     43    def __init__(self, diff_image, difference, error_string):
     44        self.diff_image = diff_image
     45        self.diff_percent = difference
     46        self.error_string = error_string
     47
     48    def __eq__(self, other):
     49        if isinstance(other, self.__class__):
     50            return (self.diff_image == other.diff_image and
     51                    self.diff_percent == other.diff_percent and
     52                    self.error_string == other.error_string)
     53
     54        return False
     55
     56    def __ne__(self, other):
     57        return not self.__eq__(other)
     58
     59    def __repr__(self):
     60        return 'ImageDiffResult({} {} {})'.format(self.diff_image, self.diff_percent, self.error_string)
    4261
    4362class ImageDiffer(object):
    … …  
    110129            m = re.match(b'diff: (.+)% (passed|failed)', output)
    111130            if m.group(2) == b'passed':
    112                 return (None, 0, None)
     131                return ImageDiffResult(None, 0, None)
    113132            diff_percent = float(string_utils.decode(m.group(1), target_type=str))
    114133
    115         return (output_image, diff_percent, err_str or None)
     134        return ImageDiffResult(output_image, diff_percent, err_str or None)
    116135
    117136    def stop(self):

  • trunk/Tools/Scripts/webkitpy/port/port_testcase.py

    r278174 r284785  
    4646from webkitpy.port.base import Port
    4747from webkitpy.port.config import apple_additions, clear_cached_configuration
    48 from webkitpy.port.image_diff import ImageDiffer
     48from webkitpy.port.image_diff import ImageDiffer, ImageDiffResult
    4949from webkitpy.port.server_process_mock import MockServerProcess
    5050from webkitpy.layout_tests.servers import http_server_base
    … …  
    256256        tmpfd.close()
    257257
    258         self.assertFalse(port.diff_image(contents1, contents1)[0])
    259         self.assertTrue(port.diff_image(contents1, contents2)[0])
    260 
    261         self.assertTrue(port.diff_image(contents1, contents2, tmpfile)[0])
     258        self.assertFalse(port.diff_image(contents1, contents1).diff_image)
     259        self.assertTrue(port.diff_image(contents1, contents2).diff_image)
     260
     261        self.assertTrue(port.diff_image(contents1, contents2, tmpfile).diff_image)
    262262
    263263        port._filesystem.remove(tmpfile)
    … …  
    265265    def test_diff_image__missing_both(self):
    266266        port = self.make_port()
    267         self.assertFalse(port.diff_image(None, None)[0])
    268         self.assertFalse(port.diff_image(None, b'')[0])
    269         self.assertFalse(port.diff_image(b'', None)[0])
    270 
    271         self.assertFalse(port.diff_image(b'', b'')[0])
     267        self.assertFalse(port.diff_image(None, None).diff_image)
     268        self.assertFalse(port.diff_image(None, b'').diff_image)
     269        self.assertFalse(port.diff_image(b'', None).diff_image)
     270
     271        self.assertFalse(port.diff_image(b'', b'').diff_image)
    272272
    273273    def test_diff_image__missing_actual(self):
    274274        port = self.make_port()
    275         self.assertTrue(port.diff_image(None, b'foo')[0])
    276         self.assertTrue(port.diff_image(b'', b'foo')[0])
     275        self.assertTrue(port.diff_image(None, b'foo').diff_image)
     276        self.assertTrue(port.diff_image(b'', b'foo').diff_image)
    277277
    278278    def test_diff_image__missing_expected(self):
    279279        port = self.make_port()
    280         self.assertTrue(port.diff_image(b'foo', None)[0])
    281         self.assertTrue(port.diff_image(b'foo', b'')[0])
     280        self.assertTrue(port.diff_image(b'foo', None).diff_image)
     281        self.assertTrue(port.diff_image(b'foo', b'').diff_image)
    282282
    283283    def test_diff_image(self):
    … …  
    299299        self.assertFalse(port._should_use_jhbuild())
    300300
    301         self.assertEqual(port.diff_image(b'foo', b'bar'), (b'', 100.0, None))
     301        self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(b'', 100.0, None))
    302302        self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0.1"])
    303         self.assertEqual(port.diff_image(b'foo', b'bar', None), (b'', 100.0, None))
     303        self.assertEqual(port.diff_image(b'foo', b'bar', None), ImageDiffResult(b'', 100.0, None))
    304304        self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0.1"])
    305         self.assertEqual(port.diff_image(b'foo', b'bar', 0), (b'', 100.0, None))
     305        self.assertEqual(port.diff_image(b'foo', b'bar', 0), ImageDiffResult(b'', 100.0, None))
    306306        self.assertEqual(self.proc.cmd, [port._path_to_image_diff(), "--tolerance", "0"])
    307307
    … …  
    310310        self.assertTrue(port._should_use_jhbuild())
    311311
    312         self.assertEqual(port.diff_image(b'foo', b'bar'), (b'', 100.0, None))
     312        self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(b'', 100.0, None))
    313313        self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0.1"])
    314         self.assertEqual(port.diff_image(b'foo', b'bar', None), (b'', 100.0, None))
     314        self.assertEqual(port.diff_image(b'foo', b'bar', None), ImageDiffResult(b'', 100.0, None))
    315315        self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0.1"])
    316         self.assertEqual(port.diff_image(b'foo', b'bar', 0), (b'', 100.0, None))
     316        self.assertEqual(port.diff_image(b'foo', b'bar', 0), ImageDiffResult(b'', 100.0, None))
    317317        self.assertEqual(self.proc.cmd, port._jhbuild_wrapper + [port._path_to_image_diff(), "--tolerance", "0"])
    318318
    … …  
    325325        port._server_process_constructor = lambda port, nm, cmd, env, crash_message=None: MockServerProcess(lines=['diff: 0% passed\n'])
    326326        image_differ = ImageDiffer(port)
    327         self.assertEqual(image_differ.diff_image(b'foo', b'bar', 0.1), (None, 0, None))
     327        self.assertEqual(image_differ.diff_image(b'foo', b'bar', 0.1), ImageDiffResult(None, 0, None))
    328328
    329329    def test_diff_image_failed(self):
    … …  
    331331        port._server_process_constructor = lambda port, nm, cmd, env, crash_message=None: MockServerProcess(lines=['diff: 100% failed\n'])
    332332        image_differ = ImageDiffer(port)
    333         self.assertEqual(image_differ.diff_image(b'foo', b'bar', 0.1), (b'', 100.0, None))
     333        self.assertEqual(image_differ.diff_image(b'foo', b'bar', 0.1), ImageDiffResult(b'', 100.0, None))
    334334
    335335    def test_diff_image_crashed(self):
    … …  
    347347        port._server_process_constructor = make_proc
    348348        port.setup_test_run()
    349         self.assertEqual(port.diff_image(b'foo', b'bar'), (b'', 0, 'ImageDiff crashed\n'))
     349        self.assertEqual(port.diff_image(b'foo', b'bar'), ImageDiffResult(b'', 0, 'ImageDiff crashed\n'))
    350350        port.clean_up_test_run()
    351351

  • trunk/Tools/Scripts/webkitpy/port/test.py

    r278454 r284785  
    3535from webkitpy.common.system.crashlogs import CrashLogs
    3636from webkitpy.common.version_name_map import PUBLIC_TABLE, VersionNameMap
     37from webkitpy.port.image_diff import ImageDiffResult
    3738
    3839
    … …  
    408409        diffed = actual_contents != expected_contents
    409410        if not actual_contents and not expected_contents:
    410             return (None, 0, None)
     411            return ImageDiffResult(None, 0, None)
     412
    411413        if not actual_contents or not expected_contents:
    412             return (True, 0, None)
     414            return ImageDiffResult(True, 0, None)
     415
    413416        if b'ref' in expected_contents:
    414417            assert tolerance == 0
    415418        if diffed:
    416             return ("< {}\n---\n> {}\n".format(
     419            return ImageDiffResult("< {}\n---\n> {}\n".format(
    417420                string_utils.decode(expected_contents, target_type=str),
    418421                string_utils.decode(actual_contents, target_type=str),
    419422            ), 1, None)
    420         return (None, 0, None)
     423
     424        return ImageDiffResult(None, 0, None)
    421425
    422426    def layout_tests_dir(self):
Note: See TracChangeset for help on using the changeset viewer.