Seperate out bidi_session from session in fixtures.py #27734
Conversation
wpt-pr-bot
requested review from
andreastt,
AutomatedTester,
mjzffr,
shs96c and
whimboo
|
@jgraham @stephenmcgruer PTAL |
Removed bidi markers and created bidi_session fixtures Add handling in bidi_session and session start function to disguish bidi_session and session so that async functions can be awaited. Modify tests to have better names.
Also delete some comments requiring specific order for test. Added comment for first and last test to avoid breaking classic webdriver session tests.
Thanks! I removed an infrastructure changes and removed some comments so that they are not intended for infrastructure tests anymore. Kept the first and last test so that bidi tests do not impact classic tests. |
| await session.websocket_transport.send("test_bidi_session_2") | ||
| await session.websocket_transport.close() | ||
| async def test_bidi_session_send(bidi_session): | ||
| await bidi_session.websocket_transport.send("test_bidi_session: send") |
There was a problem hiding this comment.
It would be good to have a follow-up here for BiDiSession.send_command(). That way we won't have to expose the websocket_transport to all of the test files, and can then even build BiDiSession.send_session_command() on top of it.
| @pytest.mark.capabilities({"acceptInsecureCerts": True}) | ||
| async def test_bidi_session_3(session): | ||
| await session.websocket_transport.send("test_bidi_session_3") | ||
| async def test_bidi_session_with_different_capability(bidi_session): |
There was a problem hiding this comment.
As mentioned in my reviews for the initial PR (and what @jgraham also referred to earlier in this PR) lets get an issue filed so we can make sure to have a single test for that. It would most likely need the new_session fixture, or maybe a new one for BiDiSession.
| is_cur_bidi = isinstance(_current_session, webdriver.BidiSession) | ||
| # If there is a session with different capabilities active or the current session | ||
| # is of different type than the one we would like to create, end it now. | ||
| if _current_session is not None and ( | ||
| (not _current_session.match(caps)) or | ||
| (is_cur_bidi != bidi)): | ||
| if is_cur_bidi: | ||
| await _current_session.end() | ||
| else: | ||
| _current_session.end() | ||
| _current_session = None | ||
| if _current_session is not None: | ||
| is_bidi = isinstance(_current_session, webdriver.BidiSession) | ||
| if is_bidi or caps != _current_session.requested_capabilities: | ||
| if is_bidi: | ||
| await _current_session.end() | ||
| else: | ||
| _current_session.end() | ||
| _current_session = None |
There was a problem hiding this comment.
This code is duplicated. Maybe we can just move it out to its own function.
PR for #27687
Seperated out bidi_session from session in fixtures.py
Removed bidi markers and created bidi_session fixtures
Added handling in bidi_session and session start function
to disguish bidi_session and session so that async functions
can be awaited.
Modified tests to have better names.