Skip to content

Conversation

bluetech
Copy link
Member

PyCollector is technically private in pytest and might change/break. The DescribeBlocks act like modules, so instead treat them as ones directly.


Hi, over in pytest we are considering whether to expose PyCollector or treat it as an internal implementation detail (ref: pytest-dev/pytest#9264). My opinion is that it should be private. Looking at plugins which might be affected by this, I found one case - pytest-describe. I believe that pytest-describe can avoid PyCollector with the change proposed here.

BTW, this plugin is really ingenious - treating functions as an extra Python namespacing mechanism by treating them as modules, that's a cool concept :)

PyCollector is technically private in pytest and might change/break.
The DescribeBlocks act like modules, so instead treat them as ones
directly.
@@ -78,10 +89,6 @@ def collect(self):
def _getobj(self):
return self._importtestmodule()

def _makeid(self):
Copy link
Member Author

Choose a reason for hiding this comment

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

By passing nodeid directly, we don't need this. Also, newer versions of pytest no longer use _makeid.

Copy link
Member

@Cito Cito Nov 13, 2021

Choose a reason for hiding this comment

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

Right, this was removed here in pytest 3.5, and we now require pytest 4.6.

Copy link
Contributor

@jacebrowning jacebrowning 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 looks reasonable to me.

@Cito, we might want to make a beta release with this change.

@jacebrowning jacebrowning merged commit 88cbe22 into pytest-dev:main Nov 13, 2021
@Cito
Copy link
Member

Cito commented Nov 13, 2021

Thanks for the heads-up and PR, @bluetech.

@jacebrowning Will check this with a larger real-world project, and I think will then just publish a 2.0.1 patch release. There is no pytest 7 alpha or beta yet, and our test matrix does not cover it, so we will need to check this again when pytest 7 comes out.

@Cito
Copy link
Member

Cito commented Nov 13, 2021

Version 2.01 is out now.

@bluetech
Copy link
Member Author

Great, thanks!

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