Bug 183356 – Add run-web-platform-tests script

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Zan Dobersek]

This script would allow us to run the web-platform-tests suite.
https://github.com/w3c/web-platform-tests

[Zan Dobersek]

Created attachment 335070 [details]
WIP patch

Work-in-progress run-web-platform-tests script that supports running the W3C web-platform-tests suite for the GTK+ port.

- MiniBrowser change shouldn't be necessary, but SSL certificates have to be correctly generated.
- The Python script has to be cleaned up and segmented into testable bits.
- Path to a web-platform-tests checkout still has to be passed via --source argument, but this should instead be automatically downloaded, as is the case with the import-w3c-tests script.
- 2dcontext and WebCryptoAPI tests are enabled -- more should follow, but it's an OK start.

[Zan Dobersek]

Created attachment 338095 [details]
Patch

[youenn fablet]

I would tend to make the main script as a class with various methods.
That would allow to improve the readability and maybe we could then write some unit tests for these.

As a side note, the manifest.json generation is quite long iirc, so maybe it could be cached somewhere based on the revision of the repo.

[Zan Dobersek]

Created attachment 338410 [details]
WIP

Still missing unit tests.

[Zan Dobersek]

Created attachment 339209 [details]
Patch

Now with unittests.

[EWS Watchlist]

Attachment 339209 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/w3c/wpt_runner_unittest.py:88:  whitespace before '}'  [pep8/E202] [5]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zan Dobersek]

Youenn, any feedback?

[youenn fablet]

Comment on attachment 339209 [details]
Patch

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

> Tools/Scripts/webkitpy/w3c/wpt_runner.py:54
> +        top_level_directory = os.path.normpath(os.path.join(os.path.dirname(__file__), '..', '..', '..', '..'))

We usually prefer using filesystem instead of os.path here and below.

> Tools/Scripts/webkitpy/w3c/wpt_runner.py:61
> +            print '***'

Could we use _log.warning instead?

> Tools/Scripts/webkitpy/w3c/wpt_runner.py:65
> +    display_driver = port.create_driver(worker_number=0, no_timeout=True)._make_driver(pixel_tests=False)

We are not supposed to use private methods directly. I see that _make_driver and _setup_environ_for_test are used directly in webdriver_test_runner.py already though.

> Tools/Scripts/webkitpy/w3c/wpt_runner.py:133
> +            self._options.wpt_checkout = self._finder.path_from_webkit_base("WebKitBuild", "w3c-tests", "web-platform-tests")

The default checkout path could be shared with the importer and exporter.

[youenn fablet]

Woups, my comments were uploaded to quickly.
Anyway, looks good to me. It would be nice to have some additional feedback from others.

[Zan Dobersek]

Comment on attachment 339209 [details]
Patch

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

>> Tools/Scripts/webkitpy/w3c/wpt_runner.py:54
>> +        top_level_directory = os.path.normpath(os.path.join(os.path.dirname(__file__), '..', '..', '..', '..'))
> 
> We usually prefer using filesystem instead of os.path here and below.

OK.

>> Tools/Scripts/webkitpy/w3c/wpt_runner.py:61
>> +            print '***'
> 
> Could we use _log.warning instead?

OK.

>> Tools/Scripts/webkitpy/w3c/wpt_runner.py:65
>> +    display_driver = port.create_driver(worker_number=0, no_timeout=True)._make_driver(pixel_tests=False)
> 
> We are not supposed to use private methods directly. I see that _make_driver and _setup_environ_for_test are used directly in webdriver_test_runner.py already though.

That's right, both scripts require such driver setup and I just mirrored webdriver_test_runner.py for that.

>> Tools/Scripts/webkitpy/w3c/wpt_runner.py:133
>> +            self._options.wpt_checkout = self._finder.path_from_webkit_base("WebKitBuild", "w3c-tests", "web-platform-tests")
> 
> The default checkout path could be shared with the importer and exporter.

I can move this into a single location, perhaps a new Python module under webkitpy.w3c.

[Amal Hussein]

Hi Zan, 

I'm curious how this is different than the existing test runners? There is ongoing work to

[Amal Hussein]

Hi Zan, 

I'm curious how this is different than the existing test runner? There is ongoing work to automate the importing, running tests and auto-updating the expectations file. There is also ongoing work to automate the export flow. 

Amal

[Zan Dobersek]

(In reply to Amal Hussein from comment #12)
> Hi Zan, 
> 
> I'm curious how this is different than the existing test runner? There is
> ongoing work to automate the importing, running tests and auto-updating the
> expectations file. There is also ongoing work to automate the export flow. 
> 
> Amal

This work doesn't mean to replace the existing W3C testing process that's based on importing the web-platform-tests test cases and running them inside the regression testing process. At some point it might come to that, but it's far from it today (or whenever this script lands). This hasn't changed since the debate in webkit-dev:
https://lists.webkit.org/pipermail/webkit-dev/2018-February/029869.html

Value of this script is that it enables running an 'original' WPT checkout against a WebKit build. While it can't replace the current W3C testing process, I think it can serve as a simple standards compliance scoreboard, and CI bots producing that would be set up.

So to a degree, this would duplicate the testing coverage. I would still argue there's value in adopting both approaches -- the current one aims to integrate these tests into the regression testing process for the purposes of developing compliant implementations inside WebKit, while the new one would test WebKit against the official test repository. Issue I'm most worried about is how much maintenance the additional testing process would require (in terms of gardening, i.e. adjusting expectations).

[Zan Dobersek]

Created attachment 341651 [details]
Patch

[EWS Watchlist]

Attachment 341651 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/w3c/wpt_runner_unittest.py:88:  whitespace before '}'  [pep8/E202] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Alberto Lopez Perez]

Comment on attachment 341651 [details]
Patch

Patch LGTM, but I think it needs a rebase to apply on last master.

[Zan Dobersek]

Created attachment 342519 [details]
Patch for landing

[EWS Watchlist]

Attachment 342519 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/w3c/wpt_runner_unittest.py:88:  whitespace before '}'  [pep8/E202] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zan Dobersek]

Comment on attachment 342519 [details]
Patch for landing

Clearing flags on attachment: 342519

Committed r232746: <https://trac.webkit.org/changeset/232746>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41036622>