Summary: | Add run-web-platform-tests script | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||||||||||
Component: | Tools / Tests | Assignee: | Zan Dobersek <zan> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | amal, bburg, brendan, cdumez, clopez, ews-watchlist, fred.wang, glenn, jbedard, lforschler, rego, webkit-bug-importer, youennf | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Zan Dobersek
2018-03-05 23:50:07 PST
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.
Created attachment 338095 [details]
Patch
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. Created attachment 338410 [details]
WIP
Still missing unit tests.
Created attachment 339209 [details]
Patch
Now with unittests.
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.
Youenn, any feedback? 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. Woups, my comments were uploaded to quickly. Anyway, looks good to me. It would be nice to have some additional feedback from others. 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. Hi Zan, I'm curious how this is different than the existing test runners? There is ongoing work to 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 (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). Created attachment 341651 [details]
Patch
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.
Comment on attachment 341651 [details]
Patch
Patch LGTM, but I think it needs a rebase to apply on last master.
Created attachment 342519 [details]
Patch for landing
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.
Comment on attachment 342519 [details] Patch for landing Clearing flags on attachment: 342519 Committed r232746: <https://trac.webkit.org/changeset/232746> All reviewed patches have been landed. Closing bug. |