Patch: stdout wrapper breaks some subprocess calls

[jpellerin]

Currently nose uses StringIO directly as a stdout wrapper which works fine
99% of the time.

subprocess.Popen's default stdout is sys.stdout. subprocess assumes this is
a real filehandler and tends to check sys.stdout.fileno(). Since the method
doesn't exist, bad things happen.

Attached is a patch that removes the direct use of StringIO with a Buffer
class that has a fileno() method that returns 1 the way stdout normall does.

_SpoofOut has also been modified to provide this method.

I ran all the unittests, they all still pass.

Google Code Info:
Issue #: 290
Author: mikeal.rogers
Created On: 2009-09-08T00:29:05.000Z
Closed On:

[jpellerin]

Google Code Info:
Author: [email protected]
Created On: 2009-09-08T14:03:17.000Z

[jpellerin]

Has this landed yet?

It is by far the largest support issue we have in #windmill, which is good because it
means that nose is becoming the preferred method of running windmill tests but bad in
that we're basically telling them to apply my patch in order to fix it.

Google Code Info:
Author: mikeal.rogers
Created On: 2009-12-11T00:35:18.000Z

[jpellerin]

I haven't had time to evaluate the patch yet. My time to work on nose is severely
limited these days. If you want to get it looked at faster than I'll be able to (I can
only say that I should have time before the end of the year) then you should bring this
topic up on the dev list and see if one of the other developers can evaluate the patch.
That may speed things up a lot.

Google Code Info:
Author: [email protected]
Created On: 2009-12-11T15:11:02.000Z

[jpellerin]

hi even with this patch, if i have a test like:
{{{
from subprocess import Popen
import sys

def sh(cmd, blast_log=None):
"""run a commmand in the shell
>>> sh("echo 'hi'")
TODO
"""
if not blast_log is None:
cmd += " 2>%s" % blast_log
#log.debug(cmd)
proc = Popen(cmd, stdout=sys.stdout, stderr=sys.stderr, shell=True)
r = proc.communicate()
return r
}}}

and run with
{{{
nosetests --with-doctest t.py
}}}

i get
{{{

F

FAIL: Doctest: t.sh

Traceback (most recent call last):
File "/usr/lib/python2.6/doctest.py", line 2145, in runTest
raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for t.sh
File "/opt/src/python-nose/t/t.py", line 4, in sh

---

File "/opt/src/python-nose/t/t.py", line 6, in t.sh
Failed example:
sh("echo 'hi'")
Exception raised:
Traceback (most recent call last):
File "/usr/lib/python2.6/doctest.py", line 1241, in run
compileflags, 1) in test.globs
File "<doctest t.sh[0]>", line 1, in
sh("echo 'hi'")
File "/opt/src/python-nose/t/t.py", line 12, in sh
proc = Popen(cmd, stdout=sys.stdout, stderr=sys.stderr, shell=True)
File "/usr/lib/python2.6/subprocess.py", line 588, in __init
errread, errwrite) = self._get_handles(stdin, stdout, stderr)
File "/usr/lib/python2.6/subprocess.py", line 945, in _get_handles
c2pwrite = stdout.fileno()
AttributeError: _SpoofOut instance has no attribute 'fileno'

---

Ran 1 test in 0.005s

FAILED (failures=1)
}}}

Google Code Info:
Author: bpederse
Created On: 2009-12-14T19:59:54.000Z

[jpellerin]

I wonder if the patch is out of date and there is another _SpoofOut class somewhere
that I haven't added this to.

Google Code Info:
Author: mikeal.rogers
Created On: 2009-12-14T20:13:25.000Z

[jpellerin]

The bug exists both in doctest and in nose, so that's why the doctest example fails.
That's out of scope for nose to fix.

The nose patch looks fine -- except that it doesn't include any tests. If this is going
to land in 0.11.2, the patch needs a functional test to support the change (one that
fails w/out the change and passes with it). So I'm setting the patch status to rejected
for now, and I'll re-evaluate when a patch or bb/gcode fork with tests is posted, or
when I have time to write the tests myself (post-0.11.2 probably).

Google Code Info:
Author: [email protected]
Created On: 2009-12-14T20:48:42.000Z

[jpellerin]

