test(hiroz-tests): improve guard robustness and diagnostics

Author: YuanYuYuan

crates/hiroz-tests/tests/common/mod.rs

@@ -453,8 +453,32 @@ impl Drop for ProducerGuard {
         // immediately (no lingering 40s hold, no reconnect-spin), and the
         // teardown completes in the background (or is reaped at process exit).
         self.stop.store(true, Ordering::Relaxed);
-        // Drop the JoinHandle without joining, detaching the thread.
-        self.handle.take();
+        if let Some(handle) = self.handle.take() {
+            // A well-behaved producer is still running (holding its entities)
+            // right up until we set `stop`, so it will normally NOT be finished
+            // here. If it *is* already finished, the producer exited early —
+            // typically a panic — which would otherwise be swallowed silently and
+            // make a later "entity never appeared" failure baffling. Since the
+            // thread is finished, `join()` returns immediately (still
+            // non-blocking), so we can observe and surface the panic.
+            if handle.is_finished() {
+                if let Err(panic) = handle.join() {
+                    let msg = panic
+                        .downcast_ref::<&str>()
+                        .map(|s| s.to_string())
+                        .or_else(|| panic.downcast_ref::<String>().cloned())
+                        .unwrap_or_else(|| "<non-string panic payload>".to_string());
+                    eprintln!(
+                        "WARNING: test producer thread exited early (before teardown) \
+                         with a panic: {msg}. Downstream 'entity not discovered' \
+                         failures in this test are likely caused by this."
+                    );
+                }
+            }
+            // Otherwise: drop the JoinHandle without joining, detaching the
+            // still-running thread (its Zenoh-session teardown can block, and we
+            // must not block the test thread on it).
+        }
     }
 }
 

crates/hiroz-tests/tests/demo_nodes.rs

@@ -401,7 +401,7 @@ fn test_hiroz_add_two_ints_server_to_rcl_client() {
     wait_for_ready(Duration::from_secs(5));
 
     // Start RCL client
-    let mut client = Command::new("ros2")
+    let client = Command::new("ros2")
         .args(["run", "demo_nodes_cpp", "add_two_ints_client"])
         .env("RMW_IMPLEMENTATION", "rmw_zenoh_cpp")
         .env("ZENOH_CONFIG_OVERRIDE", router.rmw_zenoh_env())
@@ -411,21 +411,31 @@ fn test_hiroz_add_two_ints_server_to_rcl_client() {
         .spawn()
         .expect("Failed to start RCL client");
 
+    // Wrap the child in its ProcessGuard *before* any reaping so the guard owns
+    // it throughout. Polling `try_wait` on a separate handle and only then
+    // handing the (already-reaped) child to a fresh guard would let the guard's
+    // Drop signal a process group whose PID may have been recycled. Poll through
+    // the guard's own handle instead.
+    let mut client_guard = ProcessGuard::new(client, "RCL add_two_ints client");
+
     // Bound the wait on the RCL client's own exit instead of a fixed sleep,
     // so a slow-to-discover run fails fast with a clear message rather than
     // hanging the hiroz server thread (blocked on its one expected request)
     // until nextest's hard kill.
     let client_deadline = std::time::Instant::now() + Duration::from_secs(30);
     let client_status = loop {
-        if let Some(status) = client.try_wait().expect("Failed to poll RCL client") {
+        let child = client_guard
+            .child
+            .as_mut()
+            .expect("client child owned by guard");
+        if let Some(status) = child.try_wait().expect("Failed to poll RCL client") {
             break Some(status);
         }
         if std::time::Instant::now() >= client_deadline {
             break None;
         }
         thread::sleep(Duration::from_millis(200));
     };
-    let _client_guard = ProcessGuard::new(client, "RCL add_two_ints client");
     // Check the exit status is actually success, not just that the process
     // exited -- an instantly-failing `ros2 run` (e.g. missing verb plugin,
     // bad args) also "exits within 30s" and would otherwise false-pass here.

crates/hiroz-tests/tests/parameter_interop.rs

@@ -56,9 +56,15 @@ fn wait_for_node(node_name: &str, router: &TestRouter, timeout: Duration) {
         }
 
         if start.elapsed() > timeout {
+            let discovered: Vec<String> = ctx
+                .graph()
+                .get_node_names()
+                .iter()
+                .map(|(name, ns)| format!("{ns}/{name}"))
+                .collect();
             panic!(
-                "Timed out waiting for node {} after {:?}",
-                node_name, timeout
+                "Timed out waiting for node {} after {:?}; nodes currently discovered: {:?}",
+                node_name, timeout, discovered
             );
         }