From 7d9b198f734f7d1968c88476545da29fb90c1040 Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 19 Aug 2018 02:32:36 +0200 Subject: [PATCH 1/8] Refactoring: Separated suspend from snapping (stopped always snapping when suspending - solves bug but still missing tests), reorganized functions and context managers. --- src/_pytest/capture.py | 135 +++++++++++++++++++++++------------------ 1 file changed, 77 insertions(+), 58 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index c84ba825e..f10a13a1f 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -62,8 +62,9 @@ def pytest_load_initial_conftests(early_config, parser, args): # finally trigger conftest loading but while capturing (issue93) capman.start_global_capturing() outcome = yield - out, err = capman.suspend_global_capture() + capman.suspend_global_capture() if outcome.excinfo is not None: + out, err = capman.snap_global_capture() sys.stdout.write(out) sys.stderr.write(err) @@ -96,6 +97,8 @@ class CaptureManager(object): else: raise ValueError("unknown capturing method: %r" % method) + # Global capturing control + def start_global_capturing(self): assert self._global_capturing is None self._global_capturing = self._getcapture(self._method) @@ -110,29 +113,15 @@ class CaptureManager(object): def resume_global_capture(self): self._global_capturing.resume_capturing() - def suspend_global_capture(self, item=None, in_=False): - if item is not None: - self.deactivate_fixture(item) + def suspend_global_capture(self, in_=False): cap = getattr(self, "_global_capturing", None) if cap is not None: - try: - outerr = cap.readouterr() - finally: - cap.suspend_capturing(in_=in_) - return outerr + cap.suspend_capturing(in_=in_) - @contextlib.contextmanager - def global_and_fixture_disabled(self): - """Context manager to temporarily disables global and current fixture capturing.""" - # Need to undo local capsys-et-al if exists before disabling global capture - fixture = getattr(self._current_item, "_capture_fixture", None) - ctx_manager = fixture._suspend() if fixture else dummy_context_manager() - with ctx_manager: - self.suspend_global_capture(item=None, in_=False) - try: - yield - finally: - self.resume_global_capture() + def snap_global_capture(self): + return self._global_capturing.readouterr() + + # Fixture Control (its just forwarding, think about removing this later) def activate_fixture(self, item): """If the current item is using ``capsys`` or ``capfd``, activate them so they take precedence over @@ -148,12 +137,53 @@ class CaptureManager(object): if fixture is not None: fixture.close() + def suspend_fixture(self, item): + fixture = getattr(item, "_capture_fixture", None) + if fixture is not None: + fixture._suspend() + + def resume_fixture(self, item): + fixture = getattr(item, "_capture_fixture", None) + if fixture is not None: + fixture._resume() + + # Helper context managers + + @contextlib.contextmanager + def global_and_fixture_disabled(self): + """Context manager to temporarily disables global and current fixture capturing.""" + # Need to undo local capsys-et-al if exists before disabling global capture + self.suspend_fixture(self._current_item) + self.suspend_global_capture(in_=False) + try: + yield + finally: + self.resume_global_capture() + self.resume_fixture(self._current_item) + + @contextlib.contextmanager + def item_capture(self, when, item): + self.resume_global_capture() + self.activate_fixture(item) + try: + yield + finally: + self.deactivate_fixture(item) + self.suspend_global_capture(in_=False) + + out, err = self.snap_global_capture() + item.add_report_section(when, "stdout", out) + item.add_report_section(when, "stderr", err) + + # Hooks + @pytest.hookimpl(hookwrapper=True) def pytest_make_collect_report(self, collector): if isinstance(collector, pytest.File): self.resume_global_capture() outcome = yield - out, err = self.suspend_global_capture() + self.suspend_global_capture() + out, err = self.snap_global_capture() rep = outcome.get_result() if out: rep.sections.append(("Captured stdout", out)) @@ -163,35 +193,27 @@ class CaptureManager(object): yield @pytest.hookimpl(hookwrapper=True) - def pytest_runtest_setup(self, item): + def pytest_runtest_logstart(self, item): self._current_item = item - self.resume_global_capture() - # no need to activate a capture fixture because they activate themselves during creation; this - # only makes sense when a fixture uses a capture fixture, otherwise the capture fixture will - # be activated during pytest_runtest_call - yield - self.suspend_capture_item(item, "setup") - self._current_item = None + + @pytest.hookimpl(hookwrapper=True) + def pytest_runtest_logfinish(self, item): + self._current_item = item + + @pytest.hookimpl(hookwrapper=True) + def pytest_runtest_setup(self, item): + with self.item_capture("setup", item): + yield @pytest.hookimpl(hookwrapper=True) def pytest_runtest_call(self, item): - self._current_item = item - self.resume_global_capture() - # it is important to activate this fixture during the call phase so it overwrites the "global" - # capture - self.activate_fixture(item) - yield - self.suspend_capture_item(item, "call") - self._current_item = None + with self.item_capture("call", item): + yield @pytest.hookimpl(hookwrapper=True) def pytest_runtest_teardown(self, item): - self._current_item = item - self.resume_global_capture() - self.activate_fixture(item) - yield - self.suspend_capture_item(item, "teardown") - self._current_item = None + with self.item_capture("teardown", item): + yield @pytest.hookimpl(tryfirst=True) def pytest_keyboard_interrupt(self, excinfo): @@ -201,11 +223,6 @@ class CaptureManager(object): def pytest_internalerror(self, excinfo): self.stop_global_capturing() - def suspend_capture_item(self, item, when, in_=False): - out, err = self.suspend_global_capture(item, in_=in_) - item.add_report_section(when, "stdout", out) - item.add_report_section(when, "stderr", err) - capture_fixtures = {"capfd", "capfdbinary", "capsys", "capsysbinary"} @@ -311,10 +328,12 @@ class CaptureFixture(object): self.request = request def _start(self): - self._capture = MultiCapture( - out=True, err=True, in_=False, Capture=self.captureclass - ) - self._capture.start_capturing() + # Start if not started yet + if getattr(self, "_capture", None) is not None: + self._capture = MultiCapture( + out=True, err=True, in_=False, Capture=self.captureclass + ) + self._capture.start_capturing() def close(self): cap = self.__dict__.pop("_capture", None) @@ -332,14 +351,13 @@ class CaptureFixture(object): except AttributeError: return self._outerr - @contextlib.contextmanager def _suspend(self): """Suspends this fixture's own capturing temporarily.""" self._capture.suspend_capturing() - try: - yield - finally: - self._capture.resume_capturing() + + def _resume(self): + """Resumes this fixture's own capturing temporarily.""" + self._capture.resume_capturing() @contextlib.contextmanager def disabled(self): @@ -743,3 +761,4 @@ def _attempt_to_close_capture_file(f): pass else: f.close() + From 2255892d65810cc4db2b82e2621ed313a0c47f7f Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 19 Aug 2018 13:44:12 +0200 Subject: [PATCH 2/8] Improved test to cover more cases. --- testing/test_capture.py | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/testing/test_capture.py b/testing/test_capture.py index 782971af0..e47689c9c 100644 --- a/testing/test_capture.py +++ b/testing/test_capture.py @@ -1387,17 +1387,46 @@ def test_pickling_and_unpickling_encoded_file(): pickle.loads(ef_as_str) -def test_capsys_with_cli_logging(testdir): +def test_capture_with_live_logging(testdir): # Issue 3819 - # capsys should work with real-time cli logging + # capture should work with live cli logging + + # Teardown report seems to have the capture for the whole process (setup, capture, teardown) + testdir.makeconftest(""" + def pytest_runtest_logreport(report): + if "test_global" in report.nodeid: + if report.when == "teardown": + assert "fix setup" in report.caplog + assert "something in test" in report.caplog + assert "fix teardown" in report.caplog + + assert "fix setup" in report.capstdout + assert "begin test" in report.capstdout + assert "end test" in report.capstdout + assert "fix teardown" in report.capstdout + """) + testdir.makepyfile( """ import logging import sys logger = logging.getLogger(__name__) + + @pytest.fixture + def fix1(): + print("fix setup") + logging("fix setup") + yield + logging("fix teardown") + print("fix teardown") + + def test_global(): + print("begin test") + logging.info("something in test") + print("end test") - def test_myoutput(capsys): # or use "capfd" for fd-level + def test_capsys(capsys): # or use "capfd" for fd-level print("hello") sys.stderr.write("world\\n") captured = capsys.readouterr() From 9e382e8d29f0563925eedb3dc5139aaee72a1e1c Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 19 Aug 2018 14:29:57 +0200 Subject: [PATCH 3/8] Fixed test. --- testing/test_capture.py | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/testing/test_capture.py b/testing/test_capture.py index e47689c9c..b2185822a 100644 --- a/testing/test_capture.py +++ b/testing/test_capture.py @@ -1396,32 +1396,29 @@ def test_capture_with_live_logging(testdir): def pytest_runtest_logreport(report): if "test_global" in report.nodeid: if report.when == "teardown": - assert "fix setup" in report.caplog - assert "something in test" in report.caplog - assert "fix teardown" in report.caplog - - assert "fix setup" in report.capstdout - assert "begin test" in report.capstdout - assert "end test" in report.capstdout - assert "fix teardown" in report.capstdout + with open("caplog", "w") as f: + f.write(report.caplog) + with open("capstdout", "w") as f: + f.write(report.capstdout) """) testdir.makepyfile( """ import logging import sys + import pytest logger = logging.getLogger(__name__) @pytest.fixture def fix1(): print("fix setup") - logging("fix setup") + logging.info("fix setup") yield - logging("fix teardown") + logging.info("fix teardown") print("fix teardown") - - def test_global(): + + def test_global(fix1): print("begin test") logging.info("something in test") print("end test") @@ -1434,9 +1431,7 @@ def test_capture_with_live_logging(testdir): assert captured.err == "world\\n" logging.info("something") - print("next") - logging.info("something") captured = capsys.readouterr() @@ -1445,3 +1440,18 @@ def test_capture_with_live_logging(testdir): ) result = testdir.runpytest_subprocess("--log-cli-level=INFO") assert result.ret == 0 + + with open("caplog", "r") as f: + caplog = f.read() + + assert "fix setup" in caplog + assert "something in test" in caplog + assert "fix teardown" in caplog + + with open("capstdout", "r") as f: + capstdout = f.read() + + assert "fix setup" in capstdout + assert "begin test" in capstdout + assert "end test" in capstdout + assert "fix teardown" in capstdout From 8b2c91836b507727141b12b1f47c8d29ef744d1e Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 19 Aug 2018 14:30:50 +0200 Subject: [PATCH 4/8] Fixed activation and used just runtest_protocol hook --- src/_pytest/capture.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index f10a13a1f..658632e00 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -193,12 +193,10 @@ class CaptureManager(object): yield @pytest.hookimpl(hookwrapper=True) - def pytest_runtest_logstart(self, item): - self._current_item = item - - @pytest.hookimpl(hookwrapper=True) - def pytest_runtest_logfinish(self, item): + def pytest_runtest_protocol(self, item): self._current_item = item + yield + self._current_item = None @pytest.hookimpl(hookwrapper=True) def pytest_runtest_setup(self, item): @@ -329,7 +327,7 @@ class CaptureFixture(object): def _start(self): # Start if not started yet - if getattr(self, "_capture", None) is not None: + if getattr(self, "_capture", None) is None: self._capture = MultiCapture( out=True, err=True, in_=False, Capture=self.captureclass ) From 0564b52c0e1ecdce87b98f902091f3e06a01cc13 Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 19 Aug 2018 15:26:57 +0200 Subject: [PATCH 5/8] Fixed integration with other modules/tests --- src/_pytest/debugging.py | 3 ++- src/_pytest/setuponly.py | 3 ++- testing/logging/test_reporting.py | 4 ---- testing/test_capture.py | 12 ++++++++---- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/_pytest/debugging.py b/src/_pytest/debugging.py index 9991307d0..a0594e3e8 100644 --- a/src/_pytest/debugging.py +++ b/src/_pytest/debugging.py @@ -102,7 +102,8 @@ class PdbInvoke(object): def pytest_exception_interact(self, node, call, report): capman = node.config.pluginmanager.getplugin("capturemanager") if capman: - out, err = capman.suspend_global_capture(in_=True) + capman.suspend_global_capture(in_=True) + out, err = capman.snap_global_capture() sys.stdout.write(out) sys.stdout.write(err) _enter_pdb(node, call.excinfo, report) diff --git a/src/_pytest/setuponly.py b/src/_pytest/setuponly.py index 81240d9d0..721b0a942 100644 --- a/src/_pytest/setuponly.py +++ b/src/_pytest/setuponly.py @@ -51,7 +51,8 @@ def _show_fixture_action(fixturedef, msg): config = fixturedef._fixturemanager.config capman = config.pluginmanager.getplugin("capturemanager") if capman: - out, err = capman.suspend_global_capture() + capman.suspend_global_capture() + out, err = capman.snap_global_capture() tw = config.get_terminal_writer() tw.line() diff --git a/testing/logging/test_reporting.py b/testing/logging/test_reporting.py index 820295886..b8fc371d4 100644 --- a/testing/logging/test_reporting.py +++ b/testing/logging/test_reporting.py @@ -890,10 +890,6 @@ def test_live_logging_suspends_capture(has_capture_manager, request): yield self.calls.append("exit disabled") - # sanity check - assert CaptureManager.suspend_capture_item - assert CaptureManager.resume_global_capture - class DummyTerminal(six.StringIO): def section(self, *args, **kwargs): pass diff --git a/testing/test_capture.py b/testing/test_capture.py index b2185822a..626c4414a 100644 --- a/testing/test_capture.py +++ b/testing/test_capture.py @@ -70,19 +70,23 @@ class TestCaptureManager(object): try: capman = CaptureManager(method) capman.start_global_capturing() - outerr = capman.suspend_global_capture() + capman.suspend_global_capture() + outerr = capman.snap_global_capture() assert outerr == ("", "") - outerr = capman.suspend_global_capture() + capman.suspend_global_capture() + outerr = capman.snap_global_capture() assert outerr == ("", "") print("hello") - out, err = capman.suspend_global_capture() + capman.suspend_global_capture() + out, err = capman.snap_global_capture() if method == "no": assert old == (sys.stdout, sys.stderr, sys.stdin) else: assert not out capman.resume_global_capture() print("hello") - out, err = capman.suspend_global_capture() + capman.suspend_global_capture() + out, err = capman.snap_global_capture() if method != "no": assert out == "hello\n" capman.stop_global_capturing() From 7ea4992f169edf84eec10735be688523759d4543 Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 19 Aug 2018 15:46:02 +0200 Subject: [PATCH 6/8] Fixed linting. --- src/_pytest/capture.py | 3 +-- testing/logging/test_reporting.py | 1 - testing/test_capture.py | 8 +++++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 658632e00..3147d9728 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -14,7 +14,7 @@ from tempfile import TemporaryFile import six import pytest -from _pytest.compat import CaptureIO, dummy_context_manager +from _pytest.compat import CaptureIO patchsysdict = {0: "stdin", 1: "stdout", 2: "stderr"} @@ -759,4 +759,3 @@ def _attempt_to_close_capture_file(f): pass else: f.close() - diff --git a/testing/logging/test_reporting.py b/testing/logging/test_reporting.py index b8fc371d4..363982cf9 100644 --- a/testing/logging/test_reporting.py +++ b/testing/logging/test_reporting.py @@ -878,7 +878,6 @@ def test_live_logging_suspends_capture(has_capture_manager, request): import logging import contextlib from functools import partial - from _pytest.capture import CaptureManager from _pytest.logging import _LiveLoggingStreamHandler class MockCaptureManager: diff --git a/testing/test_capture.py b/testing/test_capture.py index 626c4414a..ec8c682e2 100644 --- a/testing/test_capture.py +++ b/testing/test_capture.py @@ -1396,7 +1396,8 @@ def test_capture_with_live_logging(testdir): # capture should work with live cli logging # Teardown report seems to have the capture for the whole process (setup, capture, teardown) - testdir.makeconftest(""" + testdir.makeconftest( + """ def pytest_runtest_logreport(report): if "test_global" in report.nodeid: if report.when == "teardown": @@ -1404,7 +1405,8 @@ def test_capture_with_live_logging(testdir): f.write(report.caplog) with open("capstdout", "w") as f: f.write(report.capstdout) - """) + """ + ) testdir.makepyfile( """ @@ -1413,7 +1415,7 @@ def test_capture_with_live_logging(testdir): import pytest logger = logging.getLogger(__name__) - + @pytest.fixture def fix1(): print("fix setup") From d611b035891f481570721d105d392db642412368 Mon Sep 17 00:00:00 2001 From: Victor Date: Mon, 20 Aug 2018 12:23:59 +0200 Subject: [PATCH 7/8] Parametrized tests for capfd as well. Separated global capture test. --- testing/test_capture.py | 52 ++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/testing/test_capture.py b/testing/test_capture.py index ec8c682e2..c029a21f9 100644 --- a/testing/test_capture.py +++ b/testing/test_capture.py @@ -1391,7 +1391,7 @@ def test_pickling_and_unpickling_encoded_file(): pickle.loads(ef_as_str) -def test_capture_with_live_logging(testdir): +def test_global_capture_with_live_logging(testdir): # Issue 3819 # capture should work with live cli logging @@ -1405,7 +1405,7 @@ def test_capture_with_live_logging(testdir): f.write(report.caplog) with open("capstdout", "w") as f: f.write(report.capstdout) - """ + """ ) testdir.makepyfile( @@ -1428,20 +1428,6 @@ def test_capture_with_live_logging(testdir): print("begin test") logging.info("something in test") print("end test") - - def test_capsys(capsys): # or use "capfd" for fd-level - print("hello") - sys.stderr.write("world\\n") - captured = capsys.readouterr() - assert captured.out == "hello\\n" - assert captured.err == "world\\n" - - logging.info("something") - print("next") - logging.info("something") - - captured = capsys.readouterr() - assert captured.out == "next\\n" """ ) result = testdir.runpytest_subprocess("--log-cli-level=INFO") @@ -1461,3 +1447,37 @@ def test_capture_with_live_logging(testdir): assert "begin test" in capstdout assert "end test" in capstdout assert "fix teardown" in capstdout + + +@pytest.mark.parametrize("capture_fixture", ["capsys", "capfd"]) +def test_capture_with_live_logging(testdir, capture_fixture): + # Issue 3819 + # capture should work with live cli logging + + testdir.makepyfile( + """ + import logging + import sys + + logger = logging.getLogger(__name__) + + def test_capture({0}): + print("hello") + sys.stderr.write("world\\n") + captured = {0}.readouterr() + assert captured.out == "hello\\n" + assert captured.err == "world\\n" + + logging.info("something") + print("next") + logging.info("something") + + captured = {0}.readouterr() + assert captured.out == "next\\n" + """.format( + capture_fixture + ) + ) + + result = testdir.runpytest_subprocess("--log-cli-level=INFO") + assert result.ret == 0 From 70ebab3537f52163989df6ca6832d6dbb77f41bf Mon Sep 17 00:00:00 2001 From: Victor Date: Mon, 20 Aug 2018 17:48:14 +0200 Subject: [PATCH 8/8] Renamed snap_global_capture to read_global_capture. --- src/_pytest/capture.py | 8 ++++---- src/_pytest/debugging.py | 2 +- src/_pytest/setuponly.py | 2 +- testing/test_capture.py | 8 ++++---- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 3147d9728..8e9715cd8 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -64,7 +64,7 @@ def pytest_load_initial_conftests(early_config, parser, args): outcome = yield capman.suspend_global_capture() if outcome.excinfo is not None: - out, err = capman.snap_global_capture() + out, err = capman.read_global_capture() sys.stdout.write(out) sys.stderr.write(err) @@ -118,7 +118,7 @@ class CaptureManager(object): if cap is not None: cap.suspend_capturing(in_=in_) - def snap_global_capture(self): + def read_global_capture(self): return self._global_capturing.readouterr() # Fixture Control (its just forwarding, think about removing this later) @@ -171,7 +171,7 @@ class CaptureManager(object): self.deactivate_fixture(item) self.suspend_global_capture(in_=False) - out, err = self.snap_global_capture() + out, err = self.read_global_capture() item.add_report_section(when, "stdout", out) item.add_report_section(when, "stderr", err) @@ -183,7 +183,7 @@ class CaptureManager(object): self.resume_global_capture() outcome = yield self.suspend_global_capture() - out, err = self.snap_global_capture() + out, err = self.read_global_capture() rep = outcome.get_result() if out: rep.sections.append(("Captured stdout", out)) diff --git a/src/_pytest/debugging.py b/src/_pytest/debugging.py index a0594e3e8..f51dff373 100644 --- a/src/_pytest/debugging.py +++ b/src/_pytest/debugging.py @@ -103,7 +103,7 @@ class PdbInvoke(object): capman = node.config.pluginmanager.getplugin("capturemanager") if capman: capman.suspend_global_capture(in_=True) - out, err = capman.snap_global_capture() + out, err = capman.read_global_capture() sys.stdout.write(out) sys.stdout.write(err) _enter_pdb(node, call.excinfo, report) diff --git a/src/_pytest/setuponly.py b/src/_pytest/setuponly.py index 721b0a942..c3edc5f81 100644 --- a/src/_pytest/setuponly.py +++ b/src/_pytest/setuponly.py @@ -52,7 +52,7 @@ def _show_fixture_action(fixturedef, msg): capman = config.pluginmanager.getplugin("capturemanager") if capman: capman.suspend_global_capture() - out, err = capman.snap_global_capture() + out, err = capman.read_global_capture() tw = config.get_terminal_writer() tw.line() diff --git a/testing/test_capture.py b/testing/test_capture.py index c029a21f9..fb0e14b97 100644 --- a/testing/test_capture.py +++ b/testing/test_capture.py @@ -71,14 +71,14 @@ class TestCaptureManager(object): capman = CaptureManager(method) capman.start_global_capturing() capman.suspend_global_capture() - outerr = capman.snap_global_capture() + outerr = capman.read_global_capture() assert outerr == ("", "") capman.suspend_global_capture() - outerr = capman.snap_global_capture() + outerr = capman.read_global_capture() assert outerr == ("", "") print("hello") capman.suspend_global_capture() - out, err = capman.snap_global_capture() + out, err = capman.read_global_capture() if method == "no": assert old == (sys.stdout, sys.stderr, sys.stdin) else: @@ -86,7 +86,7 @@ class TestCaptureManager(object): capman.resume_global_capture() print("hello") capman.suspend_global_capture() - out, err = capman.snap_global_capture() + out, err = capman.read_global_capture() if method != "no": assert out == "hello\n" capman.stop_global_capturing()