feat: implement pytest_test rule#3931
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new pytest_test rule to support running pytest tests under Bazel, along with corresponding dependency updates and integration tests. Feedback on the changes highlights several critical issues: calling pytest_bazel.main in the bootstrap template will cause an AttributeError as pytest.main should be called instead; the config_settings attribute is unsupported in pytest_test and will cause a loading-phase error; the "pypi" repository is not exported by dev_pip in MODULE.bazel; list concatenation with srcs in pytest_test.bzl will fail if srcs is a select(); template expansion can be simplified by avoiding computed_substitutions; and the bzl_library target for pytest_test is missing its srcs attribute.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| py_test( | ||
| name = name, | ||
| main = main_file, | ||
| srcs = [bootstrap_target] + srcs, |
There was a problem hiding this comment.
If srcs is a select(), concatenating it as [bootstrap_target] + srcs will fail with a Starlark evaluation error because list-plus-select concatenation is not supported (only select-plus-list is supported). Reversing the order to srcs + [bootstrap_target] resolves this and makes the rule compatible with select()s.
| srcs = [bootstrap_target] + srcs, | |
| srcs = srcs + [bootstrap_target], |
There was a problem hiding this comment.
The bootstrap is not needed, we can just use the main_module that already exists in the py_test rule.
| computed_subs = ctx.actions.template_dict() | ||
|
|
||
| computed_subs.add_joined( | ||
| "%TEST_FILES%", | ||
| depset(ctx.files.srcs), | ||
| join_with = "\n", | ||
| map_each = _map_file, | ||
| ) | ||
|
|
||
| ctx.actions.expand_template( | ||
| output = output, | ||
| template = ctx.file._bootstrap_template, | ||
| substitutions = {}, | ||
| computed_substitutions = computed_subs, | ||
| ) |
There was a problem hiding this comment.
Since ctx.files.srcs is already a flat list of files available at analysis time, using computed_substitutions and template_dict is unnecessary and adds extra complexity. We can simplify this by using standard substitutions with a simple string join, which is also more compatible with older Bazel versions.
ctx.actions.expand_template(
output = output,
template = ctx.file._bootstrap_template,
substitutions = {
"%TEST_FILES%": "\n".join([f.short_path for f in ctx.files.srcs]),
},
)
aignas
left a comment
aignas
left a comment
There was a problem hiding this comment.
I need a little bit of time to think how:
- We can handle sharding related optional dependencies.
- We can handle other features that may require extra plugins.
Since pytest_bazel is its own repo, either we have to migrate the code to rules_python or migrate the rule imple to pytest_bazel and then proxy it in rules_python. Let me think about it a little.
Other ideas/thoughts here are welcome.
| py_test( | ||
| name = name, | ||
| main = main_file, | ||
| srcs = [bootstrap_target] + srcs, |
There was a problem hiding this comment.
The bootstrap is not needed, we can just use the main_module that already exists in the py_test rule.
How so? I originally tried that, but it didn't work. I didn't see how it could work in practice -- it would have to somehow auto discover the subset of files to test, but it has no info about what that subset is. |
Implements a new rule to support running Python tests with pytest. Exposes default pytest targets and includes basic tests.
b925c46 to
0dba5a1
Compare
That's OK with me, but: How about I move it to tests/support for now? I'd like to port the tests for e.g. the release tool to pytest, but I'm somewhat blocked on having a functional rule for it. This would also help give us some ideas of what basic usage looks like. |
|
+1 on having it in support for some time. |
It will discover all of the tests that are available in a particular sandbox. I remember those working at some point. And it should ignore any tests in the external dependencies. |
2 participants
This implements a pytest_test rule based upon the pytest_bazel library.
The core implementation of this is fairly simple: a file is generated that calls
pytest.main()with thesrcsto test. This generated file is the file that isrun by
py_test.DO NOT MERGE: Needs discussion. This was quickly thrown together. Haven't put any thought into how to handle conf.py or other pytest flags/settings. Works in the simple example, but haven't vetted it much.