Bug 177304 - WebDriver: Add support to import and run W3C tests
Summary: WebDriver: Add support to import and run W3C tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-09-21 09:02 PDT by Carlos Garcia Campos
Modified: 2017-11-15 13:12 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.46 MB, patch)
2017-09-21 09:25 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (1.49 MB, patch)
2017-09-22 10:40 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (1.49 MB, patch)
2017-09-22 10:42 PDT, Carlos Garcia Campos
bburg: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2017-09-21 09:02:23 PDT
WPT has now several web driver tests, and new ones are going to be added to cover the whole spec. See 

https://docs.google.com/spreadsheets/d/1GUK_sdY2cv59VAJNDxZQIfypnOpapSQhMjfcJ9Wc42U/edit#gid=0

We should be able to run those tests in our bots. We might want to import the selenium tests, or even add our owns, but for now, passing the w3c tests should ensure we properly implement the spec.
Comment 1 Carlos Garcia Campos 2017-09-21 09:25:53 PDT
Created attachment 321434 [details]
Patch
Comment 2 Build Bot 2017-09-21 09:32:58 PDT
Attachment 321434 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:29:  No name 'mozlog' in module 'webkitpy.thirdparty.autoinstalled'  [pylint/E0611] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:30:  No name 'mozprocess' in module 'webkitpy.thirdparty.autoinstalled'  [pylint/E0611] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:115:  [WebKitDriverServer.make_command] Instance of 'WebKitDriverServer' has no 'binary' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:115:  [WebKitDriverServer.make_command] Instance of 'WebKitDriverServer' has no 'port' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:115:  [WebKitDriverServer.make_command] Instance of 'WebKitDriverServer' has no '_args' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:132:  [WebDriverW3CExecutor.setup] Instance of 'WebDriverW3CExecutor' has no 'protocol' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:135:  [WebDriverW3CExecutor.teardown] Instance of 'WebDriverW3CExecutor' has no 'protocol' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:138:  [WebDriverW3CExecutor.run] Instance of 'WebDriverW3CExecutor' has no 'do_wdspec' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:138:  [WebDriverW3CExecutor.run] Instance of 'WebDriverW3CExecutor' has no 'protocol' member  [pylint/E1101] [5]
Total errors found: 9 in 237 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 BJ Burg 2017-09-21 10:21:36 PDT
Comment on attachment 321434 [details]
Patch

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

Overall, importing WebDriver tests to a separate directory on a different timescale seems like the right thing to do. We don't want WebDriver test updates to be blocked on rebaselining updated W3C layout tests.

However, I really don't want two copies of the same WPT tools in the tree. Especially because they import pytest, html5lib, and other dependencies into their tree instead of using versioned modules, so there is a lot of 3rd party code sitting around. This is an invitation for cruft and problems.

Carlos and I talked in IRC about ways to avoid doing this. Imported layout tests probably only use wptserve, so we can try to import just that for layout tests and import the full tools to WebDriverTests. Or, we can have a shared copy of tools and update the two test suites independently.

> Tools/Scripts/run-webdriver-tests:46
> +option_parser.add_option('--display-server', choices=['xvfb', 'xorg', 'weston', 'wayland'], default='xvfb',

I think this option won't make any sense on other platforms than GTK?
Comment 4 Carlos Garcia Campos 2017-09-22 00:47:48 PDT
(In reply to Brian Burg from comment #3)
> Comment on attachment 321434 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=321434&action=review
> 
> Overall, importing WebDriver tests to a separate directory on a different
> timescale seems like the right thing to do. We don't want WebDriver test
> updates to be blocked on rebaselining updated W3C layout tests.

Exactly!

> However, I really don't want two copies of the same WPT tools in the tree.
> Especially because they import pytest, html5lib, and other dependencies into
> their tree instead of using versioned modules, so there is a lot of 3rd
> party code sitting around. This is an invitation for cruft and problems.

Agree.

> Carlos and I talked in IRC about ways to avoid doing this. Imported layout
> tests probably only use wptserve, so we can try to import just that for
> layout tests and import the full tools to WebDriverTests. Or, we can have a
> shared copy of tools and update the two test suites independently.

I've checked it and layout tests don't need wptrunner and pytest at all, so we can update the importer to import only wptserve and its dependencies from tools directory. This way we won't have two copies of wptrunner, and we'll also reduce a bit the size since there are more things in tools that are unused.

> > Tools/Scripts/run-webdriver-tests:46
> > +option_parser.add_option('--display-server', choices=['xvfb', 'xorg', 'weston', 'wayland'], default='xvfb',
> 
> I think this option won't make any sense on other platforms than GTK?

Right, same way it happens in run-webkit-tests, I don't think we can add options that depends on the value of other values. What we could do is adding groups to include the port specific options when we add support for more ports.
Comment 5 Carlos Garcia Campos 2017-09-22 10:40:25 PDT
Created attachment 321562 [details]
Updated patch

I've updated the WPT tests, since it fixes tests, and now it also needs wptserve. Before the update all html content was loaded as data urls, but it seems edge doens't like those data urls (really?), so they changed to use http urls, which now requires wptserve to handle those requests. This patch adds a simple server implementation that launches the layout tests wpt server, but with a different configuration. This way we can reuse the implementation and we don't need to import wptserve for WebDriver tests too.
Comment 6 Carlos Garcia Campos 2017-09-22 10:42:03 PDT
Created attachment 321564 [details]
Patch

I forgot to git add the wpt server config file.
Comment 7 Build Bot 2017-09-22 10:49:46 PDT
Attachment 321564 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:29:  No name 'mozlog' in module 'webkitpy.thirdparty.autoinstalled'  [pylint/E0611] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:30:  No name 'mozprocess' in module 'webkitpy.thirdparty.autoinstalled'  [pylint/E0611] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:115:  [WebKitDriverServer.make_command] Instance of 'WebKitDriverServer' has no 'binary' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:115:  [WebKitDriverServer.make_command] Instance of 'WebKitDriverServer' has no 'port' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:115:  [WebKitDriverServer.make_command] Instance of 'WebKitDriverServer' has no '_args' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:132:  [WebDriverW3CExecutor.setup] Instance of 'WebDriverW3CExecutor' has no 'protocol' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:135:  [WebDriverW3CExecutor.teardown] Instance of 'WebDriverW3CExecutor' has no 'protocol' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:138:  [WebDriverW3CExecutor.run] Instance of 'WebDriverW3CExecutor' has no 'do_wdspec' member  [pylint/E1101] [5]
ERROR: Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_executor.py:138:  [WebDriverW3CExecutor.run] Instance of 'WebDriverW3CExecutor' has no 'protocol' member  [pylint/E1101] [5]
Total errors found: 9 in 244 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 BJ Burg 2017-09-25 12:08:38 PDT
Comment on attachment 321564 [details]
Patch

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

r=me

> Tools/Scripts/webkitpy/webdriver_tests/webdriver_w3c_web_server.py:48
> +        if self._port.host.platform.is_mac():

Shouldn't this use mkdtemp or similar? Why is Mac a special case?
Comment 9 Carlos Garcia Campos 2017-10-26 03:58:33 PDT
Committed r224014: <https://trac.webkit.org/changeset/224014>
Comment 10 Radar WebKit Bug Importer 2017-11-15 13:12:46 PST
<rdar://problem/35569015>