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

fix(gazelle): generate a single py_test for coarse_grained setups #1538

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Nov 6, 2023

With a recent change to generate a single test target per python file
we introduced a regression for projects using
# gazelle:python_generation_mode project configuration directive
if there are multiple files with the same filenames but under different
directories within the source tree. This PR fixes the behaviour so that
we just generate a single target containing all test files as there is
no sure way to denormalize the paths so that they never clash.

Fixes #1442.

With a recent change to generate a single test target per python file
we introduced a regression for projects using
`# gazelle:python_generation_mode project` configuration directive
if there are multiple files with the same filenames but under different
directories within the source tree. This PR fixes the behaviour so that
we just generate a single target containing all test files as there is
no sure way to denormalize the paths so that they never clash.

Fixes bazelbuild#1442.
@rickeylev rickeylev added this pull request to the merge queue Nov 6, 2023
Merged via the queue into bazelbuild:main with commit 8afbe99 Nov 6, 2023
3 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Mar 21, 2024
…tion mode (#1809)

Since #1538, when using
`gazelle:python_generation_mode project`, a `py_test` rule is created
even when there are no test files in the project.
fb673ee (first commit on the PR branch)
reproduces this issue - it shows that a `py_test` rule is created even
when there is no test entrypoint file (`__test__.py`) nor any test file
in the entire project. Additionally, test rules created on `project` or
`package` generation mode will always set `main = "__test__.py"`, even
when that file doesn't exist.

This PR fixes it by only generating a `py_test` rule if it can find some
test file - either an explicit `__test__.py`, or any other file prefixed
with `test_` or suffixed with `_test.py`. It also changes the generated
test rule to only add `main = "__test__.py"` if there is an actual
`__test__.py` file. Note that, in the case where a `__test__.py` file
doesn't exist, the generated `py_test` rule is likely invalid as-is due
to the lack of `main`; this can be fixed by mapping to some other
`py_test` implementation, and I believe the new behavior makes more
sense than pointing `main` to a non-existing file.

It may be useful to review per-commit (all tests pass on each commit):
- First commit reproduces the issue;
- Second commit fixes the issue and the corresponding tests;
- Third commit adds additional test cases.
wingsofovnia pushed a commit to wingsofovnia/rules_python_gazelle_plugin that referenced this pull request May 3, 2024
…tion mode (#1809)

Since bazelbuild/rules_python#1538, when using
`gazelle:python_generation_mode project`, a `py_test` rule is created
even when there are no test files in the project.
fb673ee47b3268a65a18a154edd574b6509c38c7 (first commit on the PR branch)
reproduces this issue - it shows that a `py_test` rule is created even
when there is no test entrypoint file (`__test__.py`) nor any test file
in the entire project. Additionally, test rules created on `project` or
`package` generation mode will always set `main = "__test__.py"`, even
when that file doesn't exist.

This PR fixes it by only generating a `py_test` rule if it can find some
test file - either an explicit `__test__.py`, or any other file prefixed
with `test_` or suffixed with `_test.py`. It also changes the generated
test rule to only add `main = "__test__.py"` if there is an actual
`__test__.py` file. Note that, in the case where a `__test__.py` file
doesn't exist, the generated `py_test` rule is likely invalid as-is due
to the lack of `main`; this can be fixed by mapping to some other
`py_test` implementation, and I believe the new behavior makes more
sense than pointing `main` to a non-existing file.

It may be useful to review per-commit (all tests pass on each commit):
- First commit reproduces the issue;
- Second commit fixes the issue and the corresponding tests;
- Third commit adds additional test cases.
wingsofovnia pushed a commit to wingsofovnia/rules_python_gazelle_plugin that referenced this pull request May 3, 2024
…tion mode (#1809)

Since bazelbuild/rules_python#1538, when using
`gazelle:python_generation_mode project`, a `py_test` rule is created
even when there are no test files in the project.
fb673ee47b3268a65a18a154edd574b6509c38c7 (first commit on the PR branch)
reproduces this issue - it shows that a `py_test` rule is created even
when there is no test entrypoint file (`__test__.py`) nor any test file
in the entire project. Additionally, test rules created on `project` or
`package` generation mode will always set `main = "__test__.py"`, even
when that file doesn't exist.

This PR fixes it by only generating a `py_test` rule if it can find some
test file - either an explicit `__test__.py`, or any other file prefixed
with `test_` or suffixed with `_test.py`. It also changes the generated
test rule to only add `main = "__test__.py"` if there is an actual
`__test__.py` file. Note that, in the case where a `__test__.py` file
doesn't exist, the generated `py_test` rule is likely invalid as-is due
to the lack of `main`; this can be fixed by mapping to some other
`py_test` implementation, and I believe the new behavior makes more
sense than pointing `main` to a non-existing file.

It may be useful to review per-commit (all tests pass on each commit):
- First commit reproduces the issue;
- Second commit fixes the issue and the corresponding tests;
- Third commit adds additional test cases.
@aignas aignas deleted the test/gazelle-test-coarse-multiple-test-targets branch May 13, 2024 06:49
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.

Gazelle with python_generation_mode project can generate duplicate py_test targets
2 participants