diff --git a/gazelle/docs/annotations.md b/gazelle/docs/annotations.md index b3f06b9991..596bb7f0f8 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 {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 {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/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..1e54fde93d --- /dev/null +++ b/gazelle/python/testdata/annotation_include_pytest_conftest_conflict/test.yaml @@ -0,0 +1,6 @@ +--- +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..89b2533988 --- /dev/null +++ b/news/gazelle-pytest-conftest.fixed.md @@ -0,0 +1,4 @@ +(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)).