WebDriver Bidi tests with new Py3 WebSockets dependencies #27195

k7z45 · 2021-01-15T00:31:31Z

Created a prototype involving WebDriver Bidi client according to [1].

The dependencies for the WebSockets lib[2] is added in third party lib. Added the dependency for pytest-asyncio[3] in this PR as well.

Later versions of pytest[4] and pluggy[5] are needed for pytest-asyncio and they are included here. Pluggy is also needed and it's already included from rebase.

The change includes:

A simple WebDriver Bidi capabilities test to demonstrate enabling WebSockets according to the protocol.
Upgraded pytest and pluggy third party dependencies
Added an async style test that enables WebSocket by directly specifying the corresponding capability.
Add BidiSession class in WebDriver client for use in fixture to conveniently create a Bidi Session and tests to demonstrate behavior between tests.
[1] #26015 (comment)
[2] https://github.com/aaugustin/websockets
[3] https://github.com/pytest-dev/pytest-asyncio
[4] https://github.com/pytest-dev/pytest
[5] https://github.com/pytest-dev/pluggy

k7z45 · 2021-01-15T01:43:21Z

To run the test, I believe still need to do
pip install pytest-asyncio
locally. (This is no longer needed after adding pytest-asynio from untar)
Then
./wpt --py3 run --log-mach=- --channel=dev chrome webdriver/tests/new_session/websocket_url.py
and
./wpt --py3 run --log-mach=- --channel=dev chrome webdriver/tests/websockets/connect.py

Actually, use python3 wpt run ... instead.

stephenmcgruer

Hi Shengfa, thanks for the PR! I've started to take a look at it, but a quick question:

The actual dependencies for the WebSockets lib[2] [...] are not included in this PR. They are installed locally using pip install.

Is this comment (from the description) still accurate? It looks like you vendored websockets into third_party and added it to localpaths.py so I wasn't sure.

webdriver/tests/support/fixtures.py

tools/webdriver/webdriver/client.py

k7z45 · 2021-01-18T18:06:17Z

Hi Shengfa, thanks for the PR! I've started to take a look at it, but a quick question:

The actual dependencies for the WebSockets lib[2] [...] are not included in this PR. They are installed locally using pip install.

Is this comment (from the description) still accurate? It looks like you vendored websockets into third_party and added it to localpaths.py so I wasn't sure.

Thanks for looking at it. That's not true anymore. I will modify the description tomorrow.

jgraham · 2021-01-21T10:35:32Z

Is it possible to clean up the commits here to make reviewing easier? In particular it would be great to have a single initial commit that vendors in third party code, and then one or more subsequent commits with the additional changes.

k7z45 · 2021-01-21T17:16:11Z

Is it possible to clean up the commits here to make reviewing easier? In particular it would be great to have a single initial commit that vendors in third party code, and then one or more subsequent commits with the additional changes.

Hi James,
I tried to do git rebase -i HEAD~16
with
pick 4b64d94 Upgrade pytest to 6.1.1 to be used with pytest-asyncio
squash ff59ff2 Remove tools/third_party/pytest for subtree add
squash bcc3874 Squashed 'tools/third_party/pytest/' content from commit 0ad20b533f
squash 0b4aec0 Squashed 'tools/third_party/websockets/' content from commit 139085fe26
squash 182e94d Add websockets to localpaths
squash fbedffb Squashed 'tools/third_party/iniconfig/' content from commit 4e20f6bcde
squash 0e7f71f Add iniconfig in localpaths,
unfortunately, I don't know how to resolve all the merge conflicts correctly.

stephenmcgruer · 2021-01-22T19:17:56Z

unfortunately, I don't know how to resolve all the merge conflicts correctly.

Trying to rebase a set of changes which include a subtree add and a merge (89e15e08325f8a8c4ce062477ecb8e5f6d49cdeb Merge branch 'master' into websockets) is difficult. It may be easier building the branch from scratch via cherry-picks (and re-doing the subtree add). Let me know if you need some assistance doing that.

k7z45 · 2021-01-22T20:50:59Z

Hi James,
Could you clarify what commits you expect to see, please?
I am having trouble trying to squash the subtree add commits into one commit.

jgraham · 2021-01-26T15:29:35Z

Baiscally I want all the commits that vendor in libraries first and then either a single commit with your changes or a number of logically seperate commits with your changes, with no merge commits except those from subtree. The use case is to be able to filter out all the vendored code when doing the review.

k7z45 · 2021-01-26T17:02:15Z

Got it, thanks for the clarification.

jgraham

This is really nearly ready to merge. I'm very excited for this to be landed!

tools/webdriver/webdriver/__init__.py

stephenmcgruer

LGTM, but (a) please wait for jgraham to also approve, and (b) can we rebase the commits before landing so that the review changes are squashed back into the appropriate original commit? (E.g. 102b879 would be squashed into 8a43e9f, etc)

k7z45 · 2021-02-10T20:32:39Z

LGTM, but (a) please wait for jgraham to also approve, and (b) can we rebase the commits before landing so that the review changes are squashed back into the appropriate original commit? (E.g. 102b879 would be squashed into 8a43e9f, etc)

Will do that once approved. Thanks for the reminder!

webdriver/tests/support/fixtures.py

webdriver/tests/websockets/connect.py

whimboo · 2021-02-11T06:31:19Z

webdriver/tests/websockets/connect.py

+    await session.websocket_transport.send("test_bidi_session_1")
+    await session.websocket_transport.close()
+
+# bidi session following a bidi session with the same capabilities


This makes the test dependent on a strict order, and test_bidi_session_1 being enabled. To get rid of this all the code should be put into a single test.

One of the reasons for the tests is testing the behavior from re-using websocket_transport between tests with/without capability change. I don't think I can achieve that with a single test.

We might need a similar new_session fixture for BiDi so that you can create multiple new sessions with different capabilities within a single test.

webdriver/tests/websockets/connect.py

webdriver/tests/support/fixtures.py

k7z45 · 2021-02-12T17:01:10Z

Reverted the latest commit for dedicated bidi_session fixture but kept the new bidi test folder.
Rebased commits from comments into appropriate original commit.

Will open another issue for remaining opened conservations after merge for

Creating a bidi_session fixture instead of existing bidi marker to reduce user test code setup for the marker.
Re-use global current_session instead of creating another global bidi session
We might need a similar new_session fixture for BiDi so that you can create multiple new sessions with different capabilities within a single test.

whimboo

I'm ok with moving the remaining parts to a new issue / PR. @jgraham mind doing a review pass yourself?

jgraham

OK, I'm happy for this to land to get things unblocked, but I agree that the proposed followups will create a nicer API.

jgraham · 2021-02-18T06:49:36Z

I think think one needs to be landed by manual push to preserve the merge commits?

stephenmcgruer · 2021-02-18T15:32:22Z

I think think one needs to be landed by manual push to preserve the merge commits?

Yes, I believe so. I will own syncing with Shengfa to get this landed (I'm not sure what permissions we'll need, but I assume I have them and he might not...)

Use git add -f tools/third_party/pytest to include version file. This includes _version.py and pytest.egg-info folder.

Test establishing a Bidirectional Session capability support

Test connecting to WebDriver session using websockets and send a message in async/await style

1. Added a BidiSession class in bidi.py under tools/webdriver/webdriver that will preset capability for websocket connection. Created async versions of start and end for the class to handle establishing and closing websocket connection. 2. Modify session fixture to take an extra boolean argument "bidi" for creating bidi session. Change the fixture to async and use await when dealing with bidi session start/end. 3. Set event loop scope to session(assuming it would reuse the same event loop for all tests) to avoid event loop is closed error when closing websocket connection. 4. Added tests for different bidi/classic session creation scenario and tested connect/send/close.

stephenmcgruer · 2021-02-18T20:48:44Z

