Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for WebKit product #9666

Merged

Conversation

Projects
None yet
6 participants
@zdobersek
Copy link
Contributor

zdobersek commented Feb 26, 2018

Allow testing development builds of various WebKit ports against the
web-platform-tests suite. The generic Selenium executors are used for
the testharness and reference tests. Wdspec tests are also supported.

Port-specific WebDriver capabilities have to be set on the executor.
This port can be defined through the --webkit-port command line flag.
At the moment only the GTK+ port is supported.


This change is Reviewable

Add support for WebKit product
Allow testing development builds of various WebKit ports against the
web-platform-tests suite. The generic Selenium executors are used for
the testharness and reference tests. Wdspec tests are also supported.

Port-specific WebDriver capabilities have to be set on the executor.
This port can be defined through the --webkit-port command line flag.
At the moment only the GTK+ port is supported.

@wpt-pr-bot wpt-pr-bot added the infra label Feb 26, 2018

@wpt-pr-bot wpt-pr-bot requested review from gsnedders and jgraham Feb 26, 2018

@foolip

This comment has been minimized.

Copy link
Contributor

foolip commented Feb 26, 2018

Oh, this definitely has overlap with #8979 which I worked on last week. Will this support running Safari as well? What WebDriver implementation is used here?


def capabilities_for_port(webkit_port, binary, binary_args):
if webkit_port == "gtk":
return {"browserName": "MiniBrowser",

This comment has been minimized.

Copy link
@foolip

foolip Feb 26, 2018

Contributor

What is MiniBrowser?

This comment has been minimized.

Copy link
@zdobersek

zdobersek Feb 26, 2018

Author Contributor

It's a small prototyping browser application that's used for quick tests on a WebKit build. As such, it's also usable as a WebDriver automation target.

This comment has been minimized.

Copy link
@foolip

foolip Feb 26, 2018

Contributor

I found https://github.com/WebKit/webkit/tree/master/Tools/MiniBrowser. So it's not specific to the GTK port, but multiple ports have this "product"?

This comment has been minimized.

Copy link
@zdobersek

zdobersek Feb 26, 2018

Author Contributor

Correct. Implementations are different, but in principle it's a limited Web view application, not a full-fledged browsing experience.

There's actually a DesiredCapabilities.WEBKITGTK dict already added to the Selenium project, it could be used here directly like it is for other products, instead of being replicated.

This comment has been minimized.

Copy link
@foolip

foolip Feb 26, 2018

Contributor

I'm not clear on how the capabilities are used, but consistency sounds good.

@zdobersek

This comment has been minimized.

Copy link
Contributor Author

zdobersek commented Feb 26, 2018

Safari requires safaridriver, a separate WebDriver implementation. These changes have been tested against the WebDriver implementation developed inside WebKit:
https://trac.webkit.org/browser/trunk/Source/WebDriver

I don't think it's likely that Safari will switch to that implementation, but majority of your work and mine could be combined if the WebDriver binary selection can be flexible enough to search for safaridriver in case of the MacOS WebKit port.

I raised a discussion about this work on webkit-dev, but it wasn't terribly extensive:
https://lists.webkit.org/pipermail/webkit-dev/2018-February/029865.html

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Feb 26, 2018

Build PASSED

Started: 2018-03-01 18:22:31
Finished: 2018-03-01 18:35:52

View more information about this build on:

class WebKit(Browser):
"""WebKit-specific interface.
Includes installation, webdriver installation, and wptrunner setup methods.

This comment has been minimized.

Copy link
@foolip

foolip Feb 26, 2018

Contributor

I changed the other docstrings to be more accurate, could do the same here.



def env_options():
return {"host": "web-platform.test",

This comment has been minimized.

Copy link
@gsnedders

gsnedders Feb 28, 2018

Contributor

host isn't needed here.

}
return capabilities

return None

This comment has been minimized.

Copy link
@jgraham

jgraham Mar 1, 2018

Contributor

Don't you want to return an empty dict here?

This comment has been minimized.

Copy link
@zdobersek

zdobersek Mar 1, 2018

Author Contributor

Right.

@jgraham

jgraham approved these changes Mar 1, 2018

@foolip

This comment has been minimized.

Copy link
Contributor

foolip commented Mar 3, 2018

I have no further comments on this. @jgraham, do you want to go ahead and merge?

@jgraham jgraham merged commit adf8750 into web-platform-tests:master Mar 3, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@foolip

This comment has been minimized.

Copy link
Contributor

foolip commented Nov 4, 2018

@zdobersek, I see that webkit isn't tested in tools/wptrunner tox tests, and also requirements_webkit.txt isn't updated by pyup. Perhaps this is something you'd like to address?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.