bazelbuild / rules_python

Bazel Python Rules
https://rules-python.readthedocs.io
Apache License 2.0
513 stars 521 forks source link

gazelle: generate py_pytest_main #1972

Closed alexeagle closed 1 day ago

alexeagle commented 1 month ago

Related to pytest support #240

The outcome of https://github.com/bazelbuild/rules_python/pull/723 is that pytest-specifics landed in other repos. https://github.com/caseyduquettesc/rules_python_pytest and https://github.com/aspect-build/rules_py/blob/main/docs/rules.md#py_pytest_main are a couple examples of code generators that create the entrypoint file that shims between py_test#entry_point and what pytest expects to run.

[!WARNING] Note, without the shim, py_test(srcs=["some.pytest.test.py"]) silently passes without running any tests!

The something added in this test case https://github.com/bazelbuild/rules_python/blob/main/gazelle/python/testdata/generated_test_entrypoint/BUILD.out#L3 alludes to the real problem, which is that gazelle knows to use that __test__.py as the entrypoint, but doesn't know how to generate the something.

It should generate a py_pytest_main, allowing users to control via map_kind which ruleset they'd like to use to provide that rule.

linzhp commented 2 weeks ago

I think there are two ways to fix this issue:

  1. we can create separate Gazelle extension to generate py_pytest_main with the name __test__. The extension will have to run before the Python extension, so the latter is aware of the newly generated __test__ target and create single py_test target with the main pointing to the it. This is probably less intrusive. The new extension can live in aspect_rules_py together with py_pytest_main.
  2. generating py_pytest_main in the existing Python extension like #2040 attempted to do. However, we need to be careful here. The existing extension will generate a py_test target per test file if no __test__.py or :__test__ target is found. If py_pytest_main is generated, those per-file py_test targets should be removed. We may need to introduce a new Gazelle directive in case people want to generate per-file targets instead.

allowing users to control via map_kind which ruleset they'd like to use to provide that rule.

I am not sure how useful this is. If people decide to use a different rule set to provide py_pytest_main, they may need different attributes, unless we can make a convention that py_pytest_main should be a macro without any attribute besides name = "__test__".

cc @jbedard

alexeagle commented 2 weeks ago

Thanks @linzhp I think that's a good idea for us to put the extension next to the rule that it produces. In aspect configure we pre-compile a gazelle binary, so this wouldn't increase complexity for our users either.

@jbedard is concerned that the order of extensions isn't a very durable abstraction. Are you concerned about it at all? I guess concerns are that the partial ordering constraints are subtle, and nothing verifies that the absolute order satisfies all the invisible constraints

aignas commented 2 weeks ago

An alternative approach that I am looking at right now is https://pypi.org/project/pytest-bazel, it does not require any extra code in the gazelle plugin and should just work with or without gazelle.

linzhp commented 1 week ago

I am not concerned about the partial ordering, as long as it's well-documented. Regular developers don't add or remove Gazelle extensions. Developer platform teams do that occasionally, but when they do that, they don't change the order of existing extensions.

linzhp commented 1 week ago

Proposing another alternative here: #2044