Skip to content

[3.10] gh-91607: Fix several test_concurrent_futures tests to test what they claim #91612

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

Merged
merged 1 commit into from
Apr 16, 2022

Conversation

gpshead
Copy link
Member

@gpshead gpshead commented Apr 16, 2022

  • Fix test_concurrent_futures to actually test what it says.

Many ProcessPoolExecutor based tests were ignoring the mp_context
and using the default instead. This meant we lacked proper test
coverage of all of them.

Also removes the old _prime_executor() worker delay seeding code
as it appears to have no point and causes 20-30 seconds extra
latency on this already long test. It also interfered with some
of the refactoring to fix the above to not needlessly create their
own executor when setUp has already created an appropriate one.

  • Don't import the name from multiprocessing directly to avoid confusion.

(cherry picked from commit 7fa3a5a)

Co-authored-by: Gregory P. Smith greg@krypto.org

…ctually test what they claim (pythonGH-91600)

* Fix test_concurrent_futures to actually test what it says.

Many ProcessPoolExecutor based tests were ignoring the mp_context
and using the default instead.  This meant we lacked proper test
coverage of all of them.

Also removes the old _prime_executor() worker delay seeding code
as it appears to have no point and causes 20-30 seconds extra
latency on this already long test.  It also interfered with some
of the refactoring to fix the above to not needlessly create their
own executor when setUp has already created an appropriate one.

* Don't import the name from multiprocessing directly to avoid confusion.

* 📜🤖 Added by blurb_it.

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>.
(cherry picked from commit 7fa3a5a)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
@gpshead gpshead added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir needs backport to 3.9 only security fixes labels Apr 16, 2022
@gpshead gpshead self-assigned this Apr 16, 2022
@gpshead gpshead merged commit 9a45893 into python:3.10 Apr 16, 2022
@miss-islington
Copy link
Contributor

Thanks @gpshead for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@gpshead gpshead deleted the backport-7fa3a5a-3.10 branch April 16, 2022 20:48
@miss-islington
Copy link
Contributor

Sorry, @gpshead, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 9a458934f7be37c59525350f1f9ecbdcd69903c1 3.9

gpshead added a commit to gpshead/cpython that referenced this pull request Apr 16, 2022
…s to actually test what they claim (pythonGH-91600) (pythonGH-91612)

* Fix test_concurrent_futures to actually test what it says.

Many ProcessPoolExecutor based tests were ignoring the mp_context
and using the default instead.  This meant we lacked proper test
coverage of all of them.

Also removes the old _prime_executor() worker delay seeding code
as it appears to have no point and causes 20-30 seconds extra
latency on this already long test.  It also interfered with some
of the refactoring to fix the above to not needlessly create their
own executor when setUp has already created an appropriate one.

* Don't import the name from multiprocessing directly to avoid confusion.

(cherry picked from commit 7fa3a5a)

Co-authored-by: Gregory P. Smith <greg@krypto.org>.
(cherry picked from commit 9a45893)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
@bedevere-bot
Copy link

GH-91617 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Apr 16, 2022
gpshead added a commit that referenced this pull request Apr 17, 2022
…ctually test what they claim (GH-91600) (GH-91612) (#91617)

* Fix test_concurrent_futures to actually test what it says.

Many ProcessPoolExecutor based tests were ignoring the mp_context
and using the default instead.  This meant we lacked proper test
coverage of all of them.

Also removes the old _prime_executor() worker delay seeding code
as it appears to have no point and causes 20-30 seconds extra
latency on this already long test.  It also interfered with some
of the refactoring to fix the above to not needlessly create their
own executor when setUp has already created an appropriate one.

* Don't import the name from multiprocessing directly to avoid confusion.

(cherry picked from commit 7fa3a5a)
(cherry picked from commit 9a45893)
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
…s to actually test what they claim (pythonGH-91600) (pythonGH-91612) (python#91617)

* Fix test_concurrent_futures to actually test what it says.

Many ProcessPoolExecutor based tests were ignoring the mp_context
and using the default instead.  This meant we lacked proper test
coverage of all of them.

Also removes the old _prime_executor() worker delay seeding code
as it appears to have no point and causes 20-30 seconds extra
latency on this already long test.  It also interfered with some
of the refactoring to fix the above to not needlessly create their
own executor when setUp has already created an appropriate one.

* Don't import the name from multiprocessing directly to avoid confusion.

(cherry picked from commit 7fa3a5a)
(cherry picked from commit 9a45893)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_concurrent_futures does not exercise each mp_context method reliably due to code smell
3 participants