search

osrf / osrf_testing_tools_cpp

Common testing tools for C++ which are used for testing in various OSRF projects.
Apache License 2.0
33 stars 29 forks source link

Add option to skip memory tool tests for address sanitizer #24

Closed bhatsach closed 5 years ago

bhatsach commented 5 years ago

Introduce a new cmake option -DSKIP_ASAN_INCOMPATIBLE_TESTS to turn off memory tool tests. This is needed to prevent the override of LD_PRELOAD or library path to interfere from the working of address sanitizer

Connects to ros2/ci#245

bhatsach commented 5 years ago

Closing this in favor of PR where we are getting address sanitizer to work successfully with memory tool tests

nuclearsandwich commented 5 years ago

Given the lack of easy success with the approach in #25 we are revisiting disabling the memory tools tests again.

I have yet to definitively determine how the aarch64 tests are skipped in each consumer package and I was hoping to set something in this repository that disables the tests gracefully for all consumers without requiring code changes or configuration changes downstream.

I'm also still tracing threads in this PR trying to determine how the SKIP_ASAN_INCOMPATIBLE_TESTS option from https://github.com/osrf/osrf_testing_tools_cpp/pull/24/files#diff-3f4d09ee9787baef1badbc32bd224d14R45 is used.

nuclearsandwich commented 5 years ago

With another hint from @wjwwood it seems like only a few packages may actually be using other components of osrf_testing_tools that don't require LD_PRELOAD and don't need to hook allocation functions. For example, test_communication is only using the scope_exit header. https://github.com/ros2/system_tests/blob/187b7d988e5866957519c9ab56d3123cb0046b7d/test_communication/test/test_messages_c.cpp#L24

nuclearsandwich commented 5 years ago

An inventory of packages depending on osrf_testing_tools_cpp:

❯ rg 'depend.*osrf_testing_tools' src/
src/ros2/rcutils/package.xml
19:  <test_depend>osrf_testing_tools_cpp</test_depend>

src/ros2/system_tests/test_communication/package.xml
27:  <test_depend>osrf_testing_tools_cpp</test_depend>

src/ros2/rcl/rcl_action/package.xml
27:  <test_depend>osrf_testing_tools_cpp</test_depend>

src/ros2/rcl/rcl/package.xml
40:  <test_depend>osrf_testing_tools_cpp</test_depend>

src/ros2/rcl/rcl_lifecycle/package.xml
27:  <test_depend>osrf_testing_tools_cpp</test_depend>

src/osrf/osrf_testing_tools_cpp/test_osrf_testing_tools_cpp/package.xml
14:  <test_depend>osrf_testing_tools_cpp</test_depend>

Of these only three pull in memory_tools:

❯ rg -l osrf_testing_tools_cpp\/memory_tools  | cut -d'/' -f1-2 | sort | uniq
osrf/osrf_testing_tools_cpp
ros2/rcl
ros2/rcutils

osrf_testing_tools_cpp itself and rcutils already contain code for disabling the memory_tools tests on aarch64. I've got more hints from @wjwwood as to why rcl might not need to at the moment but haven't finished the paperwork.

nuclearsandwich commented 5 years ago

If I invoke the test command manually, removing the LD_PRELOAD from the environment, the test still passes.

nuclearsandwich commented 5 years ago

Superceded by #28