Skip to content

bpo-46442: improve testExceptionCleanupNames - #30758

Merged
ericvsmith merged 1 commit into
python:mainfrom
yellowdusk1590:main
Jan 22, 2022
Merged

bpo-46442: improve testExceptionCleanupNames#30758
ericvsmith merged 1 commit into
python:mainfrom
yellowdusk1590:main

Conversation

@yellowdusk1590

@yellowdusk1590yellowdusk1590 commented Jan 21, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

The test tested that explicitly deleting the local variable bound to the exception
did not cause problems, but it did not test what it actually claimed to test, i.e.
that the variable is deleted automatically.

https://bugs.python.org/issue46442

@bedevere-botbedevere-bot added tests Tests in the Lib/test dir awaiting review labels Jan 21, 2022
JelleZijlstra
JelleZijlstra reviewed Jan 21, 2022
View reviewed changes
Comment threadLib/test/test_exceptions.py Outdated
sobolevn
sobolevn reviewed Jan 21, 2022
View reviewed changes
Comment threadLib/test/test_exceptions.py Outdated
ericvsmith
ericvsmith requested changes Jan 21, 2022
View reviewed changes

@ericvsmithericvsmith 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.

With the small noted changes, I think this is ready to commit.

Comment threadLib/test/test_exceptions.py Outdated
Comment threadLib/test/test_exceptions.py Outdated
@bedevere-bot

bedevere-bot commented Jan 21, 2022

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ericvsmithericvsmith self-assigned this Jan 21, 2022
@yellowdusk1590
bpo-46442: improve testExceptionCleanupNames
2e32a54
The test tested that explicitly deleting the local variable bound to the exception
did not cause problems, but it did not test what it actually claimed to test, i.e.
that the variable is deleted automatically.
@yellowdusk1590

yellowdusk1590 commented Jan 21, 2022

Copy link
Copy Markdown
ContributorAuthor

I have made the requested changes; please review again

@bedevere-bot

bedevere-bot commented Jan 21, 2022

Copy link
Copy Markdown

Thanks for making the requested changes!

@ericvsmith: please review the changes made to this pull request.

@ericvsmith
ericvsmith merged commit 82c5322 into python:mainJan 22, 2022
@miss-islington

miss-islington commented Jan 22, 2022

Copy link
Copy Markdown
Contributor

Thanks @yellowdusk1590 for the PR, and @ericvsmith for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 22, 2022
@yellowdusk1590@miss-islington
bpo-46442: improve and rename testExceptionCleanupNames (pythonGH-30758)
41a54e6
The test tested that explicitly deleting the local variable bound to the exception
did not cause problems, but it did not test what it actually claimed to test, i.e.
that the variable is deleted automatically.
(cherry picked from commit 82c5322)
Co-authored-by: Yellow Dusk <yellow.dusk1590@fastmail.com>
@bedevere-botbedevere-bot removed the needs backport to 3.10 only security fixes label Jan 22, 2022
@bedevere-bot

bedevere-bot commented Jan 22, 2022

Copy link
Copy Markdown

GH-30778 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 22, 2022
@yellowdusk1590@miss-islington
bpo-46442: improve and rename testExceptionCleanupNames (pythonGH-30758)
3229f51
The test tested that explicitly deleting the local variable bound to the exception
did not cause problems, but it did not test what it actually claimed to test, i.e.
that the variable is deleted automatically.
(cherry picked from commit 82c5322)
Co-authored-by: Yellow Dusk <yellow.dusk1590@fastmail.com>
@bedevere-bot

bedevere-bot commented Jan 22, 2022

Copy link
Copy Markdown

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

miss-islington added a commit that referenced this pull request Jan 22, 2022
@miss-islington@yellowdusk1590
bpo-46442: improve and rename testExceptionCleanupNames (GH-30758)
d4a9e34
The test tested that explicitly deleting the local variable bound to the exception
did not cause problems, but it did not test what it actually claimed to test, i.e.
that the variable is deleted automatically.
(cherry picked from commit 82c5322)
Co-authored-by: Yellow Dusk <yellow.dusk1590@fastmail.com>
miss-islington added a commit that referenced this pull request Jan 22, 2022
@miss-islington@yellowdusk1590
bpo-46442: improve and rename testExceptionCleanupNames (GH-30758)
e064af5
The test tested that explicitly deleting the local variable bound to the exception
did not cause problems, but it did not test what it actually claimed to test, i.e.
that the variable is deleted automatically.
(cherry picked from commit 82c5322)
Co-authored-by: Yellow Dusk <yellow.dusk1590@fastmail.com>
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
@miss-islington@yellowdusk1590@hello-adam
bpo-46442: improve and rename testExceptionCleanupNames (pythonGH-30758)
d96d8a5
The test tested that explicitly deleting the local variable bound to the exception
did not cause problems, but it did not test what it actually claimed to test, i.e.
that the variable is deleted automatically.
(cherry picked from commit 82c5322)
Co-authored-by: Yellow Dusk <yellow.dusk1590@fastmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstraJelleZijlstraJelleZijlstra left review comments
@sobolevnsobolevnsobolevn left review comments
@ericvsmithericvsmithericvsmith approved these changes
@iritkatrieliritkatrielAwaiting requested review from iritkatrieliritkatriel is a code owner

Assignees

@ericvsmithericvsmith

Labels

skip newstestsTests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@yellowdusk1590@bedevere-bot@miss-islington@ericvsmith@JelleZijlstra@iritkatriel@sobolevn@the-knights-who-say-ni