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

fixtures: avoid mutable arg2index state in favor of looking up the request chain #12038

Merged
merged 2 commits into from Mar 3, 2024

Conversation

bluetech
Copy link
Member

pytest allows a fixture to request its own name (directly or indirectly), in which case the fixture with the same name but one level up is used.

To know which fixture should be used next, pytest keeps a mutable item-global dict _arg2index which maintains this state. This is not great:

  • Mutable state like this is hard to understand and reason about.

  • It is conceptually buggy; the indexing is global (e.g. if requesting fix1 and fix2, the indexing is shared between them), but actually different branches of the subrequest tree should not affect each other.

    This is not an issue in practice because pytest keeps a cache of the fixturedefs it resolved anyway (_fixture_defs), but if the cache is removed it becomes evident.

Instead of the _arg2index state, count how many argnames deep we are in the subrequest tree ("the fixture stack") and use that for the index.

This way, no global mutable state and the logic is very localized and easier to understand.

This is slower, however fixture stacks should not be so deep that this matters much, I hope.

Will be reused in the next commit.
…request chain

pytest allows a fixture to request its own name (directly or
indirectly), in which case the fixture with the same name but one level
up is used.

To know which fixture should be used next, pytest keeps a mutable
item-global dict `_arg2index` which maintains this state. This is not
great:

- Mutable state like this is hard to understand and reason about.

- It is conceptually buggy; the indexing is global (e.g. if requesting
  `fix1` and `fix2`, the indexing is shared between them), but actually
  different branches of the subrequest tree should not affect each
  other.

  This is not an issue in practice because pytest keeps a cache of the
  fixturedefs it resolved anyway (`_fixture_defs`), but if the cache is
  removed it becomes evident.

Instead of the `_arg2index` state, count how many `argname`s deep we are
in the subrequest tree ("the fixture stack") and use that for the index.

This way, no global mutable state and the logic is very localized and
easier to understand.

This is slower, however fixture stacks should not be so deep that this
matters much, I hope.
@bluetech bluetech merged commit 00043f7 into pytest-dev:main Mar 3, 2024
24 checks passed
@bluetech bluetech deleted the fixtures-rm-arg2index branch March 3, 2024 13:45
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.

None yet

2 participants