◐ Shell
reader mode source ↗
Skip to content

bpo-16968: Clean up test_concurrent_futures#8099

Closed
zware wants to merge 2 commits into
python:masterfrom
zware:issue16968
Closed

bpo-16968: Clean up test_concurrent_futures#8099
zware wants to merge 2 commits into
python:masterfrom
zware:issue16968

Conversation

@zware

@zware zware commented Jul 5, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

The old test_main function would cause any dangling threads or processes
to be reaped silently rather than allowing regrtest to detect and warn
about them. Since the shutdown methods of the module should do all of
the cleanup for us, we want to get those warnings if they don't.

https://bugs.python.org/issue16968

@zware zware added tests Tests in the Lib/test dir skip news needs backport to 3.7 labels Jul 5, 2018
@zware zware self-assigned this Jul 5, 2018
zware and others added 2 commits December 2, 2018 16:00
@zware
bpo-16968: Clean up test_concurrent_futures …
02c632d 
The old test_main function would cause any dangling threads or processes
to be reaped silently rather than allowing regrtest to detect and warn
about them.  Since the shutdown methods of the module should do all of
the cleanup for us, we want to get those warnings if they don't.
@zware
Run post_test_cleanup within the saved_test_environment block …
96e5104 
This allows a change in `support.environment_altered` by `support.reap_children`
to be recorded by `saved_test_environment.__exit__`.
@zware zware requested a review from vstinner December 2, 2018 22:13
@bedevere-bot

bedevere-bot commented Dec 3, 2018

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

And if you don't make the requested changes, you will be poked with soft cushions!

vstinner
vstinner reviewed Dec 3, 2018
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Maybe just revert Lib/test/libregrtest/runtest.py change? If my runtest.py comment is wrong, I would prefer to see the change as a different PR.

@csabella

csabella commented May 31, 2019

Copy link
Copy Markdown
Contributor

@zware, not sure if you saw the review from @vstinner.

@zware

zware commented May 31, 2019

Copy link
Copy Markdown
Member Author

@csabella Thanks for the ping. I did, but haven't had the time to swap this back in to think it through :)

This PR can be freely rejected or taken over if anyone is sufficiently confident in their understanding of what's going on here; otherwise I'll get back to it one of these years :)

@vstinner

vstinner commented Jun 4, 2019

Copy link
Copy Markdown
Member

This PR can be freely rejected or taken over if anyone is sufficiently confident in their understanding of what's going on here; otherwise I'll get back to it one of these years :)

Ok. I close your PR.

@vstinner vstinner closed this Jun 4, 2019
@zware zware deleted the issue16968 branch July 2, 2019 22:25
@zware zware mentioned this pull request May 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner requested changes

Assignees

@zware zware

Labels

awaiting changes skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@zware @bedevere-bot @csabella @vstinner @the-knights-who-say-ni