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

Actions automation #8159

Closed
wants to merge 10 commits into from
Closed

Actions automation #8159

wants to merge 10 commits into from

Conversation

kereliuk
Copy link
Contributor

@kereliuk kereliuk commented Nov 13, 2017

This is a PR to get the ball rolling about automating more of the selenium capability in testdriver.js.

Want to know what people think about the way we will be extending the python selenium bindings.

I've included

  • basic functionality for sending actions in testdriver.js tests
  • an example of part of a manual test that can now be automated

AFAIK you cannot add input sources with the current selenium python bindings (It's there just not exposed?) So that would be next so we can take advantage of the W3C actions with multiple inputs.

Still a WIP (no optional parameters and missing error checks) and needs some clean up and documentation.

cc/ @gsnedders @foolip

#7434
#7435


This change is Reviewable

@ghost
Copy link

ghost commented Nov 13, 2017

Build PASSED

Started: 2017-11-14 21:09:14
Finished: 2017-11-14 21:23:47

View more information about this build on:

<head>
<meta charset="utf-8">
<title>Action</title>
<link rel="help" href="https://w3c.github.io/uievents/#events-clickevent-event-order">
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@@ -0,0 +1,27 @@
<!DOCTYPE html>
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is just a proof of concept test here. Right? Wasn't there a simple test already in pointerevents that only needed a move or something that you could reuse?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this is just proof of concept. What is the test you are thinking of?

Copy link
Member

Choose a reason for hiding this comment

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

@kereliuk kereliuk self-assigned this Nov 13, 2017
@@ -30,5 +30,8 @@ Note that if the element to be clicked does not have a unique ID, the
document must not have any DOM mutations made between the function
being called and the promise settling.

### `testdriver.actions`

http://selenium-python.readthedocs.io/api.html#module-selenium.webdriver.common.action_chains
Copy link
Member

Choose a reason for hiding this comment

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

Can this be written in terms of the WebDriver API surface, to not assume that testdriver is implemented using Selenium?

Copy link
Contributor

@sideshowbarker sideshowbarker left a comment

Choose a reason for hiding this comment

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

Only reviewed the docs change, but that at least LGTM

@kereliuk
Copy link
Contributor Author

@jgraham @gsnedders Do you have any comments for this :)

@gsnedders
Copy link
Member

At a glance this looks good, will look closer tomorrow.

@jgraham
Copy link
Contributor

jgraham commented Nov 20, 2017

Sorry, missed this somehow. I'm worried about the way this appears to be tightly coupled to the Selenium API rather than the underlying actions model, but I need to look more closely.

@kereliuk
Copy link
Contributor Author

@jgraham I have the same worry. Right now the existing click automation is in terms of the selenium API. I think we should be consistent in either using selenium or using WebDriver directly.
WDYT?

@gsnedders
Copy link
Member

Right now the existing click automation is in terms of the selenium API. I think we should be consistent in either using selenium or using WebDriver directly.

In principle it should be possible to implement all of these things not using Selenium or WebDriver without having to implement too overhead (e.g., click using gpuBenchmarking). It's not accidental that click does all of the complex WebDriver-like validation in the non-_internal function, making it trivial to implement _internal.click. We should definitely expect people will want to implement this without using Selenium or WebDriver (e.g., in content_shell).

@kereliuk
Copy link
Contributor Author

In principle it should be possible to implement all of these things not using Selenium or WebDriver without having to implement too overhead (e.g., click using gpuBenchmarking). It's not accidental that click does all of the complex WebDriver-like validation in the non-_internal function, making it trivial to implement _internal.click.

I think it still makes sense to provide WebDriver like capability for automation. Also in this PR we have all of the validation in the non-_internal function.

We should definitely expect people will want to implement this without using Selenium or WebDriver (e.g., in content_shell).

Why would people need other implementations of clicks (i.e. for content shell)?

If we agree to use WebDriver for automation in WPT, I think we should either use the protocol directly (send requests directly), or have a wrapper on the existing selenium API.

@jgraham
Copy link
Contributor

jgraham commented Nov 21, 2017

Sorry I don't have more time to look at this at the moment.

Being able to reimplement (some/all) of the API in terms of browser-internal functions was one of the design requirements, both potentially for performance reasons, but also for functional ones (i.e. WebDriver itself isn't available in all environments).

I am specifically worried about using the Selenium API in this case because it outsources the specification of what various actions mean to the Selenium implementation. That is subject to change, invalidating tests, and also hard to implement if Selenium itself is not being used. For actions it's particularly problematic because the Selenium API is much higher level than the underlying WebDriver primitives.

@foolip
Copy link
Member

foolip commented Nov 21, 2017

Why would people need other implementations of clicks (i.e. for content shell)?

@JKereliuk, this is actually what we did, see https://bugs.chromium.org/p/chromium/issues/detail?id=781231

We had the Q3 OKR for "Enable web-platform-tests using WebDriver to run in content_shell" that we decided not to pursue then, but something like that would be required (in all engines) in order to assume WebDriver. So defining the behavior in terms of WebDriver but allowing other implementations seems like the way to go.

@kereliuk
Copy link
Contributor Author

kereliuk commented Jan 8, 2018

Closing this as I will write a similar PR that uses the WebDriver API directly and not Selenium

@kereliuk kereliuk closed this Jan 8, 2018
@sideshowbarker sideshowbarker deleted the actions-automation branch November 22, 2018 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants