Skip to content

gh-112414: Add additional unit tests for calling repr() on a namespace package#112475

Merged
AlexWaygood merged 5 commits intopython:mainfrom
AlexWaygood:more-module-repr-tests
Nov 28, 2023
Merged

gh-112414: Add additional unit tests for calling repr() on a namespace package#112475
AlexWaygood merged 5 commits intopython:mainfrom
AlexWaygood:more-module-repr-tests

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Nov 27, 2023 •
edited by bedevere-app bot
Loading

Addressing @ericsnowcurrently's post-merge review at #112425 (comment). I've verified locally that the second test here fails if you build CPython with e954ac7 (the commit prior to 0622839).

@ericsnowcurrently, would you also like me to remove the test I added in 0622839, or is that okay to stay? :)

@bedevere-app bedevere-app bot mentioned this pull request Nov 27, 2023
Closed
@AlexWaygood AlexWaygood added the tests Tests in the Lib/test dir label Nov 27, 2023
@AlexWaygood AlexWaygood mentioned this pull request Nov 27, 2023
Merged
@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Nov 27, 2023

@ericsnowcurrently, would you also like me to remove the test I added in 0622839, or is that okay to stay? :)

Removing that test makes sense, since the test you add here covers that bit and the former is a bit out of place.

ericsnowcurrently
ericsnowcurrently requested changes Nov 27, 2023
View reviewed changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

mostly LGTM

@bedevere-app
Copy link

bedevere-app bot commented Nov 27, 2023

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

ericsnowcurrently
ericsnowcurrently requested changes Nov 27, 2023
View reviewed changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

Also, s/"/'/g

AlexWaygood and others added 3 commits November 27, 2023 23:49
@AlexWaygood @ericsnowcurrently
assert "from" not in repr of module with strange loader
4293ced 
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@AlexWaygood @ericsnowcurrently
Test the presence of a _path attribute does not affect repr necessa…
647bafa 
…rily

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@AlexWaygood
cleanup
a25bb4e
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Nov 27, 2023

Thanks! I have made the requested changes; please review again

ericsnowcurrently
ericsnowcurrently approved these changes Nov 28, 2023
View reviewed changes
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexWaygood AlexWaygood merged commit cf20540 into python:main Nov 28, 2023
@AlexWaygood AlexWaygood deleted the more-module-repr-tests branch November 28, 2023 00:10
@miss-islington-app
Copy link

miss-islington-app bot commented Nov 28, 2023

Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 28, 2023
@AlexWaygood @ericsnowcurrently @miss-islington
pythongh-112414: Add additional unit tests for calling repr() on a …
39d5c89 
…namespace package (pythonGH-112475)

(cherry picked from commit cf20540)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Nov 28, 2023

GH-112480 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Nov 28, 2023
@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Nov 28, 2023

Thanks for working on this, @AlexWaygood!

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Nov 28, 2023

You're very welcome! :D

AlexWaygood added a commit that referenced this pull request Nov 28, 2023
@miss-islington @AlexWaygood @ericsnowcurrently
[3.12] gh-112414: Add additional unit tests for calling repr() on a…
daf9ff9 
… namespace package (GH-112475) (#112480)

gh-112414: Add additional unit tests for calling `repr()` on a namespace package (GH-112475)
(cherry picked from commit cf20540)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@AlexWaygood @ericsnowcurrently @aisk
pythongh-112414: Add additional unit tests for calling repr() on a …
7de710c 
…namespace package (python#112475)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@AlexWaygood @ericsnowcurrently @Glyphack
pythongh-112414: Add additional unit tests for calling repr() on a …
ba9e5f2 
…namespace package (python#112475)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes

@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon is a code owner

@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner

@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir topic-importlib

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AlexWaygood @ericsnowcurrently