Add fault injection macros and unit tests to rcl_action by brawner · Pull Request #730 · ros2/rcl

brawner · 2020-07-29T01:39:13Z

Adding fault injection unit tests and macros to rcl_action. This CI will fail without a PR to rmw_fastrtps_cpp, which will come shortly. As will CI tests.

Edit: rmw_fastrtps_cpp bug fix: ros2/rmw_fastrtps#416

Signed-off-by: Stephen Brawner brawner@gmail.com

brawner · 2020-08-12T23:08:09Z

Builds and tests of the following packages rcutils, rcl_logging_spdlog, rosidl_typesupport_c, rosidl_typesupport_cpp, rcl, rcl_action, rcl_lifecycle, rosidl_runtime_c, rmw_dds_common, rmw

Linux
Linux-aarch64
macOS
Windows

brawner · 2020-08-27T18:32:27Z

Rebased this PR on master after merging #727. This depends on ros2/rmw#254

Testing against current ros 2 branches, but with the fault-injection branch for rmw and this PR branch (--packages-select rcl_action)

Linux
Linux-aarch64
macOS
Windows

Signed-off-by: Stephen Brawner <brawner@gmail.com>

brawner · 2020-08-27T20:50:21Z

I believe the failures in these unit tests are addressed by:
ros2/rmw_fastrtps#427
#770

Blast545

Some questions

rcl_action/test/rcl_action/test_action_client.cpp

rcl_action/test/rcl_action/test_action_communication.cpp

rcl_action/test/rcl_action/test_action_server.cpp

rcl_action/test/rcl_action/test_graph.cpp

Signed-off-by: Stephen Brawner <brawner@gmail.com>

Blast545

LGTM, pending green CI

hidmic · 2020-08-28T20:21:39Z

rcl_action/test/rcl_action/test_action_client.cpp

+    rcl_node_t node = rcl_get_zero_initialized_node();
+    rcl_node_options_t node_options = rcl_node_get_default_options();
+    std::string node_name = std::string("test_action_client_node_") + std::to_string(count);
+    rcl_ret_t ret = rcl_node_init(&node, node_name.c_str(), "", &this->context, &node_options);


@brawner meta: should this be outside the loop?

We haven't agreed on this, but I think it'd be good to minimize the overlap between fault injection tests. It's an expensive test and the more stuff we add the harder it is to track it down. WDYT?

Seems reasonable. Moved non rcl_action code outside of this macro loop.

Uh, I don't see it here. Refreshing Github's UI won't do either 🤔

Hmm, phantom code change I see. Fixed again

hidmic · 2020-08-28T20:23:20Z

rcl_action/test/rcl_action/test_action_server.cpp

+  ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
+  rcl_context_t context = rcl_get_zero_initialized_context();
+  ret = rcl_init(0, nullptr, &init_options, &context);
+  ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;


@brawner missing shutdown/fini counterpart?

hidmic · 2020-08-28T20:23:33Z

rcl_action/test/rcl_action/test_action_server.cpp

+  rcl_node_t node = rcl_get_zero_initialized_node();
+  rcl_node_options_t node_options = rcl_node_get_default_options();
+  ret = rcl_node_init(&node, "test_action_server_node", "", &context, &node_options);
+  ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;


@brawner missing fini counterpart?

hidmic · 2020-08-28T20:23:59Z

rcl_action/test/rcl_action/test_action_server.cpp

+  rcl_allocator_t allocator = rcl_get_default_allocator();
+  rcl_init_options_t init_options = rcl_get_zero_initialized_init_options();
+  rcl_ret_t ret = rcl_init_options_init(&init_options, allocator);
+  ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;


@brawner missing fini counterpart?

Signed-off-by: Stephen Brawner <brawner@gmail.com>

hidmic

Overall LGTM!

hidmic · 2020-08-31T14:08:18Z

rcl_action/test/rcl_action/test_action_server.cpp

+    EXPECT_EQ(RCL_RET_OK, rcl_node_fini(&node));
+    EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(&node_options));
+  }
+  );


@brawner uncrustify here?

Uncrustify was happy, but combined lines 997 and 998.

hidmic · 2020-08-31T14:09:58Z

rcl_action/test/rcl_action/test_action_client.cpp

+    rcl_node_t node = rcl_get_zero_initialized_node();
+    rcl_node_options_t node_options = rcl_node_get_default_options();
+    std::string node_name = std::string("test_action_client_node_") + std::to_string(count);
+    rcl_ret_t ret = rcl_node_init(&node, node_name.c_str(), "", &this->context, &node_options);


Uh, I don't see it here. Refreshing Github's UI won't do either 🤔

Signed-off-by: Stephen Brawner <brawner@gmail.com>

brawner · 2020-08-31T18:25:08Z

Above bug fixes I referenced above have been merged. Testing again

Linux
Linux-aarch64
macOS
Windows

brawner · 2020-08-31T19:16:17Z

Rpr job is still waiting for debian package of rcutils, but all ci.ros2.org jobs succeeded.

* Add fault injection macros and unit tests to rcl_action Signed-off-by: Stephen Brawner <brawner@gmail.com> * Addressing feedback Signed-off-by: Stephen Brawner <brawner@gmail.com> * PR Fixup Signed-off-by: Stephen Brawner <brawner@gmail.com> * PR Fixup Signed-off-by: Stephen Brawner <brawner@gmail.com>

brawner mentioned this pull request Jul 29, 2020

Unregister typesupport rather than delete ros2/rmw_fastrtps#416

Closed

brawner self-assigned this Jul 29, 2020

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch 3 times, most recently from 22f5b89 to cbf2ceb Compare August 12, 2020 21:14

brawner force-pushed the brawner/fault-injection-macros branch 2 times, most recently from d516d81 to 0b47bc0 Compare August 12, 2020 21:56

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch from cbf2ceb to 72389f8 Compare August 12, 2020 21:56

brawner force-pushed the brawner/fault-injection-macros branch 2 times, most recently from d46603e to 0c9dfc5 Compare August 12, 2020 22:00

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch 2 times, most recently from 6f74d7d to 95b9d7b Compare August 12, 2020 22:06

brawner force-pushed the brawner/fault-injection-macros branch from 0c9dfc5 to 9fb197f Compare August 12, 2020 22:09

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch from 95b9d7b to 523fc90 Compare August 12, 2020 22:10

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch 3 times, most recently from 52b17b4 to 931c347 Compare August 17, 2020 17:42

brawner mentioned this pull request Aug 17, 2020

Fault injection macros and functionality (plus example) ros2/rcutils#264

Merged

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch from 931c347 to fc4ac47 Compare August 27, 2020 18:26

brawner changed the base branch from brawner/fault-injection-macros to master August 27, 2020 18:27

brawner marked this pull request as ready for review August 27, 2020 18:27

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch from fc4ac47 to 3cbe382 Compare August 27, 2020 18:27

brawner requested review from Blast545 and hidmic August 27, 2020 18:33

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch from 3cbe382 to 8d502e2 Compare August 27, 2020 19:00

Add fault injection macros and unit tests to rcl_action

907cd21

Signed-off-by: Stephen Brawner <brawner@gmail.com>

Blast545 reviewed Aug 27, 2020

View reviewed changes

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch from 8d502e2 to 7e508d4 Compare August 27, 2020 23:11

Addressing feedback

e4f7f0c

Signed-off-by: Stephen Brawner <brawner@gmail.com>

brawner force-pushed the brawner/rcl_action-fault-injection-macros branch from 7e508d4 to e4f7f0c Compare August 27, 2020 23:15

Blast545 approved these changes Aug 28, 2020

View reviewed changes

hidmic reviewed Aug 28, 2020

View reviewed changes

PR Fixup

cddccfb

Signed-off-by: Stephen Brawner <brawner@gmail.com>

hidmic approved these changes Aug 31, 2020

View reviewed changes

PR Fixup

8ee0870

Signed-off-by: Stephen Brawner <brawner@gmail.com>

brawner merged commit 3683c6f into master Aug 31, 2020

delete-merged-branch bot deleted the brawner/rcl_action-fault-injection-macros branch August 31, 2020 19:16

Conversation

brawner commented Jul 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brawner commented Aug 12, 2020

Uh oh!

brawner commented Aug 27, 2020

Uh oh!

brawner commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Blast545 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Blast545 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hidmic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brawner commented Aug 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brawner commented Aug 31, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

brawner commented Jul 29, 2020 •

edited

Loading

brawner commented Aug 27, 2020 •

edited

Loading

Blast545 left a comment •

edited

Loading

brawner commented Aug 31, 2020 •

edited

Loading