Skip to content

pdb: import pdbcls lazily #5307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 27, 2019
Merged
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/2064.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The debugging plugin imports the wrapped ``Pdb`` class (``--pdbcls``) on-demand now.
62 changes: 33 additions & 29 deletions src/_pytest/debugging.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,42 +49,18 @@ def pytest_addoption(parser):
)


def _import_pdbcls(modname, classname):
try:
__import__(modname)
mod = sys.modules[modname]

# Handle --pdbcls=pdb:pdb.Pdb (useful e.g. with pdbpp).
parts = classname.split(".")
pdb_cls = getattr(mod, parts[0])
for part in parts[1:]:
pdb_cls = getattr(pdb_cls, part)

return pdb_cls
except Exception as exc:
value = ":".join((modname, classname))
raise UsageError("--pdbcls: could not import {!r}: {}".format(value, exc))


def pytest_configure(config):
pdb_cls = config.getvalue("usepdb_cls")
if pdb_cls:
pdb_cls = _import_pdbcls(*pdb_cls)
else:
pdb_cls = pdb.Pdb

if config.getvalue("trace"):
config.pluginmanager.register(PdbTrace(), "pdbtrace")
if config.getvalue("usepdb"):
config.pluginmanager.register(PdbInvoke(), "pdbinvoke")

pytestPDB._saved.append(
(pdb.set_trace, pytestPDB._pluginmanager, pytestPDB._config, pytestPDB._pdb_cls)
(pdb.set_trace, pytestPDB._pluginmanager, pytestPDB._config)
)
pdb.set_trace = pytestPDB.set_trace
pytestPDB._pluginmanager = config.pluginmanager
pytestPDB._config = config
pytestPDB._pdb_cls = pdb_cls

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

config._cleanup.append(fin)
Expand All @@ -104,7 +79,6 @@ class pytestPDB(object):

_pluginmanager = None
_config = None
_pdb_cls = pdb.Pdb
_saved = []
_recursive_debug = 0

Expand All @@ -114,6 +88,33 @@ def _is_capturing(cls, capman):
return capman.is_capturing()
return False

@classmethod
def _import_pdb_cls(cls):
if not cls._config:
# Happens when using pytest.set_trace outside of a test.
return pdb.Pdb

pdb_cls = cls._config.getvalue("usepdb_cls")
if not pdb_cls:
return pdb.Pdb

modname, classname = pdb_cls

try:
__import__(modname)
mod = sys.modules[modname]

# Handle --pdbcls=pdb:pdb.Pdb (useful e.g. with pdbpp).
parts = classname.split(".")
pdb_cls = getattr(mod, parts[0])
for part in parts[1:]:
pdb_cls = getattr(pdb_cls, part)

return pdb_cls
except Exception as exc:
value = ":".join((modname, classname))
raise UsageError("--pdbcls: could not import {!r}: {}".format(value, exc))

@classmethod
def _init_pdb(cls, *args, **kwargs):
""" Initialize PDB debugging, dropping any IO capturing. """
Expand Down Expand Up @@ -144,7 +145,9 @@ def _init_pdb(cls, *args, **kwargs):
else:
tw.sep(">", "PDB set_trace")

class PytestPdbWrapper(cls._pdb_cls, object):
pdb_cls = cls._import_pdb_cls()

class PytestPdbWrapper(pdb_cls, object):
Copy link
Contributor Author

@blueyed blueyed May 26, 2019

Choose a reason for hiding this comment

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

@asottile @nicoddemus
I'd also like to move the PytestPdbWrapper class out of the method, likely into a method that gets it cached based on pdb_cls then (typically only one cache entry then).
This is good for less overhead in general, but allows for testing if the returned instance is the same as before (useful for pdbpp's global pdb handling).
This should be done before 5.0 to avoid conflicts in this area.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, sounds reasonable. I suspect this started as a small class so it was put inside the method for tersiness, but it has grown quite a lot since then.

Please do it right after this gets merged. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After #5319.

_pytest_capman = capman
_continued = False

Expand Down Expand Up @@ -227,7 +230,8 @@ def get_stack(self, f, t):
_pdb = PytestPdbWrapper(**kwargs)
cls._pluginmanager.hook.pytest_enter_pdb(config=cls._config, pdb=_pdb)
else:
_pdb = cls._pdb_cls(**kwargs)
pdb_cls = cls._import_pdb_cls()
_pdb = pdb_cls(**kwargs)
return _pdb

@classmethod
Expand Down
7 changes: 4 additions & 3 deletions testing/test_pdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -1157,12 +1157,13 @@ def runcall(self, *args, **kwds):
result = testdir.runpytest(
str(p1), "--pdbcls=really.invalid:Value", syspathinsert=True
)
result.stderr.fnmatch_lines(
result.stdout.fnmatch_lines(
[
"ERROR: --pdbcls: could not import 'really.invalid:Value': No module named *really*"
"*= FAILURES =*",
"E * --pdbcls: could not import 'really.invalid:Value': No module named *really*",
]
)
assert result.ret == 4
assert result.ret == 1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that this delays the UsageError now, which is not handled too well by pytest (e.g. via exitcode).
IIRC UsageErrors are only handled during config parsing/setup.


result = testdir.runpytest(
str(p1), "--pdbcls=mypdb:Wrapped.MyPdb", syspathinsert=True
Expand Down