-
Notifications
You must be signed in to change notification settings - Fork 476
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
interact() missing coverage for (OSError, EIO), reading after child exit #23
Conversation
Is there something in particular I should do to cause it to reproduce while using vim ?
I'm in VIM as expected, but, now what? (edited for formatting) |
It may require tmux, though I'm able to duplicate it either with tmux or not. The original situation that caused this was wrapping a git merge --no-ff which opens vim as my default editor inside of tmux |
Ahh maybe I see it now? Reproduced when using pexpect as available on pypi.org (version 2.4).
Can you also reproduce this, using pexpect, from github.com?
I can't... |
Ah, nope, it looks good, sorry. The problem was that I still had a On Sat, Nov 9, 2013 at 6:50 PM, Jeff Quast notifications@github.com wrote:
|
So the conclusion is that the current version in git doesn't exhibit this bug, then? Do we want to close this PR, or are there still situations where the problem might come up? |
I'm wondering if it may be related to (and hey, maybe even possibly resolve) issue #20 (strange EOF/Timeout behavior on BSD systems) |
But this only changes code used by |
agreed, i came to the same conclusion and tabled it as "Compare and consider... later" ;-) hehe |
OK, but there's nothing we think we should be aiming to fix before 3.0, then? @finder , is that OK? I'm aiming to release 3.0 tomorrow if everything's clear. |
The git tip seems ok for me, I have no other complaints. :)
|
Great :-) |
All said, I think we leave this alone for the 3.0 release, yet edit the summary of this issue and retain this bug. What troubles me: the variable "data" is re-used, for both stdin of controlling process, and stdout of child pty, within a loop. In the case of an I/O Error reading from child process, an implicit pass occurs, and any of three conditions then occur:
All said -- an IO Error occurs when reading from a child pty that has exited. What we're looking to test, then, is a very brief moment of time where the child process has not yet exited (self.isalive() returns True), has data to read (fd returned in select() call), but then exits before it is read by the parent process (causing an IOError exception). We're also looking at some very specific operating systems: In Linux, OSError raises to signal that an EOF occurred. BSD on the other hand would return an empty bytestring. I'm going to re-name the title of this and mark it as a minor bug, as something that needs test coverage -- and fixing, if necessary. It may take some tinkering to write correct coverage for. |
actually, will close and open a bug instead. thanks for reading :-) |
I was able to duplicate the OSError while using stdin and vim:
pexpect.spawn('vim')
pexpect.interact()
added the breaks since there shouldn't be a local data that's valid after the exception
jquast: this bug was for pexpect2.4 and does not occur with 3.0.
However, some related tests should be implemented (and possibly, fixes).