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

Generic executor that uses WebDriver protocol directly #10197

Closed
wants to merge 10 commits into from

Conversation

kereliuk
Copy link
Contributor

@kereliuk kereliuk commented Mar 27, 2018

The motivation behind this is to give the option to not use Selenium with WebDriver. There are multiple reasons one might want to do this:

  • Wanting to test the WebDriver protocol directly
  • Not wanting to worry about changes in Selenium affecting WPT
  • Using functionality in a WebDriver implementation that is not implemented in Selenium

This change is Reviewable

@ghost
Copy link

ghost commented Mar 27, 2018

Build BROKEN

Started: 2018-04-03 18:53:33
Finished: 2018-04-03 19:11:39

Failing Jobs

  • tools_unittest in py27
  • tools_unittest in pypy

View more information about this build on:

@kereliuk
Copy link
Contributor Author

kereliuk commented May 8, 2018

@gsnedders @jgraham PTAL :)

@@ -51,7 +53,15 @@ def executor_kwargs(test_type, server_config, cache_manager, run_info_data,
capabilities["chromeOptions"]["excludeSwitches"] = ["enable-automation"]
if test_type == "wdspec":
capabilities["chromeOptions"]["w3c"] = True
executor_kwargs["capabilities"] = capabilities

if __wptrunner__["executor"]["testharness"] == "WebDriverTestharnessExecutor" \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use line contnuation characters.

@@ -42,6 +42,11 @@ def _set(self, key, secs):
timeouts = self.session.send_session_command("POST", "timeouts", body)
return None

def set_w3c(self, timeout, secs):
body = {"type": timeout, "ms": secs * 1000}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add this into the webdriver client rather than here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean, this is in tools/webdriver/webdriver/client.py which is the webdriver client?

return self.protocol.is_alive()

def do_test(self, test):
self.logger.info("Test requires OS-level window focus")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's only really true for Chrome, and maybe isn't true anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guilty of copy paste here. Also I'm not sure if its true anymore either. I will delete this.

@@ -3,6 +3,8 @@
from ..executors import executor_kwargs as base_executor_kwargs
from ..executors.executorselenium import (SeleniumTestharnessExecutor,
SeleniumRefTestExecutor)
from ..executors.executorwebdriver import (WebDriverTestharnessExecutor,
WebDriverRefTestExecutor)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent error

capabilities["chromeOptions"]["w3c"] = True
always_match = {"alwaysMatch": capabilities}
executor_kwargs["capabilities"] = always_match

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blank line here.

executor_kwargs["capabilities"] = capabilities

if __wptrunner__["executor"]["testharness"] == "WebDriverTestharnessExecutor" \
or __wptrunner__["executor"]["reftest"] == "WebDriverRefTestExecutor":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really a swappable thing, so I wonder what the intent here is? Per the current design you would have to make this a different product like chrome-webdriver.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't consider that for some reason... that sounds like a cleaner way to do it. Good suggestion :)

@foolip foolip mentioned this pull request Aug 9, 2018
7 tasks
@gsnedders gsnedders closed this Aug 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants