Skip to content

Commit

Permalink
fix: make conftest.py special with gazelle (#879)
Browse files Browse the repository at this point in the history
* fix: add conftest.py to py_test generated targets

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>

* fix: use separate py_library for conftest.py

This allows the conftest.py to be used on sub-directories
as pytest would pick them up.

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>

* fix: add testonly to conftest py_library

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>

* fix: testonly is a boolean, not a string

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>

Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com>
  • Loading branch information
f0rmiga committed Nov 10, 2022
1 parent 3f0d62d commit 912a505
Show file tree
Hide file tree
Showing 11 changed files with 121 additions and 12 deletions.
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

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

0 comments on commit 912a505

Please sign in to comment.