From 3febdff9963dc411a3a9da42dd354c352f269874 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 22 Oct 2019 20:36:36 +0200 Subject: [PATCH 1/8] capture: do not close tmpfile buffer, but redirect Fixes https://github.com/pytest-dev/pytest/issues/5502 --- src/_pytest/capture.py | 8 ++++++-- testing/test_capture.py | 44 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 0cd3ce60427..df4a785a765 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -594,7 +594,10 @@ def _done(self): os.dup2(targetfd_save, self.targetfd) os.close(targetfd_save) self.syscapture.done() - self.tmpfile.close() + # Redirect any remaining output. + os.dup2(self.targetfd, self.tmpfile.buffer.fileno()) + # TODO: atexit? + # self.tmpfile.close() self._state = "done" def suspend(self): @@ -665,7 +668,8 @@ def snap(self): def done(self): setattr(sys, self.name, self._old) del self._old - self.tmpfile.close() + # TODO: atexit? + # self.tmpfile.close() self._state = "done" def suspend(self): diff --git a/testing/test_capture.py b/testing/test_capture.py index 94af3aef75c..c6837f96c3a 100644 --- a/testing/test_capture.py +++ b/testing/test_capture.py @@ -1490,3 +1490,47 @@ def test_fails(): result_with_capture.stdout.fnmatch_lines( ["E * TypeError: write() argument must be str, not bytes"] ) + + +def test_logging_in_atexit(testdir): + p = testdir.makepyfile( + """ + import atexit + import logging + import sys + + cur_stdout = sys.stdout + LOGGER = logging.getLogger(__name__) + + def test_fail(): + assert 0 + + def _atexit(): + print("test-print in atexit", cur_stdout) + LOGGER.error("test-log in atexit") + + print() + print("test-register") + print() + atexit.register(_atexit) + logging.basicConfig() + + LOGGER.error("log_setup_not_shown_from_collection") + + print(sys.stderr, id(sys.stderr)) + """ + ) + result = testdir.runpytest_subprocess(str(p)) + result.stdout.fnmatch_lines( + [ + "*= 1 failed in *", + "test-print in atexit <_pytest.capture.EncodedFile object *", + ] + ) + assert result.stderr.lines == ["ERROR:test_logging_in_atexit:test-log in atexit"] + assert result.ret == 1 + + output = str(result.stdout) + str(result.stderr) + assert "test-register" not in output + assert "*- Captured stderr call -*" not in output + assert "log_setup_not_shown_from_collection" not in output From 678ad6590c5c3aa557d6bb0833c6075603638a82 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 22 Oct 2019 21:01:05 +0200 Subject: [PATCH 2/8] close atexit --- src/_pytest/capture.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index df4a785a765..9560148fd5b 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -2,6 +2,7 @@ per-test stdout/stderr capturing mechanism. """ +import atexit import collections import contextlib import io @@ -595,9 +596,8 @@ def _done(self): os.close(targetfd_save) self.syscapture.done() # Redirect any remaining output. - os.dup2(self.targetfd, self.tmpfile.buffer.fileno()) - # TODO: atexit? - # self.tmpfile.close() + os.dup2(self.targetfd, self.tmpfile_fd) + atexit.register(self.tmpfile.close) self._state = "done" def suspend(self): @@ -668,8 +668,7 @@ def snap(self): def done(self): setattr(sys, self.name, self._old) del self._old - # TODO: atexit? - # self.tmpfile.close() + atexit.register(self.tmpfile.close) self._state = "done" def suspend(self): From 33f41d33c30456bb3d15966ea98804181cdd62ad Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 22 Oct 2019 21:48:43 +0200 Subject: [PATCH 3/8] wip: atexit --- src/_pytest/capture.py | 47 +++++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 9560148fd5b..3058534088b 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -10,6 +10,8 @@ import sys from io import UnsupportedOperation from tempfile import TemporaryFile +from typing import Callable +from typing import List import pytest from _pytest.compat import CaptureIO @@ -78,19 +80,28 @@ def __init__(self, method): self._method = method self._global_capturing = None self._current_item = None + self._atexit_funcs: List[Callable] = [] + atexit.register(self._atexit_run) def __repr__(self): return "".format( self._method, self._global_capturing, self._current_item ) + def _atexit_register(self, func): + self._atexit_funcs.append(func) + + def _atexit_run(self): + for func in self._atexit_funcs: + func() + def _getcapture(self, method): if method == "fd": - return MultiCapture(out=True, err=True, Capture=FDCapture) + return MultiCapture(out=True, err=True, Capture=FDCapture, capman=self) elif method == "sys": - return MultiCapture(out=True, err=True, Capture=SysCapture) + return MultiCapture(out=True, err=True, Capture=SysCapture, capman=self) elif method == "no": - return MultiCapture(out=False, err=False, in_=False) + return MultiCapture(out=False, err=False, in_=False, capman=self) raise ValueError("unknown capturing method: %r" % method) # pragma: no cover def is_capturing(self): @@ -451,13 +462,13 @@ class MultiCapture: out = err = in_ = None _state = None - def __init__(self, out=True, err=True, in_=True, Capture=None): + def __init__(self, out=True, err=True, in_=True, Capture=None, capman: CaptureManager = None): if in_: - self.in_ = Capture(0) + self.in_ = Capture(0, capman=capman) if out: - self.out = Capture(1) + self.out = Capture(1, capman=capman) if err: - self.err = Capture(2) + self.err = Capture(2, capman=capman) def __repr__(self): return "".format( @@ -540,8 +551,9 @@ class FDCaptureBinary: EMPTY_BUFFER = b"" _state = None - def __init__(self, targetfd, tmpfile=None): + def __init__(self, targetfd, tmpfile=None, capman: CaptureManager = None): self.targetfd = targetfd + self._capman = capman try: self.targetfd_save = os.dup(self.targetfd) except OSError: @@ -560,7 +572,7 @@ def __init__(self, targetfd, tmpfile=None): with f: tmpfile = safe_text_dupfile(f, mode="wb+") if targetfd in patchsysdict: - self.syscapture = SysCapture(targetfd, tmpfile) + self.syscapture = SysCapture(targetfd, tmpfile, capman) else: self.syscapture = NoCapture() self.tmpfile = tmpfile @@ -595,9 +607,12 @@ def _done(self): os.dup2(targetfd_save, self.targetfd) os.close(targetfd_save) self.syscapture.done() - # Redirect any remaining output. - os.dup2(self.targetfd, self.tmpfile_fd) - atexit.register(self.tmpfile.close) + if self._capman: + # Redirect any remaining output. + os.dup2(self.targetfd, self.tmpfile_fd) + self._capman._atexit_register(self.tmpfile.close) + else: + self.tmpfile.close() self._state = "done" def suspend(self): @@ -639,8 +654,9 @@ class SysCapture: EMPTY_BUFFER = str() _state = None - def __init__(self, fd, tmpfile=None): + def __init__(self, fd, tmpfile=None, capman: CaptureManager = None): name = patchsysdict[fd] + self._capman = capman self._old = getattr(sys, name) self.name = name if tmpfile is None: @@ -668,7 +684,10 @@ def snap(self): def done(self): setattr(sys, self.name, self._old) del self._old - atexit.register(self.tmpfile.close) + if self._capman: + self._capman._atexit_register(self.tmpfile.close) + else: + self.tmpfile.close() self._state = "done" def suspend(self): From ad87e064ba2938b31fd91aa06bb5a4a426dd2a1a Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 23 Oct 2019 04:06:42 +0200 Subject: [PATCH 4/8] re-use tmpfiles per capman --- src/_pytest/capture.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 3058534088b..b168722d038 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -82,6 +82,7 @@ def __init__(self, method): self._current_item = None self._atexit_funcs: List[Callable] = [] atexit.register(self._atexit_run) + self._tmpfiles = {} def __repr__(self): return "".format( @@ -462,7 +463,9 @@ class MultiCapture: out = err = in_ = None _state = None - def __init__(self, out=True, err=True, in_=True, Capture=None, capman: CaptureManager = None): + def __init__( + self, out=True, err=True, in_=True, Capture=None, capman: CaptureManager = None + ): if in_: self.in_ = Capture(0, capman=capman) if out: @@ -568,9 +571,19 @@ def __init__(self, targetfd, tmpfile=None, capman: CaptureManager = None): self.syscapture = SysCapture(targetfd) else: if tmpfile is None: - f = TemporaryFile() - with f: - tmpfile = safe_text_dupfile(f, mode="wb+") + if capman: + try: + tmpfile = capman._tmpfiles[targetfd] + assert not tmpfile.closed + except KeyError: + f = TemporaryFile() + with f: + tmpfile = safe_text_dupfile(f, mode="wb+") + capman._tmpfiles[targetfd] = tmpfile + else: + f = TemporaryFile() + with f: + tmpfile = safe_text_dupfile(f, mode="wb+") if targetfd in patchsysdict: self.syscapture = SysCapture(targetfd, tmpfile, capman) else: From e2e43e507ffcefac3165ed5ff864300a738602da Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 23 Oct 2019 04:09:00 +0200 Subject: [PATCH 5/8] fix typing --- src/_pytest/capture.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index b168722d038..0570e29e76f 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -80,7 +80,7 @@ def __init__(self, method): self._method = method self._global_capturing = None self._current_item = None - self._atexit_funcs: List[Callable] = [] + self._atexit_funcs = [] # type: List[Callable] atexit.register(self._atexit_run) self._tmpfiles = {} From 96be106fdbc3ff341b10978e44658ea7288259d3 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 23 Oct 2019 04:10:32 +0200 Subject: [PATCH 6/8] use Optional Ref: https://github.com/pytest-dev/pytest/pull/6036 --- src/_pytest/capture.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 0570e29e76f..38a25ac3f8a 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -12,6 +12,7 @@ from tempfile import TemporaryFile from typing import Callable from typing import List +from typing import Optional import pytest from _pytest.compat import CaptureIO @@ -464,7 +465,12 @@ class MultiCapture: _state = None def __init__( - self, out=True, err=True, in_=True, Capture=None, capman: CaptureManager = None + self, + out=True, + err=True, + in_=True, + Capture=None, + capman: Optional[CaptureManager] = None, ): if in_: self.in_ = Capture(0, capman=capman) @@ -554,7 +560,7 @@ class FDCaptureBinary: EMPTY_BUFFER = b"" _state = None - def __init__(self, targetfd, tmpfile=None, capman: CaptureManager = None): + def __init__(self, targetfd, tmpfile=None, capman: Optional[CaptureManager] = None): self.targetfd = targetfd self._capman = capman try: @@ -667,7 +673,7 @@ class SysCapture: EMPTY_BUFFER = str() _state = None - def __init__(self, fd, tmpfile=None, capman: CaptureManager = None): + def __init__(self, fd, tmpfile=None, capman: Optional[CaptureManager] = None): name = patchsysdict[fd] self._capman = capman self._old = getattr(sys, name) From 1ca22832a1f00b3796abda92361980403c8da572 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 28 Oct 2019 13:03:26 +0100 Subject: [PATCH 7/8] re-use tmpfile for devnull --- src/_pytest/capture.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 38a25ac3f8a..979eae660ff 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -573,7 +573,14 @@ def __init__(self, targetfd, tmpfile=None, capman: Optional[CaptureManager] = No self.done = self._done if targetfd == 0: assert not tmpfile, "cannot set tmpfile with stdin" - tmpfile = open(os.devnull, "r") + if capman: + try: + tmpfile = capman._tmpfiles[0] + assert not tmpfile.closed + except KeyError: + tmpfile = open(os.devnull, "r") + else: + tmpfile = open(os.devnull, "r") self.syscapture = SysCapture(targetfd) else: if tmpfile is None: From 06f2154caccf12721c827c38f6eb4361fb7f74a4 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 28 Oct 2019 14:28:42 +0100 Subject: [PATCH 8/8] fixup! re-use tmpfile for devnull --- src/_pytest/capture.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 979eae660ff..63bbea46556 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -579,6 +579,7 @@ def __init__(self, targetfd, tmpfile=None, capman: Optional[CaptureManager] = No assert not tmpfile.closed except KeyError: tmpfile = open(os.devnull, "r") + capman._tmpfiles[targetfd] = tmpfile else: tmpfile = open(os.devnull, "r") self.syscapture = SysCapture(targetfd)