Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increasing test coverage rclcpp_lifecycle #1045

Merged
merged 1 commit into from
Apr 30, 2020

Conversation

brawner
Copy link
Contributor

@brawner brawner commented Mar 31, 2020

As with #1043 and #1044, this PR increases the test coverage for rclcpp_lifecycle. The test coverage increases from 61% to 100% of the include files and 87% of the src files (88% combined)

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

Depends on #1043, #1044
New commit: 7441e1d

@@ -98,6 +98,16 @@ if(BUILD_TESTING)
${PROJECT_NAME}
)
endif()

Copy link
Member

Choose a reason for hiding this comment

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

this PR also adds a lot of tests for rclcpp_action, were those added intentionally here?
Seems that the same tests were added in #1043

set(components "${components}test_rclcpp_components::TestComponentThrows;$<TARGET_FILE:test_component>\n")

# Used to catch invalid resource entry
set(invalid_components "test_rclcpp_components::TestComponentFoo;;$<TARGET_FILE:test_component>\n")
Copy link
Member

Choose a reason for hiding this comment

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

Same about adding tests to rclcpp_components, sounds like a duplicate of #1044.

@brawner
Copy link
Contributor Author

brawner commented Apr 29, 2020

@ivanpauno Because there was a lot of files added in these PRs, I broke them out into three different ones stacked on top of one another. There is no reason why they should be dependent on one another. I'll clean that up.

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Contributor Author

brawner commented Apr 29, 2020

Rebased on master and removed dependency from #1043 and #1044.

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

@brawner brawner merged commit f69b182 into ros2:master Apr 30, 2020

void TearDown() override
{
rclcpp::shutdown();
Copy link
Member

Choose a reason for hiding this comment

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

Calling shutdown from a different thread causes a race which was making this test fail in the nightly jobs since it has been added. See #1204 for the proposed fix.

DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Jul 7, 2020
Signed-off-by: Stephen Brawner <brawner@gmail.com>
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
Signed-off-by: Esteve Fernandez <esteve.fernandez@tier4.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants