Skip to content

Added missing tests for rclcpp lifecycle#1240

Merged
ahcorde merged 5 commits intomasterfrom
ahcorde/test/add_missing_rclcpp_lifecycle
Aug 12, 2020
Merged

Added missing tests for rclcpp lifecycle#1240
ahcorde merged 5 commits intomasterfrom
ahcorde/test/add_missing_rclcpp_lifecycle

Conversation

@ahcorde
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde commented Jul 27, 2020

  • remove_on_set_parameters_callback
  • notify_graph_change
  • get_service_names_and_types_by_node

Related to this https://ci.ros2.org/job/nightly_linux_coverage/lastBuild/cobertura/src_ros2_rclcpp_rclcpp_lifecycle_src/lifecycle_node_cpp/lifecycle_node_cpp/

Signed-off-by: ahcorde ahcorde@gmail.com

 - remove_on_set_parameters_callback
 - notify_graph_change
 - get_service_names_and_types_by_node

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde added the enhancement New feature or request label Jul 27, 2020
@ahcorde ahcorde requested a review from brawner July 27, 2020 08:54
@ahcorde ahcorde self-assigned this Jul 27, 2020
@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Jul 27, 2020

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner brawner changed the title Added missing test rclcpp lifecycle Added missing tests for rclcpp lifecycle Jul 27, 2020
{
auto node = std::make_shared<LifecycleServiceClient>("client_wait_for_graph_change");
auto * node_graph =
dynamic_cast<rclcpp::node_interfaces::NodeGraph *>(node->get_node_graph_interface().get());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the dynamic_cast necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copy this line from the original test

const auto * node_graph =
dynamic_cast<rclcpp::node_interfaces::NodeGraph *>(node->get_node_graph_interface().get());

Copy link
Copy Markdown
Contributor

@brawner brawner Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That test was for node_interfaces explicitly, and I wanted to verify that the cast to NodeGraph* was doing what I thought it was.

I think for this test, the std::shared_ptr<NodeGraphInterface> from get_node_graph_interface() should be sufficient.

ahcorde added 2 commits July 28, 2020 10:34
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from brawner July 28, 2020 19:46
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Jul 29, 2020

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@ahcorde ahcorde requested a review from brawner August 10, 2020 07:10
Copy link
Copy Markdown
Contributor

@brawner brawner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this review slipped through the cracks

EXPECT_EQ(transitions.size(), 0u);
}


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra line

@ahcorde
Copy link
Copy Markdown
Contributor Author

ahcorde commented Aug 12, 2020

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde force-pushed the ahcorde/test/add_missing_rclcpp_lifecycle branch from 3b4bfba to 49dd1bd Compare August 12, 2020 07:46
@ahcorde ahcorde requested a review from brawner August 12, 2020 08:07
@ahcorde ahcorde merged commit 42682d1 into master Aug 12, 2020
peterpena pushed a commit that referenced this pull request Aug 18, 2020
* Added missing test rclcpp lifecycle
 - remove_on_set_parameters_callback
 - notify_graph_change
 - get_service_names_and_types_by_node

Signed-off-by: ahcorde <ahcorde@gmail.com>

* omit the name of the argument in lambda function

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Removed extra line

Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Pedro Pena <peter.a.pena@gmail.com>
ahcorde added a commit that referenced this pull request Oct 22, 2020
* Added missing test rclcpp lifecycle
 - remove_on_set_parameters_callback
 - notify_graph_change
 - get_service_names_and_types_by_node

Signed-off-by: ahcorde <ahcorde@gmail.com>

* omit the name of the argument in lambda function

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Removed extra line

Signed-off-by: ahcorde <ahcorde@gmail.com>
ahcorde added a commit that referenced this pull request Nov 3, 2020
* Added missing test rclcpp lifecycle
 - remove_on_set_parameters_callback
 - notify_graph_change
 - get_service_names_and_types_by_node

Signed-off-by: ahcorde <ahcorde@gmail.com>

* omit the name of the argument in lambda function

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Added feedback

Signed-off-by: ahcorde <ahcorde@gmail.com>

* Removed extra line

Signed-off-by: ahcorde <ahcorde@gmail.com>
@clalancette clalancette deleted the ahcorde/test/add_missing_rclcpp_lifecycle branch January 15, 2021 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants