Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ 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.


{#v0-0-0-removed}
### Removed
Expand Down
85 changes: 85 additions & 0 deletions gazelle/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,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`
Expand Down Expand Up @@ -585,6 +587,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`.
Expand Down
17 changes: 15 additions & 2 deletions gazelle/python/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,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))
}
Expand Down Expand Up @@ -301,6 +303,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()]) {
Expand Down Expand Up @@ -353,6 +356,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
addSrc(pyBinaryEntrypointFilename).
addModuleDependencies(deps).
addResolvedDependencies(annotations.includeDeps).
setAnnotations(*annotations).
generateImportsAttribute()

pyBinary := pyBinaryTarget.build()
Expand Down Expand Up @@ -383,6 +387,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
addSrc(conftestFilename).
addModuleDependencies(deps).
addResolvedDependencies(annotations.includeDeps).
setAnnotations(*annotations).
addVisibility(visibility).
setTestonly().
generateImportsAttribute()
Expand Down Expand Up @@ -414,6 +419,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() {
Expand Down Expand Up @@ -466,7 +472,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()

Expand Down
30 changes: 26 additions & 4 deletions gazelle/python/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"context"
_ "embed"
"fmt"
"log"
"strconv"
"strings"

"github.com/emirpasic/gods/sets/treeset"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -183,8 +186,12 @@ const (
// The Gazelle annotation prefix.
annotationPrefix string = "gazelle:"
// The ignore annotation kind. E.g. '# gazelle:ignore <module_name>'.
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.
Expand Down Expand Up @@ -222,13 +229,18 @@ 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
// comments of a Python module.
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 {
Expand All @@ -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
}

Expand Down
9 changes: 9 additions & 0 deletions gazelle/python/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type targetBuilder struct {
main *string
imports []string
testonly bool
annotations *annotations
}

// newTargetBuilder constructs a new targetBuilder.
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Empty file.
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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"],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:include_pytest_conftest foo
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# gazelle:include_pytest_conftest true
if __name__ == "__main__":
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import conftest

# gazelle:include_pytest_conftest false
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# gazelle:include_dep :conftest
# gazelle:include_pytest_conftest false
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:include_pytest_conftest false
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:include_pytest_conftest 0
Loading