-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
bpo-45019: Silence a warning in test_ctypes. #28362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-45019: Silence a warning in test_ctypes. #28362
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Lib/ctypes/test/test_values.py
Outdated
# Hide the message written by the __hello__ module. | ||
with captured_stdout(): | ||
spec = importlib.util.find_spec(modname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it wouldn't be useful to just not print anything from the __hello__
module unless it's invoked as a __main__
module? That could possibly allow us to remove a lot of captured_stdout() calls. What is this output actually needed for (other than tradition)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure we didn't think about it when the module was added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change the title of the PR and the commit when it's merged to mention the change to flag.py/__hello__
. (Also, why are we pulling the data file out of an outdated tooling script? We might as well stuff an actually __hello__.py
in the stdlib?)
Lib/ctypes/test/test_values.py
Outdated
@@ -73,6 +73,7 @@ class struct_frozen(Structure): | |||
self.assertTrue([entry.code[i] for i in range(abs(entry.size))]) | |||
# Check the module's package-ness. | |||
with import_helper.frozen_modules(): | |||
# Hide the message written by the __hello__ module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can drop this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, meant to approve as well.
I'm going to move the |
This was missed in PR gh-28319.
https://bugs.python.org/issue45019