PYTHON-5087 - Convert test.test_load_balancer to async #2103
Conversation
|
|
||
| # Generate unified tests. | ||
| globals().update(generate_test_classes(TEST_PATH, module=__name__)) |
There was a problem hiding this comment.
i don't think it really matters but _TEST_PATH vs TEST_PATH?
//should i be changing them to TEST_PATH (my brain would appreciate it if it was consistent across the entire test suite haha)
There was a problem hiding this comment.
Good catch. Let's stick with TEST_PATH for now. I'll open a ticket to rename all the ones named _TEST_PATH in one go.
|
Note: I've opened PYTHON-5113 to refactor our test utils once all conversions are complete. |
test/utils.py
Outdated
| def __init__(self, target): | ||
| self.exc = None | ||
| self.target = target | ||
| self.task = create_task(self.run()) |
There was a problem hiding this comment.
create_task should be called in run() to mirror the behavior of Thread.
There was a problem hiding this comment.
It should be called in start() to mirror Thread, right?
There was a problem hiding this comment.
Yeah that's right. A Thread doesn't start running until start() is called.
test/utils.py
Outdated
|
|
||
| async def run(self): | ||
| try: | ||
| await self.target() |
There was a problem hiding this comment.
Awaiting the task should happen in join() to mirror the behavior of Thread.
| self.assertEqual(pool.active_sockets, 1) # Pinned. | ||
|
|
||
| if _IS_SYNC: | ||
| thread = PoolLocker(pool) |
There was a problem hiding this comment.
If we make the ExceptionCatchingTask changes I suggested, will that let use remove this branch?
There was a problem hiding this comment.
I believe so, yes.
No description provided.