yes doctest.py from stdlib defines its own _SpoofOut
doing this:
{{{
from nose.ext.dtcompat import _SpoofOut
doctest._SpoofOut = _SpoofOut
}}}

at the top of my code works for me. (i realize it's probably a bad idea for a general
solution)
thanks.

Google Code Info:
Author: bpederse
Created On: 2009-12-14T21:01:29.000Z

[jpellerin]

Google Code Info:
Author: [email protected]
Created On: 2010-03-03T16:20:30.000Z

[jpellerin]

I really need this fixed, and posted to the mailing list today not knowing of this ticket. Is
there an ETA on this getting fixed and released?

Thanks!

Google Code Info:
Author: [email protected]
Created On: 2010-03-24T23:16:10.000Z

[jpellerin]

I bet the MultiProcessFile in plugintest has this issue as well...

Google Code Info:
Author: [email protected]
Created On: 2011-05-03T04:27:20.000Z

[jpellerin]

Same issue here. In my case it is a "simple" unit test where I happens to launch a process with subprocess. The StringIO wrapper kills the test.

All tests run fine with unittest.

nosetests version 1.0.0

Google Code Info:
Author: [email protected]
Created On: 2011-06-28T19:13:00.000Z

[jpellerin]

I was able to bypass this problem with the following short/horrible monkeypatch in my test code:

#monkey patch to get nose working with subprocess...
def fileno_monkeypatch(self):
import sys
return sys.stdout.fileno()
import StringIO
StringIO.StringIO.fileno = fileno_monkeypatch

That hack just treats all attempts at .fileno() access on a StringIO instance as if it were stdout that was wrapped. It worked ok for me, but it may need to get uglier to deal with stderr as well if it doesn't work for you.

Google Code Info:
Author: [email protected]
Created On: 2011-06-28T19:36:03.000Z

[jpellerin]

@russ: Please show us this simple testcase, and how to make it fail. We'll add it as a test for nose.

Google Code Info:
Author: [email protected]
Created On: 2011-06-28T19:51:46.000Z

[jpellerin]

@workitharder: This little file will crash with nose:

import subprocess, sys
def testSubprocessCrash():
subprocess.Popen(["ls", ], stdout = sys.stdout)

Note that that little test can also be fixed by changing sys.stdout to sys.stdout, but you may not always comfortably have direct access to the Popen call that is handing sys.stdout around.

Cleaner would be to simply have nose subclass StringIO and add a bogus .fileno() member that will play nicely with subprocess. I see that that is what the initial patch posted seemed to do.

Google Code Info:
Author: [email protected]
Created On: 2011-06-28T20:04:25.000Z

[jpellerin]

Thanks.
It should be a simple matter to add that as a test, and to fix up plugintest
for both the multiprocessing and StringIO cases.
Would you provide us with a patch?
--Buck

Google Code Info:
Author: [email protected]
Created On: 2011-06-28T20:24:06.000Z

[jpellerin]

@workitharder: I don't have a patch, and that nasty monkey patch is exactly that... nasty! It is unclear whether or not it will cause other problems, although it did seem to work for my case. In particular it very likely prevents the spawned subprocess outputs to what it thinks stdout is from getting captured by the StringIO wrapper in the parent. I have not tested this, though.

Sorry - no time to work through nose to try and figure out a real patch, although I would like to help. First reason being that I just started looking at using nose today and have no clue of the nose architecture (eg: why is stdout wrapped with StringIO anyway). Secondly, a clean patch may not be possible due to subprocess's heavy use of file descriptors.

Google Code Info:
Author: [email protected]
Created On: 2011-06-29T05:14:38.000Z

[jpellerin]

(Old?) patch: https://gist.github.com/1477878

[Nodd]

I'm having the same issue with nose and subprocess. The monkeypatch works, but it shouldn't be necessary.

[jrimestad]

A fine workaround outside nose is to replace check_call with check_output or Popen

# returnCode = subprocess.check_call(shlex.split(command), stdout=cout, stderr=cerr)
p = Popen(shlex.split(command), stdout=PIPE, stderr=PIPE)
out, err = p.communicate(b"input data that is passed to subprocess' stdin")
if cout:
cout.write(out.decode(encoding='UTF-8'))
if cerr:
cerr.write(err.decode(encoding='UTF-8'))
returnCode = p.returncode