Bug 160833 – Allow a port to run tests with a custom device setup

• Component: New Bugs
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Allow a port to run tests with a custom device setup

[Simon Fraser (smfr)]

Created attachment 285982 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 285988 [details]
Patch

[Daniel Bates]

Comment on attachment 285988 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285988&action=review

It would be good if we had more test coverage for the changes. The changes look sane.

> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:106
> +    def _custom_device_for_test(self, test):
> +        for device_class in self._port.CUSTOM_DEVICE_CLASSES:
> +            directory_suffix = device_class + self._port.TEST_PATH_SEPARATOR
> +            if directory_suffix in test:
> +                return device_class
> +        return None

We could add a test for this function in manager_unittest.py.

> Tools/Scripts/webkitpy/layout_tests/controllers/manager.py:220
> +                _log.debug('%s tests use device %s', len(custom_device_tests[device_class]), device_class)

Nit: I know this is a debug logging. We could use pluralize() here to emit "test" or "tests".

> Tools/Scripts/webkitpy/port/driver.py:56
> +        return "DriverInput(test_name='%s', timeout=%s, image_hash=%s, should_run_pixel_test=%s'" % (self.test_name, self.timeout, self.image_hash, self.should_run_pixel_test)

Nit: I believe we prefer using String.format() instead of the % operator towards making this code compatible with Python 3. I'm unclear of the status of our effort.

> Tools/Scripts/webkitpy/port/ios.py:89
> +            'ipad': 'iPad Air'},

For your consideration, I suggest we put the "}," on its on own line. This has the benefit that when appending a new device type to the end of this dictionary that we do not need to modify this line.

> Tools/Scripts/webkitpy/port/ios.py:92
> +            'ipad': 'iPad Retina'},

Ditto.

> Tools/Scripts/webkitpy/port/port_testcase.py:254
> +        print port

Did you intend to add this line?

[Simon Fraser (smfr)]

https://trac.webkit.org/r204477