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).
@@ -0,0 +1,2 @@ | |||
``sys.stderr`` is always line-buffered now, even if ``stderr`` is |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
serhiy-storchaka
Dec 21, 2019
Member
This is your first contribution. Please add your name in Misc/ACKS
. You can also add "Patch by yourname." here.
This comment has been minimized.
This comment has been minimized.
LGTM. @serhiy-storchaka, @pitrou: does it look good to you? I let you merge the change. |
jendrikseipp commentedDec 17, 2019
•
edited by bedevere-bot
Fixes https://bugs.python.org/issue13601
https://bugs.python.org/issue13601