Pytest 8 error: FixtureManager.getfixtureclosure() missing 1 required positional argument: 'ignore_args'

[sscherfke]

Pytest 8 was just released and it seems that this breaks sybil:

../../.virtualenvs/typed-settings/lib/python3.12/site-packages/sybil/integration/pytest.py:51: in __init__
    self.request_fixtures(sybil.fixtures)
../../.virtualenvs/typed-settings/lib/python3.12/site-packages/sybil/integration/pytest.py:56: in request_fixtures
    closure = fm.getfixtureclosure(names, self)
E   TypeError: FixtureManager.getfixtureclosure() missing 1 required positional argument: 'ignore_args'

During handling of the above exception, another exception occurred:
../../.virtualenvs/typed-settings/lib/python3.12/site-packages/sybil/integration/pytest.py:120: in collect
    yield SybilItem.from_parent(self, sybil=self.sybil, example=example)
../../.virtualenvs/typed-settings/lib/python3.12/site-packages/_pytest/nodes.py:275: in from_parent
    return cls._create(parent=parent, **kw)
../../.virtualenvs/typed-settings/lib/python3.12/site-packages/_pytest/nodes.py:167: in _create
    return super().__call__(*k, **known_kw)
../../.virtualenvs/typed-settings/lib/python3.12/site-packages/sybil/integration/pytest.py:51: in __init__
    self.request_fixtures(sybil.fixtures)
../../.virtualenvs/typed-settings/lib/python3.12/site-packages/sybil/integration/pytest.py:56: in request_fixtures
    closure = fm.getfixtureclosure(names, self)
E   TypeError: FixtureManager.getfixtureclosure() missing 1 required positional argument: 'ignore_args'

Found nothing related in the Changelogs and did not have time to dig deeper into it.

Maybe I can provide a fix later this day. :-)

[cjw296]

Oh good, another pytest major release, another set of breakage. If they could actually make stable APIs tools like Sybil could use, this wouldn't happen. For now, I'm just going to pin to "less than 8"

[sscherfke]

pytest-dev/pytest#11868 (comment)

The change with getfixtureclosure() is that ignore_args: Sequence[str] = () became ignore_args: AbstractSet[str] (w/o a default). Maybe Pytest could change that to ignore_args: AbstractSet[str] = frozenset(). This immutable default might help to keep the API more or less backwards compatible.

@cjw296 you can just change the call to closure = fm.getfixtureclosure(names, self, {}) closure = fm.getfixtureclosure(names, self, set()) and it should work.

[The-Compiler]

set() (empty set) rather than {} (empty dict) probably, no?

[adamtheturtle]

@cjw296 you can just change the call to closure = fm.getfixtureclosure(names, self, {}) and it should work.

That will not work.

This is because the order of arguments changed between Pytest 7 and Pytest 8:

https://github.com/pytest-dev/pytest/blob/23906106968eb95afbd61adfbc7bbb795fc9aaa9/src/_pytest/fixtures.py#L1487-L1491

def getfixtureclosure(
        self,
        fixturenames: Tuple[str, ...],
        parentnode: nodes.Node,
        ignore_args: Sequence[str] = (),

changed to:

https://github.com/pytest-dev/pytest/blob/478f8233bca8147445f0c5129f04ada892cc6c91/src/_pytest/fixtures.py#L1534-L1538

def getfixtureclosure(
        self,
        parentnode: nodes.Node,
        initialnames: Tuple[str, ...],
        ignore_args: AbstractSet[str],

For this particular error, I think that the best solution is:

        if PYTEST_VERSION >= (8, 0, 0):
            closure = fm.getfixtureclosure(initialnames=names, parentnode=self, ignore_args=set())
        else:
            closure = fm.getfixtureclosure(parentnode=self, initialnames=names)

I changed to keyword arguments for clarity and hopefully more stability.

That said, once this particular error is fixed, more show up.

[adamtheturtle]

For now, I'm just going to pin to "less than 8"

@cjw296 For Sybil's own tests, that's fine, but for users that requires pytest to be a dependency. That was rejected in #82.

[cjw296]

@adamtheturtle - when pytest continually break APIs that plugins depend on, users of pytest have to deal with it as well as plugin authors. If you see the upstream issue report around this, a bunch of other plugins were also affected.

Use of Sybil does not have a dependency on pytest, and Python's dependency specifications have no ways of expressing "requires version X of package A, but only if package A is in use", unless you know different?

[cjw296]

This is fixed in 6.0.3 which is now on pypi: https://pypi.org/project/sybil/6.0.3/

[sscherfke]

Thank you very much <3

[adamtheturtle]

@adamtheturtle - when pytest continually break APIs that plugins depend on, users of pytest have to deal with it as well as plugin authors. If you see the upstream issue report around this, a bunch of other plugins were also affected.

Use of Sybil does not have a dependency on pytest, and Python's dependency specifications have no ways of expressing "requires version X of package A, but only if package A is in use", unless you know different?

@cjw296 I appreciate that

• If Pytest API changes will cause breakages, and it may be better if that didn't happen.
• It would be good to somehow only require pytest when needed. I do not know of a way to do that that is not already discussed (e.g. a separate sybil-pytest package, or a pip install sybil[pytest] extra).
• It is better to not install pytest when that is not absolutely necessary.
• No solution is perfect.

As a user of Sybil who does use Pytest, I would prefer to have pip error on resolving Sybil and Pytest when they have incompatible versions, rather than have an unintelligible traceback. This feature of pip is not taken advantage of by the current dependency choice.

You have a better understanding of the desire to not have an "unnecessary" dependency than I do, but from what I understand, my opinion is that having Pytest as a dependency of Sybil (maybe in a [pytest] extra) is a better trade-off overall than the current situation.

[sneakers-the-rat]

coming across this later and just wanna say the frustration is heard and i want to thank you for maintaining this tool <3

[cjw296]

@adamtheturtle:

sybil[pytest]

But what would that pin pytest to? These breakages have occurred on just about every type of pytest release from major down to third point? Chasing that is going to likely be as much work as just getting out a "fix" release for Sybil as soon as possible :-/

[adamtheturtle]

That's a shame! I'd suggest pinning to the latest major version, and that would decrease the number of times that this hits users.

[cjw296]

Can you provide evidence to support your claim? Especially since it directly contradicts my previous statement that, anecdotally, I've seen breakage on every type of release...

[adamtheturtle]

@cjw296 I believe it would have avoided the issues which arose when Pytest bumped to 8.0.0. It wouldn't have avoided the issues on minor version changes.