diff --git a/CHANGELOG b/CHANGELOG index e6e3eace1..40444fb82 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,11 @@ Changes between 1.3.0 and 1.3.1 ================================================== +- fix issue96: make capturing more resilient against Control-C + interruptions (involved somewhat substantial refactoring + to the underlying capturing functionality to avoid race + conditions). + - make py.test.cmdline.main() return the exitstatus instead of raising (which is still done by py.cmdline.pytest()) and make it so that py.test.cmdline.main() can be called diff --git a/py/_io/capture.py b/py/_io/capture.py index fe03dee9a..56dfe603a 100644 --- a/py/_io/capture.py +++ b/py/_io/capture.py @@ -29,21 +29,25 @@ except ImportError: class FDCapture: """ Capture IO to/from a given os-level filedescriptor. """ - def __init__(self, targetfd, tmpfile=None): + def __init__(self, targetfd, tmpfile=None, now=True): """ save targetfd descriptor, and open a new temporary file there. If no tmpfile is specified a tempfile.Tempfile() will be opened in text mode. """ self.targetfd = targetfd + self._patched = [] if tmpfile is None: f = tempfile.TemporaryFile('wb+') tmpfile = dupfile(f, encoding="UTF-8") f.close() self.tmpfile = tmpfile - self._savefd = os.dup(targetfd) - os.dup2(self.tmpfile.fileno(), targetfd) - self._patched = [] + if now: + self.start() + + def start(self): + self._savefd = os.dup(self.targetfd) + os.dup2(self.tmpfile.fileno(), self.targetfd) def setasfile(self, name, module=sys): """ patch . to self.tmpfile @@ -62,10 +66,13 @@ class FDCapture: def done(self): """ unpatch and clean up, returns the self.tmpfile (file object) """ - os.dup2(self._savefd, self.targetfd) + try: + os.dup2(self._savefd, self.targetfd) + os.close(self._savefd) + self.tmpfile.seek(0) + except (AttributeError, ValueError, OSError): + pass self.unsetfiles() - os.close(self._savefd) - self.tmpfile.seek(0) return self.tmpfile def writeorg(self, data): @@ -146,89 +153,92 @@ class Capture(object): def reset(self): """ reset sys.stdout/stderr and return captured output as strings. """ - if hasattr(self, '_suspended'): - outfile = self._kwargs['out'] - errfile = self._kwargs['err'] - del self._kwargs - else: - outfile, errfile = self.done() + outfile, errfile = self.done() out, err = "", "" - if outfile: + if outfile and not outfile.closed: out = outfile.read() outfile.close() - if errfile and errfile != outfile: + if errfile and errfile != outfile and not errfile.closed: err = errfile.read() errfile.close() return out, err def suspend(self): """ return current snapshot captures, memorize tempfiles. """ - assert not hasattr(self, '_suspended') - self._suspended = True outerr = self.readouterr() outfile, errfile = self.done() - self._kwargs['out'] = outfile - self._kwargs['err'] = errfile return outerr - def resume(self): - """ resume capturing with original temp files. """ - assert self._suspended - self._initialize(**self._kwargs) - del self._suspended - class StdCaptureFD(Capture): """ This class allows to capture writes to FD1 and FD2 and may connect a NULL file to FD0 (and prevent reads from sys.stdin) """ - def __init__(self, out=True, err=True, - mixed=False, in_=True, patchsys=True): - self._kwargs = locals().copy() - del self._kwargs['self'] - self._initialize(**self._kwargs) - - def _initialize(self, out=True, err=True, - mixed=False, in_=True, patchsys=True): + def __init__(self, out=True, err=True, mixed=False, + in_=True, patchsys=True, now=True): + self.in_ = in_ if in_: self._oldin = (sys.stdin, os.dup(0)) - sys.stdin = DontReadFromInput() - fd = os.open(devnullpath, os.O_RDONLY) - os.dup2(fd, 0) - os.close(fd) - if out: + if out: tmpfile = None if hasattr(out, 'write'): - tmpfile = out - self.out = py.io.FDCapture(1, tmpfile=tmpfile) - if patchsys: - self.out.setasfile('stdout') - if err: - if mixed and out: + tmpfile = None + self.out = py.io.FDCapture(1, tmpfile=tmpfile, now=False) + self.out_tmpfile = tmpfile + if err: + if out and mixed: tmpfile = self.out.tmpfile elif hasattr(err, 'write'): tmpfile = err else: tmpfile = None - self.err = py.io.FDCapture(2, tmpfile=tmpfile) - if patchsys: - self.err.setasfile('stderr') + self.err = py.io.FDCapture(2, tmpfile=tmpfile, now=False) + self.err_tmpfile = tmpfile + self.patchsys = patchsys + if now: + self.startall() + + def startall(self): + if self.in_: + sys.stdin = DontReadFromInput() + fd = os.open(devnullpath, os.O_RDONLY) + os.dup2(fd, 0) + os.close(fd) + + out = getattr(self, 'out', None) + if out: + out.start() + if self.patchsys: + out.setasfile('stdout') + err = getattr(self, 'err', None) + if err: + err.start() + if self.patchsys: + err.setasfile('stderr') + + def resume(self): + """ resume capturing with original temp files. """ + #if hasattr(self, 'out'): + # self.out.restart() + #if hasattr(self, 'err'): + # self.err.restart() + self.startall() def done(self): """ return (outfile, errfile) and stop capturing. """ + outfile = errfile = None if hasattr(self, 'out'): outfile = self.out.done() - else: - outfile = None if hasattr(self, 'err'): errfile = self.err.done() - else: - errfile = None if hasattr(self, '_oldin'): oldsys, oldfd = self._oldin - os.dup2(oldfd, 0) - os.close(oldfd) + try: + os.dup2(oldfd, 0) + os.close(oldfd) + except OSError: + pass sys.stdin = oldsys return outfile, errfile @@ -252,69 +262,61 @@ class StdCapture(Capture): modifies sys.stdout|stderr|stdin attributes and does not touch underlying File Descriptors (use StdCaptureFD for that). """ - def __init__(self, out=True, err=True, in_=True, mixed=False): - self._kwargs = locals().copy() - del self._kwargs['self'] - self._initialize(**self._kwargs) - - def _initialize(self, out, err, in_, mixed): - self._out = out - self._err = err - self._in = in_ - if out: - self._oldout = sys.stdout - if not hasattr(out, 'write'): - out = TextIO() - sys.stdout = self.out = out - if err: - self._olderr = sys.stderr - if out and mixed: - err = self.out + def __init__(self, out=True, err=True, in_=True, mixed=False, now=True): + self._oldout = sys.stdout + self._olderr = sys.stderr + self._oldin = sys.stdin + if out and not hasattr(out, 'file'): + out = TextIO() + self.out = out + if err: + if mixed: + err = out elif not hasattr(err, 'write'): err = TextIO() - sys.stderr = self.err = err - if in_: - self._oldin = sys.stdin - sys.stdin = self.newin = DontReadFromInput() + self.err = err + self.in_ = in_ + if now: + self.startall() + + def startall(self): + if self.out: + sys.stdout = self.out + if self.err: + sys.stderr = self.err + if self.in_: + sys.stdin = self.in_ = DontReadFromInput() def done(self): """ return (outfile, errfile) and stop capturing. """ - o,e = sys.stdout, sys.stderr - if self._out: - try: - sys.stdout = self._oldout - except AttributeError: - raise IOError("stdout capturing already reset") - del self._oldout + outfile = errfile = None + if self.out and not self.out.closed: + sys.stdout = self._oldout outfile = self.out outfile.seek(0) - else: - outfile = None - if self._err: - try: - sys.stderr = self._olderr - except AttributeError: - raise IOError("stderr capturing already reset") - del self._olderr + if self.err and not self.err.closed: + sys.stderr = self._olderr errfile = self.err errfile.seek(0) - else: - errfile = None - if self._in: + if self.in_: sys.stdin = self._oldin return outfile, errfile + def resume(self): + """ resume capturing with original temp files. """ + self.startall() + def readouterr(self): """ return snapshot value of stdout/stderr capturings. """ out = err = "" - if self._out: - out = sys.stdout.getvalue() - sys.stdout.truncate(0) - sys.stdout.seek(0) - if self._err: - err = sys.stderr.getvalue() - sys.stderr.truncate(0) - sys.stderr.seek(0) + if self.out: + out = self.out.getvalue() + self.out.truncate(0) + self.out.seek(0) + if self.err: + err = self.err.getvalue() + self.err.truncate(0) + self.err.seek(0) return out, err class DontReadFromInput: @@ -344,5 +346,3 @@ except AttributeError: devnullpath = 'NUL' else: devnullpath = '/dev/null' - - diff --git a/py/_plugin/pytest_capture.py b/py/_plugin/pytest_capture.py index 1f06ba8df..eb3c60342 100644 --- a/py/_plugin/pytest_capture.py +++ b/py/_plugin/pytest_capture.py @@ -106,6 +106,14 @@ def addouterr(rep, outerr): def pytest_configure(config): config.pluginmanager.register(CaptureManager(), 'capturemanager') +class NoCapture: + def startall(self): + pass + def resume(self): + pass + def suspend(self): + return "", "" + class CaptureManager: def __init__(self): self._method2capture = {} @@ -118,15 +126,17 @@ class CaptureManager: def _makestringio(self): return py.io.TextIO() - def _startcapture(self, method): + def _getcapture(self, method): if method == "fd": - return py.io.StdCaptureFD( + return py.io.StdCaptureFD(now=False, out=self._maketempfile(), err=self._maketempfile() ) elif method == "sys": - return py.io.StdCapture( + return py.io.StdCapture(now=False, out=self._makestringio(), err=self._makestringio() ) + elif method == "no": + return NoCapture() else: raise ValueError("unknown capturing method: %r" % method) @@ -152,27 +162,25 @@ class CaptureManager: if hasattr(self, '_capturing'): raise ValueError("cannot resume, already capturing with %r" % (self._capturing,)) - if method != "no": - cap = self._method2capture.get(method) - if cap is None: - cap = self._startcapture(method) - self._method2capture[method] = cap - else: - cap.resume() + cap = self._method2capture.get(method) self._capturing = method + if cap is None: + self._method2capture[method] = cap = self._getcapture(method) + cap.startall() + else: + cap.resume() def suspendcapture(self, item=None): self.deactivate_funcargs() if hasattr(self, '_capturing'): method = self._capturing - if method != "no": - cap = self._method2capture[method] + cap = self._method2capture.get(method) + if cap is not None: outerr = cap.suspend() - else: - outerr = "", "" del self._capturing if item: - outerr = (item.outerr[0] + outerr[0], item.outerr[1] + outerr[1]) + outerr = (item.outerr[0] + outerr[0], + item.outerr[1] + outerr[1]) return outerr return "", "" @@ -180,19 +188,17 @@ class CaptureManager: if not hasattr(pyfuncitem, 'funcargs'): return assert not hasattr(self, '_capturing_funcargs') - l = [] - for name, obj in pyfuncitem.funcargs.items(): - if name == 'capfd' and not hasattr(os, 'dup'): - py.test.skip("capfd funcarg needs os.dup") + self._capturing_funcargs = capturing_funcargs = [] + for name, capfuncarg in pyfuncitem.funcargs.items(): if name in ('capsys', 'capfd'): - obj._start() - l.append(obj) - if l: - self._capturing_funcargs = l + capturing_funcargs.append(capfuncarg) + capfuncarg._start() def deactivate_funcargs(self): - if hasattr(self, '_capturing_funcargs'): - for capfuncarg in self._capturing_funcargs: + capturing_funcargs = getattr(self, '_capturing_funcargs', None) + if capturing_funcargs is not None: + while capturing_funcargs: + capfuncarg = capturing_funcargs.pop() capfuncarg._finalize() del self._capturing_funcargs @@ -256,16 +262,19 @@ def pytest_funcarg__capfd(request): platform does not have ``os.dup`` (e.g. Jython) tests using this funcarg will automatically skip. """ + if not hasattr(os, 'dup'): + py.test.skip("capfd funcarg needs os.dup") return CaptureFuncarg(request, py.io.StdCaptureFD) class CaptureFuncarg: def __init__(self, request, captureclass): self._cclass = captureclass + self.capture = self._cclass(now=False) #request.addfinalizer(self._finalize) def _start(self): - self.capture = self._cclass() + self.capture.startall() def _finalize(self): if hasattr(self, 'capture'): @@ -276,6 +285,4 @@ class CaptureFuncarg: return self.capture.readouterr() def close(self): - self.capture.reset() - del self.capture - + self._finalize() diff --git a/testing/io_/test_capture.py b/testing/io_/test_capture.py index fa64208a0..de2552b76 100644 --- a/testing/io_/test_capture.py +++ b/testing/io_/test_capture.py @@ -96,6 +96,21 @@ def test_dupfile(tmpfile): class TestFDCapture: pytestmark = needsdup + def test_not_now(self, tmpfile): + fd = tmpfile.fileno() + cap = py.io.FDCapture(fd, now=False) + data = tobytes("hello") + os.write(fd, data) + f = cap.done() + s = f.read() + assert not s + cap = py.io.FDCapture(fd, now=False) + cap.start() + os.write(fd, data) + f = cap.done() + s = f.read() + assert s == "hello" + def test_stdout(self, tmpfile): fd = tmpfile.fileno() cap = py.io.FDCapture(fd) @@ -185,8 +200,11 @@ class TestStdCapture: def test_capturing_twice_error(self): cap = self.getcapture() print ("hello") - cap.reset() - py.test.raises(Exception, "cap.reset()") + out, err = cap.reset() + print ("world") + out2, err = cap.reset() + assert out == "hello\n" + assert not err def test_capturing_modify_sysouterr_in_between(self): oldout = sys.stdout @@ -210,7 +228,6 @@ class TestStdCapture: cap2 = self.getcapture() print ("cap2") out2, err2 = cap2.reset() - py.test.raises(Exception, "cap2.reset()") out1, err1 = cap1.reset() assert out1 == "cap1\n" assert out2 == "cap2\n" @@ -265,6 +282,13 @@ class TestStdCapture: assert out == "after\n" assert not err +class TestStdCaptureNotNow(TestStdCapture): + def getcapture(self, **kw): + kw['now'] = False + cap = py.io.StdCapture(**kw) + cap.startall() + return cap + class TestStdCaptureFD(TestStdCapture): pytestmark = needsdup @@ -296,6 +320,22 @@ class TestStdCaptureFD(TestStdCapture): assert out.startswith("3") assert err.startswith("4") +class TestStdCaptureFDNotNow(TestStdCaptureFD): + pytestmark = needsdup + + def getcapture(self, **kw): + kw['now'] = False + cap = py.io.StdCaptureFD(**kw) + cap.startall() + return cap + +def test_capture_not_started_but_reset(): + capsys = py.io.StdCapture(now=False) + capsys.done() + capsys.done() + capsys.reset() + capsys.reset() + @needsdup def test_capture_no_sys(): capsys = py.io.StdCapture() diff --git a/testing/plugin/test_pytest_capture.py b/testing/plugin/test_pytest_capture.py index c96769609..f5015db58 100644 --- a/testing/plugin/test_pytest_capture.py +++ b/testing/plugin/test_pytest_capture.py @@ -39,6 +39,10 @@ class TestCaptureManager: old = sys.stdout, sys.stderr, sys.stdin try: capman = CaptureManager() + # call suspend without resume or start + outerr = capman.suspendcapture() + outerr = capman.suspendcapture() + assert outerr == ("", "") capman.resumecapture(method) print ("hello") out, err = capman.suspendcapture()