-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
Closed
Labels
testsTests in the Lib/test dirTests in the Lib/test dirtype-featureA feature request or enhancementA feature request or enhancement
Description
Feature or enhancement
There are 3 TODO comments that should be resolved in the test_concurrent_futures.py
file[0]
cpython/Lib/test/test_concurrent_futures.py
Line 714 in e16d4ed
# TODO([email protected]): Should have a test with a non-zero timeout. |
2.
cpython/Lib/test/test_concurrent_futures.py
Line 1534 in e16d4ed
# TODO([email protected]): This test is timing dependent. |
3.
cpython/Lib/test/test_concurrent_futures.py
Line 1548 in e16d4ed
# TODO([email protected]): This test is timing dependent. |
Pitch
Resolving 1), i.e adding a test to cover the non-zero timeout case would simply make the tests more robust.resolved in: gh-100522 Add a test for 'futures.as_completed' timing out with a non-zero timeout value #100523- Resolving 2) would have two benefits. Firstly, it would make the tests run faster by ~1s (there is a 1s sleep). Secondly, sleeping in the tests can occasionally cause flakiness[1] and so I would consider it good practice to remove them where practical.
- Resolving 3) Same reasons as for 2).
Is there a preferred way to tackle this?
[0]. https://github.com/python/cpython/blob/e16d4ed59072839b49bda4b447f260201aae7e39/Lib/test/test_concurrent_futures.py
[1]. This seems to be acknowledged in the related timeouts that are used:
cpython/Lib/test/support/__init__.py
Line 96 in 745545b
# If a test using SHORT_TIMEOUT starts to fail randomly on slow buildbots, use |
CC: @brianquinlan
Linked PRs
Metadata
Metadata
Assignees
Labels
testsTests in the Lib/test dirTests in the Lib/test dirtype-featureA feature request or enhancementA feature request or enhancement