use the loop fixture in more tests #6642
Conversation
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 15 files ±0 15 suites ±0 6h 15m 28s ⏱️ - 26m 24s For more details on these failures, see this check. Results for commit a3c663c. ± Comparison against base commit 7b24c94. ♻️ This comment has been updated with latest results. |
graingert
force-pushed
the
use-the-loop-fixture-for-more-tests
branch
from
338e321 to
58b7a13
Compare
* avoid loop fixture in test_basic_no_loop this test intentionally passes loop=None and currently results in a "No running loop" deprecation warning * split test_client_loop tests into two
graingert
force-pushed
the
use-the-loop-fixture-for-more-tests
branch
from
58b7a13 to
72a1e6b
Compare
graingert
force-pushed
the
use-the-loop-fixture-for-more-tests
branch
from
72a1e6b to
a3c663c
Compare
| ): | ||
| pass | ||
| def test_repeated(loop_in_thread): | ||
| loop = loop_in_thread |
There was a problem hiding this comment.
What makes you need loop_in_thread instead of just loop?
There was a problem hiding this comment.
the LocalCluster.__exit__ closes the loop that's passed in - unless it's already running in another thread
| pass | ||
| sleep(0.1) | ||
| with Client(s["address"]) as c: | ||
| with Client(s["address"], loop=None) as c: |
There was a problem hiding this comment.
I don't understand the purpose of this test?
There was a problem hiding this comment.
Everything LGTM with the exception of this.
It makes little sense to me - particularly after comparing it to the async test directly above.
Maybe the omission of the explicit loop may have just been a mistake from the previous developer?
There was a problem hiding this comment.
introduced here 9bd1091#diff-09c46b6d46629b09a6659806dc2788dc30133fa5cb414872c6535bb572fe8f12R2299-R2308 #172
see also:
1f9ac65
There was a problem hiding this comment.
it's explicitly testing the case where the Client is started up without a loop, so to make this intention clear I pass loop=None
|
Thank you |
Refs #6163
This allows us to make sure the tornado loop is cleaned up after interacting with LoopRunner
this test intentionally passes loop=None and currently results
in a "No running loop" deprecation warning