I have manually pushed the two subtree adds (which generate merge commits and so cannot be landed via GitHub's UI). We believe the rest of the PR can be landed via GitHub's UI, so shengfa has rebased it and we're waiting for the CI to run.

moz-wptsync-bot added the mozilla:gecko-blocked label Jan 15, 2021

k7z45 force-pushed the websockets branch from bc27953 to 4bec81f Compare January 15, 2021 01:09

k7z45 marked this pull request as ready for review January 15, 2021 01:41

k7z45 requested a review from jgraham as a code owner January 15, 2021 01:41

wpt-pr-bot added infra localpaths.py third_party webdriver wg-testing_browser labels Jan 15, 2021

wpt-pr-bot assigned mjzffr Jan 15, 2021

wpt-pr-bot requested review from andreastt, AutomatedTester, LukeZielinski, mjzffr, shs96c, stephenmcgruer and whimboo January 15, 2021 01:42

stephenmcgruer assigned stephenmcgruer and unassigned mjzffr Jan 15, 2021

stephenmcgruer reviewed Jan 18, 2021

View reviewed changes

webdriver/tests/support/fixtures.py Outdated Show resolved Hide resolved

tools/webdriver/webdriver/client.py Outdated Show resolved Hide resolved

tools/webdriver/webdriver/client.py Outdated Show resolved Hide resolved

stephenmcgruer self-requested a review January 21, 2021 21:00

k7z45 force-pushed the websockets branch from 1840291 to 4cbf884 Compare January 26, 2021 17:33

jgraham reviewed Feb 10, 2021

View reviewed changes

tools/webdriver/webdriver/__init__.py Outdated Show resolved Hide resolved

stephenmcgruer requested review from jgraham and stephenmcgruer February 10, 2021 19:17

stephenmcgruer approved these changes Feb 10, 2021

View reviewed changes

whimboo reviewed Feb 11, 2021

View reviewed changes

jgraham reviewed Feb 12, 2021

View reviewed changes

webdriver/tests/support/fixtures.py Outdated Show resolved Hide resolved

k7z45 force-pushed the websockets branch from f71f47c to 7fb0819 Compare February 12, 2021 16:44

whimboo approved these changes Feb 15, 2021

View reviewed changes

k7z45 requested a review from jgraham February 17, 2021 16:57

jgraham approved these changes Feb 18, 2021

View reviewed changes

k7z45 added 9 commits February 18, 2021 20:39

Add websockets and iniconfig to tools/localpaths.py

43af802

Add pytest from pip directly

b4e5050

Use git add -f tools/third_party/pytest to include version file. This includes _version.py and pytest.egg-info folder.

Add pytest-asyncio to tools third_party

7f0bfbf

Add pytest-asyncio to localpaths

2ccab64

Add a test for WebDriver capabilities for webSocketUrl

2f99020

Test establishing a Bidirectional Session capability support

Add an async style test for connecting to WebDriver

965b5de

Test connecting to WebDriver session using websockets and send a message in async/await style

Import localpaths in resources/test/conftest.py

743d342

Moved bidi test to its own folder

8ee4f2c

k7z45 force-pushed the websockets branch from a397240 to 8ee4f2c Compare February 18, 2021 20:46

stephenmcgruer merged commit def808a into web-platform-tests:master Feb 18, 2021

moz-wptsync-bot removed the mozilla:gecko-blocked label Feb 18, 2021

k7z45 mentioned this pull request Feb 18, 2021

Create better API for WebDriver Bidi Tests #27687

Closed

k7z45 mentioned this pull request Feb 26, 2021

Have a single test to test bidi session with different capabilities #27808

Open

foolip mentioned this pull request May 3, 2021

Survey of vendored dependencies (tools/third_party/) #28801

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WebDriver Bidi tests with new Py3 WebSockets dependencies #27195

WebDriver Bidi tests with new Py3 WebSockets dependencies #27195

k7z45 commented Jan 15, 2021 •

edited

k7z45 commented Jan 15, 2021 •

edited

stephenmcgruer left a comment

k7z45 commented Jan 18, 2021

jgraham commented Jan 21, 2021 •

edited

k7z45 commented Jan 21, 2021 •

edited

stephenmcgruer commented Jan 22, 2021

k7z45 commented Jan 22, 2021

jgraham commented Jan 26, 2021 •

edited

k7z45 commented Jan 26, 2021

jgraham left a comment

stephenmcgruer left a comment

k7z45 commented Feb 10, 2021

whimboo Feb 11, 2021

k7z45 Feb 11, 2021

whimboo Feb 12, 2021

k7z45 commented Feb 12, 2021 •

edited

whimboo left a comment

jgraham left a comment

jgraham commented Feb 18, 2021 •

edited

stephenmcgruer commented Feb 18, 2021

stephenmcgruer commented Feb 18, 2021

WebDriver Bidi tests with new Py3 WebSockets dependencies #27195

WebDriver Bidi tests with new Py3 WebSockets dependencies #27195

Conversation

k7z45 commented Jan 15, 2021 • edited

k7z45 commented Jan 15, 2021 • edited

stephenmcgruer left a comment

Choose a reason for hiding this comment

k7z45 commented Jan 18, 2021

jgraham commented Jan 21, 2021 • edited

k7z45 commented Jan 21, 2021 • edited

stephenmcgruer commented Jan 22, 2021

k7z45 commented Jan 22, 2021

jgraham commented Jan 26, 2021 • edited

k7z45 commented Jan 26, 2021

jgraham left a comment

Choose a reason for hiding this comment

stephenmcgruer left a comment

Choose a reason for hiding this comment

k7z45 commented Feb 10, 2021

whimboo Feb 11, 2021

Choose a reason for hiding this comment

k7z45 Feb 11, 2021

Choose a reason for hiding this comment

whimboo Feb 12, 2021

Choose a reason for hiding this comment

k7z45 commented Feb 12, 2021 • edited

whimboo left a comment

Choose a reason for hiding this comment

jgraham left a comment

Choose a reason for hiding this comment

jgraham commented Feb 18, 2021 • edited

stephenmcgruer commented Feb 18, 2021

stephenmcgruer commented Feb 18, 2021

k7z45 commented Jan 15, 2021 •

edited

k7z45 commented Jan 15, 2021 •

edited

jgraham commented Jan 21, 2021 •

edited

k7z45 commented Jan 21, 2021 •

edited

jgraham commented Jan 26, 2021 •

edited

k7z45 commented Feb 12, 2021 •

edited

jgraham commented Feb 18, 2021 •

edited