GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [3.10] gh-91676 gh-91260 unittest.IsolatedAsyncioTestCase no longer leaks its executor by gpshead · Pull Request #91680 · python/cpython

Conversation

@gpshead

@gpshead gpshead commented Apr 19, 2022

Copy link
Copy Markdown
Member

For things like test_asyncio.test_thread this was causing frequent
"environment modified by test" errors as the executor threads had not
always stopped running after the test was over.

issues #91676 and #91260 alternate fix to #91667.

This only happened in 3.10 and 3.9 after #32086 as the main 3.11 branch has a completely rewritten unittest.IsolatedAsyncioTestCase that uses asyncio APIs itself that do not appear to leak the executor.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@gpshead
unittest.IsolatedAsyncioTestCase no longer leaks its executor.
404894a 
For things like test_asyncio.test_thread this was causing frequent
"environment modified by test" errors as the executor threads had not
always stopped running after the test was over.
@gpshead gpshead added type-bug An unexpected behavior, bug, or error needs backport to 3.9 stdlib Standard Library Python modules in the Lib/ directory 3.10 (EOL) end of life 3.9 (EOL) end of life labels Apr 19, 2022
@gpshead
gpshead requested a review from asvetlov April 19, 2022 04:25
@bedevere-bot bedevere-bot added the awaiting core review label Apr 19, 2022
@gpshead

gpshead commented Apr 19, 2022

Copy link
Copy Markdown
Member Author

@asvetlov is this unittest.IsolatedAsyncioTestCase change to make in stable releases or should #91667 proceed to simply revert your test_asyncio.test_threads.py PR in the 3.10 and 3.9 branches?

I believe it to be what people want this class to do. But https://docs.python.org/3.11/library/unittest.html#unittest.IsolatedAsyncioTestCase.run says nothing more than "sets up a new event loop to run the test ... at the end tasks in the event loop are cancelled" without explicitly stating what should be obvious that upon exit this new private event loop should go away.

So I believe this unittest bugfix is desirable. There appears no nice way to inject this shutdown_default_executor() call as a workaround for anyone already using this class given the way the code in 3.10 and earlier is structured.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@gpshead gpshead changed the title gh-91676 gh-91260 unittest.IsolatedAsyncioTestCase no longer leaks its executor [3.10] gh-91676 gh-91260 unittest.IsolatedAsyncioTestCase no longer leaks its executor Apr 19, 2022
@gpshead
News entry.
522e6e4
@gpshead
gpshead requested a review from pablogsal April 19, 2022 04:35
@gpshead gpshead self-assigned this Apr 19, 2022
pablogsal
pablogsal approved these changes Apr 19, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Apr 19, 2022
@pablogsal
pablogsal merged commit 61570ae into python:3.10 Apr 19, 2022
@miss-islington

miss-islington commented Apr 19, 2022

Copy link
Copy Markdown
Contributor

Thanks @gpshead for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 19, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 19, 2022
@gpshead @miss-islington
[3.10] pythongh-91676 pythongh-91260 unittest.IsolatedAsyncioTestCase…
d7f6bed 
… no longer leaks its executor (pythonGH-91680)

For things like test_asyncio.test_thread this was causing frequent
"environment modified by test" errors as the executor threads had not
always stopped running after the test was over.
(cherry picked from commit 61570ae)

Co-authored-by: Gregory P. Smith <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 label Apr 19, 2022
@bedevere-bot

bedevere-bot commented Apr 19, 2022

Copy link
Copy Markdown

GH-91692 is a backport of this pull request to the 3.9 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@arhadthedev arhadthedev mentioned this pull request Apr 19, 2022
[3.10] gh-91118: Fix docstrings that do not honor --without-doc-strings (GH-31769) #91662
Merged
@kumaraditya303 kumaraditya303 mentioned this pull request Apr 19, 2022
[3.10] Revert "[3.10] bpo-47104: Rewrite asyncio.to_thread tests to use IsolatedAsyncioTestCase (GH-32086)" #91667
Closed
miss-islington added a commit that referenced this pull request Apr 19, 2022
@miss-islington @gpshead
[3.10] gh-91676 gh-91260 unittest.IsolatedAsyncioTestCase no longer l…
cb3c85d 
…eaks its executor (GH-91680)

For things like test_asyncio.test_thread this was causing frequent
"environment modified by test" errors as the executor threads had not
always stopped running after the test was over.
(cherry picked from commit 61570ae)

Co-authored-by: Gregory P. Smith <[email protected]>
@gpshead gpshead linked an issue Apr 19, 2022 that may be closed by this pull request
test_asyncio environment changed during test execution due to empty threading._dangling without MainThread before some tests #91676
Closed
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
@miss-islington @gpshead @hello-adam
[3.10] pythongh-91676 pythongh-91260 unittest.IsolatedAsyncioTestCase…
ff493ea 
… no longer leaks its executor (pythonGH-91680)

For things like test_asyncio.test_thread this was causing frequent
"environment modified by test" errors as the executor threads had not
always stopped running after the test was over.
(cherry picked from commit 61570ae)

Co-authored-by: Gregory P. Smith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pablogsal pablogsal pablogsal approved these changes
@asvetlov asvetlov Awaiting requested review from asvetlov

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@gpshead gpshead

Labels

3.9 (EOL) end of life 3.10 (EOL) end of life stdlib Standard Library Python modules in the Lib/ directory type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test_asyncio environment changed during test execution due to empty threading._dangling without MainThread before some tests

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@gpshead @miss-islington @bedevere-bot @pablogsal
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.