feat(py_test): add opt-in safeguard against accidental no-op tests by thirtyseven · Pull Request #3825 · bazel-contrib/rules_python

thirtyseven · 2026-06-16T16:28:14Z

Summary

Implements #3824.

py_test runs the main module directly and does not automatically invoke a test runner (e.g. unittest/pytest). A common pitfall is to define test classes/functions but forget to run them, so the test silently passes without executing anything.

This adds a validation action to py_test that statically analyzes the main module with ast and fails the build if the module's top-level body is "inert" — only definitions, imports, assignments, and docstrings, with nothing that actually runs tests (such as an if __name__ == "__main__": guard invoking a runner).

Because this is technically a breaking change, it is gated behind a new flag, //python/config_settings:validate_test_main, with values auto/enabled/ disabled. The default is auto, which currently resolves to disabled, and can be flipped to enabled in rules_python 3.0.

Notes/limitations:

Only applies to py_test targets with a main source file; main_module targets are not checked.
Enabling requires the exec-tools toolchain (default for hermetic toolchains); if it's missing, the build fails with a message pointing at the disable flag.

Test plan

Analysis tests (tests/base_rules/py_test) verify the PyValidateTestMain
action and _validation output group are present when enabled and absent
when disabled.
Unit test (tests/validate_test_main) covers the ast classification
(inert vs. runs-something).
Bazel-in-bazel integration test (tests/integration/validate_test_main)
builds real py_test targets with the flag and asserts an inert test fails
the build with a descriptive error when enabled, while a correct test builds;
and that an inert test builds when disabled / by default.

py_test runs the main module directly and does not automatically invoke a test runner. A common pitfall is to define test classes/functions but forget to run them, causing the test to silently pass. Add a validation action, gated behind the new //python/config_settings:validate_test_main flag (auto/enabled/disabled, default off), that statically analyzes the main module with ast and fails the build if its top-level body is inert (only definitions, imports, assignments, docstrings) with nothing that runs tests. Fixes bazel-contrib#3824

Add an integration test that builds real py_test targets with the validate_test_main flag and asserts: - an inert test main fails the build with a descriptive error when enabled - a main that invokes a runner builds when enabled - an inert test builds when disabled and by default

gemini-code-assist

Code Review

This pull request introduces an opt-in safeguard (validate_test_main) to prevent py_test targets from silently passing when they do not actually execute any tests. It implements a static analysis validator using Python's ast module to detect inert main modules and fail the build if no active test runner is invoked. The feedback suggests extending the validator's inert node list to include ast.Global and ast.TypeAlias (for Python 3.12+ compatibility) and adding defensive checks in Starlark to safely access the exec_tools toolchain.

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.

Address review feedback: top-level `global x` and PEP 695 `type X = ...` aliases don't run any test code, so they should count as inert when deciding whether a test main runs something. `ast.TypeAlias` only exists on Python 3.12+, so it's added dynamically.

The uv module extension transitively loads common_labels.bzl, so adding the VALIDATE_TEST_MAIN label changed the extension's bzlTransitiveDigest. Regenerate the integration test's MODULE.bazel.lock to match.

Only flag a main module as inert when it actually defines test classes or functions but never runs them. A module that defines nothing and only imports other modules (e.g. an aggregator that relies on import side effects) is now always allowed, avoiding false positives. Update unit tests, docs, and add an import-only e2e case.

The import-only fixture intentionally has an unused import; add a noqa so ruff (pinned v0.15.14 in CI) passes. The comment doesn't affect the AST, so the module stays import-only for the validator.

gemini-code-assist

Code Review

This pull request introduces an opt-in build-time validation safeguard (validate_test_main) for py_test targets to prevent them from silently passing when they define tests but fail to invoke a test runner. The validation statically analyzes the main module using an AST parser to ensure it contains active statements. The review feedback highlights a critical gap where compound statements like ast.If (e.g., if TYPE_CHECKING:) and ast.Try (e.g., try/except ImportError:) are not recursively checked, allowing inert files to bypass validation. Suggestions were provided to recursively check these blocks and add corresponding unit tests.

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.

