Skip to content

Commit 6789bfe

Browse files
Restrict take_event deadlock test to fastrtps and skip when not exercised
The test was registered for every rmw implementation via call_for_each_rmw_implementation(test_api). On non-fastrtps stacks FASTRTPS_DEFAULT_PROFILES_FILE is a no-op and a depth-1 best-effort reader that never takes does not reliably produce SAMPLE_LOST, so events_seen stayed 0 and the final EXPECT_GT(events_seen, 0u) failed for a reason unrelated to any deadlock. Guard the registration with if(rmw_implementation MATCHES "fastrtps") so it only runs for the fastrtps variants, and change the events_seen == 0 guard from EXPECT_GT to GTEST_SKIP so an un-exercised scenario is reported as skipped rather than a misleading pass or a spurious failure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Moore <thomas.moore@torc.ai>
1 parent 6a91217 commit 6789bfe

2 files changed

Lines changed: 25 additions & 14 deletions

File tree

test_rmw_implementation/CMakeLists.txt

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -309,15 +309,21 @@ if(BUILD_TESTING)
309309
)
310310

311311
# Reproduces an AB-BA lock-order inversion between rmw_take_event(MESSAGE_LOST)
312-
# and on_sample_lost. Requires real SAMPLE_LOST, so intra-process delivery is
313-
# disabled via the profile below (Fast DDS only; ignored by other implementations).
314-
ament_add_ros_isolated_gtest_test(test_event_message_lost_deadlock
315-
TEST_NAME test_event_message_lost_deadlock${target_suffix}
316-
TIMEOUT 60
317-
ENV
318-
${rmw_implementation_env_var}
319-
FASTRTPS_DEFAULT_PROFILES_FILE=${CMAKE_CURRENT_SOURCE_DIR}/test/no_intraprocess_profile.xml
320-
)
312+
# and on_sample_lost. This is specific to rmw_fastrtps: it relies on disabling
313+
# intra-process delivery (via the Fast DDS profile below, a no-op for other
314+
# implementations) to force real SAMPLE_LOST. Other implementations do not
315+
# reliably produce SAMPLE_LOST for this pattern, so registering it for them only
316+
# yields false failures; restrict registration to the fastrtps variants. (The test
317+
# itself also skips cleanly if no loss is generated.)
318+
if(rmw_implementation MATCHES "fastrtps")
319+
ament_add_ros_isolated_gtest_test(test_event_message_lost_deadlock
320+
TEST_NAME test_event_message_lost_deadlock${target_suffix}
321+
TIMEOUT 60
322+
ENV
323+
${rmw_implementation_env_var}
324+
FASTRTPS_DEFAULT_PROFILES_FILE=${CMAKE_CURRENT_SOURCE_DIR}/test/no_intraprocess_profile.xml
325+
)
326+
endif()
321327
endfunction()
322328

323329
call_for_each_rmw_implementation(test_api)

test_rmw_implementation/test/test_event_message_lost_deadlock.cpp

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,14 @@ TEST_F(TestEventMessageLostDeadlock, take_event_does_not_deadlock_with_on_sample
210210
EXPECT_EQ(RMW_RET_OK, rmw_destroy_subscription(node, sub));
211211
EXPECT_EQ(RMW_RET_OK, rmw_destroy_publisher(node, pub));
212212

213-
// Guard against a misleading pass: if no loss was ever generated the R->E edge never
214-
// ran. Most commonly this means intra-process delivery was left enabled.
215-
EXPECT_GT(events_seen.load(), 0u)
216-
<< "No SAMPLE_LOST was generated, so the lock-order inversion was not exercised. "
217-
"Ensure intra-process delivery is disabled (see the profile in the CMake env).";
213+
// If no loss was ever generated, the R->E edge never ran, so the scenario was not
214+
// exercised and "no deadlock" proves nothing. This happens when intra-process delivery
215+
// was not disabled, or on an implementation that does not produce SAMPLE_LOST for a
216+
// depth-1 best-effort reader that never takes. Skip cleanly rather than reporting a
217+
// misleading pass or a failure unrelated to any deadlock.
218+
if (events_seen.load() == 0u) {
219+
GTEST_SKIP()
220+
<< "No SAMPLE_LOST was generated, so the lock-order inversion was not exercised. "
221+
"Ensure intra-process delivery is disabled (see the profile in the CMake env).";
222+
}
218223
}

0 commit comments

Comments
 (0)