Skip to content

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

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

AlexWaygood merged 5 commits into
python:mainfrom
AlexWaygood:more-module-repr-tests

Conversation

@AlexWaygood

@AlexWaygood AlexWaygood commented Nov 27, 2023 •

Copy link
Copy Markdown
Member

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? :)

@ericsnowcurrently

Copy link
Copy Markdown
Member

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

mostly LGTM

Comment thread Lib/test/test_module/__init__.py
Comment thread Lib/test/test_module/__init__.py
@bedevere-app

bedevere-app Bot commented Nov 27, 2023

Copy link
Copy Markdown

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

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

Also, s/"/'/g

@AlexWaygood

Copy link
Copy Markdown
Member Author

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

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

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
Copy Markdown

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
…namespace package (pythonGH-112475)

(cherry picked from commit cf20540)

Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Eric Snow <[email protected]>
@bedevere-app

bedevere-app Bot commented Nov 28, 2023

Copy link
Copy Markdown

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
Copy Markdown
Member

Thanks for working on this, @AlexWaygood!

@AlexWaygood

Copy link
Copy Markdown
Member Author

You're very welcome! :D

AlexWaygood added a commit that referenced this pull request Nov 28, 2023
… 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 <[email protected]>
Co-authored-by: Eric Snow <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants