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

Create better API for WebDriver Bidi Tests #27687

Closed
k7z45 opened this issue Feb 18, 2021 · 3 comments
Closed

Create better API for WebDriver Bidi Tests #27687

k7z45 opened this issue Feb 18, 2021 · 3 comments
Assignees

Comments

@k7z45
Copy link
Contributor

k7z45 commented Feb 18, 2021

This is a follow up issue to 27195. There were some remaining conversations but we decided to merge what we had to get things unblocked.

Summary:

  1. Creating a bidi_session fixture instead of existing bidi marker to reduce user test code setup for the marker. [1][2]
  2. Re-use global current_session instead of creating another global bidi session. [4]
  3. [3]

[1] Instead of a marker, which would mean that we have to add it before each and every test method, having a dedicated bidi_session fixture would cause way lesser code to write:

async def test_log_events(bidi_session, url):
    bidi_session.url = url("/tests/bidi/support/log_events.html")
    ...

That way we can also factor out all the bidi specific code from the session fixture itself, and have a clear separation for the BiDi extension.

What do you think?

Originally posted by @whimboo in #27195 (comment)
[2] With the dedicated session approach this can also become part of thebidi_session fixture.

Originally posted by @whimboo in #27195 (comment)
[3] 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.

Originally posted by @whimboo in #27195 (comment)
[4] I'm worried about having more than one global session. I think it would make sense to have a bidi_session fixture but for it to actually share the same session as the _current_session; if the bidi status changes we'd need to restart the session just like we do with the capabilities.

Originally posted by @jgraham in #27195 (comment)

@whimboo
Copy link
Contributor

whimboo commented Feb 19, 2021

That sounds good. @k7z45, will you be able to work on that? Thanks!

@k7z45
Copy link
Contributor Author

k7z45 commented Feb 22, 2021

Sure, thanks!

@whimboo
Copy link
Contributor

whimboo commented Mar 22, 2023

I think that we can close this issue given that the bidi_session fixture has been implemented quite some time ago. For anything else we can file a follow-up issue if needed.

@whimboo whimboo closed this as completed Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants