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

Compatibility issue with pytest 7.2: Package py is used but not listed as dependency #226

Closed
abey79 opened this issue Oct 25, 2022 · 10 comments · Fixed by #227
Closed

Compatibility issue with pytest 7.2: Package py is used but not listed as dependency #226

abey79 opened this issue Oct 25, 2022 · 10 comments · Fixed by #227

Comments

@abey79
Copy link
Contributor

abey79 commented Oct 25, 2022

The package py is used at least here but is not listed in the dependencies.

Until version 7.1.x, Pytest would require py, but this has been dropped with Pytest 7.2, leading to the following error:

INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/_pytest/main.py", line 266, in wrap_session
INTERNALERROR>     config._do_configure()
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/_pytest/config/__init__.py", line 1037, in _do_configure
INTERNALERROR>     self.hook.pytest_configure.call_historic(kwargs=dict(config=self))
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/pluggy/_hooks.py", line 277, in call_historic
INTERNALERROR>     res = self._hookexec(self.name, self.get_hookimpls(), kwargs, False)
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/pluggy/_manager.py", line 80, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/pluggy/_callers.py", line 60, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/pluggy/_result.py", line 60, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/pluggy/_callers.py", line 39, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/pytest_benchmark/plugin.py", line 440, in pytest_configure
INTERNALERROR>     bs = config._benchmarksession = BenchmarkSession(config)
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/pytest_benchmark/session.py", line 38, in __init__
INTERNALERROR>     self.logger = Logger(level, config=config)
INTERNALERROR>   File "/Users/hhip/Library/Caches/pypoetry/virtualenvs/vpype-l5n3HO8W-py3.10/lib/python3.10/site-packages/pytest_benchmark/logger.py", line 20, in __init__
INTERNALERROR>     self.term = py.io.TerminalWriter(file=sys.stderr)
INTERNALERROR> AttributeError: module 'py' has no attribute 'io'

This is fixed by explicitly manually installing py or adding py to the project.

@abey79 abey79 changed the title Package py is used but not listed as dependency Compatibility issue with pytest 7.2: Package py is used but not listed as dependency Oct 25, 2022
ionelmc pushed a commit that referenced this issue Oct 25, 2022
This is needed since `py` is used but not explicitly listed. Pytest 7.2 no longer requires `py`, which breaks pytest-benchmark with this version of pytest.

Fixes #226
@alexprengere
Copy link

It would be great if a release could be made with that change 😉

@nicoddemus
Copy link

Just FYI, instead of depending on py, if the only dependency is the TerminalWriter, is to import it from from _pytest._io import TerminalWriter. While it is a private module, might be cleaner than depend on the deprecated py library.

@ionelmc
Copy link
Owner

ionelmc commented Oct 25, 2022

@nicoddemus what's the difference between those 2 TerminalWriters?

@nicoddemus
Copy link

None, we vendored py.io into pytest a long time ago, and nothing on them has changed since then. 👍

pytest-xdist just did the same thing (importing TerminalWriter from _pytest._io).

@ionelmc
Copy link
Owner

ionelmc commented Oct 25, 2022

@nicoddemus alright I've changed to what you've suggested but it turns out there were other issues, not sure if I've fixed correctly, can you take a look at c2e860f ?

@nicoddemus
Copy link

nicoddemus commented Oct 25, 2022

At first glance it looks good!

Given there were more changes than just an import, you might consider releasing a hotfix which just adds the py dependency, and after that make a new release which drops py completely (with the changes from c2e860f).

@pyrito
Copy link

pyrito commented Oct 25, 2022

Hi, thanks for the quick fix on the issue! When will pytest-benchmark make the new release?

@ionelmc
Copy link
Owner

ionelmc commented Oct 25, 2022

Just released 4.0.0.

@pyrito
Copy link

pyrito commented Oct 25, 2022

@ionelmc when will it be on conda-forge? Referring to this specifically: https://github.com/conda-forge/pytest-benchmark-feedstock

dagardner-nv added a commit to dagardner-nv/Morpheus that referenced this issue Oct 25, 2022
@ionelmc
Copy link
Owner

ionelmc commented Oct 25, 2022

@pyrito I don't own that repository not do I know much about conda packaging so that question would be best answered by opening an issue in that repository. I doubt they have notifications for releases so it's best to just ping the maintainers.

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 a pull request may close this issue.

5 participants