Skip to content

Commit c9783c8

Browse files
authored
feat(gazelle): Directive controlling pytest ancestor dependencies (#3596)
Fixes #3595. Add a new directive `python_include_ancestor_conftest`, defaulting to `true`, that configures whether or not ancestor `conftest` targets are added to a `py_test` target's dependencies.
1 parent cd34111 commit c9783c8

31 files changed

Lines changed: 226 additions & 6 deletions

CHANGELOG.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,9 @@ END_UNRELEASED_TEMPLATE
8282
* (tests) No more coverage warnings are being printed if there are no sources.
8383
([#2762](https://github.com/bazel-contrib/rules_python/issues/2762))
8484
* (gazelle) Ancestor `conftest.py` files are added in addition to sibling `conftest.py`.
85-
([#3497](https://github.com/bazel-contrib/rules_python/issues/3497))
85+
([#3497](https://github.com/bazel-contrib/rules_python/issues/3497)) Note
86+
that this behavior can be reverted to the pre-VERSION_NEXT_FEATURE behavior by setting the new
87+
`python_include_ancestor_conftest` directive to `false`.
8688
* (pypi) `pip_parse` no longer silently drops PEP 508 URL-based requirements
8789
(`pkg @ https://...`) when `extract_url_srcs=False` (the default for
8890
`pip_repository`).
@@ -115,6 +117,12 @@ END_UNRELEASED_TEMPLATE
115117
{obj}`PyExecutableInfo.venv_python_exe`.
116118
* (tools/wheelmaker.py) Added support for URL requirements according to PEP 508
117119
in Requires-Dist metadata. ([#3569](https://github.com/bazel-contrib/rules_python/pull/3569))
120+
* (gazelle) A new directive `python_include_ancestor_conftest` has been added.
121+
When `false`, ancestor `conftest` targets are not automatically added to
122+
{bzl:obj}`py_test` target dependencies. This `false` behavior is how things
123+
were in `rules_python` before VERSION_NEXT_FEATURE. The default is `true`, as the prior behavior
124+
was technically incorrect.
125+
([#3596](https://github.com/bazel-contrib/rules_python/pull/3596))
118126

119127
{#v1-8-4}
120128
## [1.8.4] - 2026-02-10

gazelle/docs/annotations.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ deps = [
116116
```
117117

118118

119+
(annotation-include-pytest-conftest)=
119120
## `include_pytest_conftest`
120121

121122
:::{versionadded} 1.6.0

gazelle/docs/directives.md

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,11 @@ The Python-specific directives are:
175175
* Default: `false`
176176
* Allowed Values: `true`, `false`
177177

178+
[`# gazelle:python_include_ancestor_conftest bool`](#python-include-ancestor-conftest)
179+
: Controls whether ancestor conftest targets are added to {bzl:obj}`py_test` target
180+
dependencies.
181+
* Default: `true`
182+
* Allowed Values: `true`, `false`
178183

179184
## `python_extension`
180185

@@ -720,3 +725,67 @@ previously-generated or hand-created rules.
720725
:::{error}
721726
Detailed docs are not yet written.
722727
:::
728+
729+
## `python_include_ancestor_conftest`
730+
731+
Version VERSION_NEXT_FEATURE includes a fix ({gh-pr}`3498`) for a long-standing issue
732+
({gh-issue}`3497`) where ancestor `conftest.py` files were not automatically
733+
added as dependencies of {bzl:obj}`py_test` targets.
734+
735+
However, some people may not want this behavior (see https://xkcd.com/1172/).
736+
Thus the `python_include_ancestor_conftest` directive controls this behavior.
737+
It defaults to `true`, which causes all ancestor `conftest.py` files to be
738+
included as dependencies for {bzl:obj}`py_test` targets.
739+
740+
Setting the directive to `false` reverts to the pre-VERSION_NEXT_FEATURE behavior.
741+
742+
For example, given this directory tree (not shown: intermediary `BUILD.bazel`
743+
files)
744+
745+
```
746+
./
747+
├── conftest.py
748+
└── one/
749+
├── conftest.py
750+
└── two/
751+
├── conftest.py
752+
└── three/
753+
├── BUILD.bazel
754+
├── conftest.py
755+
└── my_test.py
756+
```
757+
758+
Gazelle will generate this target for `foo_test.py` by default:
759+
760+
```starlark
761+
py_test(
762+
name = "foo_test",
763+
srcs = ["foo_test.py"],
764+
deps = [
765+
":conftest", # same as "//one:two/three:conftest"
766+
"//:conftest",
767+
"//one:conftest",
768+
"//one/two:conftest",
769+
],
770+
)
771+
```
772+
773+
But when `python_include_ancestor_conftest` is `false`, only the sibling
774+
`:conftest` target will be included as a dependency:
775+
776+
:::{tip}
777+
The [`include_pytest_conftest` annotation](annotation-include-pytest-conftest)
778+
controls whether the sibling `:conftest` target is added to {bzl:obj}`py_test`
779+
target dependency list.
780+
:::
781+
782+
```starlark
783+
# gazelle:python_include_ancestor_conftest false
784+
py_test(
785+
name = "foo_test",
786+
srcs = ["foo_test.py"],
787+
deps = [
788+
":conftest",
789+
],
790+
)
791+
```

gazelle/python/configure.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ func (py *Configurer) KnownDirectives() []string {
7474
pythonconfig.ExperimentalAllowRelativeImports,
7575
pythonconfig.GenerateProto,
7676
pythonconfig.PythonResolveSiblingImports,
77+
pythonconfig.PythonIncludeAncestorConftest,
7778
}
7879
}
7980

@@ -261,6 +262,12 @@ func (py *Configurer) Configure(c *config.Config, rel string, f *rule.File) {
261262
log.Fatal(err)
262263
}
263264
config.SetResolveSiblingImports(v)
265+
case pythonconfig.PythonIncludeAncestorConftest:
266+
v, err := strconv.ParseBool(strings.TrimSpace(d.Value))
267+
if err != nil {
268+
log.Fatal(err)
269+
}
270+
config.SetIncludeAncestorConftest(v)
264271
}
265272
}
266273

gazelle/python/generate.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func matchesAnyGlob(s string, globs []string) bool {
6868

6969
// findConftestPaths returns package paths containing conftest.py, from currentPkg
7070
// up through ancestors, stopping at module root.
71-
func findConftestPaths(repoRoot, currentPkg, pythonProjectRoot string) []string {
71+
func findConftestPaths(repoRoot, currentPkg, pythonProjectRoot string, includeAncestorConftest bool) []string {
7272
var result []string
7373
for pkg := currentPkg; ; pkg = filepath.Dir(pkg) {
7474
if pkg == "." {
@@ -77,6 +77,12 @@ func findConftestPaths(repoRoot, currentPkg, pythonProjectRoot string) []string
7777
if _, err := os.Stat(filepath.Join(repoRoot, pkg, conftestFilename)); err == nil {
7878
result = append(result, pkg)
7979
}
80+
// We traverse up the tree to find conftest files and we start in
81+
// the current package. Thus if we find one in the current package
82+
// and do not want ancestors, we break early.
83+
if !includeAncestorConftest {
84+
break
85+
}
8086
if pkg == "" {
8187
break
8288
}
@@ -402,7 +408,6 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
402408
setAnnotations(*annotations).
403409
generateImportsAttribute()
404410

405-
406411
pyBinary := pyBinaryTarget.build()
407412

408413
result.Gen = append(result.Gen, pyBinary)
@@ -526,13 +531,13 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
526531

527532
for _, pyTestTarget := range pyTestTargets {
528533
shouldAddConftest := pyTestTarget.annotations.includePytestConftest == nil ||
529-
*pyTestTarget.annotations.includePytestConftest
534+
*pyTestTarget.annotations.includePytestConftest
530535

531536
if shouldAddConftest {
532-
for _, conftestPkg := range findConftestPaths(args.Config.RepoRoot, args.Rel, pythonProjectRoot) {
537+
for _, conftestPkg := range findConftestPaths(args.Config.RepoRoot, args.Rel, pythonProjectRoot, cfg.IncludeAncestorConftest()) {
533538
pyTestTarget.addModuleDependency(
534539
Module{
535-
Name: importSpecFromSrc(pythonProjectRoot, conftestPkg, conftestFilename).Imp,
540+
Name: importSpecFromSrc(pythonProjectRoot, conftestPkg, conftestFilename).Imp,
536541
Filepath: filepath.Join(conftestPkg, conftestFilename),
537542
},
538543
)

gazelle/python/testdata/directive_python_include_ancestor_conftest/BUILD.in

Whitespace-only changes.
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
load("@rules_python//python:defs.bzl", "py_library")
2+
3+
py_library(
4+
name = "conftest",
5+
testonly = True,
6+
srcs = ["conftest.py"],
7+
visibility = ["//:__subpackages__"],
8+
)

gazelle/python/testdata/directive_python_include_ancestor_conftest/MODULE.bazel

Whitespace-only changes.
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Directive: `python_include_ancestor_conftest`
2+
3+
This test case asserts that the `# gazelle:python_include_ancestor_conftest`
4+
directive correctly includes or excludes ancestor `conftest` targets in
5+
`py_test` target dependencies.
6+
7+
The test also asserts that the directive can be applied at any level and that
8+
child levels will inherit the value:
9+
10+
+ The root level does not set the directive (it defaults to True).
11+
+ The next level, `one/`, inherits that value.
12+
+ The next level, `one/two/`, sets the directive to False; consequently the
13+
`py_test` target only includes the sibling `:conftest` target.
14+
+ The `one/two/no_conftest/` directory does not contain a `conftest.py` file
15+
thereby asserting that we correctly do not include any `conftest` targets
16+
whatsoever.
17+
+ The final level, `one/two/three/`, sets the directive back to True, meaning
18+
the `py_test` target includes a total of 4 `conftest` targets.
19+
+ The `one/two/three/no_conftest/` directory does not contain a `conftest.py`
20+
file and thus asserts that the code includes _only_ ancestor `conftest`
21+
targets.
22+
23+
See [Issue #3595](https://github.com/bazel-contrib/rules_python/issues/3595).

gazelle/python/testdata/directive_python_include_ancestor_conftest/WORKSPACE

Whitespace-only changes.

0 commit comments

Comments
 (0)