fix(ros): make the interop gate count interop tests - #276
Open
YuanYuYuan wants to merge 2 commits into
Open
Conversation
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.
The suite is `#![cfg(not(ros_humble))]` -- Humble has no type description service to interoperate with -- so it compiles to an empty binary there and contributes no tests. Requiring it on every distro failed a healthy humble run. Every other interop suite stays mandatory everywhere.
This was referenced Jul 29, 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.
Summary
The interop gate merged in #271 does not count interop tests. It asserts a non-zero package total and then reports it as interop:
Only 41 of those are interop binaries —
pubsub_interop14,demo_nodes9,service_interop7,type_description_interop4,action_interop4,parameter_interop3. The other 84 arelifecycle,cache,parameter_testsand friends. Delete every interop test and the gate still prints a confident number.That is the defect #271 was written to remove, reintroduced in the fix.
Key Changes
type_description_interopis exempt on Humble, where the suite is#![cfg(not(ros_humble))]because there is no type description service to interoperate with; every other suite is mandatory on every distro.ros2CLI. Each interop test returns early — still passing, still counted — whencheck_ros2_available()is false. test(ros): fail the interop job when it runs no tests #271 named this failure mode in its own description and did not close it.check_ros2_availableusesCommand::new("ros2").output().is_ok(), which is true if the process merely spawned, so a runner with a brokenrmw_zenoh_cppproduces a green job full of skips.0 tests runbranch. cargo-nextest 0.9.138 defaults--no-teststo fail, so it was unreachable. The repo already knew this —.github/workflows/test.ymlpasses--no-tests=warnprecisely because the default fails.Output becomes a per-suite breakdown rather than one aggregate.
What fails without this
On
maintoday, the gate's own output is the evidence: it printed125 ROS interop tests ran against rmw_zenoh_cppfor a run containing 41.The gate then detected a real empty suite on its own first CI run. The humble job failed with:
That was a correct detection of a genuinely-empty test binary, caught by the new check and invisible to the old one — which had reported the same humble run as
116 ROS interop tests ran. It turned out to be a legitimate distro difference rather than a defect, and is now exempted, but it is the proof the detector detects rather than merely never firing.Verified in five directions against real nextest output:
ros2 CLI not availablelineNotes
Found by an adversarial review of the merged PR. The remaining gap, not addressed here: the four
Interop tests with ROS 2 *jobs intest.ymlrun nextest directly and are ungated — one of them producedSummary [0.000s] 0 tests run: 0 passed, 13 skippedand passed, on the very commit that merged #271.Breaking Changes
None. CI-only; the gate becomes stricter.