Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upbpo-13601: always use line-buffering for sys.stderr #17646
Conversation
This comment has been minimized.
This comment has been minimized.
LGTM, but it would be nice if you could add a test, for example in |
This comment has been minimized.
This comment has been minimized.
Thanks for your comments! I hope my changes address them adequately. |
code = f'import os, sys; {buf}.write({txt1!r}); {buf}.write({txt2!r}); os._exit(0)' | ||
rc, out, err = assert_python_ok('-c', code) | ||
self.assertEqual(out, out_ok) | ||
self.assertEqual(err, err_ok) |
This comment has been minimized.
This comment has been minimized.
vstinner
Dec 19, 2019
Member
I'm not sure that a functional test is required here to validate manually that buffered are flushed properly. Maybe testing the following attributes are enough?
>>> sys.stdout.write_through, sys.stdout.write_through
(False, False)
What do you think @pitrou?
My worry is that functional tests are more likely to fail and so give me more to work to maintain the CI :-)
This comment has been minimized.
This comment has been minimized.
Thanks for the documentation update! |
('sys.stderr.line_buffering', True), | ||
] | ||
for attr, value in cases: | ||
self.assertEqual(get_value(attr), value, f'{attr} is not {value}') |
This comment has been minimized.
This comment has been minimized.
vstinner
Dec 20, 2019
Member
I suggest to rewrite the test to run Python a single time rather than 6 times, to make the test faster:
def test_non_interactive_output_buffering(self):
code = textwrap.dedent("""
import sys
out = sys.stdout
print(out.isatty(), out.write_through, out.line_buffering)
err = sys.stderr
print(err.isatty(), err.write_through, err.line_buffering)
""")
args = [sys.executable, '-c', code]
proc = subprocess.run(args, stdout=subprocess.PIPE,
text=True, check=True)
self.assertEqual(proc.stdout,
'False False False\n'
'True False True\n')
This comment has been minimized.
This comment has been minimized.
jendrikseipp
Dec 20, 2019
Author
Good point! I used your code, but added stderr=subprocess.PIPE
to ensure that sys.stderr
is not a TTY (without this both the old and the new code would pass the test).
jendrikseipp commentedDec 17, 2019
•
edited by bedevere-bot
Fixes https://bugs.python.org/issue13601
https://bugs.python.org/issue13601