Changeset 100326 in webkit


Ignore:
Timestamp:
Nov 15, 2011 2:30:19 PM (12 years ago)
Author:
tony@chromium.org
Message:

[NRWT] Reftests should run even when pixel tests are disabled.
https://bugs.webkit.org/show_bug.cgi?id=60605

Reviewed by Dirk Pranke.

  • Scripts/webkitpy/layout_tests/controllers/single_test_runner.py: Only skip ref tests if --no-ref-tests is passed.

Also add an assert to make sure we get image hashes back when running ref tests.

  • Scripts/webkitpy/layout_tests/controllers/worker.py: Use Driver.has_crashed() instead of poll().
  • Scripts/webkitpy/layout_tests/port/base_unittest.py:
  • Scripts/webkitpy/layout_tests/port/chromium.py: Use DriverProxy.
  • Scripts/webkitpy/layout_tests/port/driver.py: Add DriverProxy which does the work

of starting a pixel driver if needed. It handles the logic of sending the test
to the correct driver. Also renamed Driver.poll() to Driver.has_crashed().

  • Scripts/webkitpy/layout_tests/port/dryrun.py:
  • Scripts/webkitpy/layout_tests/port/test.py: Switch to using DriverProxy so we get test coverage.
  • Scripts/webkitpy/layout_tests/port/webkit.py:
  • Scripts/webkitpy/layout_tests/run_webkit_tests.py: Add --no-ref-tests.
  • Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py: Test --no-ref-tests.
Location:
trunk/Tools
Files:
13 edited

Legend:

Unmodified
Added
Removed
  • trunk/Tools/ChangeLog

    r100287 r100326  
     12011-11-14  Tony Chang  <tony@chromium.org>
     2
     3        [NRWT] Reftests should run even when pixel tests are disabled.
     4        https://bugs.webkit.org/show_bug.cgi?id=60605
     5
     6        Reviewed by Dirk Pranke.
     7
     8        * Scripts/webkitpy/layout_tests/controllers/single_test_runner.py: Only skip ref tests if --no-ref-tests is passed.
     9            Also add an assert to make sure we get image hashes back when running ref tests.
     10        * Scripts/webkitpy/layout_tests/controllers/worker.py: Use Driver.has_crashed() instead of poll().
     11        * Scripts/webkitpy/layout_tests/port/base_unittest.py:
     12        * Scripts/webkitpy/layout_tests/port/chromium.py: Use DriverProxy.
     13        * Scripts/webkitpy/layout_tests/port/driver.py: Add DriverProxy which does the work
     14            of starting a pixel driver if needed.  It handles the logic of sending the test
     15            to the correct driver.  Also renamed Driver.poll() to Driver.has_crashed().
     16        * Scripts/webkitpy/layout_tests/port/dryrun.py:
     17        * Scripts/webkitpy/layout_tests/port/test.py: Switch to using DriverProxy so we get test coverage.
     18        * Scripts/webkitpy/layout_tests/port/webkit.py:
     19        * Scripts/webkitpy/layout_tests/run_webkit_tests.py: Add --no-ref-tests.
     20        * Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py: Test --no-ref-tests.
     21
    1222011-11-15  David Kilzer  <ddkilzer@apple.com>
    223
  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/single_test_runner.py

    r99651 r100326  
    114114        if self._should_fetch_expected_checksum():
    115115            image_hash = self._port.expected_checksum(self._test_name)
    116         return DriverInput(self._test_name, self._timeout, image_hash)
     116        return DriverInput(self._test_name, self._timeout, image_hash, self._is_reftest)
    117117
    118118    def run(self):
     119        if self._is_reftest:
     120            if self._port.get_option('no_ref_tests') or self._options.new_baseline or self._options.reset_results:
     121                result = TestResult(self._test_name)
     122                result.type = test_expectations.SKIP
     123                return result
     124            return self._run_reftest()
    119125        if self._options.new_baseline or self._options.reset_results:
    120             if self._is_reftest:
    121                 # Returns a dummy TestResult. We don't have to rebase for reftests.
    122                 return TestResult(self._test_name)
    123             else:
    124                 return self._run_rebaseline()
    125         if self._is_reftest:
    126             if self._port.get_option('pixel_tests'):
    127                 return self._run_reftest()
    128             result = TestResult(self._test_name)
    129             result.type = test_expectations.SKIP
    130             return result
     126            return self._run_rebaseline()
    131127        return self._run_compare_test()
    132128
     
    291287        driver_output1 = self._driver.run_test(self._driver_input())
    292288        reference_test_name = self._port.relative_test_filename(self._reference_filename)
    293         driver_output2 = self._driver.run_test(DriverInput(reference_test_name, self._timeout, driver_output1.image_hash))
     289        driver_output2 = self._driver.run_test(DriverInput(reference_test_name, self._timeout, driver_output1.image_hash, self._is_reftest))
    294290        test_result = self._compare_output_with_reference(driver_output1, driver_output2)
    295291
     
    309305            return TestResult(self._test_name, failures, total_test_time, has_stderr)
    310306
     307        assert(driver_output1.image_hash or driver_output2.image_hash)
     308
    311309        if self._is_mismatch_reftest:
    312310            if driver_output1.image_hash == driver_output2.image_hash:
  • trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py

    r99907 r100326  
    244244        Returns: a TestResult object.
    245245        """
    246         if not self._driver or self._driver.poll() is not None:
     246        if self._driver and self._driver.has_crashed():
     247            self.kill_driver()
     248        if not self._driver:
    247249            self._driver = self._port.create_driver(self._worker_number)
    248250        return self.run_single_test(self._driver, test_input)
  • trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py

    r100216 r100326  
    5454from webkitpy.layout_tests.models.test_configuration import TestConfiguration
    5555from webkitpy.layout_tests.port import config as port_config
     56from webkitpy.layout_tests.port import driver
    5657from webkitpy.layout_tests.port import http_lock
    5758from webkitpy.layout_tests.port import test_files
     
    664665    def create_driver(self, worker_number):
    665666        """Return a newly created Driver subclass for starting/stopping the test driver."""
    666         raise NotImplementedError('Port.create_driver')
     667        return driver.DriverProxy(self, worker_number, self._driver_class(), pixel_tests=self.get_option('pixel_tests'))
    667668
    668669    def start_helper(self):
     
    958959        for a platform across all OSes, architectures, build and graphics types."""
    959960        raise NotImplementedError('Port._generate_test_configurations')
     961
     962    def _driver_class(self):
     963        """Returns the port's driver implementation."""
     964        raise NotImplementedError('Port._driver_class')
  • trunk/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py

    r100216 r100326  
    303303
    304304    def test_virtual_driver_methods(self):
    305         driver = Driver(None, None)
     305        driver = Driver(Port(MockHost()), None, pixel_tests=False)
    306306        self.assertVirtual(driver.run_test, None)
    307         self.assertVirtual(driver.poll)
    308307        self.assertVirtual(driver.stop)
     308        self.assertVirtual(driver.cmd_line)
    309309
    310310
     
    313313
    314314    def _assert_wrapper(self, wrapper_string, expected_wrapper):
    315         wrapper = Driver(None, None)._command_wrapper(wrapper_string)
     315        wrapper = Driver(Port(MockHost()), None, pixel_tests=False)._command_wrapper(wrapper_string)
    316316        self.assertEqual(wrapper, expected_wrapper)
    317317
  • trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium.py

    r99989 r100326  
    253253            self._filesystem.rmtree(cachedir)
    254254
    255     def create_driver(self, worker_number):
    256         """Starts a new Driver and returns a handle to it."""
    257         return ChromiumDriver(self, worker_number)
     255    def _driver_class(self):
     256        return ChromiumDriver
    258257
    259258    def start_helper(self):
     
    401400# FIXME: This should inherit from WebKitDriver now that Chromium has a DumpRenderTree process like the rest of WebKit.
    402401class ChromiumDriver(Driver):
    403     def __init__(self, port, worker_number):
    404         Driver.__init__(self, port, worker_number)
     402    def __init__(self, port, worker_number, pixel_tests=True):
     403        Driver.__init__(self, port, worker_number, pixel_tests)
    405404        self._proc = None
    406405        self._image_path = None
    407         if self._port.get_option('pixel_tests'):
     406        if self._pixel_tests:
    408407            self._image_path = self._port._filesystem.join(self._port.results_directory(), 'png_result%s.png' % self._worker_number)
    409408
    410409    def _wrapper_options(self):
    411410        cmd = []
    412         if self._port.get_option('pixel_tests'):
     411        if self._pixel_tests:
    413412            # See note above in diff_image() for why we need _convert_path().
    414413            cmd.append("--pixel-tests=" + self._port._convert_path(self._image_path))
     
    456455        self._proc = subprocess.Popen(self.cmd_line(), stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, close_fds=close_fds)
    457456
    458     def poll(self):
    459         return self._proc.poll()
     457    def has_crashed(self):
     458        if self._proc is None:
     459            return False
     460        return self._proc.poll() is not None
    460461
    461462    def _write_command_and_read_line(self, input=None):
     
    532533        while not crash and line.rstrip() != "#EOF":
    533534            # Make sure we haven't crashed.
    534             if line == '' and self.poll() is not None:
     535            if line == '' and self._proc.poll() is not None:
    535536                # This is hex code 0xc000001d, which is used for abrupt
    536537                # termination. This happens if we hit ctrl+c from the prompt
  • trunk/Tools/Scripts/webkitpy/layout_tests/port/driver.py

    r97931 r100326  
    3131
    3232class DriverInput(object):
    33     def __init__(self, test_name, timeout, image_hash):
     33    def __init__(self, test_name, timeout, image_hash, is_reftest):
    3434        self.test_name = test_name
    3535        self.timeout = timeout  # in ms
    3636        self.image_hash = image_hash
     37        self.is_reftest = is_reftest
    3738
    3839
     
    5859
    5960
    60 class Driver:
     61class Driver(object):
    6162    """Abstract interface for the DumpRenderTree interface."""
    6263
    63     def __init__(self, port, worker_number):
     64    def __init__(self, port, worker_number, pixel_tests):
    6465        """Initialize a Driver to subsequently run tests.
    6566
     
    7273        self._port = port
    7374        self._worker_number = worker_number
     75        self._pixel_tests = pixel_tests
    7476
    7577    def run_test(self, driver_input):
     
    9092        return shlex.split(wrapper_option) if wrapper_option else []
    9193
    92     def poll(self):
    93         """Returns None if the Driver is still running. Returns the returncode if it has exited."""
    94         raise NotImplementedError('Driver.poll')
     94    def has_crashed(self):
     95        return False
    9596
    9697    def stop(self):
    9798        raise NotImplementedError('Driver.stop')
     99
     100    def cmd_line(self):
     101        raise NotImplementedError('Driver.cmd_line')
     102
     103
     104class DriverProxy(object):
     105    """A wrapper for managing two Driver instances, one with pixel tests and
     106    one without. This allows us to handle plain text tests and ref tests with a
     107    single driver."""
     108
     109    def __init__(self, port, worker_number, driver_instance_constructor, pixel_tests):
     110        self._driver = driver_instance_constructor(port, worker_number, pixel_tests)
     111        if pixel_tests:
     112            self._reftest_driver = self._driver
     113        else:
     114            self._reftest_driver = driver_instance_constructor(port, worker_number, pixel_tests=True)
     115
     116    def run_test(self, driver_input):
     117        if driver_input.is_reftest:
     118            return self._reftest_driver.run_test(driver_input)
     119        return self._driver.run_test(driver_input)
     120
     121    def has_crashed(self):
     122        return self._driver.has_crashed() or self._reftest_driver.has_crashed()
     123
     124    def stop(self):
     125        self._driver.stop()
     126        self._reftest_driver.stop()
     127
     128    def cmd_line(self):
     129        cmd_line = self._driver.cmd_line()
     130        if self._driver != self._reftest_driver:
     131            cmd_line += ['; '] + self._reftest_driver.cmd_line()
     132        return cmd_line
  • trunk/Tools/Scripts/webkitpy/layout_tests/port/dryrun.py

    r99907 r100326  
    100100        pass
    101101
    102     def create_driver(self, worker_number):
    103         return DryrunDriver(self, worker_number)
     102    def driver_class(self):
     103        return DryrunDriver
    104104
    105105
     
    109109    def cmd_line(self):
    110110        return ['None']
    111 
    112     def poll(self):
    113         return None
    114111
    115112    def run_test(self, driver_input):
  • trunk/Tools/Scripts/webkitpy/layout_tests/port/gtk.py

    r99935 r100326  
    7373        return "--gtk"
    7474
    75     def create_driver(self, worker_number):
    76         return GtkDriver(self, worker_number)
     75    def _driver_class(self):
     76        return GtkDriver
    7777
    7878    def setup_environ_for_server(self, server_name=None):
  • trunk/Tools/Scripts/webkitpy/layout_tests/port/test.py

    r99907 r100326  
    360360        pass
    361361
    362     def create_driver(self, worker_number):
    363         return TestDriver(self, worker_number)
     362    def _driver_class(self):
     363        return TestDriver
    364364
    365365    def start_http_server(self):
     
    489489        return [self._port._path_to_driver()] + self._port.get_option('additional_drt_flag', [])
    490490
    491     def poll(self):
    492         return True
    493 
    494491    def run_test(self, test_input):
    495492        start_time = time.time()
  • trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py

    r99907 r100326  
    226226        return self._build_path('layout-test-results')
    227227
    228     def create_driver(self, worker_number):
    229         return WebKitDriver(self, worker_number)
     228    def _driver_class(self):
     229        return WebKitDriver
    230230
    231231    def _tests_for_other_platforms(self):
     
    441441    """WebKit implementation of the DumpRenderTree/WebKitTestRunner interface."""
    442442
    443     def __init__(self, port, worker_number):
    444         Driver.__init__(self, port, worker_number)
     443    def __init__(self, port, worker_number, pixel_tests=False):
     444        Driver.__init__(self, port, worker_number, pixel_tests)
    445445        self._driver_tempdir = port._filesystem.mkdtemp(prefix='%s-' % self._port.driver_name())
    446446        # WebKitTestRunner can report back subprocess crashes by printing
     
    458458        self._server_process = None
    459459
    460 
    461460    # FIXME: This may be unsafe, as python does not guarentee any ordering of __del__ calls
    462461    # I believe it's possible that self._port or self._port._filesystem may already be destroyed.
     
    467466        cmd = self._command_wrapper(self._port.get_option('wrapper'))
    468467        cmd.append(self._port._path_to_driver())
    469         if self._port.get_option('pixel_tests'):
     468        if self._pixel_tests:
    470469            cmd.append('--pixel-tests')
    471470        if self._port.get_option('gc_between_tests'):
     
    491490        self._server_process = server_process.ServerProcess(self._port, server_name, self.cmd_line(), environment)
    492491
    493     def poll(self):
    494         return self._server_process.poll()
     492    def has_crashed(self):
     493        if self._server_process is None:
     494            return False
     495        return self._server_process.poll() is not None
    495496
    496497    def _check_for_driver_crash(self, error_line):
  • trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py

    r100118 r100326  
    275275        optparse.make_option("--no-sample-on-timeout", action="store_false",
    276276            dest="sample_on_timeout", help="Don't run sample on timeout (Mac OS X only)"),
     277        optparse.make_option("--no-ref-tests", action="store_true",
     278            dest="no_ref_tests", help="Skip all ref tests"),
    277279        optparse.make_option("--tolerance",
    278280            help="Ignore image differences less than this percentage (some "
  • trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py

    r99773 r100326  
    153153    class RecordingTestDriver(TestDriver):
    154154        def __init__(self, port, worker_number):
    155             TestDriver.__init__(self, port, worker_number)
     155            TestDriver.__init__(self, port, worker_number, pixel_tests=port.get_option('pixel_test'))
    156156            self._current_test_batch = None
    157 
    158         def poll(self):
    159             # So that we don't create a new driver for every test
    160             return None
    161157
    162158        def stop(self):
     
    681677        self.assertEquals(['passes/reftest.html'], tests_run)
    682678
    683     def test_reftest_skip_reftests_if_pixel_tests_are_disabled(self):
     679    def test_reftest_run_reftests_if_pixel_tests_are_disabled(self):
    684680        tests_run = get_tests_run(['--no-pixel-tests', 'passes/reftest.html'], tests_included=True, flatten_batches=True)
     681        self.assertEquals(['passes/reftest.html'], tests_run)
     682
     683    def test_reftest_skip_reftests_if_no_ref_tests(self):
     684        tests_run = get_tests_run(['--no-ref-tests', 'passes/reftest.html'], tests_included=True, flatten_batches=True)
     685        self.assertEquals([], tests_run)
     686        tests_run = get_tests_run(['--no-ref-tests', '--no-pixel-tests', 'passes/reftest.html'], tests_included=True, flatten_batches=True)
    685687        self.assertEquals([], tests_run)
    686688
Note: See TracChangeset for help on using the changeset viewer.