Bug 225516 – Merge WebDriverTests into LayoutTests

• Status: NEW
• Component: Tools / Tests

[Sam Sneddon [:gsnedders]]

As we move to having WPT automatically imported, it's probably worthwhile revisiting the decision to put the WebDriverTests in their own top-level directory. While they're certainly somewhat different to the majority of the tests in LayoutTests, they don't necessarily seem any more odd than having the inspector tests there.

[Radar WebKit Bug Importer]

<rdar://problem/78018630>

[Sam Sneddon [:gsnedders]]

One thing I've noticed recently is that there's a variety of tests in the WebDriverTests import that rely on /common, which doesn't exist in that import. This does seem like a notable downside of having a second import of part of WPT?

gsnedders@gsnedders-marsha OpenSource % rg '/common' WebDriverTests/imported/w3c --glob '!tools'
WebDriverTests/imported/w3c/webdriver/tests/forward/forward.py
87:        url("/common/blank.html"),
88:        url("/common/blank.html#1234"),
89:        url("/common/blank.html#5678"),

WebDriverTests/imported/w3c/webdriver/tests/get_named_cookie/get.py
28:    session.url = url("/common/blank.html")
60:    session.url = url("/common/blank.html")
98:    session.url = url("/common/blank.html")
127:    session.url = url("/common/blank.html", protocol="https")

WebDriverTests/imported/w3c/webdriver/tests/get_named_cookie/user_prompts.py
20:        create_cookie("foo", value="bar", path="/common/blank.html")
38:        create_cookie("foo", value="bar", path="/common/blank.html")
53:        create_cookie("foo", value="bar", path="/common/blank.html")

WebDriverTests/imported/w3c/webdriver/tests/add_cookie/user_prompts.py
24:        session.url = url("/common/blank.html")
46:        session.url = url("/common/blank.html")
69:        session.url = url("/common/blank.html")

WebDriverTests/imported/w3c/webdriver/tests/add_cookie/add.py
29:    session.url = url("/common/blank.html")
104:    session.url = url("/common/blank.html")
134:    session.url = "http://127.0.0.1:%s/common/blank.html" % (server_config["ports"]["http"][0])
163:    session.url = url("/common/blank.html")
188:    session.url = url("/common/blank.html")
213:    session.url = url("/common/blank.html")
241:    session.url = url("/common/blank.html")
267:    session.url = url("/common/blank.html")
282:    session.url = url("/common/blank.html")

WebDriverTests/imported/w3c/webdriver/tests/permissions/set.py
35:    session.url = url("/common/blank.html", protocol="https")
48:    session.url = url("/common/blank.html", protocol="http")
64:    session.url = url("/common/blank.html", protocol="https")
104:    session.url = url("/common/blank.html", protocol="https")
107:    session.url = url("/common/blank.html", protocol="https")

WebDriverTests/imported/w3c/webdriver/tests/delete_all_cookies/user_prompts.py
18:        create_cookie("foo", value="bar", path="/common/blank.html")
35:        create_cookie("foo", value="bar", path="/common/blank.html")
52:        create_cookie("foo", value="bar", path="/common/blank.html")

WebDriverTests/imported/w3c/webdriver/tests/delete_cookie/user_prompts.py
17:        create_cookie("foo", value="bar", path="/common/blank.html")
35:        create_cookie("foo", value="bar", path="/common/blank.html")
52:        create_cookie("foo", value="bar", path="/common/blank.html")

WebDriverTests/imported/w3c/webdriver/tests/back/back.py
75:        url("/common/blank.html"),
76:        url("/common/blank.html#1234"),
77:        url("/common/blank.html#5678"),

[Carlos Alberto Lopez Perez]

WebDriver tests are run with a different tooling.

The runner for this tests is Tools/Scripts/run-webdriver-tests

Their expectations are also different (they use json files instead of TestExpectation text-based files). Check WebDriverTests/TestExpectations.json

Also to the best of my knowledge the run-webdriver-tests script doesn't support the Mac ports currently. 

Only GTK and WPE ports are currently supported.
We have 4 post-commit bots running this tests:

https://build.webkit.org/#/builders/730
https://build.webkit.org/#/builders/732
https://build.webkit.org/#/builders/37
https://build.webkit.org/#/builders/18

But there are no EWS bots testing this, so take into account when doing pull-requests.

So I'm not sure if having then inside LayoutTests directory is a good idea. Doing that would likely require a rewrite of the current tooling.

[Sam Sneddon [:gsnedders]]

To be clear: I'm not suggesting here we change any of the tooling or expectation format or anything, just that we change the path of the import from WebDriverTests/imported/w3c/webdriver to LayoutTests/imported/w3c/web-platform-tests/webdriver (well, not changing the tooling _beyond_ changing the path the tests are located at, which doesn't at first glance look too invasive?).

And yes, it's correct that it doesn't support the Mac ports—but that's also because the SafariDriver implementation isn't in WebKit.

[Carlos Alberto Lopez Perez]

(In reply to Sam Sneddon [:gsnedders] from comment #4)
> To be clear: I'm not suggesting here we change any of the tooling or
> expectation format or anything, just that we change the path of the import
> from WebDriverTests/imported/w3c/webdriver to
> LayoutTests/imported/w3c/web-platform-tests/webdriver (well, not changing
> the tooling _beyond_ changing the path the tests are located at, which
> doesn't at first glance look too invasive?).
> 
> And yes, it's correct that it doesn't support the Mac ports—but that's also
> because the SafariDriver implementation isn't in WebKit.

In theory sounds something doable but there may be unexpected issues in practice.

As far as I remember everything imported under LayoutTests/imported/w3c/web-platform-tests gets imported with the script Tools/Scripts/import-w3c-tests that sometimes does funny thing to the imports, rewriting stuff and somethings breaking it.

There was a meta bug here where I tried to link the issues that I found with the tool, but is already old. Still I guess many issues remain unfixed. https://bugs.webkit.org/show_bug.cgi?id=207734

On the other hand the current tests at WebDriver directory are imported with another script Tools/Scripts/import-webdriver-tests that is simple and doesn't rewrite stuff

To me it doesn't sound right moving the WebDriver tests inside the Layout test one: they work differently and are not handled the same way. Gardening them requires specialized knowledge about how the TestExpectations are different. They use a different runner, etc. 

What would be the benefit of moving the tests inside that directory?

Also take into account that inside WebDriverTests/imported there are not only the w3c tests, but also the selenium ones that get imported from https://github.com/SeleniumHQ/selenium.git

In any case I'm not an expert on this. I would wait for Lauro Moura and Carlos Garcia to comment on this.