-
-
Notifications
You must be signed in to change notification settings - Fork 33.1k
gh-140082: Forward colorizing from libregrtest to unittest #140083
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
base: main
Are you sure you want to change the base?
Conversation
libregrtest redirects test output to a file as part of its operation. When `unittest` checks to see if it should colorize with `isatty(sys.stdout)` that fails resulting in no colorizing of the unittest output. Update `libregrtest` to set `FORCE_COLOR=1` when redirecting test output so that unittest will do color printing.
I think this can be skip news as it's a strictly internal change / libregrtest isn't externally exposed |
timeout = runtests.timeout | ||
|
||
if can_colorize(file=sys.stdout): | ||
os.environ['FORCE_COLOR'] = "1" |
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.
Is this change also needed?
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.
Yes for single-process mode coloring of unittest
output because unittest
gets given a io.StringIO
as its stdout
(on which isatty
returns False). ./python -m test -W --single-process
with a failing test output is uncolored without this, colored with it.
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've been pondering if this would be better done by setting os.environ['FORCE_COLOR']
inside libregrtest
main (
cpython/Lib/test/libregrtest/main.py
Lines 755 to 779 in 1e1f435
def main(self, tests: TestList | None = None) -> NoReturn: | |
if self.want_add_python_opts: | |
self._add_python_opts() | |
self._init() | |
if self.want_cleanup: | |
cleanup_temp_dir(self.tmp_dir) | |
sys.exit(0) | |
if self.want_wait: | |
input("Press any key to continue...") | |
setup_test_dir(self.test_dir) | |
selected, tests = self.find_tests(tests) | |
exitcode = 0 | |
if self.want_list_tests: | |
self.list_tests(selected) | |
elif self.want_list_cases: | |
list_cases(selected, | |
match_tests=self.match_tests, | |
test_dir=self.test_dir) | |
else: | |
exitcode = self.run_tests(selected, tests) |
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.
Would it be possible to move these 2 lines in setup_tests()?
You might add a NEWS entry in the Tests category. |
timeout = runtests.timeout | ||
|
||
if can_colorize(file=sys.stdout): | ||
os.environ['FORCE_COLOR'] = "1" |
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.
Would it be possible to move these 2 lines in setup_tests()?
env['TEMP'] = tmp_dir | ||
env['TMP'] = tmp_dir | ||
|
||
# The subcommand is run with a temporary output which means it is not a tty |
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.
# The subcommand is run with a temporary output which means it is not a tty | |
# The subcommand is run with a temporary output which means it is not a TTY |
libregrtest
redirects test output to a file as part of its operation. Whenunittest
checks to see if it should colorize withisatty(sys.stdout)
that returns False resulting in no colorizing of theunittest
output.Update
libregrtest
to setFORCE_COLOR=1
when redirecting test output so thatunittest
will do color printing.python -m test -W
#140082