PYTHON-5080 - Convert test.test_examples to async #2097
Conversation
|
The remaining failure is |
test/test_examples.py
Outdated
| db = self.db | ||
|
|
||
| # Start Aggregation Example 1 | ||
|
|
There was a problem hiding this comment.
doesn't matter that much, but new line?
| session=s, | ||
| ) | ||
| ).next() | ||
| )["adoptableCatsCount"] |
There was a problem hiding this comment.
Is this really ruff's formatting? Ew.
There was a problem hiding this comment.
Yeah it's not great.
test/test_examples.py
Outdated
| db.inventory.insert_one({"username": "alice"}) | ||
| db.inventory.delete_one({"username": "alice"}) | ||
|
|
||
| t = asyncio.create_task(insert_docs()) |
There was a problem hiding this comment.
Could we utilize a Thread/Task wrapper api to avoid duplicating tests like this? For example, like ExceptionCatchingTask/ExceptionCatchingThread.
There was a problem hiding this comment.
Similar to how SpecRunnerThread/SpecRunnerTask work? We could move that abstraction up to be a generic class we use everywhere we need a similar pattern.
test/asynchronous/helpers.py
Outdated
| if self.args: | ||
| await self.target(*self.args) | ||
| else: | ||
| await self.target() |
There was a problem hiding this comment.
await self.target(*self.args) will work even if self.args is an empty list so this code can be simplified.
test/asynchronous/helpers.py
Outdated
| return not self.stopped | ||
|
|
||
| async def run(self): | ||
| if self.target: |
There was a problem hiding this comment.
When would "target" ever be None?
There was a problem hiding this comment.
Good point, we'd want it to throw an error here instead of being a silent no-op.
| nonlocal done | ||
| while not done: | ||
| db.inventory.insert_one({"username": "alice"}) | ||
| db.inventory.delete_one({"username": "alice"}) |
There was a problem hiding this comment.
We should probably put a short sleep in here (5ms maybe) to avoid a tight loop.
test/asynchronous/helpers.py
Outdated
| if self.target: | ||
| await self.target() | ||
| await self.target(*self.args) | ||
| self.stopped = True |
There was a problem hiding this comment.
Do we need to set stopped to try even when target raises? If so I suggest using try/finally.
There was a problem hiding this comment.
Good point, I'd say so.
3 participants
No description provided.