avoid using __file__ in pytest_plugin_registered as can be wrong on Windows#11825
Conversation
0d169d8 to
ef99b0e
Compare
src/_pytest/fixtures.py
Outdated
| nodeid = "" | ||
| if os.sep != nodes.SEP: | ||
| nodeid = nodeid.replace(os.sep, nodes.SEP) | ||
| plugin_name = self.config.pluginmanager.get_name(plugin) |
There was a problem hiding this comment.
@bluetech Is that what you meant in #11816 (comment)?
bluetech
left a comment
There was a problem hiding this comment.
Yep, exactly, this LGTM.
Test failures seem unrelated. "hypothesis.errors.HypothesisSideeffectWarning: Slow code in plugin: avoid lazy evaluation of text() at import time!" never seen this before. Unless the get_name slowness is actually the culprit... I will re-run the jobs later.
As for optimizing the get_name call, I can do it in a separate PR, unless you want to do it here already. The best approach I think is to add a plugin_name parameter to the pytest_plugin_registered hook.
No idea what this means so if you could handle it in another MR, that would be great. |
|
The Hypothesis issue seems to reproduce when re-running the jobs, though some jobs pass. The (existing, seems to be unrelated to this PR) test is: @hypothesis.given(strategies.text() | strategies.binary())
@hypothesis.settings(
deadline=400.0
) # very close to std deadline and CI boxes are not reliable in CPU power
def test_idval_hypothesis(self, value) -> None:
escaped = IdMaker([], [], None, None, None, None, None)._idval(value, "a", 6)
assert isinstance(escaped, str)
escaped.encode("ascii")The error is: @Zac-HD Maybe you have an idea? The test seems run of the mill to me and I don't see any import-time evaluation going on but I don't have much experience with Hypothesis. |
|
We just added that warning last night, and I suspect a pytest-specific interaction... I suggest globally ignoring |
Otherwise mypy doesn't fully recognize pluggy's typing for some reason or another.
ef99b0e to
d0c531d
Compare
|
Added the change to the hook. Did it in this PR to make backporting easier. |
…ed` hook We have a use case for this in the next commit. The name can be obtained by using `manager.get_name(plugin)`, however this is currently O(num plugins) in pluggy, which would be good to avoid. Besides, it seems generally useful.
…gin_name` hook parameter
d0c531d to
9ea2e0a
Compare
closes #11816
Alternative fix to #11821.
The plugin name is equal to the path for conftest plugins (suggested in #11816 (comment)). Use that instead of
plugin.__file__.Test with the h5py CI that was broken by the issue: h5py/h5py#2370
Another way of testing this MR: