-
Notifications
You must be signed in to change notification settings - Fork 371
feat(server): add async context manager support to EventQueue #743
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,6 +70,28 @@ def test_constructor_invalid_max_queue_size() -> None: | |
| EventQueue(max_queue_size=-10) | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_event_queue_async_context_manager( | ||
| event_queue: EventQueue, | ||
| ) -> None: | ||
| """Test that EventQueue can be used as an async context manager.""" | ||
| async with event_queue as q: | ||
| assert q is event_queue | ||
| assert event_queue.is_closed() is False | ||
| assert event_queue.is_closed() is True | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_event_queue_async_context_manager_on_exception( | ||
| event_queue: EventQueue, | ||
| ) -> None: | ||
| """Test that close() is called even when an exception occurs inside the context.""" | ||
| with pytest.raises(RuntimeError, match='boom'): | ||
| async with event_queue: | ||
| raise RuntimeError('boom') | ||
| assert event_queue.is_closed() is True | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the suggestion! I kept is True/is False for consistency with the existing assertions in this file (e.g., test_close_idempotent, test_is_closed_reflects_state, etc.), which all use the same style. Happy to change if maintainers prefer updating. |
||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_enqueue_and_dequeue_event(event_queue: EventQueue) -> None: | ||
| """Test that an event can be enqueued and dequeued.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For boolean comparisons, it's more idiomatic and generally preferred to use
assert not ...instead ofassert ... is False.