Add a publish-and-take test for rmw_take_sequence - #283
Conversation
Signed-off-by: xiaoyang <xy1453417528@gmail.com>
|
Tick the box to add this pull request to the merge queue (same as
|
| } | ||
| total_taken += taken; | ||
| } | ||
| EXPECT_EQ(message_count, total_taken); |
There was a problem hiding this comment.
can we add more info here? e.g., how many were taken before timing out. this would make CI flake triage much easier.
| OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( | ||
| { | ||
| EXPECT_EQ( | ||
| RMW_RET_OK, rmw_message_sequence_fini(&sequence)) << rmw_get_error_string().str; | ||
| }); |
There was a problem hiding this comment.
this ordering leaves dangling pointers in sequence.data? test_msgs__msg__BasicTypes__Sequence__destroy(seq) runs before rmw_message_sequence_fini(&sequence), meaning the fini runs while sequence.data[i] are dangling?
| sub, message_count, &sequence, &info_sequence, &taken, null_allocation); | ||
| ASSERT_EQ(RMW_RET_OK, ret) << rmw_get_error_string().str; | ||
| ASSERT_EQ(taken, sequence.size); | ||
| ASSERT_EQ(taken, info_sequence.size); |
There was a problem hiding this comment.
we can inspect more entries as proper test? it would meaningfully strengthen the test to check, e.g., that info_sequence.data[i].publisher_gid matches the publisher.
There was a problem hiding this comment.
Addressed all three review comments in 1ba23ba:
- The final assertion now reports both
total_takenandmessage_countwhen the deadline is reached, making CI timeout failures easier to diagnose. - The cleanup order now ensures that
rmw_message_sequence_fini(&sequence)runs before the backing message sequence is destroyed, sosequence.datadoes not contain dangling pointers during finalization. - The test now obtains the publisher GID with
rmw_get_gid_for_publisher()and verifies it against every entry returned ininfo_sequence.
Signed-off-by: xiaoyang <xy1453417528@gmail.com>
Summary
rmw_take_sequencetest to publish and receive multiple messages.BasicTypessequence storage matching the subscription type instead of the previousStringsstorage.Testing
Tested commit:
ba9916bEnvironment:
rmw_fastrtps_cppCommands:
Result:
Fixes #176