bpo-45019: Silence a warning in test_ctypes. by ericsnowcurrently · Pull Request #28362

bpo-45019: Silence a warning in test_ctypes. by ericsnowcurrently · Pull Request #28362 · python/cpython

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +76 to +78

		# Hide the message written by the __hello__ module.
		with captured_stdout():
		spec = importlib.util.find_spec(modname)

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

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.

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

		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.

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.

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 __hello__ stuff to a different PR.