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

Broken compatibility with pytest<7 (pytest.Parser type annotation) #755

Closed
martinhoyer opened this issue Feb 13, 2024 · 6 comments · Fixed by #756
Closed

Broken compatibility with pytest<7 (pytest.Parser type annotation) #755

martinhoyer opened this issue Feb 13, 2024 · 6 comments · Fixed by #756

Comments

@martinhoyer
Copy link
Contributor

The parser annotation in plugin.py introduced in 10.0 does not work with pytest 6.

def pytest_addoption(parser: pytest.Parser) -> None:

pytest.Parser is only available from pytest 7.0.0

>>> from testinfra import plugin
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.9/site-packages/testinfra/plugin.py", line 40, in <module>
    def pytest_addoption(parser: pytest.Parser) -> None:
AttributeError: module 'pytest' has no attribute 'Parser'

While specifying minimal pytest version would be a solution, I would rather recommend finding another solution, as RHEL-9/Alma Linux 9/CentOS Stream 9 are running pytest 6 (and Python 3.9).

I'd be happy to help with finding a solution, should you decide to keep pytest 6 compatibility.

@RonnyPfannschmidt
Copy link
Member

It's possible to use the future annotations features Plus and conditional TYPE_CHECKING imports to sidestep this

Since those old pytest versions are unmaintained it's best if someone affected steps up for changing this

There's need for feedback form the primary maintainers as they are well within reason to just set requires-python

@philpep
Copy link
Contributor

philpep commented Feb 14, 2024

I think we should require pytest>=7 then and maybe document how to install testinfra within a virtualenv or with pipx.

@martinhoyer
Copy link
Contributor Author

Since those old pytest versions are unmaintained it's best if someone affected steps up for changing this

Well, pytest 6 is maintained in the enterprise linux world.

Allow me to explain the Fedora packaing as best I can:
pytest-testinfra is being packaged in Fedora, incl. EPEL(extra packagers for enterprise linux).
CentOS Stream 8 builds will end this year in May, allowing Fedora Python projects packagers to only focus on EPEL9, which generally do not require changes from the Fedora versions.

I'm looking into refactoring the rpm packaging with the latest testinfra version in mind now, which is how I found the issue in the first place. (I'm not the maintainer, just contributing). And I believe this one line with type annotation is preventing the el9 package from being built. It would be a shame if the rpm version would have to include patches instead of using upstream sources 'as is'.

I'd be happy to step-up! :) Unless there is a fundamental incompability with pytest 6.2. Will try to run some tests later on.

There's need for feedback form the primary maintainers as they are well within reason to just set requires-python

Required minimal python is already 3.9 here afaik.

@martinhoyer
Copy link
Contributor Author

So it was indeed only one future annotations import in the plugin.py.

[unrelated] I've also noticed flake8 check is broken on Python 3.12. Will take a look later. Hopefully there are no objections against using ruff :)

@RonnyPfannschmidt
Copy link
Member

I'd like to make it abundantly clear that pytest 6 is upstream eol

Open source projects in general can't be expected to pay the enterprise tax

The expectation is that the enterprise vendor will have to handle the details

@martinhoyer
Copy link
Contributor Author

I'd like to make it abundantly clear that pytest 6 is upstream eol

Nobody says otherwise.

Open source projects in general can't be expected to pay the enterprise tax

I mean, there is no minimal pytest version set here currently, so what tax are you talking about? :)

That said, it makes perfect sense to require pytest>=7, I just thought it would be cool to have parity with EPEL9 rpm (and let people use pytest6 if they wish so for whatever reason)

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.

3 participants