Repository navigation
ci: fail rust_test targets that ran zero tests - #650
Merged
Merged
Conversation
EnmanuelParache
marked this pull request as ready for review
November 2, 2026 18:27
EnmanuelParache
force-pushed
the
zero-test-audit
branch
from
November 2, 2026 18:27
8d25c38 to
c13de5f
Compare
Contributor
Author
|
@rusty1968 with these changes ci can now detect empty rust tests |
chrysh
reviewed
Nov 2, 2026
chrysh
left a comment
Contributor
There was a problem hiding this comment.
I verified the mechanism by running it locally: bazel test --build_event_json_file=... emits targetKind: "rust_test rule", matching _RUST_TEST_KIND exactly, and a fully cached re-run still emits a testResult event naming test.log (with cachedLocally: true), so the docstring's claim about cached results holds. The testResult event also carries a status field, which the second inline comment relies on.
Three comments inline. Two more points that have no line to sit on:
- The first two commits (
bd258d9a4,11fa722a7) are PR 648, so this diff currently shows both PRs. A rebase once 648 lands trims it to the audit. ${PROJECT_ROOT}/.pw_ci_tests_bep.jsonputs a build output into the source tree. Nothing breaks today (the rootBUILD.bazeldeclares no glob, and.gitignorecovers git), but a path under Bazel's output base would keep the workspace clean.
EnmanuelParache
force-pushed
the
zero-test-audit
branch
2 times, most recently
from
November 3, 2026 14:46
316d655 to
9ea94de
Compare
Contributor
Author
|
@chrysh thank you for taking the time reviewing these changes. I pushed a few more commits based on your observations. |
`rust_test` reports PASSED when its binary contains no tests, so a target
whose sources are stripped by `cfg` (or that has no tests at all) looks the
same as one that ran.
- `ci_tests` writes a Bazel Build Event Protocol file to
`${BUILD_ROOT}/pw_ci_tests_bep.json`, under pw's output directory rather
than the source tree.
- New `zero_test_audit` analyzer in the `ci` group runs
`//presubmit:zero_tests` after the builds. It reads each `rust_test`'s
`test.log` from the BEP file and fails if the summed
`test result: ... N passed` count is 0 or no summary line exists. BEP names
the log for cached results too, and pw moves the `bazel-*` symlinks, so
`bazel-testlogs` is not used.
- pw expands `${BUILD_ROOT}` only inside a config's own args, so the analyzer
gets its own config whose `--run_under=env ZERO_TESTS_BEP=...` points at
the `ci_tests_config` build directory. A missing file exits 2, so an
analyzer that runs before `ci_tests` fails instead of passing on stale data.
- A BEP stream without `buildFinished` is rejected as incomplete, and results
whose status is not PASSED are skipped, since Bazel already fails the run
for them and "0 passed; 5 failed" is not "ran no tests".
- Unit tests: `//presubmit:zero_tests_test`.
`//services/pldm:pldm_service_test` and
`//services/spdm/transport-mctp:spdm_transport_mctp_test` have no tests in
their crates; they are allowlisted until the targets are deleted or given
tests.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
EnmanuelParache
force-pushed
the
zero-test-audit
branch
from
November 3, 2026 18:42
bec7a94 to
d220e17
Compare
rusty1968
approved these changes
Nov 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Check 2 of #462. Follows #648 (check 1), which has merged.
rust_testreports PASSED when its binary contains no tests, so a target whose sources are stripped bycfg(or that has no tests at all) looks the same as one that ran.ci_testsnow passes--build_event_json_file=${BUILD_ROOT}/pw_ci_tests_bep.json, so the Bazel Build Event Protocol (BEP) file lands in pw's output directory (out/ci_tests_config/), not the source tree.zero_test_auditanalyzer in thecigroup runs//presubmit:zero_testsafter the builds. It reads eachrust_test'stest.logfrom the BEP file and fails if the summedtest result: ... N passedcount is 0 (or no summary line exists).bazel-testlogs: BEP names the log for cached results too, and pw runs bazel with--symlink_prefix=${BUILD_ROOT}/bazel-, so the symlink isn't where one would expect.${BUILD_ROOT}only inside a config's own args (not in toolargs),canonicalize-flagsrejects--run_env, and--client_envdoes not reach the program. The analyzer therefore has its own config (zero_test_audit_config) whose--run_under=env ZERO_TESTS_BEP=${BUILD_ROOT}/../ci_tests_config/pw_ci_tests_bep.jsonhands the script the path ofci_tests' build directory. This relies on pw laying out each config's build directory as a sibling under--output-dir.buildFinishedis rejected as incomplete, and results whose status is not PASSED are skipped, so a target whose tests all failed is not misreported as having run none (Bazel already fails the run for it).ci_testsfails instead of passing on stale data.//presubmit:zero_tests_test.Existing zero-test targets. The audit found two with no tests in their crates:
//services/pldm:pldm_service_testand//services/spdm/transport-mctp:spdm_transport_mctp_test. They are allowlisted inzero_tests.pyfor now. Better to delete those targets or add tests; I'd like a maintainer's view.Tested. CI ran the audit end to end on an earlier revision of this branch: it ran after
ci_testsand reportedaudited 39 rust_test targets, 0 ran zero tests. Locally,./pw presubmitand the unit tests pass, and./pw zero_test_auditreceives the expanded path. The final squashed and rebased revision has only had the unit tests re-run locally; CI on this PR is its end-to-end run.Not covered. A stale BEP file left in
out/ci_tests_config/by an earlier local run would be re-read if the analyzer is launched alone. CI starts from a clean tree, so this only affects local runs.Does not close #462 on its own; together with #648 it covers both checks proposed there.
🤖 Generated with Claude Code