test(ros): fail the interop job when it runs no tests - #271
Merged
Conversation
The ROS interop step captured nextest's output with `complete` and never printed it, so a green job showed the command echo followed by "All ROS 2 <distro> tests passed!" and nothing in between. That banner could not be falsified: nextest exits 0 having run zero tests, and each interop test returns early -- still passing -- when check_ros2_available says no. Print the captured output and require a nextest summary reporting a non-zero count. Split out of #250, where it rode along because the swallowed output was found while gathering evidence for that fix. It is CI hygiene and has nothing to do with subscriber re-entrancy.
This was referenced Jul 29, 2026
YuanYuYuan
added a commit
that referenced
this pull request
Aug 5, 2026
This branch predates #271 and carried an older scripts/test-ros.nu. Rebasing replayed it, deleting the two `print` lines #271 added -- so merging would have restored a banner that cannot fail: nextest exits 0 when it runs zero tests, and without the output nothing distinguishes 57 passing interop tests from a binary that matched none. Restores the file to main's version. The extraction commit's own message says it split the CI gate out to #271; the file did not follow.
YuanYuYuan
added a commit
that referenced
this pull request
Aug 5, 2026
This branch predates #271 and carried an older scripts/test-ros.nu. Rebasing replayed it, deleting the two `print` lines #271 added -- so merging would have restored a banner that cannot fail: nextest exits 0 when it runs zero tests, and without the output nothing distinguishes 57 passing interop tests from a binary that matched none. Restores the file to main's version. The extraction commit's own message says it split the CI gate out to #271; the file did not follow.
YuanYuYuan
added a commit
that referenced
this pull request
Aug 6, 2026
This branch predates #271 and carried an older scripts/test-ros.nu. Rebasing replayed it, deleting the two `print` lines #271 added -- so merging would have restored a banner that cannot fail: nextest exits 0 when it runs zero tests, and without the output nothing distinguishes 57 passing interop tests from a binary that matched none. Restores the file to main's version. The extraction commit's own message says it split the CI gate out to #271; the file did not follow.
YuanYuYuan
added a commit
that referenced
this pull request
Aug 14, 2026
This branch predates #271 and carried an older scripts/test-ros.nu. Rebasing replayed it, deleting the two `print` lines #271 added -- so merging would have restored a banner that cannot fail: nextest exits 0 when it runs zero tests, and without the output nothing distinguishes 57 passing interop tests from a binary that matched none. Restores the file to main's version. The extraction commit's own message says it split the CI gate out to #271; the file did not follow.
YuanYuYuan
added a commit
that referenced
this pull request
Aug 14, 2026
The gate added in #271 asserted a non-zero total and then reported it as "N ROS interop tests ran against rmw_zenoh_cpp". That total is the whole hiroz-tests package. On a healthy run it printed 125, of which only 41 are interop -- the rest are lifecycle, cache, parameter_tests and friends. Deleting every interop test would still have printed a confident number. That is the defect #271 existed to remove, reintroduced in the fix. Three changes: - Count the six interop binaries by name and require each to be present. A run with 84 passing non-interop tests now fails instead of reporting "84 ROS interop tests ran". - Fail when any test self-skipped for a missing ros2 CLI. Those return early, still pass, and are still counted -- a pass with no interop in it. #271 named this failure mode in its own description and did not close it. - Gate the run that decided the outcome. The retry path re-ran nextest but the gate still parsed the first attempt, so a passing retry was validated against the discarded output, and a first attempt that died before producing a summary failed an otherwise-green run. Also drops the `0 tests run` branch: cargo-nextest 0.9.138 defaults --no-tests to fail, so it was unreachable. The repo already knew this -- test.yml passes --no-tests=warn precisely because the default fails. Proven in four directions: healthy output passes; 84 non-interop tests fail; a self-skip line fails; empty output fails.
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
1 participant
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.
Summary
The ROS interop step captured nextest's output with
completeand never printed it, so a passing job logged the command and then✅ All ROS 2 <distro> tests passed!with nothing in between.That banner could not be falsified. nextest exits 0 when it runs zero tests, and every interop test additionally returns early — still passing — when
check_ros2_availablesays no. Nothing in the log distinguished "137 interop tests passed against rmw_zenoh_cpp" from "the binary matched no tests".Key Changes
Print the captured output, and require a nextest summary reporting a non-zero count. A run that produces no summary line, or reports
0 tests run, is now a hard error rather than a pass.What fails without this
Before, the entire step output between the echoed command and the success banner was empty. After, the same job prints every test and its own assertion:
The gate was checked in all three directions before being relied on: a 57-test summary passes, a
0 tests runsummary errors, and absent output errors.Notes
Split out of #250, where it rode along because the swallowed output was found while gathering evidence for that fix. It is CI hygiene and has nothing to do with subscriber re-entrancy.
Breaking Changes
None.