Skip to content

generate py_test without __test__ #999

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

Merged
merged 1 commit into from
Jan 23, 2023
Merged

Conversation

linzhp
Copy link
Contributor

@linzhp linzhp commented Jan 20, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

The Gazelle extension requires the existence of __test__.py or a target named __test__ to generate py_test targets. Otherwise, all _test.py and test_*.py will be included into py_library.

Issue Number: #714

What is the new behavior?

__test__.py is not part of official Python specification. In fact, it is rarely used in existing open source Python projects. A more common scenario is to have _test.py or test_*.py files, each with something like:

if __name__ == '__main__':
    unittest.main()

This PR creates a py_test target per test Python file when neither __test__.py or a target named __test__ exists.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@linzhp linzhp requested a review from f0rmiga as a code owner January 20, 2023 18:32
Copy link
Member

@f0rmiga f0rmiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know at least a few repositories that this will be a breaking change. I've seen files using these patterns to contain test utilities and be reused by multiple tests. Ideally, those should be renamed to adhere to better practices. Having said that, we will need warning notes on the next release. cc @groodt @rickeylev

@f0rmiga f0rmiga merged commit 0943375 into bazel-contrib:main Jan 23, 2023
@linzhp linzhp deleted the py_test branch January 23, 2023 22:43
f0rmiga pushed a commit that referenced this pull request Apr 8, 2023
Since #999, gazelle can
generate multiple `py_test` rules in a single package (when it finds
multiple `*_test.py` or `test_*.py` files and no `__test__.py` file). In
this case, adding new test files to a package with pre-existing
`py_test` rules is not handled properly due to the `MatchAny` property
on the `py_test` kind - it will match the existing `py_test` rule and
edit it instead of adding a new test rule. This PR disables the matching
so that new `py_test` rules are properly generated.
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.

2 participants