-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Improve assert mod not in mods error message #11795
Improve assert mod not in mods error message #11795
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed having a better error message is something we should have for all internal assert
calls, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this would have been useful with the issue.
src/_pytest/config/__init__.py
Outdated
@@ -656,7 +656,8 @@ def _importconftest( | |||
if dirpath in self._dirpath2confmods: | |||
for path, mods in self._dirpath2confmods.items(): | |||
if dirpath in path.parents or path == dirpath: | |||
assert mod not in mods | |||
error_message = f"{str(conftestpath)=} has already been loaded as the module {mod=}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think it will be useful to include the __file__
as a debugging aid - it may no longer be at fault but it's useful to see where mod
came from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made this update
[ran: tweaked message, made the formatting lazy]
be9d154
to
1c9d683
Compare
Related to #9765, add a better error message than
assert mod not in mods
. This would have been super useful to figure out from the error message the difference fromconftestpath
andmod.__file__
. Hopefully the #11708 will be merged and this error is less likely to occur, but at least if a variation of the issue occurs again there will be a better error message.Here is the error on my reproducer https://github.com/lesteve/pytest-issue-9765-reproducer:
For now I keep an
AssertionError
but let me know if you think another kind of error is more appropriate.It does not seem that easy to add a test for this ...
Let me know if you think this is worth adding a changelog for this or not!
Depending on how #11708 goes, this may need to be tweaked since a discrepancy between
mod.__file__
andstr(contestpath)
is likely not how this problem will show up. It could even lead to red-herring, since it will be potentially fine thatmod.__file__
andstr(conftestpath)
are not the consistent and people may be misled by reading comments like #9765 (comment) ...