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 tests for proposed WebDriver Shadow DOM support #27132

Merged
merged 24 commits into from Jan 14, 2021

Conversation

jimevans
Copy link
Contributor

The tests within this PR correspond to the changes proposed by the PR to the WebDriver Specification for supporting Shadow DOM

Copy link
Contributor

@whimboo whimboo left a comment

Choose a reason for hiding this comment

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

Thanks @jimevans for taking a stab on it. I have a couple of comments that I added inline. I would appreciate to have good test coverage for scenarios we are already aware of for other commands. I hope that this will be fine with you.

tools/webdriver/webdriver/client.py Outdated Show resolved Hide resolved
tools/webdriver/webdriver/client.py Outdated Show resolved Hide resolved
tools/webdriver/webdriver/client.py Show resolved Hide resolved
webdriver/tests/find_element_from_shadow_root/find.py Outdated Show resolved Hide resolved
webdriver/tests/find_element_from_shadow_root/find.py Outdated Show resolved Hide resolved
webdriver/tests/find_element_from_shadow_root/find.py Outdated Show resolved Hide resolved
Copy link
Contributor

@whimboo whimboo left a comment

Choose a reason for hiding this comment

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

It's all great work. Thanks Jim! I have some more comments here. Most apply to all of the different commands.

@@ -0,0 +1,71 @@
@pytest.fixture
def get_shadow_page(inline, shadow_content):
return inline("""
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually wonder how this can work with the current implementation. What you return here is a string as produced by inline(). In the tests you call get_shadow_page(...), which has to fail on a string.

So what you want is to return an inner function; the same as what we do for most of the global fixtures. And only the inner method has the shadow_content argument.

This also applies to the other commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged. Will work on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Took a stab at this in c9921ba. I've probably botched it, so do feel free to re-review.

webdriver/tests/find_element_from_shadow_root/find.py Outdated Show resolved Hide resolved
Copy link
Contributor

@whimboo whimboo left a comment

Choose a reason for hiding this comment

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

This looks all fine to me now (by not being able to run any of these tests). Thanks a lot.

@jgraham jgraham merged commit a7ccbc6 into web-platform-tests:master Jan 14, 2021
@jimevans jimevans deleted the webdriver-shadow-dom branch November 10, 2021 17:31
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.

None yet

7 participants