Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make conftest.py special with gazelle #879

Merged
merged 4 commits into from
Nov 10, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
70 changes: 58 additions & 12 deletions gazelle/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const (
pyBinaryEntrypointFilename = "__main__.py"
pyTestEntrypointFilename = "__test__.py"
pyTestEntrypointTargetname = "__test__"
conftestFilename = "conftest.py"
conftestTargetname = "conftest"
)

var (
Expand Down Expand Up @@ -71,6 +73,7 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
// be generated for this package or not.
hasPyTestFile := false
hasPyTestTarget := false
hasConftestFile := false

for _, f := range args.RegularFiles {
if cfg.IgnoresFile(filepath.Base(f)) {
Expand All @@ -81,6 +84,8 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
hasPyBinary = true
} else if !hasPyTestFile && f == pyTestEntrypointFilename {
hasPyTestFile = true
} else if f == conftestFilename {
hasConftestFile = true
} else if strings.HasSuffix(f, "_test.py") || (strings.HasPrefix(f, "test_") && ext == ".py") {
pyTestFilenames.Add(f)
} else if ext == ".py" {
Expand Down Expand Up @@ -196,10 +201,10 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes

pyLibraryTargetName := cfg.RenderLibraryName(packageName)

// Check if a target with the same name we are generating alredy exists,
// and if it is of a different kind from the one we are generating. If
// so, we have to throw an error since Gazelle won't generate it
// correctly.
// Check if a target with the same name we are generating already
// exists, and if it is of a different kind from the one we are
// generating. If so, we have to throw an error since Gazelle won't
// generate it correctly.
if args.File != nil {
for _, t := range args.File.Rules {
if t.Name() == pyLibraryTargetName && t.Kind() != pyLibraryKind {
Expand Down Expand Up @@ -233,10 +238,10 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes

pyBinaryTargetName := cfg.RenderBinaryName(packageName)

// Check if a target with the same name we are generating alredy exists,
// and if it is of a different kind from the one we are generating. If
// so, we have to throw an error since Gazelle won't generate it
// correctly.
// Check if a target with the same name we are generating already
// exists, and if it is of a different kind from the one we are
// generating. If so, we have to throw an error since Gazelle won't
// generate it correctly.
if args.File != nil {
for _, t := range args.File.Rules {
if t.Name() == pyBinaryTargetName && t.Kind() != pyBinaryKind {
Expand Down Expand Up @@ -267,6 +272,43 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
result.Imports = append(result.Imports, pyBinary.PrivateAttr(config.GazelleImportsKey))
}

var conftest *rule.Rule
if hasConftestFile {
deps, err := parser.parseSingle(conftestFilename)
if err != nil {
log.Fatalf("ERROR: %v\n", err)
}

// Check if a target with the same name we are generating already
// exists, and if it is of a different kind from the one we are
// generating. If so, we have to throw an error since Gazelle won't
// generate it correctly.
if args.File != nil {
for _, t := range args.File.Rules {
if t.Name() == conftestTargetname && t.Kind() != pyLibraryKind {
fqTarget := label.New("", args.Rel, conftestTargetname)
err := fmt.Errorf("failed to generate target %q of kind %q: "+
"a target of kind %q with the same name already exists.",
fqTarget.String(), pyLibraryKind, t.Kind())
collisionErrors.Add(err)
}
}
}

conftestTarget := newTargetBuilder(pyLibraryKind, conftestTargetname, pythonProjectRoot, args.Rel).
setUUID(uuid.Must(uuid.NewUUID()).String()).
addSrc(conftestFilename).
addModuleDependencies(deps).
addVisibility(visibility).
setTestonly().
generateImportsAttribute()

conftest = conftestTarget.build()

result.Gen = append(result.Gen, conftest)
result.Imports = append(result.Imports, conftest.PrivateAttr(config.GazelleImportsKey))
}

if hasPyTestFile || hasPyTestTarget {
if hasPyTestFile {
// Only add the pyTestEntrypointFilename to the pyTestFilenames if
Expand All @@ -280,10 +322,10 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes

pyTestTargetName := cfg.RenderTestName(packageName)

// Check if a target with the same name we are generating alredy exists,
// and if it is of a different kind from the one we are generating. If
// so, we have to throw an error since Gazelle won't generate it
// correctly.
// Check if a target with the same name we are generating already
// exists, and if it is of a different kind from the one we are
// generating. If so, we have to throw an error since Gazelle won't
// generate it correctly.
if args.File != nil {
for _, t := range args.File.Rules {
if t.Name() == pyTestTargetName && t.Kind() != pyTestKind {
Expand Down Expand Up @@ -317,6 +359,10 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes
pyTestTarget.addModuleDependency(module{Name: pyLibrary.PrivateAttr(uuidKey).(string)})
}

if conftest != nil {
pyTestTarget.addModuleDependency(module{Name: conftest.PrivateAttr(uuidKey).(string)})
}

pyTest := pyTestTarget.build()

result.Gen = append(result.Gen, pyTest)
Expand Down
10 changes: 10 additions & 0 deletions gazelle/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type targetBuilder struct {
visibility *treeset.Set
main *string
imports []string
testonly bool
}

// newTargetBuilder constructs a new targetBuilder.
Expand Down Expand Up @@ -96,6 +97,12 @@ func (t *targetBuilder) setMain(main string) *targetBuilder {
return t
}

// setTestonly sets the testonly attribute to true.
func (t *targetBuilder) setTestonly() *targetBuilder {
t.testonly = true
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 Expand Up @@ -131,6 +138,9 @@ func (t *targetBuilder) build() *rule.Rule {
if !t.deps.Empty() {
r.SetPrivateAttr(config.GazelleImportsKey, t.deps)
}
if t.testonly {
r.SetAttr("testonly", true)
}
r.SetPrivateAttr(resolvedDepsKey, t.resolvedDeps)
return r
}
1 change: 1 addition & 0 deletions gazelle/testdata/simple_test_with_conftest/BUILD.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
load("@rules_python//python:defs.bzl", "py_library")
27 changes: 27 additions & 0 deletions gazelle/testdata/simple_test_with_conftest/BUILD.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
load("@rules_python//python:defs.bzl", "py_library", "py_test")

py_library(
name = "simple_test_with_conftest",
srcs = [
"__init__.py",
"foo.py",
],
visibility = ["//:__subpackages__"],
)

py_library(
name = "conftest",
testonly = True,
srcs = ["conftest.py"],
visibility = ["//:__subpackages__"],
)

py_test(
name = "simple_test_with_conftest_test",
srcs = ["__test__.py"],
main = "__test__.py",
deps = [
":conftest",
":simple_test_with_conftest",
],
)
4 changes: 4 additions & 0 deletions gazelle/testdata/simple_test_with_conftest/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Simple test with conftest.py

This test case asserts that a simple `py_test` is generated as expected when a
`conftest.py` is present.
1 change: 1 addition & 0 deletions gazelle/testdata/simple_test_with_conftest/WORKSPACE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# This is a Bazel workspace for the Gazelle test data.
3 changes: 3 additions & 0 deletions gazelle/testdata/simple_test_with_conftest/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from foo import foo

_ = foo
12 changes: 12 additions & 0 deletions gazelle/testdata/simple_test_with_conftest/__test__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import unittest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the __test__.py file a gazelle convention that automatically becomes the test entrypoint, or is it generated? Reason I'm asking is this file name doesn't seem to match the allowed test patterns in generate, but gets included in the py_test sources.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's special for Gazelle. The following is the main logic (though they are used in some other places).

pyTestEntrypointFilename = "__test__.py"
pyTestEntrypointTargetname = "__test__"

} else if !hasPyTestFile && f == pyTestEntrypointFilename {
hasPyTestFile = true

if hasPyTestFile {
// Only add the pyTestEntrypointFilename to the pyTestFilenames if
// the file exists on disk.
pyTestFilenames.Add(pyTestEntrypointFilename)
}


from __init__ import foo


class FooTest(unittest.TestCase):
def test_foo(self):
self.assertEqual("foo", foo())


if __name__ == "__main__":
unittest.main()
Empty file.
2 changes: 2 additions & 0 deletions gazelle/testdata/simple_test_with_conftest/foo.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def foo():
return "foo"
3 changes: 3 additions & 0 deletions gazelle/testdata/simple_test_with_conftest/test.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
expect:
exit_code: 0