Annotation: include/ignore conftest

[dougthor42]

🚀 feature request

Relevant Rules

gazelle

Description

Gazelle will automatically add a dep on :conftest to py_test targets if the directory has a conftest.py file.

rules_python/gazelle/python/generate.go

Lines 126 to 127 in 5c68ff9

	} else if f == conftestFilename {
	hasConftestFile = true

rules_python/gazelle/python/generate.go

Lines 364 to 394 in 5c68ff9

	var conftest *rule.Rule
	if hasConftestFile {
	deps, _, annotations, 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 err := ensureNoCollision(args.File, conftestTargetname, actualPyLibraryKind); err != nil {
	fqTarget := label.New("", args.Rel, conftestTargetname)
	err := fmt.Errorf("failed to generate target %q of kind %q: %w. ",
	fqTarget.String(), actualPyLibraryKind, err)
	collisionErrors.Add(err)
	}
	conftestTarget := newTargetBuilder(pyLibraryKind, conftestTargetname, pythonProjectRoot, args.Rel, pyFileNames).
	addSrc(conftestFilename).
	addModuleDependencies(deps).
	addResolvedDependencies(annotations.includeDeps).
	addVisibility(visibility).
	setTestonly().
	generateImportsAttribute()
	conftest = conftestTarget.build()
	result.Gen = append(result.Gen, conftest)
	result.Imports = append(result.Imports, conftest.PrivateAttr(config.GazelleImportsKey))
	}

rules_python/gazelle/python/generate.go

Lines 468 to 470 in 5c68ff9

	if conftest != nil {
	pyTestTarget.addModuleDependency(Module{Name: strings.TrimSuffix(conftestFilename, ".py")})
	}

If the file doesn't need conftest.py, this can result in pulling in a large dep tree unnecessarily.

Describe the solution you'd like

Python annotation # gazelle:include_conftest false.

• When false, force-exclude add the dep.
• When true, force-add the dep. This is equivalent to the # gazelle:include_dep :conftest directive.
• Defaults to unset, which is the same as the current behavior

Describe alternatives you've considered

None.

[aignas]

Since conftest.py is a pytest only thing, I would love to have the annotation mention it: # gazelle: include_pytest_conftest false. Is the file useful in other places? I.e. do other test frameworks use the file as well?

[dougthor42]

+1 we can call it include_pytest_conftest.

Is the file useful in other places? I.e. do other test frameworks use the file as well?

Not that I'm aware of on both counts.

---

And really, we probably should disable the auto-include-conftest by default. If a project doesn't use pytest at all but still has a conftest.py file (because... reasons?), it will still be automatically added.

But that's breaking behavior so... maybe not till 2.0 🫤

[aignas]

I think I would love to be able to tell gazelle to:

• Add conftest.py to py_test or py_library(testonly=True) srcs.
• Always add @pypi//pytest_bazel to py_test deps.
• Set main_module to pytest_bazel to each py_test

[dougthor42]

• Add conftest.py to py_test or py_library(testonly=True) srcs.

IIRC pytest will not inject conftest.py into PYTHONPATH for non-test files, so a py_library(testonly=True) target won't pick up conftest unless it imports it (meaning Gazelle adds it to deps) or is named test_*.py or *_test.py (and which point Gazelle should be making a py_test target instead...).

I guess there could be an edge case where you have something like

py_library(
 name = "foo",
 # there's a gazelle:resolve directive or some other directive that tells Gazelle
 # to make foo_test.py a py_library instead of a py_test target.
 srcs = ["foo_test.py"]
)

but I think that those cases are pretty rare and it's not unreasonable to tell people they need to somehow manually add the conftest dep (via # keep comment, via # gazelle:include_dep :conftest annotation, or via import conftest).

...

...

Oh damn I just now see that you said srcs, not deps. Why do you want conftest.py in srcs?

---

• Always add @pypi//pytest_bazel to py_test deps.
• Set main_module to pytest_bazel to each py_test

TIL about pytest-bazel haha. I'd say that's a separate feature request, as I'll have to investigate it more. I do see you're the main developer for it though 😁

[brandonchinn178]

Thanks for this, I'm looking forward to it! One thing - pytest will "inherit" conftest files higher up in the tree. Should include_pytest_conftest include parents too?

https://docs.pytest.org/en/stable/reference/fixtures.html#conftest-py-sharing-fixtures-across-multiple-files

[dougthor42]

The behavior before this feature was implemented was to not include parent conftest files.

For example

src/
 conftest.py
 foo/
 conftest.py
 bar_test.py

Gazelle will make

# in src/foo/
py_library(
 name = "conftest",
 srcs = ["conftest.py"],
 deps = [
 ..., # note that it does not include //src:conftest"
 ],
)

py_test(
 name = "bar_test",
 srcs = ["bar_test.py"],
 deps = [":conftest"], # note that it does not include //src:conftest"
)

I opted to keep that behavior. Due to the "magic"-ness of conftest.py, it's pretty hard to figure out if a child conftest.py actually needs the parent conftest.py file. Sometimes it does, but other times it doesn't. Thus we offload this to the user.

If src/foo/conftest.py does actually depend on src/conftest.py, then it's up to to user to explicitly state that. Options are:

1. Edit the //src/foo:conftest target's deps, adding //src:conftest with a # keep comment.
2. Add # gazelle:include_dep //src:conftest annotation to src/foo/conftest.py source code
3. Add an explicit import src.conftest to src/foo/conftest.py.