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.

Sign up for GitHub

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

Wait for client on test_connection_lost_before_handshake_complete #2001

Merged
merged 17 commits into from
Jun 10, 2023
Merged

Wait for client on test_connection_lost_before_handshake_complete #2001

Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1b460d3
Wait for client on `test_connection_lost_before_handshake_complete`
Kludex Jun 8, 2023
d245cdf
Add asyncio.event
Kludex Jun 8, 2023
8abe5dd
Remove task variable
Kludex Jun 8, 2023
5864e98
Hold a reference to the task
Kludex Jun 9, 2023
35699d2
Fix linter
Kludex Jun 9, 2023
b43f6b7
Cancel instead of wait
Kludex Jun 9, 2023
507d27d
Debug tests
Kludex Jun 9, 2023
158080e
Debug tests
Kludex Jun 9, 2023
27cad4c
Debug tests
Kludex Jun 9, 2023
b241f31
Debug tests
Kludex Jun 9, 2023
baec6c4
more
Kludex Jun 9, 2023
6887835
Change the location of the set response received
Kludex Jun 10, 2023
5465855
Add another await
Kludex Jun 10, 2023
ca01658
await task
Kludex Jun 10, 2023
8dba049
remove event
Kludex Jun 10, 2023
d3505d6
remove print
Kludex Jun 10, 2023
3fb818d
Apply suggestions from code review
Kludex Jun 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions tests/protocols/test_websocket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,7 @@ async def test_connection_lost_before_handshake_complete(
ws_protocol_cls, http_protocol_cls, unused_tcp_port: int
):
send_accept_task = asyncio.Event()
response_received = asyncio.Event()
disconnect_message = {}

async def app(scope, receive, send):
Expand All @@ -735,6 +736,7 @@ async def websocket_session(uri):
"sec-websocket-key": "dGhlIHNhbXBsZSBub25jZQ==",
},
)
response_received.set()

config = Config(
app=app,
Expand All @@ -744,13 +746,11 @@ async def websocket_session(uri):
port=unused_tcp_port,
)
async with run_server(config):
task = asyncio.create_task(
websocket_session(f"ws://127.0.0.1:{unused_tcp_port}")
)
asyncio.create_task(websocket_session(f"ws://127.0.0.1:{unused_tcp_port}"))
Copy link
Member

Choose a reason for hiding this comment

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

the task = is important please keep it

Copy link
Member Author

Choose a reason for hiding this comment

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

It's important when you don't want the GC to clean it, since it's a weak ref, but in this case since all the logic happens in the same function scope, I don't think I need it?

Copy link
Member

Choose a reason for hiding this comment

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

it's been a while so I dont remember exactly but when I reworked the whole test suite and added strict settings to pytest this helped a lot.

await asyncio.sleep(0.1)
send_accept_task.set()

task.cancel()
await response_received.wait()
assert response is not None
assert response.status_code == 500, response.text
assert response.text == "Internal Server Error"
Expand Down