#3332 improve traceback for import error#3345
Closed
feuillemorte wants to merge 6 commits intopytest-dev:featuresfrom
feuillemorte:3332-improve-traceback-for-import-error
Closed
#3332 improve traceback for import error#3345feuillemorte wants to merge 6 commits intopytest-dev:featuresfrom feuillemorte:3332-improve-traceback-for-import-error
feuillemorte wants to merge 6 commits intopytest-dev:featuresfrom
feuillemorte:3332-improve-traceback-for-import-error
Conversation
_pytest/config.py
Outdated
| tw.line("ERROR: could not load %s\n" % (e.path,), red=True) | ||
| formatted_tb = safe_str(e) | ||
| tw.line( | ||
| "ImportError while importing test module '{path}'.\n" |
Member
There was a problem hiding this comment.
"test module" sounds a bit confusing here, I think "conftest module" would fit better here.
feuillemorte
commented
Mar 27, 2018
| formatted = traceback.format_tb(etb) | ||
| # The level of the tracebacks we want to print is hand crafted :( | ||
| return repr(evalue) + '\n' + ''.join(formatted[2:]) | ||
| return ''.join(formatted[2:]) + '\nE ' + etype.__name__ + ': ' + str(evalue) |
Contributor
Author
There was a problem hiding this comment.
What do you think about this line? Is it good? Now it looks like
E ModuleNotFoundError: No module named 'wrong_import'
Member
|
Hi @feuillemorte, thanks for another PR! Your change fixes the problem for root # content of root/sub/conftest.py
import invalid_module
# content of root/sub/test_foo.py
def test(): passWhen ran from the I hacked the code a bit and produced the following patch: diff --git a/_pytest/config.py b/_pytest/config.py
index 7a4622a..5927435 100644
--- a/_pytest/config.py
+++ b/_pytest/config.py
@@ -209,6 +209,7 @@ class PytestPluginManager(PluginManager):
self.rewrite_hook = _pytest.assertion.DummyRewriteHook()
# Used to know when we are importing conftests after the pytest_configure stage
self._configured = False
+ self._config = None
def addhooks(self, module_or_class):
"""
@@ -285,6 +286,7 @@ class PytestPluginManager(PluginManager):
"trylast: mark a hook implementation function such that the "
"plugin machinery will try to call it last/as late as possible.")
self._configured = True
+ self._config = config
def _warn(self, message):
kwargs = message if isinstance(message, dict) else {
@@ -351,7 +353,18 @@ class PytestPluginManager(PluginManager):
continue
conftestpath = parent.join("conftest.py")
if conftestpath.isfile():
- mod = self._importconftest(conftestpath)
+ try:
+ mod = self._importconftest(conftestpath)
+ except ConftestImportFailure as e:
+ from _pytest.nodes import Collector
+ from _pytest._code.code import ExceptionInfo
+ from _pytest.python import filter_traceback
+ exc_info = ExceptionInfo(e.excinfo)
+ if self._config.getoption('verbose') < 2:
+ exc_info.traceback = exc_info.traceback.filter(filter_traceback)
+ exc_repr = exc_info.getrepr(style='short') if exc_info.traceback else exc_info.exconly()
+ formatted_tb = safe_str(exc_repr)
+ raise Collector.CollectError(formatted_tb)
clist.append(mod)
self._path2confmods[path] = clist
This produces a better output: But I think it can still be improved, we need to investigate a bit more how to get rid of the chained exception traceback entries. Please note that my handling in the Lines 427 to 439 in ff3d13e |
Contributor
Author
|
@nicoddemus what is ret? What means '3'? Traceback is very huge in this test: if I only I can use But in this case we have 2 tracebacks: It's also huge |
Member
|
Hi @feuillemorte, I'm just passing by so I don't have much time to look deeper into the code, but to answer your question Lines 21 to 27 in ad0b433 I'm surprised that Is this code in a branch? I will take a look as soon as I have some time, although this week is turning busier than I expected. 😅 |
Contributor
Author
|
@nicoddemus I pushed some code, but some tests will be failed. |
Contributor
Author
|
Do we really need exc_info = ExceptionInfo()
if config and config.getoption('verbose') < 2:
exc_info.traceback = exc_info.traceback.filter(filter_traceback)
exc_repr = exc_info.getrepr(style='short') if exc_info.traceback else exc_info.exconly()
formatted_tb = safe_str(exc_repr)If we remove it and write smth like def print_short_traceback(error):
from _pytest.nodes import Collector
formatted_tb = safe_str(error)
sys.tracebacklimit = 0
raise Collector.CollectError(
"ImportError while importing test module '{fspath}'.\n"
"Hint: make sure your test modules/packages have valid Python names.\n"
"Traceback:\n"
"{traceback}".format(fspath=error.path, traceback=formatted_tb)
) from NoneWe get beautiful traceback :) Traceback (most recent call last):
_pytest.nodes.Collector.CollectError: ImportError while importing test module '/tmp/pytest-of-feuillemorte/pytest-63/test_issue109_sibling_conftests_not_loaded0/sub1/conftest.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
File "<frozen importlib._bootstrap>", line 971, in _find_and_load
File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 656, in _load_unlocked
File "<frozen importlib._bootstrap>", line 626, in _load_backward_compatible
File "/home/feuillemorte/github/pytest-dev/pytest/pytest_3332/_pytest/assertion/rewrite.py", line 213, in load_module
py.builtin.exec_(co, mod.__dict__)
File "/tmp/pytest-of-feuillemorte/pytest-63/test_issue109_sibling_conftests_not_loaded0/sub1/conftest.py", line 1, in <module>
assert 0
E AssertionError: assert 0 |
Member
I kinda disagree, I would prefer a traceback which shows only the error in the Also it is not a good idea to change Btw sorry I didn't have time to take a look at your PR yet. Thanks for the patience. 😁 |
Member
|
@feuillemorte thanks a lot for all the work here, but I believe this will be supplanted by #4077! 👍 🙇 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3332
Output: