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

[gazelle] Support resolving pytest fixtures #2162

Open
alex-torok opened this issue Aug 28, 2024 · 2 comments
Open

[gazelle] Support resolving pytest fixtures #2162

alex-torok opened this issue Aug 28, 2024 · 2 comments
Labels
gazelle Gazelle plugin related issues help wanted

Comments

@alex-torok
Copy link
Contributor

alex-torok commented Aug 28, 2024

🚀 feature request

Description

Gazelle should allow for the configuration and resolution of pytest fixtures. In pytest, fixtures are method parameters that are automatically discovered and injected into test functions by pytest's plugin machinery.

For example,

def test_foo(mocker):
   mocker.patch(...)
   ...

The mocker keyword is a pytest fixture that is found in the pytest_mock pip package. Fixtures can also be provided by other targets in the workspace.

Describe the solution you'd like

Update the gazelle plugin to find pytest fixtures and track them in addition to module dependencies.

Support resolution config for pytest fixtures through # gazelle:python_resolve_pytest_fixture FIXTURE_NAME TARGET_LABEL directives.

For the above example mocker, a directive line like below could be added to the root build file:

# gazelle:python_resolve_pytest_fixture mocker @pip//pytest_mock

Doing this would have the rules_python gazelle plugin add @pip//pytest_mock to any test target that has a test_* function that accepts an argument named mocker. Some additional handling will need to be done to track fixture functions that depend on other fixtures.

Describe alternatives you've considered

I considered using # gazelle:resolve pytest_fixture FIXTURE_NAME TARGET_LABEL, but it would require implementing a CrossResolver in the gazelle extension to resolve the pytest_fixture language. It also feels like it goes a bit against gazelle design to have a language name in a resolve directive for which there isn't actually a plugin for. Keeping it all in rules_python's directive configuration seems more appropriate given that rules_python already has special handling for conftest files. There could be a whole pytest_fixture indexing process that would allow for automatic resolution of pytest fixtures, but having a manual override seems good enough.

A second alternative is to use # gazelle:include_dep lines in every test file that needs a specific fixture dependency, but this is a bit of a pain to maintain.

@aignas
Copy link
Collaborator

aignas commented Aug 28, 2024

Thinking with a python ruleset maintainer hat, how does making gazelle understand pytest fixture model benefit other frameworks? Asking to support something like this is basically asking to special case pytest.

The alternative for now is to add pytest_mocker to the deps and use the # keep directive. It's not great, because it duplicates the list of dependencies for a test - one in build.bazel and another one elsewhere, but it at least makes things easy to understand.

Not saying yet if gazelle should have or not have the functionality, but special casing pytest in this ruleset does not have precedence yet.

@alex-torok
Copy link
Contributor Author

alex-torok commented Aug 28, 2024

Thinking with a python ruleset maintainer hat, how does making gazelle understand pytest fixture model benefit other frameworks? Asking to support something like this is basically asking to special case pytest.

It would be a special case just for pytest, which has a unique plugin registration and dependency injection/fixture system.

special casing pytest in this ruleset does not have precedence yet.

In the case of the gazelle extension, I don't think this is true. The gazelle extension already has handling for conftest.py files (originally added in #879). conftest.py files are how pytest provides fixtures across multiple files in the same directory. See pytest conftest docs. The gazelle extension was made to understand this and generate a conftest library target that gets added as a dep to all py_test targets in the same directory.

It does this unconditionally, and without the ability to disable it. Under my above proposal, the behavior would only be changed if the gazelle:python_resolve_pytest_fixture directive is used.

Thanks for the quick response by the way!

@aignas aignas added help wanted gazelle Gazelle plugin related issues labels Sep 8, 2024
@groodt groodt changed the title (gazelle): Support resolving pytest fixtures [gazelle] Support resolving pytest fixtures Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gazelle Gazelle plugin related issues help wanted
Projects
None yet
Development

No branches or pull requests

2 participants