diff --git a/CHANGELOG.md b/CHANGELOG.md index e74f14b1db..b65a233f1e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,11 @@ END_UNRELEASED_TEMPLATE * 3.12.11 * 3.13.5 * 3.14.0b3 +* (gazelle): New annotation `gazelle:include_pytest_conftest`. When not set (the + default) or `true`, gazelle will inject any `conftest.py` file found in the same + directory as a {obj}`py_test` target to that {obj}`py_test` target's `deps`. + This behavior is unchanged from previous versions. When `false`, the `:conftest` + dep is not added to the {obj}`py_test` target. * (gazelle) New directive `gazelle:python_generate_proto`; when `true`, Gazelle generates `py_proto_library` rules for `proto_library`. `false` by default. diff --git a/gazelle/README.md b/gazelle/README.md index 35a1e4f701..cf91461e39 100644 --- a/gazelle/README.md +++ b/gazelle/README.md @@ -550,6 +550,8 @@ The annotations are: | Tells Gazelle to ignore import statements. `imports` is a comma-separated list of imports to ignore. | | | [`# gazelle:include_dep targets`](#annotation-include_dep) | N/A | | Tells Gazelle to include a set of dependencies, even if they are not imported in a Python module. `targets` is a comma-separated list of target names to include as dependencies. | | +| [`# gazelle:include_pytest_conftest bool`](#annotation-include_pytest_conftest) | N/A | +| Whether or not to include a sibling `:conftest` target in the deps of a `py_test` target. Default behaviour is to include `:conftest`. | | #### Annotation: `ignore` @@ -622,6 +624,89 @@ deps = [ ] ``` +#### Annotation: `include_pytest_conftest` + +Added in [#3080][gh3080]. + +[gh3080]: https://github.com/bazel-contrib/rules_python/pull/3080 + +This annotation accepts any string that can be parsed by go's +[`strconv.ParseBool`][ParseBool]. If an unparsable string is passed, the +annotation is ignored. + +[ParseBool]: https://pkg.go.dev/strconv#ParseBool + +Starting with [`rules_python` 0.14.0][rules-python-0.14.0] (specifically [PR #879][gh879]), +Gazelle will include a `:conftest` dependency to an `py_test` target that is in +the same directory as `conftest.py`. + +[rules-python-0.14.0]: https://github.com/bazel-contrib/rules_python/releases/tag/0.14.0 +[gh879]: https://github.com/bazel-contrib/rules_python/pull/879 + +This annotation allows users to adjust that behavior. To disable the behavior, set +the annotation value to "false": + +``` +# some_file_test.py +# gazelle:include_pytest_conftest false +``` + +Example: + +Given a directory tree like: + +``` +. +├── BUILD.bazel +├── conftest.py +└── some_file_test.py +``` + +The default Gazelle behavior would create: + +```starlark +py_library( + name = "conftest", + testonly = True, + srcs = ["conftest.py"], + visibility = ["//:__subpackages__"], +) + +py_test( + name = "some_file_test", + srcs = ["some_file_test.py"], + deps = [":conftest"], +) +``` + +When `# gazelle:include_pytest_conftest false` is found in `some_file_test.py` + +```python +# some_file_test.py +# gazelle:include_pytest_conftest false +``` + +Gazelle will generate: + +```starlark +py_library( + name = "conftest", + testonly = True, + srcs = ["conftest.py"], + visibility = ["//:__subpackages__"], +) + +py_test( + name = "some_file_test", + srcs = ["some_file_test.py"], +) +``` + +See [Issue #3076][gh3076] for more information. + +[gh3076]: https://github.com/bazel-contrib/rules_python/issues/3076 + + #### Directive: `experimental_allow_relative_imports` Enables experimental support for resolving relative imports in `python_generation_mode package`. diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go index 343743559f..279bee6af7 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -264,7 +264,9 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes addSrc(filename). addModuleDependencies(mainModules[filename]). addResolvedDependencies(annotations.includeDeps). - generateImportsAttribute().build() + generateImportsAttribute(). + setAnnotations(*annotations). + build() result.Gen = append(result.Gen, pyBinary) result.Imports = append(result.Imports, pyBinary.PrivateAttr(config.GazelleImportsKey)) } @@ -305,6 +307,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes addModuleDependencies(allDeps). addResolvedDependencies(annotations.includeDeps). generateImportsAttribute(). + setAnnotations(*annotations). build() if pyLibrary.IsEmpty(py.Kinds()[pyLibrary.Kind()]) { @@ -357,6 +360,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes addSrc(pyBinaryEntrypointFilename). addModuleDependencies(deps). addResolvedDependencies(annotations.includeDeps). + setAnnotations(*annotations). generateImportsAttribute() pyBinary := pyBinaryTarget.build() @@ -387,6 +391,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes addSrc(conftestFilename). addModuleDependencies(deps). addResolvedDependencies(annotations.includeDeps). + setAnnotations(*annotations). addVisibility(visibility). setTestonly(). generateImportsAttribute() @@ -418,6 +423,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes addSrcs(srcs). addModuleDependencies(deps). addResolvedDependencies(annotations.includeDeps). + setAnnotations(*annotations). generateImportsAttribute() } if (!cfg.PerPackageGenerationRequireTestEntryPoint() || hasPyTestEntryPointFile || hasPyTestEntryPointTarget || cfg.CoarseGrainedGeneration()) && !cfg.PerFileGeneration() { @@ -470,7 +476,14 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes for _, pyTestTarget := range pyTestTargets { if conftest != nil { - pyTestTarget.addModuleDependency(Module{Name: strings.TrimSuffix(conftestFilename, ".py")}) + conftestModule := Module{Name: strings.TrimSuffix(conftestFilename, ".py")} + if pyTestTarget.annotations.includePytestConftest == nil { + // unset; default behavior + pyTestTarget.addModuleDependency(conftestModule) + } else if *pyTestTarget.annotations.includePytestConftest { + // set; add if true, do not add if false + pyTestTarget.addModuleDependency(conftestModule) + } } pyTest := pyTestTarget.build() diff --git a/gazelle/python/parser.go b/gazelle/python/parser.go index 11e01dbf51..3d0dbe7a5f 100644 --- a/gazelle/python/parser.go +++ b/gazelle/python/parser.go @@ -18,6 +18,8 @@ import ( "context" _ "embed" "fmt" + "log" + "strconv" "strings" "github.com/emirpasic/gods/sets/treeset" @@ -123,6 +125,7 @@ func (p *python3Parser) parse(pyFilenames *treeset.Set) (*treeset.Set, map[strin allAnnotations.ignore[k] = v } allAnnotations.includeDeps = append(allAnnotations.includeDeps, annotations.includeDeps...) + allAnnotations.includePytestConftest = annotations.includePytestConftest } allAnnotations.includeDeps = removeDupesFromStringTreeSetSlice(allAnnotations.includeDeps) @@ -183,8 +186,12 @@ const ( // The Gazelle annotation prefix. annotationPrefix string = "gazelle:" // The ignore annotation kind. E.g. '# gazelle:ignore '. - annotationKindIgnore annotationKind = "ignore" - annotationKindIncludeDep annotationKind = "include_dep" + annotationKindIgnore annotationKind = "ignore" + // Force a particular target to be added to `deps`. Multiple invocations are + // accumulated and the value can be comma separated. + // Eg: '# gazelle:include_dep //foo/bar:baz,@repo//:target + annotationKindIncludeDep annotationKind = "include_dep" + annotationKindIncludePytestConftest annotationKind = "include_pytest_conftest" ) // Comment represents a Python comment. @@ -222,6 +229,10 @@ type annotations struct { ignore map[string]struct{} // Labels that Gazelle should include as deps of the generated target. includeDeps []string + // Whether the conftest.py file, found in the same directory as the current + // python test file, should be added to the py_test target's `deps` attribute. + // A *bool is used so that we can handle the "not set" state. + includePytestConftest *bool } // annotationsFromComments returns all the annotations parsed out of the @@ -229,6 +240,7 @@ type annotations struct { func annotationsFromComments(comments []Comment) (*annotations, error) { ignore := make(map[string]struct{}) includeDeps := []string{} + var includePytestConftest *bool for _, comment := range comments { annotation, err := comment.asAnnotation() if err != nil { @@ -255,11 +267,21 @@ func annotationsFromComments(comments []Comment) (*annotations, error) { includeDeps = append(includeDeps, t) } } + if annotation.kind == annotationKindIncludePytestConftest { + val := annotation.value + parsedVal, err := strconv.ParseBool(val) + if err != nil { + log.Printf("WARNING: unable to cast %q to bool in %q. Ignoring annotation", val, comment) + continue + } + includePytestConftest = &parsedVal + } } } return &annotations{ - ignore: ignore, - includeDeps: includeDeps, + ignore: ignore, + includeDeps: includeDeps, + includePytestConftest: includePytestConftest, }, nil } diff --git a/gazelle/python/target.go b/gazelle/python/target.go index 06b653d915..6e6c3f4b14 100644 --- a/gazelle/python/target.go +++ b/gazelle/python/target.go @@ -37,6 +37,7 @@ type targetBuilder struct { main *string imports []string testonly bool + annotations *annotations } // newTargetBuilder constructs a new targetBuilder. @@ -51,6 +52,7 @@ func newTargetBuilder(kind, name, pythonProjectRoot, bzlPackage string, siblingS deps: treeset.NewWith(moduleComparator), resolvedDeps: treeset.NewWith(godsutils.StringComparator), visibility: treeset.NewWith(godsutils.StringComparator), + annotations: new(annotations), } } @@ -130,6 +132,13 @@ func (t *targetBuilder) setTestonly() *targetBuilder { return t } +// setAnnotations sets the annotations attribute on the target. +func (t *targetBuilder) setAnnotations(val annotations) *targetBuilder { + t.annotations = &val + return t +} + + // generateImportsAttribute generates the imports attribute. // These are a list of import directories to be added to the PYTHONPATH. In our // case, the value we add is on Bazel sub-packages to be able to perform imports diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/README.md b/gazelle/python/testdata/annotation_include_pytest_conftest/README.md new file mode 100644 index 0000000000..6a347d154e --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/README.md @@ -0,0 +1,25 @@ +# Annotation: Include Pytest Conftest + +Validate that the `# gazelle:include_pytest_conftest` annotation follows +this logic: + ++ When a `conftest.py` file does not exist: + + all values have no affect ++ When a `conftest.py` file does exist: + + Truthy values add `:conftest` to `deps`. + + Falsey values do not add `:conftest` to `deps`. + + Unset (no annotation) performs the default action. + +Additionally, we test that: + ++ invalid values (eg `foo`) print a warning and then act as if + the annotation was not present. ++ last annotation (highest line number) wins. ++ the annotation has no effect on non-test files/targets. ++ the `include_dep` can still inject `:conftest` even when `include_pytest_conftest` + is false. ++ `import conftest` will still add the dep even when `include_pytest_conftest` is + false. + +An annotation without a value is not tested, as that's part of the core +annotation framework and not specific to this annotation. diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/WORKSPACE b/gazelle/python/testdata/annotation_include_pytest_conftest/WORKSPACE new file mode 100644 index 0000000000..e69de29bb2 diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/test.yaml b/gazelle/python/testdata/annotation_include_pytest_conftest/test.yaml new file mode 100644 index 0000000000..e643d0e90c --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/test.yaml @@ -0,0 +1,5 @@ +--- +expect: + stderr: | + gazelle: WARNING: unable to cast "foo" to bool in "# gazelle:include_pytest_conftest foo". Ignoring annotation + exit_code: 0 diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/BUILD.in b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/BUILD.in new file mode 100644 index 0000000000..e69de29bb2 diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/BUILD.out b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/BUILD.out new file mode 100644 index 0000000000..60695352ca --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/BUILD.out @@ -0,0 +1,68 @@ +load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test") + +py_binary( + name = "binary", + srcs = ["binary.py"], + visibility = ["//:__subpackages__"], +) + +py_library( + name = "with_conftest", + srcs = [ + "binary.py", + "library.py", + ], + visibility = ["//:__subpackages__"], +) + +py_library( + name = "conftest", + testonly = True, + srcs = ["conftest.py"], + visibility = ["//:__subpackages__"], +) + +py_test( + name = "bad_value_test", + srcs = ["bad_value_test.py"], + deps = [":conftest"], +) + +py_test( + name = "conftest_imported_test", + srcs = ["conftest_imported_test.py"], + deps = [":conftest"], +) + +py_test( + name = "conftest_included_test", + srcs = ["conftest_included_test.py"], + deps = [":conftest"], +) + +py_test( + name = "false_test", + srcs = ["false_test.py"], +) + +py_test( + name = "falsey_test", + srcs = ["falsey_test.py"], +) + +py_test( + name = "last_value_wins_test", + srcs = ["last_value_wins_test.py"], +) + +py_test( + name = "true_test", + srcs = ["true_test.py"], + deps = [":conftest"], +) + +py_test( + name = "unset_test", + srcs = ["unset_test.py"], + deps = [":conftest"], +) diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/bad_value_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/bad_value_test.py new file mode 100644 index 0000000000..af2e8c54e0 --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/bad_value_test.py @@ -0,0 +1 @@ +# gazelle:include_pytest_conftest foo diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/binary.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/binary.py new file mode 100644 index 0000000000..d6dc8413d4 --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/binary.py @@ -0,0 +1,3 @@ +# gazelle:include_pytest_conftest true +if __name__ == "__main__": + pass diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/conftest.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/conftest.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/conftest_imported_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/conftest_imported_test.py new file mode 100644 index 0000000000..2c72ca4df1 --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/conftest_imported_test.py @@ -0,0 +1,3 @@ +import conftest + +# gazelle:include_pytest_conftest false diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/conftest_included_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/conftest_included_test.py new file mode 100644 index 0000000000..c942bfb1ab --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/conftest_included_test.py @@ -0,0 +1,2 @@ +# gazelle:include_dep :conftest +# gazelle:include_pytest_conftest false diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/false_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/false_test.py new file mode 100644 index 0000000000..ba71a2818b --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/false_test.py @@ -0,0 +1 @@ +# gazelle:include_pytest_conftest false diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/falsey_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/falsey_test.py new file mode 100644 index 0000000000..c4387b3a8c --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/falsey_test.py @@ -0,0 +1 @@ +# gazelle:include_pytest_conftest 0 diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/last_value_wins_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/last_value_wins_test.py new file mode 100644 index 0000000000..6ffc06f9c0 --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/last_value_wins_test.py @@ -0,0 +1,6 @@ +# gazelle:include_pytest_conftest true +# gazelle:include_pytest_conftest TRUE +# gazelle:include_pytest_conftest False +# gazelle:include_pytest_conftest 0 +# gazelle:include_pytest_conftest 1 +# gazelle:include_pytest_conftest F diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/library.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/library.py new file mode 100644 index 0000000000..b2d10359da --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/library.py @@ -0,0 +1 @@ +# gazelle:include_pytest_conftest true diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/true_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/true_test.py new file mode 100644 index 0000000000..b2d10359da --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/true_test.py @@ -0,0 +1 @@ +# gazelle:include_pytest_conftest true diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/unset_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/with_conftest/unset_test.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/BUILD.in b/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/BUILD.in new file mode 100644 index 0000000000..e69de29bb2 diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/BUILD.out b/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/BUILD.out new file mode 100644 index 0000000000..01383344c5 --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/BUILD.out @@ -0,0 +1,16 @@ +load("@rules_python//python:defs.bzl", "py_test") + +py_test( + name = "false_test", + srcs = ["false_test.py"], +) + +py_test( + name = "true_test", + srcs = ["true_test.py"], +) + +py_test( + name = "unset_test", + srcs = ["unset_test.py"], +) diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/false_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/false_test.py new file mode 100644 index 0000000000..ba71a2818b --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/false_test.py @@ -0,0 +1 @@ +# gazelle:include_pytest_conftest false diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/true_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/true_test.py new file mode 100644 index 0000000000..b2d10359da --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/true_test.py @@ -0,0 +1 @@ +# gazelle:include_pytest_conftest true diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/unset_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest/without_conftest/unset_test.py new file mode 100644 index 0000000000..e69de29bb2