Address review feedback: top-level `if TYPE_CHECKING:` and `try: import foo except ImportError:` blocks whose branches are entirely inert previously counted as "active" and let an otherwise-inert test main bypass the check. Inspect `if`/`try`/`try*` branches recursively so these common import guards are correctly treated as inert. The `if` condition is ignored (a guard doesn't run tests; a runner call in a branch body is still detected as active). Add unit tests for TYPE_CHECKING, try/except ImportError, and a runner invoked inside an `if` body.

thirtyseven · 2026-06-16T18:47:50Z

/gemini PTAL

gemini-code-assist

Code Review

This pull request introduces an opt-in safeguard (validate_test_main) for py_test targets to statically analyze the main module and prevent tests from silently passing without executing any test code. It adds a validator script (py_test_main_validator.py) using Python's ast module to detect inert top-level statements, along with corresponding build settings, flags, integration tests, and unit tests. The review feedback correctly identifies a critical issue where all assignments (e.g., exit_code = unittest.main()) are unconditionally treated as inert, which would cause false positives. The reviewer suggests dynamically checking assignments and expressions for active nodes (like function calls) using ast.walk and adding corresponding unit tests to verify this behavior.

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.

Address review feedback: assignments were unconditionally inert, so a main like `exit_code = unittest.main()` (which actually runs the tests) was a false positive that would fail a valid test build. Mark an assignment or expression statement inert only when its value contains no call/await/yield; class/function bodies are still skipped (those node types remain unconditionally inert), so the method-body calls don't defeat the check. Add unit tests for active assignment and active expression mains.

thirtyseven · 2026-06-16T21:19:55Z

/gemini PTAL

gemini-code-assist

Code Review

This pull request introduces an opt-in safeguard (validate_test_main) to prevent py_test targets from silently passing when they define tests but fail to invoke a test runner. It implements a static analysis tool (py_test_main_validator.py) using Python's ast module to check for active statements in the test's main module and integrates this check as a build-time validation action. Feedback on the changes includes explicitly handling ast.Assert statements as inert in the validator (with corresponding unit tests) and appending to the _validation output group in py_executable.bzl rather than overwriting it to prevent conflicts with other validation actions.

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.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Address review feedback: add unit tests for the ast.Assert handling -- an inert `assert True` with a definition is rejected, while an assert whose condition runs a call (e.g. `assert unittest.main()`) is treated as active.

thirtyseven · 2026-06-16T21:38:19Z

/gemini PTAL

gemini-code-assist

Code Review

This pull request introduces an opt-in build-time validation safeguard for py_test targets to prevent them from silently passing when no tests are actually executed. It adds the --@rules_python//python/config_settings:validate_test_main flag and a static analysis tool (py_test_main_validator.py) that parses the test's main module using the ast module to ensure it contains active statements (like invoking a test runner) rather than only inert definitions or imports. The feedback points out a potential false positive where a test runner is invoked directly inside an if condition (e.g., if unittest.main().wasSuccessful():), and suggests updating the validator to check if the condition expression itself runs code.

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.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

The if handling now also checks the condition for calls; update the comment which still claimed the condition was ignored.

aignas

The approach in general is good and goes a long way towards the rules_python being less surprising and #3594 not being as urgent.

Since this is something that may have a big performance penalty if there are a lot of actions already, I am curious in how this could be improved. The only way would be an aspect collecting all files and then running a single action against all of the test files, but that is not necessarily more performant as it is not cache friendly.

What is more, if the users are using unittest.main or something similar, they could actually just add it via main_module to work-around the penalty for most of the cases.

@rickeylev, any ideas here?

thirtyseven · 2026-06-29T16:55:56Z

Since this is something that may have a big performance penalty if there are a lot of actions already, I am curious in how this could be improved. The only way would be an aspect collecting all files and then running a single action against all of the test files, but that is not necessarily more performant as it is not cache friendly.

The cost is just one additional action per py_test, right? It's a validation action, so it won't actually be in the critical path for the py_test and can run in parallel. The set of inputs is just the single main entrypoint file.

What is more, if the users are using unittest.main or something similar, they could actually just add it via main_module to work-around the penalty for most of the cases.

True, but running unittest as a module will do automatic test discovery from runfiles vs unittest.main() which just runs the current script's tests. Probably not desired in most cases. We could implement a different escape hatch though.

# Conflicts: # python/private/BUILD.bazel # python/private/common_labels.bzl # tests/integration/BUILD.bazel # tests/integration/bzlmod_lockfile/MODULE.bazel.lock

Only the validate_test_main entry should be added; the merge had reintroduced two locale-dependent line reorderings.

The uv extension's bzlTransitiveDigest depends on rules_python's .bzl files, which this branch changed. Hand-picking main's digest during the merge left it stale, failing bzlmod_lockfile_test (--lockfile_mode=error). Regenerated via 'bazel mod deps --lockfile_mode=update'.

aignas

LGTM, just the changelog and we are ready for merging this one. Thank you very much for this. :)

Per reviewer request, use the news/ snippet approach instead of editing CHANGELOG.md directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

thirtyseven · 2026-07-01T07:04:54Z

LGTM, just the changelog and we are ready for merging this one. Thank you very much for this. :)

Done, thanks for the review! One long term maintainability issue I can think of before this goes in is that if this check ever gets tightened over time, it could cause existing py_tests to fail. I don't think it's that big of a deal, since:

it's opt-in currently
it's not dissimilar to how rules_java pulls in new Error Prone checks in minor versions via java_tools upgrades, so there is some precedent in Bazel world for this
it should remain a high SNR check and only catch legitimately erroneous code that was previously missed

thirtyseven added 2 commits June 16, 2026 09:21

thirtyseven requested review from aignas and rickeylev as code owners June 16, 2026 16:28