From 8e68984af7abd15e50aea6981085e5cacdc7671c Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Thu, 30 Jul 2026 02:29:19 +0800 Subject: [PATCH 1/2] fix(ros): make the interop gate count interop tests 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. --- scripts/test-ros.nu | 97 +++++++++++++++++++++++++++++++-------------- 1 file changed, 68 insertions(+), 29 deletions(-) diff --git a/scripts/test-ros.nu b/scripts/test-ros.nu index dc0e8484..1994ac94 100755 --- a/scripts/test-ros.nu +++ b/scripts/test-ros.nu @@ -73,49 +73,88 @@ def run-ros-interop [] { run-cmd $prebuild_cmd # Try without verbose logging first (faster) - let result = (do -i { run-cmd $cmd --distro $distro | complete }) - - # Always surface the runner's own output. - # - # This used to capture with `complete` and then never print, so a passing - # ROS job logged the nextest command, produced not one line of test output, - # and printed "All ROS 2 tests passed!". That banner was - # unfalsifiable: nextest exits 0 when it runs *zero* tests, and each interop - # test additionally returns early (still passing) when `check_ros2_available` - # says no. Nothing in the log distinguished "57 interop tests passed against - # rmw_zenoh_cpp" from "the binary matched no tests". - print $result.stdout - print $result.stderr - - # If tests failed, retry with trace logging for detailed diagnostics - # This is CRITICAL for debugging interop issues - shows type hashes, key expressions, service calls - if $result.exit_code != 0 { + let first = (do -i { run-cmd $cmd --distro $distro | complete }) + + # Always surface the runner's own output. This used to capture with + # `complete` and never print, so a passing job logged the command and then + # "All ROS 2 tests passed!" with nothing in between. + print $first.stdout + print $first.stderr + + # If tests failed, retry with trace logging for detailed diagnostics. + # This is CRITICAL for debugging interop issues - shows type hashes, key + # expressions, service calls. + let decided = if $first.exit_code != 0 { print "\n⚠️ ROS interop tests failed. Retrying with trace logging..." $env.RUST_LOG = "hiroz=trace,rmw_zenoh_cpp=debug,warn" - run-cmd $cmd --distro $distro + let retry = (do -i { run-cmd $cmd --distro $distro | complete }) + print $retry.stdout + print $retry.stderr + if $retry.exit_code != 0 { + error make {msg: $"ROS interop tests failed. Command: ($cmd)"} + } + # Gate the run that decided the outcome, not the discarded attempt. + $retry + } else { + $first } - # An exit code of 0 is necessary but not sufficient — require evidence that - # tests actually ran. nextest's last line is - # `Summary [ 12.345s] 57 tests run: 57 passed, 0 skipped`. - let summary = ([$result.stdout, $result.stderr] | str join "\n" | lines - | where {|l| $l =~ 'tests run:' }) + let out = ([$decided.stdout, $decided.stderr] | str join "\n") - if ($summary | is-empty) { + # An exit code of 0 is necessary but not sufficient. Three things have to + # hold, and each closes a distinct way this job has read as green while + # proving nothing. + + # 1. A summary exists at all. + if (($out | lines | where {|l| $l =~ 'tests run:'}) | is-empty) { error make { msg: $"ROS interop run produced no nextest summary line, so it is unknown whether any test ran. Command: ($cmd)" } } - let ran = ($summary | last | parse --regex '(?\d+) tests run' | get n.0 | into int) - if $ran == 0 { + # 2. The *interop* suites ran -- not merely `hiroz-tests`. + # + # The previous version asserted a non-zero total, then reported it as + # "N ROS interop tests". That total is the whole package: on a healthy run + # it reads 125, of which only ~41 are interop. Deleting every interop test + # would still have printed a confident number. Count the interop binaries + # by name instead, and require each to be present. + let interop_suites = [ + pubsub_interop + service_interop + action_interop + parameter_interop + type_description_interop + demo_nodes + ] + let counts = ($interop_suites | each {|suite| + { + suite: $suite + n: ($out | lines | where {|l| $l =~ $"hiroz-tests::($suite) "} | length) + } + }) + let missing = ($counts | where n == 0 | get suite) + if ($missing | is-not-empty) { error make { - msg: $"ROS interop run executed 0 tests -- a vacuous pass, not a pass. Command: ($cmd)" + msg: $"ROS interop suites produced no tests: ($missing | str join ', '). A pass here would be vacuous. Command: ($cmd)" } } - print $"\n($ran) ROS interop tests ran against rmw_zenoh_cpp." -} + # 3. No test self-skipped for a missing ros2 CLI. + # + # Each interop test returns early -- still passing, still counted -- when + # `check_ros2_available` is false. That is a pass with no interop in it. + let skipped = ($out | lines | where {|l| $l =~ 'ros2 CLI not available'}) + if ($skipped | is-not-empty) { + error make { + msg: $"ROS interop tests self-skipped because the ros2 CLI was unavailable; they report as passes but exercised nothing. Command: ($cmd)" + } + } + + let total = ($counts | get n | math sum) + print $"\n($total) ROS interop tests ran against rmw_zenoh_cpp:" + for c in $counts { print $" ($c.suite): ($c.n)" } +} # ============================================================================ # Test Suite Configuration # ============================================================================ From 47ddff36d2d99cc1bb36e7791d70de0cc51b6424 Mon Sep 17 00:00:00 2001 From: yuanyuyuan Date: Thu, 30 Jul 2026 02:46:24 +0800 Subject: [PATCH 2/2] fix(ci): exempt type_description_interop on humble 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. --- scripts/test-ros.nu | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/scripts/test-ros.nu b/scripts/test-ros.nu index 1994ac94..2f8dc635 100755 --- a/scripts/test-ros.nu +++ b/scripts/test-ros.nu @@ -119,14 +119,18 @@ def run-ros-interop [] { # it reads 125, of which only ~41 are interop. Deleting every interop test # would still have printed a confident number. Count the interop binaries # by name instead, and require each to be present. - let interop_suites = [ + # + # The list is distro-dependent. `type_description_interop.rs` is + # `#![cfg(not(ros_humble))]` -- Humble has no type description service to + # interoperate with -- so requiring it there would fail a healthy run. + # Every other suite must be present on every distro. + let interop_suites = ([ pubsub_interop service_interop action_interop parameter_interop - type_description_interop demo_nodes - ] + ] | append (if $distro == "humble" { [] } else { [type_description_interop] })) let counts = ($interop_suites | each {|suite| { suite: $suite