Skip to content

fix(ros): make the interop gate count interop tests - #276

Open
YuanYuYuan wants to merge 2 commits into
mainfrom
fix/interop-gate-counts-interop
Open

fix(ros): make the interop gate count interop tests#276
YuanYuYuan wants to merge 2 commits into
mainfrom
fix/interop-gate-counts-interop

Conversation

@YuanYuYuan

@YuanYuYuan YuanYuYuan commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

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:

Summary [47.809s] 125 tests run: 125 passed, 3 skipped
125 ROS interop tests ran against rmw_zenoh_cpp.

Only 41 of those are interop binaries — pubsub_interop 14, demo_nodes 9, service_interop 7, type_description_interop 4, action_interop 4, parameter_interop 3. The other 84 are lifecycle, cache, parameter_tests and 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

  • Count the interop binaries by name; require each to be present. A run of 84 passing non-interop tests now fails, instead of reporting "84 ROS interop tests ran". type_description_interop is 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.
  • Fail when a test self-skipped for a missing ros2 CLI. Each interop test returns early — still passing, still counted — when check_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_available uses Command::new("ros2").output().is_ok(), which is true if the process merely spawned, so a runner with a broken rmw_zenoh_cpp produces a green job full of skips.
  • Gate the run that decided the outcome. The retry path re-ran nextest but the gate parsed the first attempt: a passing retry was validated against discarded output, and a first attempt that died before producing a summary failed an otherwise-green run.
  • Drop the 0 tests run branch. cargo-nextest 0.9.138 defaults --no-tests to fail, so it was unreachable. The repo already knew this — .github/workflows/test.yml passes --no-tests=warn precisely because the default fails.

Output becomes a per-suite breakdown rather than one aggregate.

What fails without this

On main today, the gate's own output is the evidence: it printed 125 ROS interop tests ran against rmw_zenoh_cpp for a run containing 41.

The gate then detected a real empty suite on its own first CI run. The humble job failed with:

ROS interop suites produced no tests: type_description_interop.
A pass here would be vacuous.

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:

input result
healthy jazzy run, all six suites present pass — prints the 41-test breakdown
healthy humble run, five suites pass
84 passing tests, no interop suites fail — names the missing suites
healthy output plus one ros2 CLI not available line fail
empty output fail — no summary

Notes

Found by an adversarial review of the merged PR. The remaining gap, not addressed here: the four Interop tests with ROS 2 * jobs in test.yml run nextest directly and are ungated — one of them produced Summary [0.000s] 0 tests run: 0 passed, 13 skipped and passed, on the very commit that merged #271.

Breaking Changes

None. CI-only; the gate becomes stricter.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant