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.

Sign up for GitHub

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

Jump to bottom

[foxy backport] Backport all unit tests and bug fixes, feature branch #1383

Merged
merged 61 commits into from
Oct 19, 2020
Merged

[foxy backport] Backport all unit tests and bug fixes, feature branch #1383

merged 61 commits into from
Oct 19, 2020

Conversation

brawner
Copy link
Contributor

@brawner brawner commented Oct 6, 2020 •
edited
Loading

This is the start of PRs to backport unit tests and some associated bug fixes. This PR will be "rebased and merged", so commit history and hashes can be linked.

Edit: This PR will act as the feature branch, and I'll merge other backports for tests into this one. Either rebase and merging if the commits can be taken as is, or squashed if changes are needed to each backport commit. If squashed, I will only be squashing the required changes into the original commit (not squashing multiple backport commits together)

Backported PRs
Bug fix PRs: #1179, #1178, #1211, #1245, #1246, #1251, #1253, #1252, #1270, #1274, #1281, #1291, #1310,
New testing features: #1232
Unit test PRs: #1184, #1181, #1202, #1197, #1198, #1221, #1189, #1222, #1165, #1272, #1308, #1316, #1322, #1321, #1326, #1325

brawner and others added 9 commits October 6, 2020 13:45
@brawner
Throw exception if rcl_timer_init fails (#1179)
a94f6b3 
* Throw exception if rcl_timer_init fails

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

* Add bad-argument tests for GenericTimer

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

* Add comments

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

* Address feedback

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

* Address feedback

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Check period duration in create_wall_timer (#1178)
d932076 
* Check period duration in create_wall_timer

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

* Adding comments

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@hidmic @brawner
Fix rclcpp::NodeOptions::operator= (#1211)
0ee47ef 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@brawner
Add unit tests for logging functionality (#1184)
7cb96ed 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Unit tests for some header-only functions/classes (#1181)
ba38c4f 
* Unit tests for header-only functions/classes

Adds coverage for:
  * any_service_callback.hpp
  * any_subscription_callback.hpp
  * create_subscription.hpp
  * create_timer.hpp

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

* Address PR feedback

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Unit tests for node interfaces (#1202)
0580a3a 
* Unit tests for node interfaces

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

* Address PR Feedback

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

* Address PR feedback

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

* Adjusting comment

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Unit tests for allocator_memory_strategy.hpp (#1197)
81641b6 
* Unit tests for allocator_memory_strategy.hpp

Part 1 of 2 for this file, but part 2 of 3 for memory strategies
overall

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

* PR Fixup

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

* Remove find_package

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

* Remove ref to osrf_testing_tools

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Unit tests for allocator_memory_strategy.cpp part 2 (#1198)
b1c4166 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Add unit test for static_executor_entities_collector (#1221)
9cf088b 
* Add unit test for static_executor_entities_collector

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

* PR Fixup

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner requested review from ahcorde and jacobperron October 6, 2020 20:52
@brawner
Copy link
Contributor Author

brawner commented Oct 6, 2020

Testing --packages-select rclcpp

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

@jacobperron
Copy link
Member

@ros-pull-request-builder retest this please

jacobperron
jacobperron approved these changes Oct 6, 2020
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

I just reviewed for ABI compatibility. I didn't thoroughly review the code.

@brawner
Copy link
Contributor Author

brawner commented Oct 6, 2020

Thanks for checking Jacob!

Even though this is green, I'm going to wait until I open a PR with a few fixes in it.

@brawner
[foxy backport] Derive and throw exception in spin_some spin_all for …
621d3bd 
…StaticSingleThreadedExecutor (#1385)

* Derive and throw exception in spin_some spin_all for StaticSingleThreadedExecutor (#1220)

* Derive and throw exception in spin_some spin_all

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

* Fix style and add unit test

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

* Remove header changes and throw exceptions in .cpp

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner mentioned this pull request Oct 7, 2020
Merged
@brawner
[foxy backport] Parameterize test executors for all executor types (#…
6660139 
…1222) (#1386)

* Parameterize test executors for all executor types (#1222)

* Relocate test_executor to executors directory

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

* Parametrize test_executors for all executor types

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

* PR Fixup

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

* More fixup

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

* Fixup

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

* Adding issue for tracking

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

* Remove tests for non-foxy API

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner mentioned this pull request Oct 7, 2020
Merged
brawner and others added 7 commits October 7, 2020 11:33
@brawner
EXPECT_THROW_EQ and ASSERT_THROW_EQ macros for unittests (#1232)
dbf9a19 
* EXPECT_THROW_EQ and ASSERT_THROW_EQ macros for unittests

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

* Address PR Feedback

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Unittests for memory strategy files, except allocator_memory_strategy (…
d5720da 
…#1189)

* Unit tests for memory_strategy classes (part 1)

Adds unit tests for:
* strategies/message_pool_memory_strategy.hpp
* memory_strategy.cpp
* message_memory_strategy.cpp

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

* Address PR Feedback

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

* Update with new macros

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@dirk-thomas @brawner
fix node graph test with Connext and CycloneDDS returning actual data (…
9c43879 
…#1245)

* fix node graph test with Connext and CycloneDDS returning actual data

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* use ADD_FAILURE()

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@dirk-thomas @brawner
fix failing test with Connext since it doesn't wait for discovery (#1246
fea538f 
)

* fix failing test with Connext since it doesn't wait for discovery

Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>

* Check for added service in the node graph

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

Co-authored-by: Stephen Brawner <brawner@gmail.com>
@brawner
Adjust test_static_executor_entities_collector for rmw_connext_cpp (#…
a5ffa3a 
…1251)

It turns out rmw_connext_cpp adds a default waitable that other rmw
implementations do not. Adjusting the unit test to take this into
account in a non-rmw specific manner.

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Increase timeouts for connext for long tests (#1253)
12aeba2 
* Increase timeouts for connext for long tests

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

* Fix cmakelists

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Simplify and fix allocator memory strategy unit test for connext (#1252)
93839e1 
* Fix allocator memory strategy for connext

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

* PR Feedback

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner mentioned this pull request Oct 7, 2020
Merged
@fujitatomoya @brawner
Ability to configure domain_id via InitOptions. (#1165)
8272a8c 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>

Remove non-foxy api changes

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@fujitatomoya @brawner
initialize_logging_ should be copied. (#1272)
ae4bb5c 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
Blast545 and others added 11 commits October 9, 2020 11:45
@Blast545 @brawner
Add timer coverage tests (#1363)
e1b86e4 
* Add missing tests API
* Reformat style error throw
* Add internal errors tests

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @brawner
Add time API coverage tests (#1347)
580d36b 
* Change value used as max representation
* Add coverage tests time
* Add call to detach clock
* Add tests time
* Add duration construction tests
* Add const qualifier to constants
* Add check clock stays the same
* Make operator RCLCPP_PUBLIC
* Add tests exceptions duration
* Fix division by 0 on windows

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@brawner
Add test for ParameterService (#1355)
1d67840 
* Add test for ParameterService

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

* Address PR feedback

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Only exchange intra_process waitable if nonnull (#1317)
0164217 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@clalancette @brawner
Finish API coverage on executors. (#1364)
c71f8b6 
In particular, add API coverage for spin_node_until_future_complete,
spin_until_future_complete, and spin_node_once.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@brawner
Finish coverage of publisher API (#1365)
85d809c 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Add unit tests for qos and qos_event files (#1352)
5706243 
* Add unit tests for qos and qos_event files

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

* PR Feedback

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

* Address PR Feedback

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

* Fix windows CI

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Tests for LoanedMessage with mocked loaned message publisher (#1366)
01d4d5b 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Increase coverage of guard_condition.cpp to 100% (#1369)
90f0a5e 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Increase coverage of WaitSetTemplate (#1368)
7b7c491 
* Increase coverage of WaitSetTemplate

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

* PR fixup

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Make sure to clean the external client/service handle. (#1296)
a4b0442 
Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
@brawner
Copy link
Contributor Author

brawner commented Oct 9, 2020 •
edited
Loading

Standard tests --packages-up-to rclcpp

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

Cyclonedds only

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

ConnextDds only

  • Linux Build Status
  • macOS Build Status
  • Windows Build Status

Debug build

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

@brawner
Copy link
Contributor Author

brawner commented Oct 12, 2020

PR to adjust timeouts with Connext. These tests are also timing out on master, so this PR targets that and I'll add it to this PR when it's merged.
#1400

@brawner
Copy link
Contributor Author

brawner commented Oct 13, 2020 •
edited
Loading

This also now includes the following PRs:

#1165, #1272, #1329, #1270, #1274, #1281, #1291, #1310, #1308, #1316, #1321, #1326, #1325, #1232, #1189, #1245, #1246, #1251, #1253, #1252, #1336, #1330, #1346, #1343, #1350, #1349, #1344, #1342, #1353, #1358, #1359, #1360, #1363, #1347, #1355, #1317, #1364, #1365, #1352, #1366, #1369, #1368, #1296,

@brawner brawner mentioned this pull request Oct 13, 2020
Merged
@brawner
Copy link
Contributor Author

brawner commented Oct 13, 2020

Merged #1401 into foxy, which hopefully resolves the windows debug issue.

@brawner
Copy link
Contributor Author

brawner commented Oct 14, 2020

I added only the rclcpp bits from #1400 to this PR. The rest will need to backported separately because there were merge conflicts.

@brawner
Copy link
Contributor Author

brawner commented Oct 14, 2020

Standard tests:

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

Connext only:

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

Debug build:

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

@brawner
Increase test timeouts of slow running tests with rmw_connext_cpp (#1400
7060b8b 
)

* Increase test timeouts of slow running tests with rmw_connext_cpp

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

* Fix other issues with connext

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner force-pushed the brawner/rclcpp-backport-tests-part1 branch from 7cb82cb to 7060b8b Compare October 15, 2020 18:19
@brawner brawner mentioned this pull request Oct 15, 2020
Merged
@clalancette @brawner
Clear members for StaticExecutorEntitiesCollector to avoid shared_ptr…
6bd52ae 
… dependency (#1303)

* Clear members for StaticExecutorEntitiesCollector to avoid shared_ptr dependency

Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Contributor Author

brawner commented Oct 16, 2020 •
edited
Loading

Standard tests:

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

Connext only:

  • Linux Build Status
  • macOS Build Status
  • Windows Build Status

Debug:

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

@brawner
Copy link
Contributor Author

brawner commented Oct 16, 2020

Final CI results look good. The failing test with connext is the SubscriptionTopicStatisticsFixture which is an existing issue on rolling. cppcheck times out on macos debug, which is a periodic failure and would have to be addressed over at ament_lint.

I set DCO to pass because all commits are signed off by their respective authors. Github just had trouble figuring that out...

@brawner brawner merged commit 77aae40 into foxy Oct 19, 2020
@delete-merged-branch delete-merged-branch bot deleted the brawner/rclcpp-backport-tests-part1 branch October 19, 2020 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahcorde ahcorde ahcorde approved these changes

@jacobperron jacobperron jacobperron approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@brawner @jacobperron @ahcorde @hidmic @dirk-thomas @fujitatomoya @dabonnie @Blast545 @clalancette