Skip to content
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

[bug] pytest_collect_file gets called multiple times #6088

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Oct 28, 2019

testing/test_collection.py::test_collect_init_tests triggers the assert.

Unfortunately the whole Package feature was never properly done (this is not the first time I am running into issues with it).

testing/test_collection.py::test_collect_init_tests triggers the assert.
@blueyed blueyed added topic: collection related to the collection phase type: bug problem that needs to be addressed labels Oct 28, 2019
@RonnyPfannschmidt
Copy link
Member

@blueyed the package feature has been a hack since a while,

to correctly sort it out we need to reintroduce folders in the collection tree (which is also needed for aligning package scope fixtures with their actual packages)

its something im working towards via the node detanglement ideas (#5975 and others help)

@@ -575,6 +577,10 @@ def _collect(self, arg):
yield from m

def _collectfile(self, path, handle_dupes=True):
key = (path, handle_dupes)
assert key not in self._collected, key
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 not familiar with the code, how does this interact with handle_dupes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

handle_duples is also a hack on top (IIRC even by me already). I just added it here. The code is not meant to stay.
_collectfile could use a cache/lookup to work around this - the main issue is that hooks shouldn't really be called multiple times. I've only noticed this when debugging some --lf stuff.

Copy link
Member

Choose a reason for hiding this comment

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

#4319 . i actually merged it

i believe this will b better if we shift packages around so they are parents of modules and not alongside of modules
(then we can also attach conftests to packages/blank packages and finally get rid of the nodeid string matching hell)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good!

@blueyed blueyed closed this Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: collection related to the collection phase type: bug problem that needs to be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants