close
The Wayback Machine - https://web.archive.org/web/20221231133715/https://github.com/python/cpython/pull/100523
Skip to content
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

gh-100522 Add a test for 'futures.as_completed' timing out with a non-zero timeout value #100523

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JosephSBoyle
Copy link
Contributor

@JosephSBoyle JosephSBoyle commented Dec 25, 2022

Resolves a longstanding TODO in the futures tests:

TODO(brian@sweetapp.com): Should have a test with a non-zero timeout.

by fixturizing the existing test.

@bedevere-bot bedevere-bot added awaiting review tests Tests in the Lib/test dir labels Dec 25, 2022
@JosephSBoyle JosephSBoyle changed the title gh-100552 Add a test for 'futures.as_completed' timing out with a non-zero timeout value gh-100522 Add a test for 'futures.as_completed' timing out with a non-zero timeout value Dec 25, 2022

future = self.executor.submit(time.sleep, 0.1)
Copy link
Contributor Author

@JosephSBoyle JosephSBoyle Dec 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test actually sleeps this long (possibly when the thread is torn down?) so reducing the time from 2s is quite beneficial for test runtime. Particularly so as this test is expanded to run multiple times:

2s sleep:

image

0.1s sleep:
image


Python + pytest on 3.12 with the command:
./python -m pytest Lib/test/test_concurrent_futures.py -k test_future_times_out -vvv

@JosephSBoyle JosephSBoyle marked this pull request as ready for review Dec 25, 2022
@JosephSBoyle JosephSBoyle force-pushed the fix-issue-100522 branch 3 times, most recently from db6c886 to 087c339 Compare Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants