Skip to content

Conversation

lesteve
Copy link
Contributor

@lesteve lesteve commented Jan 10, 2024

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 from conftestpath and mod.__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:

AssertionError: str(conftestpath)='C:\\msys64\\home\\lesteve\\dev\\pytest-issue-9765-reproducer\\project\\conftest.py'
has already been loaded as the module mod=<module 'project.conftest' from 'c:\\msys64\\home\\lesteve\\dev\\pytest-issue
-9765-reproducer\\project\\conftest.py'>

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__ and str(contestpath) is likely not how this problem will show up. It could even lead to red-herring, since it will be potentially fine that mod.__file__ and str(conftestpath) are not the consistent and people may be misled by reading comments like #9765 (comment) ...

Copy link
Member

@nicoddemus nicoddemus left a 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!

Copy link
Member

@bluetech bluetech left a 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.

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=}"
Copy link
Member

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.

Copy link
Member

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]
@bluetech bluetech force-pushed the improve-assert-mod-not-in-mods-error-message branch from be9d154 to 1c9d683 Compare January 14, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants