From 976c76888be7eaee5f3d0aec18e55274f8fef817 Mon Sep 17 00:00:00 2001 From: Alex Martani Date: Thu, 10 Sep 2026 16:44:48 -0700 Subject: [PATCH 1/2] fix(gazelle): merge pytest conftest annotations deterministically Concurrent parsing let an unset value overwrite an explicit annotation. This made test generation depend on parser completion order. Track explicit true and false values across all source files. Preserve an agreed value and reject conflicts. --- gazelle/docs/annotations.md | 10 ++ gazelle/python/BUILD.bazel | 4 + gazelle/python/parser.go | 19 +++- gazelle/python/parser_test.go | 92 +++++++++++++++++++ .../BUILD.in | 2 + .../BUILD.out | 2 + .../README.md | 4 + .../WORKSPACE | 1 + .../conftest.py | 1 + .../false_test.py | 1 + .../test.yaml | 5 + .../true_test.py | 1 + news/gazelle-pytest-conftest.fixed.md | 2 + 13 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 gazelle/python/parser_test.go create mode 100644 gazelle/python/testdata/annotation_include_pytest_conftest_conflict/BUILD.in create mode 100644 gazelle/python/testdata/annotation_include_pytest_conftest_conflict/BUILD.out create mode 100644 gazelle/python/testdata/annotation_include_pytest_conftest_conflict/README.md create mode 100644 gazelle/python/testdata/annotation_include_pytest_conftest_conflict/WORKSPACE create mode 100644 gazelle/python/testdata/annotation_include_pytest_conftest_conflict/conftest.py create mode 100644 gazelle/python/testdata/annotation_include_pytest_conftest_conflict/false_test.py create mode 100644 gazelle/python/testdata/annotation_include_pytest_conftest_conflict/test.yaml create mode 100644 gazelle/python/testdata/annotation_include_pytest_conftest_conflict/true_test.py create mode 100644 news/gazelle-pytest-conftest.fixed.md diff --git a/gazelle/docs/annotations.md b/gazelle/docs/annotations.md index b3f06b9991..05ac4a6daa 100644 --- a/gazelle/docs/annotations.md +++ b/gazelle/docs/annotations.md @@ -198,3 +198,13 @@ py_test( ``` See {gh-issue}`3076` for more information. + +When a `py_test` has multiple source files, the annotation may be omitted from +some files. If multiple source files set the annotation, they must all set it to +the same value; Gazelle reports an error if the values conflict. + +:::{versionchanged} VERSION_NEXT_PATCH +For multi-source `py_test` targets, annotations in different source files must +agree. An annotation in one source file is no longer overwritten by an unset +value in another source file. +::: diff --git a/gazelle/python/BUILD.bazel b/gazelle/python/BUILD.bazel index 1ffa2890e1..bd6679572f 100644 --- a/gazelle/python/BUILD.bazel +++ b/gazelle/python/BUILD.bazel @@ -120,10 +120,14 @@ go_test( name = "default_test", srcs = [ "file_parser_test.go", + "parser_test.go", "std_modules_test.go", ], embed = [":python"], deps = [ + "@com_github_emirpasic_gods//sets/treeset:go_default_library", + "@com_github_emirpasic_gods//utils:go_default_library", "@com_github_stretchr_testify//assert", + "@com_github_stretchr_testify//require", ], ) diff --git a/gazelle/python/parser.go b/gazelle/python/parser.go index 3d0dbe7a5f..bead1848b1 100644 --- a/gazelle/python/parser.go +++ b/gazelle/python/parser.go @@ -92,6 +92,8 @@ func (p *python3Parser) parse(pyFilenames *treeset.Set) (*treeset.Set, map[strin mainModules := make(map[string]*treeset.Set, len(chRes)) allAnnotations := new(annotations) allAnnotations.ignore = make(map[string]struct{}) + var includesPytestConftest bool + var excludesPytestConftest bool for res := range chRes { if res.HasMain { mainModules[res.FileName] = treeset.NewWith(moduleComparator) @@ -125,9 +127,24 @@ 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 + if annotations.includePytestConftest != nil { + if *annotations.includePytestConftest { + includesPytestConftest = true + } else { + excludesPytestConftest = true + } + } } + if includesPytestConftest && excludesPytestConftest { + return nil, nil, nil, fmt.Errorf( + "conflicting values for the %q annotation across Python source files", + annotationKindIncludePytestConftest, + ) + } + if includesPytestConftest || excludesPytestConftest { + allAnnotations.includePytestConftest = &includesPytestConftest + } allAnnotations.includeDeps = removeDupesFromStringTreeSetSlice(allAnnotations.includeDeps) return modules, mainModules, allAnnotations, nil diff --git a/gazelle/python/parser_test.go b/gazelle/python/parser_test.go new file mode 100644 index 0000000000..9aff397fd2 --- /dev/null +++ b/gazelle/python/parser_test.go @@ -0,0 +1,92 @@ +package python + +import ( + "os" + "path/filepath" + "testing" + + "github.com/emirpasic/gods/sets/treeset" + godsutils "github.com/emirpasic/gods/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseIncludePytestConftestAnnotations(t *testing.T) { + t.Parallel() + + boolPointer := func(value bool) *bool { + return &value + } + tests := []struct { + name string + contents []string + expected *bool + expectErr string + }{ + { + name: "all unset", + contents: []string{"", ""}, + }, + { + name: "false and unset", + contents: []string{"# gazelle:include_pytest_conftest false", ""}, + expected: boolPointer(false), + }, + { + name: "true and unset", + contents: []string{"", "# gazelle:include_pytest_conftest true"}, + expected: boolPointer(true), + }, + { + name: "matching false values", + contents: []string{ + "# gazelle:include_pytest_conftest false", + "# gazelle:include_pytest_conftest false", + "", + }, + expected: boolPointer(false), + }, + { + name: "matching true values", + contents: []string{ + "# gazelle:include_pytest_conftest true", + "", + "# gazelle:include_pytest_conftest true", + }, + expected: boolPointer(true), + }, + { + name: "conflicting values", + contents: []string{ + "# gazelle:include_pytest_conftest false", + "", + "# gazelle:include_pytest_conftest true", + }, + expectErr: "conflicting values for the \"include_pytest_conftest\" annotation " + + "across Python source files", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + repoRoot := t.TempDir() + filenames := treeset.NewWith(godsutils.StringComparator) + for index, contents := range test.contents { + filename := string(rune('a'+index)) + "_test.py" + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, filename), []byte(contents), 0o600)) + filenames.Add(filename) + } + + parser := newPython3Parser(repoRoot, "", func(string) bool { return false }) + _, _, annotations, err := parser.parse(filenames) + if test.expectErr != "" { + assert.EqualError(t, err, test.expectErr) + return + } + + require.NoError(t, err) + assert.Equal(t, test.expected, annotations.includePytestConftest) + }) + } +} diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/BUILD.in b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/BUILD.in new file mode 100644 index 0000000000..786a959d7b --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/BUILD.in @@ -0,0 +1,2 @@ +# gazelle:python_generation_mode package +# gazelle:python_generation_mode_per_package_require_test_entry_point false diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/BUILD.out b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/BUILD.out new file mode 100644 index 0000000000..786a959d7b --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/BUILD.out @@ -0,0 +1,2 @@ +# gazelle:python_generation_mode package +# gazelle:python_generation_mode_per_package_require_test_entry_point false diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/README.md b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/README.md new file mode 100644 index 0000000000..97317ffca7 --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/README.md @@ -0,0 +1,4 @@ +# Conflicting `include_pytest_conftest` annotations + +This test case asserts that Gazelle fails when source files in the same +`py_test` set `include_pytest_conftest` to conflicting values. diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/WORKSPACE b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/WORKSPACE new file mode 100644 index 0000000000..faff6af87a --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/WORKSPACE @@ -0,0 +1 @@ +# This is a Bazel workspace for the Gazelle test data. diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/conftest.py b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/conftest.py new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/conftest.py @@ -0,0 +1 @@ + diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/false_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/false_test.py new file mode 100644 index 0000000000..ba71a2818b --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/false_test.py @@ -0,0 +1 @@ +# gazelle:include_pytest_conftest false diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/test.yaml b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/test.yaml new file mode 100644 index 0000000000..780d2148d7 --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/test.yaml @@ -0,0 +1,5 @@ +--- +expect: + exit_code: 1 + stderr: | + gazelle: ERROR: conflicting values for the "include_pytest_conftest" annotation across Python source files diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/true_test.py b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/true_test.py new file mode 100644 index 0000000000..b2d10359da --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/true_test.py @@ -0,0 +1 @@ +# gazelle:include_pytest_conftest true diff --git a/news/gazelle-pytest-conftest.fixed.md b/news/gazelle-pytest-conftest.fixed.md new file mode 100644 index 0000000000..532e13012f --- /dev/null +++ b/news/gazelle-pytest-conftest.fixed.md @@ -0,0 +1,2 @@ +(gazelle) Made `include_pytest_conftest` annotations deterministic for +multi-source tests and report conflicting explicit values. From 0d0bd997abeaa3095a3d055c59de10252ff0a639 Mon Sep 17 00:00:00 2001 From: Alex Martani Date: Fri, 18 Sep 2026 20:45:06 -0700 Subject: [PATCH 2/2] docs(gazelle): clarify pytest conftest fix The news fragment did not clearly describe the user-visible outcome. Clarify when annotations are kept and when Gazelle reports an error. Also wrap the stderr fixture and use API cross-references. --- gazelle/docs/annotations.md | 12 ++++++------ .../test.yaml | 5 +++-- news/gazelle-pytest-conftest.fixed.md | 6 ++++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/gazelle/docs/annotations.md b/gazelle/docs/annotations.md index 05ac4a6daa..596bb7f0f8 100644 --- a/gazelle/docs/annotations.md +++ b/gazelle/docs/annotations.md @@ -199,12 +199,12 @@ py_test( See {gh-issue}`3076` for more information. -When a `py_test` has multiple source files, the annotation may be omitted from -some files. If multiple source files set the annotation, they must all set it to -the same value; Gazelle reports an error if the values conflict. +When a {bzl:obj}`py_test` has multiple source files, the annotation may be +omitted from some files. If multiple source files set the annotation, they must +all set it to the same value; Gazelle reports an error if the values conflict. :::{versionchanged} VERSION_NEXT_PATCH -For multi-source `py_test` targets, annotations in different source files must -agree. An annotation in one source file is no longer overwritten by an unset -value in another source file. +For multi-source {bzl:obj}`py_test` targets, annotations in different source +files must agree. An annotation in one source file is no longer overwritten by +an unset value in another source file. ::: diff --git a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/test.yaml b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/test.yaml index 780d2148d7..1e54fde93d 100644 --- a/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/test.yaml +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/test.yaml @@ -1,5 +1,6 @@ --- expect: exit_code: 1 - stderr: | - gazelle: ERROR: conflicting values for the "include_pytest_conftest" annotation across Python source files + stderr: >- + gazelle: ERROR: conflicting values for the + "include_pytest_conftest" annotation across Python source files diff --git a/news/gazelle-pytest-conftest.fixed.md b/news/gazelle-pytest-conftest.fixed.md index 532e13012f..89b2533988 100644 --- a/news/gazelle-pytest-conftest.fixed.md +++ b/news/gazelle-pytest-conftest.fixed.md @@ -1,2 +1,4 @@ -(gazelle) Made `include_pytest_conftest` annotations deterministic for -multi-source tests and report conflicting explicit values. +(gazelle) Preserve explicitly set `include_pytest_conftest` annotations in +multi-source tests when other source files omit the annotation, and report an +error when explicitly set values conflict +([#3076](https://github.com/bazel-contrib/rules_python/issues/3076)).