test(ros): fail the interop job when it runs no tests

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. Also correct two doc claims in pubsub.rs: the dispatcher
and queue-mode capacities are not the same expression (they differ at a
zero depth, harmlessly -- now pinned by two queue tests), and catch_unwind
around a user callback is inert under the abort-on-panic opt profile.

Author: YuanYuYuan

crates/hiroz/src/pubsub.rs

@@ -110,16 +110,23 @@ pub(crate) const DISPATCH_UNBOUNDED: usize = usize::MAX;
 
 /// The dispatcher capacity implied by a subscriber's history QoS.
 ///
-/// Deliberately the *same* expression [`ZSubBuilder::build`] uses to size the
-/// queue-mode [`BoundedQueue`]: `KeepLast(depth)` keeps `depth`, `KeepAll` keeps
-/// everything. A callback subscriber and a queue subscriber declared with the
-/// same QoS therefore retain the same number of undelivered samples, which is
-/// the only reading of ROS `KEEP_LAST(depth)` that does not depend on which
-/// hiroz API the user happened to pick.
+/// Matches what [`ZSubBuilder::build`] gives the queue-mode [`BoundedQueue`]:
+/// `KeepLast(depth)` keeps `depth`, `KeepAll` keeps everything. A callback
+/// subscriber and a queue subscriber declared with the same QoS therefore retain
+/// the same number of undelivered samples, which is the only reading of ROS
+/// `KEEP_LAST(depth)` that does not depend on which hiroz API the user happened
+/// to pick.
 ///
-/// A zero depth (the rmw spelling of "system default", which cannot be produced
-/// through [`QosProfile`] but can arrive over the wire) is floored at 1 rather
-/// than being allowed to degenerate into "keep nothing".
+/// The two are *not* the same expression, and the difference is confined to a
+/// zero depth (the rmw spelling of "system default", which cannot be produced
+/// through [`QosProfile`] but can arrive over the wire). Here it is floored at 1
+/// rather than degenerating into "keep nothing"; the queue path passes the 0
+/// through. Retention still agrees, because [`BoundedQueue::push`] evicts before
+/// it inserts (`len >= capacity` → `pop_front`, then `push_back`), so a capacity
+/// of 0 also retains exactly one sample — see `queue::tests::
+/// zero_capacity_retains_one_sample`. What differs is bookkeeping, not data: at
+/// capacity 0 every push reports a drop, including the first one into an empty
+/// queue.
 pub(crate) fn dispatch_capacity(qos: &hiroz_protocol::qos::QosProfile) -> usize {
     match qos.history {
         QosHistory::KeepLast(depth) => depth.max(1),
@@ -394,6 +401,16 @@ impl CallbackDispatcher {
                 while let Some(sample) = drain_queue.dequeue() {
                     // A panicking user callback must not kill the drain thread —
                     // that would silently stop all further delivery.
+                    //
+                    // This holds only where panics unwind. Under `panic = "abort"`
+                    // — which this workspace's `[profile.opt]` sets — the panic
+                    // aborts the process before `catch_unwind` can return `Err`,
+                    // so neither the recovery below nor the log line happens. The
+                    // guard is therefore effective for dev, test and `release`
+                    // builds (including everything CI runs) and inert for `opt`.
+                    // That is a deliberate consequence of choosing `abort` for
+                    // that profile, not an oversight here: a build that opts into
+                    // aborting on panic has opted out of surviving one.
                     if std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| (*handler)(sample)))
                         .is_err()
                     {

crates/hiroz/src/queue.rs

@@ -114,3 +114,43 @@ impl<T> BoundedQueue<T> {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// A zero capacity retains one sample, not zero.
+    ///
+    /// `pubsub::dispatch_capacity` floors a zero history depth at 1 while the
+    /// queue-mode path passes the 0 straight through, so the two sizing
+    /// expressions differ. This pins the reason that divergence is harmless:
+    /// `push` evicts *before* it inserts, so capacity 0 behaves as capacity 1
+    /// for retention. If `push` is ever reordered to insert-then-evict, a
+    /// zero-depth queue starts discarding every sample and this fails.
+    #[test]
+    fn zero_capacity_retains_one_sample() {
+        let q = BoundedQueue::new(0);
+
+        assert!(q.push(1), "capacity 0 reports a drop even on the first push");
+        assert_eq!(q.len(), 1, "capacity 0 must retain one sample, not zero");
+
+        q.push(2);
+        assert_eq!(q.len(), 1);
+        assert_eq!(q.try_recv(), Some(2), "the newest sample is the one kept");
+        assert!(q.is_empty());
+    }
+
+    /// The capacity-1 comparison the doc claims equivalence against: same
+    /// retention, but no spurious drop report on the first push.
+    #[test]
+    fn capacity_one_retains_one_sample_without_reporting_a_drop() {
+        let q = BoundedQueue::new(1);
+
+        assert!(!q.push(1), "an empty capacity-1 queue must not report a drop");
+        assert_eq!(q.len(), 1);
+
+        assert!(q.push(2), "the second push evicts the first");
+        assert_eq!(q.len(), 1);
+        assert_eq!(q.try_recv(), Some(2));
+    }
+}

scripts/test-ros.nu

@@ -75,13 +75,45 @@ def run-ros-interop [] {
     # 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 <distro> 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 {
         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
     }
+
+    # 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:' })
+
+    if ($summary | 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 '(?<n>\d+) tests run' | get n.0 | into int)
+    if $ran == 0 {
+        error make {
+            msg: $"ROS interop run executed 0 tests -- a vacuous pass, not a pass. Command: ($cmd)"
+        }
+    }
+    print $"\n($ran) ROS interop tests ran against rmw_zenoh_cpp."
 }
 
 # ============================================================================