Add tests for unsupported qos checks on initialize #5

ross-desmond · 2019-04-02T23:55:00Z

Signed-off-by: Ross Desmond 44277324+ross-desmond@users.noreply.github.com

Issue #, if available:

Description of changes:
Modify tests for checking unsupported.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

rcl/src/rcl/event.c

rcl/test/rcl/test_events.cpp

emersonknapp · 2019-04-03T00:09:22Z

Mostly looks fine - nitpicking around

Signed-off-by: Ross Desmond <44277324+ross-desmond@users.noreply.github.com>

ross-desmond · 2019-04-03T00:56:31Z

@emersonknapp requested changes complete

rcl/src/rcl/event.c

nburek

One question about our return types, but I don't think it's a blocker for these.

nburek · 2019-04-03T15:14:17Z

rcl/test/rcl/test_events.cpp

+    rmw_time_t lifespan {1, 0};
+    rmw_time_t lease_duration {1, 0};
+    rmw_qos_liveliness_policy_t liveliness_policy = RMW_QOS_POLICY_LIVELINESS_AUTOMATIC;
+    EXPECT_EQ(RMW_RET_ERROR,


I would expect all of these to be receiving back RMW_RET_UNSUPPORTED if they're failing because it is unsupported. Are we masking that error code at some layer?

https://github.com/aws-robotics/rmw/blob/qos_reviewed/rmw/include/rmw/ret_types.h#L31

That will not be possible, since the init function does not return RMW_RET_T, but instead a pointer: nullptr if unable to initialize, or a filled ptr.

We could always change the function prototype 🗡️

Yeah, we can add an issue to the backlog to add a return error code from this function. It's certainly not a blocker for this PR.

Call rcutils_logging_shutdown() in TearDown to avoid leaking logging internal objects. As any change to this file makes test_logging_named fail, this releaxes the EXACT_EQ call on g_last_log_event.location->line_number[1]. Instead of looking for a particular value, we test an invariant of the field (line numbers are positive). Finally, in test_logging_function, initialize g_counter and g_function_called values to make sure the test will not fail even if `--gtest_repeat=2` is passed. Other tests are still failing when this flag is passed: [ FAILED ] TestLoggingMacros.test_logging_once [ FAILED ] TestLoggingMacros.test_logging_skipfirst [ FAILED ] TestLoggingMacros.test_logging_skipfirst_throttle However, fixing those tests would require actually changing the logging implementation which is out of the scope of this change. Before: $ ./test_logging_macros Running main() from gmock_main.cc [==========] Running 8 tests from 1 test case. [----------] Global test environment set-up. [----------] 8 tests from TestLoggingMacros [ RUN ] TestLoggingMacros.test_logging_named [ OK ] TestLoggingMacros.test_logging_named (0 ms) [ RUN ] TestLoggingMacros.test_logging_once [ OK ] TestLoggingMacros.test_logging_once (0 ms) [ RUN ] TestLoggingMacros.test_logging_expression [ OK ] TestLoggingMacros.test_logging_expression (0 ms) [ RUN ] TestLoggingMacros.test_logging_function [ OK ] TestLoggingMacros.test_logging_function (0 ms) [ RUN ] TestLoggingMacros.test_logging_skipfirst [ OK ] TestLoggingMacros.test_logging_skipfirst (0 ms) [ RUN ] TestLoggingMacros.test_logging_throttle [ OK ] TestLoggingMacros.test_logging_throttle (302 ms) [ RUN ] TestLoggingMacros.test_logging_skipfirst_throttle [ OK ] TestLoggingMacros.test_logging_skipfirst_throttle (302 ms) [ RUN ] TestLoggingMacros.test_logger_hierarchy [ OK ] TestLoggingMacros.test_logger_hierarchy (0 ms) [----------] 8 tests from TestLoggingMacros (605 ms total) [----------] Global test environment tear-down [==========] 8 tests from 1 test case ran. (605 ms total) [ PASSED ] 8 tests. ================================================================= ==19903==ERROR: LeakSanitizer: detected memory leaks Direct leak of 504 byte(s) in 7 object(s) allocated from: #0 0x7fd640c1fb50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50) #1 0x7fd64091cbb5 in __default_allocate (/home/.../ros2_ws/install-asan/rcutils/lib/librcutils.so+0x8bb5) #2 0x7fd64092f1b0 in rcutils_string_map_init (/home/.../ros2_ws/install-asan/rcutils/lib/librcutils.so+0x1b1b0) #3 0x7fd640926641 in rcutils_logging_initialize_with_allocator (/home/.../ros2_ws/install-asan/rcutils/lib/librcutils.so+0x12641) #4 0x7fd6409261ab in rcutils_logging_initialize (/home/.../ros2_ws/install-asan/rcutils/lib/librcutils.so+0x121ab) #5 0x564405fc6627 in TestLoggingMacros::SetUp() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x29627) #6 0x564406045999 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0xa8999) #7 0x564406037b7d in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x9ab7d) #8 0x564405fe4503 in testing::Test::Run() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x47503) #9 0x564405fe59e4 in testing::TestInfo::Run() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x489e4) ros2#10 0x564405fe6588 in testing::TestCase::Run() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x49588) ros2#11 0x564406001699 in testing::internal::UnitTestImpl::RunAllTests() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x64699) ros2#12 0x56440604844c in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0xab44c) ros2#13 0x564406039e46 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x9ce46) ros2#14 0x564405ffe42d in testing::UnitTest::Run() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x6142d) ros2#15 0x564405fd197d in RUN_ALL_TESTS() (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x3497d) ros2#16 0x564405fd18c3 in main (/home/.../ros2_ws/build-asan/rcutils/test_logging_macros+0x348c3) ros2#17 0x7fd63fd84b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96) After: $ ./test_logging_macros Running main() from gmock_main.cc [==========] Running 9 tests from 1 test case. [----------] Global test environment set-up. [----------] 9 tests from TestLoggingMacros [ RUN ] TestLoggingMacros.test_empty [ OK ] TestLoggingMacros.test_empty (0 ms) [ RUN ] TestLoggingMacros.test_logging_named [ OK ] TestLoggingMacros.test_logging_named (0 ms) [ RUN ] TestLoggingMacros.test_logging_once [ OK ] TestLoggingMacros.test_logging_once (0 ms) [ RUN ] TestLoggingMacros.test_logging_expression [ OK ] TestLoggingMacros.test_logging_expression (0 ms) [ RUN ] TestLoggingMacros.test_logging_function [ OK ] TestLoggingMacros.test_logging_function (0 ms) [ RUN ] TestLoggingMacros.test_logging_skipfirst [ OK ] TestLoggingMacros.test_logging_skipfirst (0 ms) [ RUN ] TestLoggingMacros.test_logging_throttle [ OK ] TestLoggingMacros.test_logging_throttle (303 ms) [ RUN ] TestLoggingMacros.test_logging_skipfirst_throttle [ OK ] TestLoggingMacros.test_logging_skipfirst_throttle (302 ms) [ RUN ] TestLoggingMacros.test_logger_hierarchy [ OK ] TestLoggingMacros.test_logger_hierarchy (0 ms) [----------] 9 tests from TestLoggingMacros (605 ms total) [----------] Global test environment tear-down [==========] 9 tests from 1 test case ran. (606 ms total) [ PASSED ] 9 tests. [1] https://testing.googleblog.com/2015/01/testing-on-toilet-change-detector-tests.html Signed-off-by: Thomas Moulard <tmoulard@amazon.com>

Add rcutils_logging_shutdown() every time needed in test_logging.cpp. Remove spurious `g_rcutils_logging_initialized = false;` preventing rcutils_logging_shutdown() from freeing memory. Before: $ ./test_logging Running main() from ../../../install-asan/gtest_vendor/src/gtest_vendor/src/gtest_main.cc [==========] Running 5 tests from 1 test case. [----------] Global test environment set-up. [----------] 5 tests from TestLogging [ RUN ] TestLogging.test_logging_initialization [ OK ] TestLogging.test_logging_initialization (0 ms) [ RUN ] TestLogging.test_logging [ OK ] TestLogging.test_logging (0 ms) [ RUN ] TestLogging.test_log_severity [ OK ] TestLogging.test_log_severity (0 ms) [ RUN ] TestLogging.test_logger_severities [ OK ] TestLogging.test_logger_severities (0 ms) [ RUN ] TestLogging.test_logger_severity_hierarchy [ OK ] TestLogging.test_logger_severity_hierarchy (0 ms) [----------] 5 tests from TestLogging (0 ms total) [----------] Global test environment tear-down [==========] 5 tests from 1 test case ran. (0 ms total) [ PASSED ] 5 tests. ================================================================= ==1676==ERROR: LeakSanitizer: detected memory leaks Direct leak of 72 byte(s) in 1 object(s) allocated from: #0 0x7f7a7229db50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50) #1 0x7f7a71f9abb5 in __default_allocate (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x8bb5) #2 0x7f7a71fad1b0 in rcutils_string_map_init (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x1b1b0) #3 0x7f7a71fa4641 in rcutils_logging_initialize_with_allocator (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x12641) #4 0x7f7a71fa41ab in rcutils_logging_initialize (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x121ab) #5 0x561e9d22c972 in TestLogging_test_logging_Test::TestBody() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x1a972) #6 0x561e9d2b545d in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0xa345d) #7 0x561e9d2a71c7 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x951c7) #8 0x561e9d2534c9 in testing::Test::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x414c9) #9 0x561e9d2548f4 in testing::TestInfo::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x428f4) ros2#10 0x561e9d255498 in testing::TestCase::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x43498) ros2#11 0x561e9d2705a9 in testing::internal::UnitTestImpl::RunAllTests() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x5e5a9) ros2#12 0x561e9d2b7f10 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0xa5f10) ros2#13 0x561e9d2a9490 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x97490) ros2#14 0x561e9d26d33d in testing::UnitTest::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x5b33d) ros2#15 0x561e9d24088c in RUN_ALL_TESTS() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x2e88c) ros2#16 0x561e9d2407d2 in main (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x2e7d2) ros2#17 0x7f7a71402b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96) Direct leak of 72 byte(s) in 1 object(s) allocated from: #0 0x7f7a7229db50 in __interceptor_malloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xdeb50) #1 0x7f7a71f9abb5 in __default_allocate (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x8bb5) #2 0x7f7a71fad1b0 in rcutils_string_map_init (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x1b1b0) #3 0x7f7a71fa4641 in rcutils_logging_initialize_with_allocator (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x12641) #4 0x7f7a71fa41ab in rcutils_logging_initialize (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/install-asan/rcutils/lib/librcutils.so+0x121ab) #5 0x561e9d22a9b6 in TestLogging_test_logging_initialization_Test::TestBody() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x189b6) #6 0x561e9d2b545d in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0xa345d) #7 0x561e9d2a71c7 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x951c7) #8 0x561e9d2534c9 in testing::Test::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x414c9) #9 0x561e9d2548f4 in testing::TestInfo::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x428f4) ros2#10 0x561e9d255498 in testing::TestCase::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x43498) ros2#11 0x561e9d2705a9 in testing::internal::UnitTestImpl::RunAllTests() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x5e5a9) ros2#12 0x561e9d2b7f10 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0xa5f10) ros2#13 0x561e9d2a9490 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x97490) ros2#14 0x561e9d26d33d in testing::UnitTest::Run() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x5b33d) ros2#15 0x561e9d24088c in RUN_ALL_TESTS() (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x2e88c) ros2#16 0x561e9d2407d2 in main (/home/ANT.AMAZON.COM/tmoulard/ros2_ws/build-asan/rcutils/test_logging+0x2e7d2) ros2#17 0x7f7a71402b96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96) SUMMARY: AddressSanitizer: 144 byte(s) leaked in 2 allocation(s). After: $ ./test_logging Running main() from ../../../install-asan/gtest_vendor/src/gtest_vendor/src/gtest_main.cc [==========] Running 5 tests from 1 test case. [----------] Global test environment set-up. [----------] 5 tests from TestLogging [ RUN ] TestLogging.test_logging_initialization [ OK ] TestLogging.test_logging_initialization (0 ms) [ RUN ] TestLogging.test_logging [ OK ] TestLogging.test_logging (0 ms) [ RUN ] TestLogging.test_log_severity [ OK ] TestLogging.test_log_severity (0 ms) [ RUN ] TestLogging.test_logger_severities [ OK ] TestLogging.test_logger_severities (0 ms) [ RUN ] TestLogging.test_logger_severity_hierarchy [ OK ] TestLogging.test_logger_severity_hierarchy (0 ms) [----------] 5 tests from TestLogging (1 ms total) [----------] Global test environment tear-down [==========] 5 tests from 1 test case ran. (1 ms total) [ PASSED ] 5 tests. Signed-off-by: Thomas Moulard <tmoulard@amazon.com>

emersonknapp suggested changes Apr 3, 2019

View reviewed changes

rcl/src/rcl/event.c Outdated Show resolved Hide resolved

rcl/test/rcl/test_events.cpp Outdated Show resolved Hide resolved

rcl/test/rcl/test_events.cpp Outdated Show resolved Hide resolved

rcl/test/rcl/test_events.cpp Outdated Show resolved Hide resolved

ross-desmond added 2 commits April 2, 2019 17:21

Add tests for unsupported qos checks on initialize

bd5fae5

Signed-off-by: Ross Desmond <44277324+ross-desmond@users.noreply.github.com>

Use std::chrono instead of manual conversions

df16ac7

ross-desmond force-pushed the qos_fastrtps_unsupported branch from 14ca683 to df16ac7 Compare April 3, 2019 00:49

Reformat spacing event.c

d4d89ce

emersonknapp approved these changes Apr 3, 2019

View reviewed changes

mm318 reviewed Apr 3, 2019

View reviewed changes

rcl/src/rcl/event.c Outdated Show resolved Hide resolved

nburek approved these changes Apr 3, 2019

View reviewed changes

Reformat subscription event init return event.c

7856492

nburek merged commit 49daf51 into qos_reviewed Apr 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests for unsupported qos checks on initialize #5

Add tests for unsupported qos checks on initialize #5

ross-desmond commented Apr 2, 2019

emersonknapp commented Apr 3, 2019

ross-desmond commented Apr 3, 2019

nburek left a comment

nburek Apr 3, 2019

ross-desmond Apr 3, 2019

ross-desmond Apr 3, 2019

nburek Apr 3, 2019

Add tests for unsupported qos checks on initialize #5

Add tests for unsupported qos checks on initialize #5

Conversation

ross-desmond commented Apr 2, 2019

emersonknapp commented Apr 3, 2019

ross-desmond commented Apr 3, 2019

nburek left a comment

Choose a reason for hiding this comment

nburek Apr 3, 2019

Choose a reason for hiding this comment

ross-desmond Apr 3, 2019

Choose a reason for hiding this comment

ross-desmond Apr 3, 2019

Choose a reason for hiding this comment

nburek Apr 3, 2019

Choose a reason for hiding this comment