From d406786a8d935cddd9c8b1388535200205527c20 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 18 Mar 2019 22:58:22 +0100 Subject: [PATCH 1/7] pdb: handle capturing with fixtures only --- src/_pytest/capture.py | 18 +++++++++++++++ src/_pytest/debugging.py | 31 ++++++++++++++++++++----- testing/test_pdb.py | 49 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 6 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index c140757c4..3f6055bd8 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -107,6 +107,16 @@ class CaptureManager(object): return MultiCapture(out=False, err=False, in_=False) raise ValueError("unknown capturing method: %r" % method) # pragma: no cover + def is_capturing(self): + if self.is_globally_capturing(): + return "global" + capture_fixture = getattr(self._current_item, "_capture_fixture", None) + if capture_fixture is not None: + return ( + "fixture %s" % self._current_item._capture_fixture.request.fixturename + ) + return False + # Global capturing control def is_globally_capturing(self): @@ -134,6 +144,14 @@ class CaptureManager(object): if cap is not None: cap.suspend_capturing(in_=in_) + def suspend(self, in_=False): + self.suspend_fixture(self._current_item) + self.suspend_global_capture(in_) + + def resume(self): + self.resume_global_capture() + self.resume_fixture(self._current_item) + def read_global_capture(self): return self._global_capturing.readouterr() diff --git a/src/_pytest/debugging.py b/src/_pytest/debugging.py index 5b780d101..abe4f65b6 100644 --- a/src/_pytest/debugging.py +++ b/src/_pytest/debugging.py @@ -109,7 +109,7 @@ class pytestPDB(object): if cls._pluginmanager is not None: capman = cls._pluginmanager.getplugin("capturemanager") if capman: - capman.suspend_global_capture(in_=True) + capman.suspend(in_=True) tw = _pytest.config.create_terminal_writer(cls._config) tw.line() if cls._recursive_debug == 0: @@ -117,10 +117,22 @@ class pytestPDB(object): header = kwargs.pop("header", None) if header is not None: tw.sep(">", header) - elif capman and capman.is_globally_capturing(): - tw.sep(">", "PDB set_trace (IO-capturing turned off)") else: - tw.sep(">", "PDB set_trace") + if capman: + capturing = capman.is_capturing() + else: + capturing = False + if capturing: + if capturing == "global": + tw.sep(">", "PDB set_trace (IO-capturing turned off)") + else: + tw.sep( + ">", + "PDB set_trace (IO-capturing turned off for %s)" + % capturing, + ) + else: + tw.sep(">", "PDB set_trace") class _PdbWrapper(cls._pdb_cls, object): _pytest_capman = capman @@ -138,11 +150,18 @@ class pytestPDB(object): tw = _pytest.config.create_terminal_writer(cls._config) tw.line() if cls._recursive_debug == 0: - if self._pytest_capman.is_globally_capturing(): + capturing = self._pytest_capman.is_capturing() + if capturing == "global": tw.sep(">", "PDB continue (IO-capturing resumed)") + elif capturing: + tw.sep( + ">", + "PDB continue (IO-capturing resumed for %s)" + % capturing, + ) else: tw.sep(">", "PDB continue") - self._pytest_capman.resume_global_capture() + self._pytest_capman.resume() cls._pluginmanager.hook.pytest_leave_pdb( config=cls._config, pdb=self ) diff --git a/testing/test_pdb.py b/testing/test_pdb.py index 05cb9bd77..e6b5fecda 100644 --- a/testing/test_pdb.py +++ b/testing/test_pdb.py @@ -970,3 +970,52 @@ def test_quit_with_swallowed_SystemExit(testdir): rest = child.read().decode("utf8") assert "no tests ran" in rest TestPDB.flush(child) + + +@pytest.mark.parametrize("fixture", ("capfd", "capsys")) +def test_pdb_suspends_fixture_capturing(testdir, fixture): + """Using "-s" with pytest should suspend/resume fixture capturing.""" + p1 = testdir.makepyfile( + """ + def test_inner({fixture}): + import sys + + print("out_inner_before") + sys.stderr.write("err_inner_before\\n") + + __import__("pdb").set_trace() + + print("out_inner_after") + sys.stderr.write("err_inner_after\\n") + + out, err = {fixture}.readouterr() + assert out =="out_inner_before\\nout_inner_after\\n" + assert err =="err_inner_before\\nerr_inner_after\\n" + """.format( + fixture=fixture + ) + ) + + child = testdir.spawn_pytest(str(p1) + " -s") + + child.expect("Pdb") + before = child.before.decode("utf8") + assert ( + "> PDB set_trace (IO-capturing turned off for fixture %s) >" % (fixture) + in before + ) + + # Test that capturing is really suspended. + child.sendline("p 40 + 2") + child.expect("Pdb") + assert "\r\n42\r\n" in child.before.decode("utf8") + + child.sendline("c") + rest = child.read().decode("utf8") + assert "out_inner" not in rest + assert "err_inner" not in rest + + TestPDB.flush(child) + assert child.exitstatus == 0 + assert "= 1 passed in " in rest + assert "> PDB continue (IO-capturing resumed for fixture %s) >" % (fixture) in rest From 40718efaccb701c6458f91da5aa9f6c9d58b9567 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 19 Mar 2019 00:47:22 +0100 Subject: [PATCH 2/7] Fix/revisit do_continue with regard to conditions --- src/_pytest/debugging.py | 13 ++++++++----- testing/test_pdb.py | 3 ++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/_pytest/debugging.py b/src/_pytest/debugging.py index abe4f65b6..5e859f0e3 100644 --- a/src/_pytest/debugging.py +++ b/src/_pytest/debugging.py @@ -146,22 +146,25 @@ class pytestPDB(object): def do_continue(self, arg): ret = super(_PdbWrapper, self).do_continue(arg) - if self._pytest_capman: + if cls._recursive_debug == 0: tw = _pytest.config.create_terminal_writer(cls._config) tw.line() - if cls._recursive_debug == 0: + if self._pytest_capman: capturing = self._pytest_capman.is_capturing() + else: + capturing = False + if capturing: if capturing == "global": tw.sep(">", "PDB continue (IO-capturing resumed)") - elif capturing: + else: tw.sep( ">", "PDB continue (IO-capturing resumed for %s)" % capturing, ) - else: - tw.sep(">", "PDB continue") self._pytest_capman.resume() + else: + tw.sep(">", "PDB continue") cls._pluginmanager.hook.pytest_leave_pdb( config=cls._config, pdb=self ) diff --git a/testing/test_pdb.py b/testing/test_pdb.py index e6b5fecda..6d4fd18e8 100644 --- a/testing/test_pdb.py +++ b/testing/test_pdb.py @@ -576,7 +576,8 @@ class TestPDB(object): child.sendline("c") child.expect("LEAVING RECURSIVE DEBUGGER") assert b"PDB continue" not in child.before - assert b"print_from_foo" in child.before + # No extra newline. + assert child.before.endswith(b"c\r\nprint_from_foo\r\n") child.sendline("c") child.expect(r"PDB continue \(IO-capturing resumed\)") rest = child.read().decode("utf8") From ae067df941d16a6b410164ac21a1d9f7d77e97e0 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 19 Mar 2019 01:04:15 +0100 Subject: [PATCH 3/7] add test_pdb_continue_with_recursive_debug --- testing/test_pdb.py | 51 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/testing/test_pdb.py b/testing/test_pdb.py index 6d4fd18e8..74d22e7ef 100644 --- a/testing/test_pdb.py +++ b/testing/test_pdb.py @@ -604,6 +604,57 @@ class TestPDB(object): child.expect("1 passed") self.flush(child) + @pytest.mark.parametrize("capture", (True, False)) + def test_pdb_continue_with_recursive_debug(self, capture, testdir): + """Full coverage for do_debug without capturing. + + This is very similar to test_pdb_interaction_continue_recursive, but + simpler, and providing more coverage. + """ + p1 = testdir.makepyfile( + """ + def set_trace(): + __import__('pdb').set_trace() + + def test_1(): + set_trace() + """ + ) + if capture: + child = testdir.spawn_pytest("%s" % p1) + else: + child = testdir.spawn_pytest("-s %s" % p1) + child.expect("Pdb") + before = child.before.decode("utf8") + if capture: + assert ">>> PDB set_trace (IO-capturing turned off) >>>" in before + else: + assert ">>> PDB set_trace >>>" in before + child.sendline("debug set_trace()") + child.expect(r"\(Pdb.*") + before = child.before.decode("utf8") + assert "\r\nENTERING RECURSIVE DEBUGGER\r\n" in before + child.sendline("c") + child.expect(r"\(Pdb.*") + + # No continue message with recursive debugging. + before = child.before.decode("utf8") + assert ">>> PDB continue " not in before + # No extra newline. + assert before.startswith("c\r\n\r\n--Return--") + + child.sendline("c") + child.expect("Pdb") + before = child.before.decode("utf8") + assert "\r\nLEAVING RECURSIVE DEBUGGER\r\n" in before + child.sendline("c") + rest = child.read().decode("utf8") + if capture: + assert "> PDB continue (IO-capturing resumed) >" in rest + else: + assert "> PDB continue >" in rest + assert "1 passed in" in rest + def test_pdb_used_outside_test(self, testdir): p1 = testdir.makepyfile( """ From 951213ee0937c3fef7ce29052fcc2004866e0ffa Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 19 Mar 2019 01:27:59 +0100 Subject: [PATCH 4/7] Use new suspend/resume in global_and_fixture_disabled --- src/_pytest/capture.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/_pytest/capture.py b/src/_pytest/capture.py index 3f6055bd8..0e8a693e8 100644 --- a/src/_pytest/capture.py +++ b/src/_pytest/capture.py @@ -145,6 +145,7 @@ class CaptureManager(object): cap.suspend_capturing(in_=in_) def suspend(self, in_=False): + # Need to undo local capsys-et-al if it exists before disabling global capture. self.suspend_fixture(self._current_item) self.suspend_global_capture(in_) @@ -186,14 +187,11 @@ class CaptureManager(object): @contextlib.contextmanager def global_and_fixture_disabled(self): """Context manager to temporarily disable global and current fixture capturing.""" - # Need to undo local capsys-et-al if it exists before disabling global capture. - self.suspend_fixture(self._current_item) - self.suspend_global_capture(in_=False) + self.suspend() try: yield finally: - self.resume_global_capture() - self.resume_fixture(self._current_item) + self.resume() @contextlib.contextmanager def item_capture(self, when, item): From d53209956b4cd9345be5576d5472760b8e6de145 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 19 Mar 2019 04:10:29 +0100 Subject: [PATCH 5/7] test_pdb_continue_with_recursive_debug: mock pdb.set_trace --- testing/test_pdb.py | 75 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 58 insertions(+), 17 deletions(-) diff --git a/testing/test_pdb.py b/testing/test_pdb.py index 74d22e7ef..d5cf17ef9 100644 --- a/testing/test_pdb.py +++ b/testing/test_pdb.py @@ -604,52 +604,93 @@ class TestPDB(object): child.expect("1 passed") self.flush(child) - @pytest.mark.parametrize("capture", (True, False)) - def test_pdb_continue_with_recursive_debug(self, capture, testdir): + @pytest.mark.parametrize("capture_arg", ("", "-s", "-p no:capture")) + def test_pdb_continue_with_recursive_debug(self, capture_arg, testdir): """Full coverage for do_debug without capturing. - This is very similar to test_pdb_interaction_continue_recursive, but - simpler, and providing more coverage. + This is very similar to test_pdb_interaction_continue_recursive in general, + but mocks out ``pdb.set_trace`` for providing more coverage. """ p1 = testdir.makepyfile( """ + try: + input = raw_input + except NameError: + pass + def set_trace(): __import__('pdb').set_trace() - def test_1(): + def test_1(monkeypatch): + import _pytest.debugging + + class pytestPDBTest(_pytest.debugging.pytestPDB): + @classmethod + def set_trace(cls, *args, **kwargs): + # Init _PdbWrapper to handle capturing. + _pdb = cls._init_pdb(*args, **kwargs) + + # Mock out pdb.Pdb.do_continue. + import pdb + pdb.Pdb.do_continue = lambda self, arg: None + + print("=== SET_TRACE ===") + assert input() == "debug set_trace()" + + # Simulate _PdbWrapper.do_debug + cls._recursive_debug += 1 + print("ENTERING RECURSIVE DEBUGGER") + print("=== SET_TRACE_2 ===") + + assert input() == "c" + _pdb.do_continue("") + print("=== SET_TRACE_3 ===") + + # Simulate _PdbWrapper.do_debug + print("LEAVING RECURSIVE DEBUGGER") + cls._recursive_debug -= 1 + + print("=== SET_TRACE_4 ===") + assert input() == "c" + _pdb.do_continue("") + + def do_continue(self, arg): + print("=== do_continue") + # _PdbWrapper.do_continue("") + + monkeypatch.setattr(_pytest.debugging, "pytestPDB", pytestPDBTest) + + import pdb + monkeypatch.setattr(pdb, "set_trace", pytestPDBTest.set_trace) + set_trace() """ ) - if capture: - child = testdir.spawn_pytest("%s" % p1) - else: - child = testdir.spawn_pytest("-s %s" % p1) - child.expect("Pdb") + child = testdir.spawn_pytest("%s %s" % (p1, capture_arg)) + child.expect("=== SET_TRACE ===") before = child.before.decode("utf8") - if capture: + if not capture_arg: assert ">>> PDB set_trace (IO-capturing turned off) >>>" in before else: assert ">>> PDB set_trace >>>" in before child.sendline("debug set_trace()") - child.expect(r"\(Pdb.*") + child.expect("=== SET_TRACE_2 ===") before = child.before.decode("utf8") assert "\r\nENTERING RECURSIVE DEBUGGER\r\n" in before child.sendline("c") - child.expect(r"\(Pdb.*") + child.expect("=== SET_TRACE_3 ===") # No continue message with recursive debugging. before = child.before.decode("utf8") assert ">>> PDB continue " not in before - # No extra newline. - assert before.startswith("c\r\n\r\n--Return--") child.sendline("c") - child.expect("Pdb") + child.expect("=== SET_TRACE_4 ===") before = child.before.decode("utf8") assert "\r\nLEAVING RECURSIVE DEBUGGER\r\n" in before child.sendline("c") rest = child.read().decode("utf8") - if capture: + if not capture_arg: assert "> PDB continue (IO-capturing resumed) >" in rest else: assert "> PDB continue >" in rest From 63a01bdb3308e36bdca7b4b006358af6f19e640f Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 20 Mar 2019 03:39:13 +0100 Subject: [PATCH 6/7] Factor out pytestPDB._is_capturing --- src/_pytest/debugging.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/_pytest/debugging.py b/src/_pytest/debugging.py index 5e859f0e3..bb90d00ca 100644 --- a/src/_pytest/debugging.py +++ b/src/_pytest/debugging.py @@ -101,6 +101,12 @@ class pytestPDB(object): _saved = [] _recursive_debug = 0 + @classmethod + def _is_capturing(cls, capman): + if capman: + return capman.is_capturing() + return False + @classmethod def _init_pdb(cls, *args, **kwargs): """ Initialize PDB debugging, dropping any IO capturing. """ @@ -118,10 +124,7 @@ class pytestPDB(object): if header is not None: tw.sep(">", header) else: - if capman: - capturing = capman.is_capturing() - else: - capturing = False + capturing = cls._is_capturing(capman) if capturing: if capturing == "global": tw.sep(">", "PDB set_trace (IO-capturing turned off)") @@ -149,10 +152,9 @@ class pytestPDB(object): if cls._recursive_debug == 0: tw = _pytest.config.create_terminal_writer(cls._config) tw.line() - if self._pytest_capman: - capturing = self._pytest_capman.is_capturing() - else: - capturing = False + + capman = self._pytest_capman + capturing = pytestPDB._is_capturing(capman) if capturing: if capturing == "global": tw.sep(">", "PDB continue (IO-capturing resumed)") @@ -162,7 +164,7 @@ class pytestPDB(object): "PDB continue (IO-capturing resumed for %s)" % capturing, ) - self._pytest_capman.resume() + capman.resume() else: tw.sep(">", "PDB continue") cls._pluginmanager.hook.pytest_leave_pdb( From 46d9243eb076df31495d6c0399142310b4b50dba Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 28 Mar 2019 11:56:53 +0100 Subject: [PATCH 7/7] changelog --- changelog/4951.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/4951.feature.rst diff --git a/changelog/4951.feature.rst b/changelog/4951.feature.rst new file mode 100644 index 000000000..b40e03af5 --- /dev/null +++ b/changelog/4951.feature.rst @@ -0,0 +1 @@ +Output capturing is handled correctly when only capturing via fixtures (capsys, capfs) with ``pdb.set_trace()``.