bpo-13886: Fix test_builtin.PtyTests tests when readline is loaded #7133
Conversation
|
Last attempt (no OS X available), noticing that |
|
Closing the PR to trigger a new build |
Lib/test/test_builtin.py
Outdated
| [master, slave] = pty.openpty() | ||
| proc = subprocess.Popen(cmd, stdin=slave, stdout=slave, stderr=slave) | ||
| os.close(slave) | ||
| output = "could not read process output" |
There was a problem hiding this comment.
Seems this string is always overwritten below (or if there is an exception, it is never used)
| '# Check we exercise the PyOS_Readline() path\n' | ||
| 'if not sys.stdin.isatty() or not sys.stdout.isatty():\n' | ||
| ' raise AssertionError("standard IO should be a tty")\n' | ||
| 'print("{sentinel}", end="")\n' |
There was a problem hiding this comment.
Why the end="" bit? Add a comment saying you are testing that input flushes stdout if that is your intention, because it’s not obvious.
There was a problem hiding this comment.
It is expected that input() flushes stdout, not sure why you think this would need to be tested here. Avoiding the \n makes it clear that the length of sentinel on input is not tied to platform new line conversions.
Lib/test/test_builtin.py
Outdated
| output = "could not read process output" | ||
| with proc, os.fdopen(master, "wb", closefd=False) as writer: | ||
| # Wait for the process to be fully started. | ||
| sentinel = os.read(master, len(expected)) |
There was a problem hiding this comment.
I’m curious why you need to synchronize the parent with the child. What happens if the process isn’t fully started? Does OS X throw away the input if the child isn’t ready or something?
There was a problem hiding this comment.
I do not have an OS X platform and have been merely guessing at solutions to fix the patch from issue 13886 on this platform. Synchronizing the parent with the child makes the test more robust (and easier to debug).
Lib/test/test_builtin.py
Outdated
| 'if result != {expected!a}:\n' | ||
| ' raise AssertionError("unexpected input " + ascii(result))\n' | ||
| ) | ||
| sentinel = 'Hé, this is before calling input()' |
There was a problem hiding this comment.
Does Python still run in environments where stdout or the command line cannot encode e-acute? I haven’t kept up with the new Python encoding changes, but this used to be the case. You can avoid the CLI problem by using the ascii function, or “!a” in the format string.
There was a problem hiding this comment.
Oops, got it now.
Simplest is to set sentinel to 123 then.
https://bugs.python.org/issue13886