Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/5228.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix ``pdb.set_trace`` wrapper when used in child threads after main thread exited.
11 changes: 2 additions & 9 deletions src/_pytest/debugging.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ def pytest_configure(config):
if config.getvalue("usepdb"):
config.pluginmanager.register(PdbInvoke(), "pdbinvoke")

pytestPDB._saved.append(
(pdb.set_trace, pytestPDB._pluginmanager, pytestPDB._config, pytestPDB._pdb_cls)
)
pytestPDB._saved.append(pdb.set_trace)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little worried about this because this leaves config and pluginmanager alive after pytest.main(), which might be a problem for someone embedding pytest I guess.

I wonder if we can only pass what is is actually needed to pytestPDB instead of the entire config and pluginmanager objects?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the main point here is to keep them alive after pytest exits, isn't it?.. ;)

(The whole wrapping can be improved in general - I'm doing this basically step by step when running into issues)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the main point here is to keep them alive after pytest exits, isn't it?.. ;)

Yep, and I'm not sure that's a good thing 😅, specially if I'm not using threads at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative would be to fix the debugging plugin to not crash in this case I think.

pdb.set_trace = pytestPDB.set_trace
pytestPDB._pluginmanager = config.pluginmanager
pytestPDB._config = config
Expand All @@ -88,12 +86,7 @@ def pytest_configure(config):
# NOTE: not using pytest_unconfigure, since it might get called although
# pytest_configure was not (if another plugin raises UsageError).
def fin():
(
pdb.set_trace,
pytestPDB._pluginmanager,
pytestPDB._config,
pytestPDB._pdb_cls,
) = pytestPDB._saved.pop()
pdb.set_trace = pytestPDB._saved.pop()

config._cleanup.append(fin)

Expand Down
67 changes: 60 additions & 7 deletions testing/test_pdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -1027,19 +1027,25 @@ def test_trace_after_runpytest(testdir):
from _pytest.debugging import pytestPDB

def test_outer(testdir):
from _pytest.debugging import pytestPDB

assert len(pytestPDB._saved) == 1

testdir.runpytest("-k test_inner")
testdir.makepyfile(
\"""
from _pytest.debugging import pytestPDB

__import__('pdb').set_trace()
def test_inner():
assert len(pytestPDB._saved) == 2
print("test_inner_" + "end")
\"""
)

def test_inner(testdir):
assert len(pytestPDB._saved) == 2
result = testdir.runpytest("-s", "-k", "test_inner")
assert result.ret == 0
__import__('pdb').set_trace()
"""
)
child = testdir.spawn_pytest("-p pytester %s -k test_outer" % p1)
child = testdir.spawn_pytest("-s -p pytester %s -k test_outer" % p1)
child.expect("test_inner_end")
child.expect(r"\(Pdb")
child.sendline("c")
rest = child.read().decode("utf8")
Expand Down Expand Up @@ -1191,3 +1197,50 @@ def test(monkeypatch):
result = testdir.runpytest(str(p1))
result.stdout.fnmatch_lines(["E *BdbQuit", "*= 1 failed in*"])
assert result.ret == 1


def test_pdb_in_thread_after_exit(testdir):
"""Ensure that pdb.set_trace works after main thread exited already.

This tests both continuation after the main thread exited, and a new
set_trace afterwards.
"""
p1 = testdir.makepyfile(
"""
import threading

main_thread = threading.current_thread()
evt = threading.Event()
evt2 = threading.Event()

def test():

def target():
print("target_" + "start")
evt.set()
assert main_thread.is_alive()
__import__('pdb').set_trace()
assert not main_thread.is_alive()
__import__('pdb').set_trace()
print("target_" + "end")

thread = threading.Thread(target=target)
thread.start()

evt.wait()
evt2.wait()
"""
)
child = testdir.spawn_pytest(str(p1) + " -s")
child.expect("target_start")
child.expect(r"\(Pdb")
child.sendline("evt2.set()")
child.expect("= 1 passed in") # main thread exited
child.sendline("c")
child.expect(r"\(Pdb")
child.sendline("c")
child.expect("target_end")
child.wait()
rest = child.read().decode("utf8")
assert "Exception in thread" not in rest
assert child.exitstatus == 0