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 added in #271 does not count interop tests. It asserts a non-zero package total, then reports that total 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 tests. Counting test functions in crates/hiroz-tests/tests/:

suite tests
pubsub_interop 14
demo_nodes 9
service_interop 7
type_description_interop 4 (absent on Humble)
action_interop 4
parameter_interop 3

The rest of the package is lifecycle, cache, hu_meter, 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.

What this PR does

One file changes: scripts/test-ros.nu.

change effect
Count the interop binaries by name; require each to be present A run of only non-interop tests now fails and names the missing suites, instead of reporting them as interop
Fail when the output contains ros2 CLI not available A run whose tests skipped or died for a missing ros2 CLI is no longer a pass
Gate the attempt that decided the outcome The retry path re-ran nextest, but the gate parsed the first attempt's output
Drop the 0 tests run branch Unreachable: nextest's --no-tests default is to fail
Print a per-suite breakdown Replaces the single aggregate number

Notes on each:

Suite list is distro-dependent. type_description_interop.rs is #![cfg(not(ros_humble))] (line 11) — Humble has no type description service to interoperate with — so it is required on every distro except Humble. The other five suites are required everywhere. python_interop is deliberately absent: it is #![cfg(feature = "python-interop")], which this command does not enable.

The self-skip check is not uniform across suites, and the message is what makes it detectable. check_ros2_available() is Command::new("ros2").arg("--version").output().is_ok() (crates/hiroz-tests/tests/common/mod.rs:295) — true if the process merely spawned, so a runner with a broken rmw_zenoh_cpp still passes it. Of the suites that call it, only pubsub_interop self-skips silently: it prints Skipping …: ros2 CLI not available and returns, so the test still passes and still counts (lines 116, 198). demo_nodes, service_interop, parameter_interop and type_description_interop panic! with the same substring, so they already fail the run; the gate's check catches both spellings. action_interop has no such guard at all. #271 named this failure mode in its own description and did not close it.

The 0 tests run branch was unreachable. Established by reading, not by executing a zero-test run: .github/workflows/test.yml:167-171 passes --no-tests=warn for the hiroz-union invocation precisely because the default fails the job.

Evidence

The new check fired on a real empty suite, on this branch's first CI run. The Humble job failed with:

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

The old gate had reported that same Humble run as 116 ROS interop tests ran. The cause turned out to be a legitimate distro difference rather than a defect, and is now exempted (47ddff36) — but it is the proof that the detector detects rather than merely never firing.

Beyond that, the gate logic was exercised by feeding recorded nextest output through it. This is a manual check; there is no automated test for the gate in this repo.

input result
healthy jazzy output, all six suites pass — prints the 41-test breakdown
healthy humble output, 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 line

Breaking changes

what changes who is affected before → after action
scripts/test-ros.nu fails when interop suites are absent or self-skipped anyone running the script locally without a working ros2 CLI silent green → explicit failure install/source ROS 2 before running, or run nextest directly

No library, API or message-format change. CI-only; the gate becomes strictly stricter.

Coverage this does not have

gap detail
The four Interop tests with ROS 2 <distro> jobs are ungated .github/workflows/test.yml runs cargo nextest run directly (lines 160-171) and never calls scripts/test-ros.nu. Its hiroz-union invocation passes --no-tests=warn, so a zero-test run there is green by design
The suite list is hand-maintained A new interop suite is not required until someone adds its name to interop_suites
The gate itself is untested Its behaviour was checked by hand against recorded output, not by a test that runs in CI
Per-suite counts are not asserted Only presence is required, so a suite that shrinks from 14 tests to 1 still passes

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