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

[Backport Foxy] Increased code coverage and some fixes #848

Merged
merged 42 commits into from
Nov 2, 2020
Merged

[Backport Foxy] Increased code coverage and some fixes #848

merged 42 commits into from
Nov 2, 2020

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Oct 30, 2020

Increased code coverage and some fixes

Related with this other PR ros2/rmw_cyclonedds#262

Blast545 and others added 30 commits October 30, 2020 10:10
@Blast545 @ahcorde
Add tests for rcl package (#668)
e0b92ef 
* Add tests for node_options usage
* Add tests for copying options arguments
* Add bad argument tests for wait sets
* Add tests for wait_set_get_allocator function
* Add test for rcl_take_response function
* Change take_request to match take_response without info
* Change specific test for sequence_number
* Add tests for client take failed
* Remove tests already done in nominal setup
* Add bad arguments tests
* Add test for init returning BAD_ALLOC
* Add test for client get_options
* Add test for already init client
* Add bad argument tests
* Add basic test get_default_domain_id
* Add test for rmw to rcl return code function
* Add test for get_localhost_only
* Add tests for localhost expected usage
* Address peer review comments
* Add test for env variable leading to ULONG_MAX
* Change return values to enum in test
* Fix rcl_get_localhost_only return value
* Address peer review comments
* Add unexpected value to get_localhost
* Add reset_rcl_error after expected fails

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @ahcorde
Add bad arguments tests for coverage (#698)
921c5ff 
* Add test context bad_fini
* Add bad name service tests
* Add default case validation result
* Add bad_arg tests rcl_lexer_lookahead2_accept
* Add bad_arg test rcl_trigger_guard_condition
* Add bad arguments events test
* Add message_lost event test
* Disable failing test
* Address peer review comments
* Remove failing test
* Separate non related API tests
* Add comments to explain error causes
* Use test fixture without params
* Replace nullptr with NULL

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @ahcorde
Add coverage tests for rcl (#703)
3fa8af8 
* Add tests not empty names_and_types
* Add tests graph.c module
* Add tests remap basic usage
* Add more tests remap module
* Add tests ini/fini publisher_node
* Refactor style and naming
* Add nullptr bad arg
* Add tests bad args
* Add test bad_alloc
* Add missing source file
* Remove not working test
* Remove extra rcl_reset_error
* Change ASSERT to EXPECT when fini scope
* Add const for const arguments
* Add missing fini parsed arguments
* Add error string to output
* Add invalid usage comments to test
* Fix lint line length issues
* Fix uncrustify lint
* Modify test to save alloc struct
* Remove internal source file
* Remove case requiring internal def

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @ahcorde
Move rcl_remap_copy to public header (#709)
8858fcc 
* Add test for remap internal functions
* Add function headers
* Make local function public
* Move rcl_remap_copy to public header

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @ahcorde
Make public ini/fini rosout publisher (#704)
f1d889d 
* Add bad param tests ini/fini rosout publisher
* Make ini/fini_publisher_for_node public

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @ahcorde
Add remap needed null check (#711)
608cbb6 
* Add needed extra null check
* Add test for added check
* Add tests for nullptrs

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@clalancette @ahcorde
Make sure to call rcl_arguments_fini at the end of the test.
12ead98 
This just prevents a memory leak.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@Blast545 @ahcorde
Reformat rmw_impl_id_check to call a testable function (#725)
b334288 
* Reformat rmw_impl_id_check to call a testable function
* Reformat to expose the function in the public header
* Reformat style return result
* Expose macro names to be tested with the function checker
* Add test for failing cases of the function
* Set error variable and log in the main caller
* Use format string for logging
* Change name of checker function
* Reset rcl error to avoid overwrite

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @ahcorde
Add mock tests, publisher 95% coverage (#732)
286de92 
* Add mimick test for rcl_publisher_get_subscription_count
* Remove const qualifiers
* Add missing mock suffix
* Improve test description
* Add mock test for rcl_publisher_assert_liveliness
* Add class to init publisher tests
* Add test for mocked rmw_publish
* Add mock test for rcl_publish_serialized
* Mock rcutils_string_map_init to make init fail
* Add mock test making rmw_publisher_get_actual_qos fail
* Add class to ease mimick usage
* Reformat tests to use helper class
* Add mocked rcutils_string_map_init to make init fail
* Add tests mocking loaned functions
* Add mock fail tests for publisher_init
* Add publisher fini fail mock tests
* Add nullptr tests
* Update mocking utilities
* Reformat with macro utility
* Add comments for mocked tests
* Check count_size value after test
* Reformat to use constexpr where possible
* Add variable making clear bad param test
* Add link to original file to help tracking changes

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @ahcorde
Add missing calls to rcl_convert_rmw_ret_to_rcl_ret (#738)
47327f8 
Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @ahcorde
Add deallocate calls to free strdup allocated memory (#737)
e485dae 
* Add deallocate calls to free strdup allocated memory
* Add variables to know if free is required
* Reformat not use extra booleans
* Address peer review comments

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@hidmic @ahcorde
Complete subscription API test coverage (#734)
c595ca1 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@Blast545 @ahcorde
Fix bug error handling get_param_files (#743)
f51f8e5 
* Add bomb allocator
* Fix memory checked for error
* Add tests for bad alloc
* Fix break statement
* Add static keyword to methods added

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@hidmic @ahcorde
Fix rcl package's logging API error specs and handling. (#746)
364e1fb 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@Blast545 @ahcorde
Fix allocation arguments copy (#748)
ad8b2b7 
* Change allocation method for copied parameter files
* Add bad allocator test

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@hidmic @ahcorde
Complete rcl enclave validation API coverage. (#751)
44235ee 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@ahcorde
Added path_to_fail to mocking_utils in rcl
cabc782 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@hidmic @ahcorde
Clean up rcl_expand_topic_name() implementation. (#757)
12e104d 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@Blast545 @ahcorde
Add coverage tests 94% service.c (#756)
f4a51b4 
* Add tests for fail init
* Add fini tests
* Add tests rcl_take_request_with_info
* Add tests send_response
* Change const strings to constexpr
* Improve test descriptions
* Remove test

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@hidmic @ahcorde
Extend rcl_expand_topic_name() API test coverage. (#758)
99efabd 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@Blast545 @ahcorde
Adding tests to arguments.c (#752)
30a9681 
* Add nullptr tests get_param_files
* Add bad alloc tests
* Add missing tests
* Add bad alloc tests rcl_arguments_copy
* Remove repeated test
* Remove spaces
* Restore erased test
* Relocate test
* Refactor bomb allocator test
* Add missing rcl_reset_error() checks

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@hidmic @ahcorde
Zero initialize guard condition on failed init. (#760)
8ee5552 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Do not invalidate context before successful shutdown. (#761)
63842fd 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Yield rcl_context_fini() error codes. (#763)
0c02c8d 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@brawner @ahcorde
Add fault injection macros to rcl functions (#727)
e79b900 
* Add fault injection macros to rcl functions

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

* PR fixup

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@hidmic @ahcorde
Cleanup rcl_get_secure_root() implementation. (#762)
1f9f627 
* Do not assume allocation will succeed.
* Do not leak allocated memory on failure.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Check rcutils_strdup() outcome immediately. (#768)
bfff4c1 
Within rcl_node_init() implementation.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Bump test coverage. (#764)
adfb2e9 
* init/shutdown API
* context fini API
* node init/fini API
* guard condition init/fini API
* security APIs

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@Blast545 @ahcorde
Fix wait allocation cleanup (#770)
e2f6526 
* Add fix wait.c init/fini allocation on error
* Add test for the fix
* Remove unit tests

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@Blast545 @ahcorde
Add coverage tests wait module (#769)
a72ae0a 
* Add coverage tests wait module
* Add fault injection test
* Add zero_init allocator test to init
* Remove bomb allocator test
* Add better handling fault injection
* Add comment to fault injection tests
* Move test to another section

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
hidmic and others added 10 commits October 30, 2020 12:35
@hidmic @ahcorde
Fix rcl arguments' API memory leaks and bugs. (#778)
7ee8e11 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic @ahcorde
Bump rcl arguments' API test coverage. (#777)
ece5251 
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@ahcorde
Fixed log level in backported foxy tests
14d1784 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
calling fini functions to avoid memory leak (#791)
8b9d9cc 
* Fix test memory leaks

1.  calling rcutils_string_map_fini to avoid memory leak
2.  Fix memory leak that not to call rcutils_string_array_fini for enclaves
3.  Fix that not to rcutils_string_array_fini for node_names_2 and node_namespaces_2
4.  Fix that not to rcl_log_levels_fini for copied_log_levels
5.  Fix that not to call rmw_security_options_fini for options
6.  Call test_msgs__srv__BasicTypes_Request__fini for service_request in the end

Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
@clalancette @ahcorde
Fix that not to deallocate event impl in some failure case (#790)
99c96ea 
* Fix that not to deallocate event impl in some failure case

Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette @ahcorde
Fix memory leak because of mock test (#800)
f3a7318 
* Fix memory leak because of mock test

Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
Co-authored-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette @ahcorde
Make sure to check the return value of rcl APIs. (#838)
f9cea34 
This is just a further check to ensure the test is correct,
and also gets rid of a slew of dead store warnings from
clang static analysis.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@jacobperron @ahcorde
Return OK when finalizing zero-initialized contexts (#842)
8d47d7c 
Resolves #814

This is consistent behavior with finalizing other types of zero-initialized objects.

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@jacobperron @ahcorde
Zero initialize events an size_of_events members of rcl_wait_set_t (#841
6222bee 
)

Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@clalancette @ahcorde
Make sure to always check return values. (#840)
caf3a16 
* Make sure to always check return values.

Pointed out by clang static analysis, we should always
check these return values.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@ahcorde ahcorde requested a review from brawner October 30, 2020 12:21
@ahcorde ahcorde self-assigned this Oct 30, 2020
@ahcorde ahcorde mentioned this pull request Oct 30, 2020
Closed
@ahcorde
Copy link
Contributor Author

ahcorde commented Oct 30, 2020 •
edited
Loading

Compiled up-to rcl and testing rcl:

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

@ahcorde
Copy link
Contributor Author

ahcorde commented Nov 2, 2020

  • Windows Build Status

clalancette and others added 2 commits November 2, 2020 09:56
@clalancette @ahcorde
Add a semicolon to RCUTILS_LOGGING_AUTOINIT. (#816)
488d54e 
This makes it look like a C statement.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@dirk-thomas @ahcorde
increase timeouts in test_services fixtures for Connext (#745)
6ac4453 
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
@ahcorde
Copy link
Contributor Author

ahcorde commented Nov 2, 2020

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

@ahcorde ahcorde merged commit 6bbe229 into foxy Nov 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the ahcorde/coverage/foxy branch November 2, 2020 15:22
@brawner brawner mentioned this pull request Nov 10, 2020
Closed
@hidmic hidmic mentioned this pull request Nov 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brawner brawner brawner approved these changes

Assignees

@ahcorde ahcorde

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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