Message285038
Dear Martin, now I understand your intention. I merged my test suite with Chris's fix and documented our insights. SmallPtyTests contains regression tests for this issue.
While testing, I found a subtle change in behavior introduced by Chris's patch: It is no longer possible to get a broken pipe easily. I made a small adjustment to preserve this existing behavior: In the _copy loop, I changed the order in which master_fd and STDIN_FILENO are copied to preserve the BrokenPipeError.
I tuned the slow tests, the complete test suite now needs less than 1 second on my system. I updated the documentation to note that the module is no longer supposed to be Linux-only.
> I realized that PtyWhiteBoxIntegrationTermiosTest is a reasonable test for the termios module, so it is beneficial. Though it may have been simpler to write it using pty.openpty(), avoiding an extra thread that just copies data between other threads.
I agree completely. However, pty.openpty() does not depend on the _copy() loop which is modified by Chris's patch. To add higher test coverage for the changes we introduce by merging Chris's fix, I decided to stick to pty.spawn().
I tested on Linux 4.4, Linux 3.13, FreeBSD 11, MacOS 10.11.6 and all tests are green. If anything goes wrong on the testbot army, here is the fallback strategy:
[I don't expect anything to go wrong, but I'm rather prepared than sorry]
If the test_pty does not finish within one minute on a system, please kill it. It would be nice to know on which platform it failed, so we can whitelist the module in the tests and the documentation for only the supported platforms. |
|
Date |
User |
Action |
Args |
2017-01-09 12:27:22 | Cornelius Diekmann | set | recipients:
+ Cornelius Diekmann, vstinner, martin.panter, Alex.Willmer, chris.torek |
2017-01-09 12:27:20 | Cornelius Diekmann | set | messageid: <1483964840.38.0.955935171615.issue29070@psf.upfronthosting.co.za> |
2017-01-09 12:27:20 | Cornelius Diekmann | link | issue29070 messages |
2017-01-09 12:27:20 | Cornelius Diekmann | create | |
|