From 756a8d716ab374e37a504aeac85913b36702771d Mon Sep 17 00:00:00 2001 From: Silvio Date: Sat, 7 Dec 2024 16:01:45 +0100 Subject: [PATCH] Reduce cmake test boilerplate All instances of mujoco_test in the code base were followed by a call to target_link_libraries( fixture gmock). As anyhow mujoco_test already was calling target_link_libraries to some predefined list of targets (mujoco and gtest_main), this PR adds to the list of default linked targets also fixture and gmock, to reduce the boilerplate. Furthermore, to completly remove the need for calling target_link_libraries after a call to mujoco_test, this PR also add to the mujoco_test macro the ADDITIONAL_LINK_LIBRARIES argument, that can be used if a given test needs to link some additional targets beside the default ones. To permit to use these new features also for mujoco benchmarks, this PR adds a MAIN_TARGET parameter to mujoco_test, to select if gtest_main or another target is used to provide the main entry point to the test executable. --- test/CMakeLists.txt | 30 ++++++++-- test/benchmark/CMakeLists.txt | 83 +++++++-------------------- test/engine/CMakeLists.txt | 43 ++------------ test/plugin/actuator/CMakeLists.txt | 5 +- test/plugin/elasticity/CMakeLists.txt | 1 - test/plugin/sensor/CMakeLists.txt | 1 - test/thread/CMakeLists.txt | 2 - test/user/CMakeLists.txt | 17 +----- test/xml/CMakeLists.txt | 10 +--- 9 files changed, 58 insertions(+), 134 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 122760a9..0f92803d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -17,10 +17,22 @@ set(MUJOCO_TEST_WORKING_DIR ${CMAKE_CURRENT_SOURCE_DIR}) include(GoogleTest) +# This macro is used to add a C++ test to MuJoCo CMake build system +# +# The CMake targets linked by default with target_link_libraries are: +# mujoco fixture gmock +# +# The macro supports the following parameters: +# * PROPERTIES: multiple value parameter, its value is set as test properties +# via the set_tests_properties( PROPERTIES ) function +# * ADDITIONAL_LINK_LIBRARIES: multiple value parameter, additional libraries linked +# to the test via target_link_libraries +# * MAIN_TARGET: single value parameter, used to specify the target linked to the tests +# that define the main entry poiny. If not indicated, gest_main is used macro(mujoco_test name) set(options) - set(oneValueArgs) - set(multiValueArgs PROPERTIES) + set(oneValueArgs MAIN_TARGET) + set(multiValueArgs PROPERTIES ADDITIONAL_LINK_LIBRARIES) cmake_parse_arguments( _ARGS "${options}" @@ -30,7 +42,15 @@ macro(mujoco_test name) ) add_executable(${name} ${name}.cc) - target_link_libraries(${name} gtest_main mujoco) + target_link_libraries(${name} mujoco fixture gmock) + if(_ARGS_MAIN_TARGET) + target_link_libraries(${name} ${_ARGS_MAIN_TARGET}) + else() + target_link_libraries(${name} gtest_main) + endif() + if(_ARGS_ADDITIONAL_LINK_LIBRARIES) + target_link_libraries(${name} ${_ARGS_ADDITIONAL_LINK_LIBRARIES}) + endif() target_include_directories(${name} PRIVATE ${MUJOCO_TEST_INCLUDE}) set_target_properties(${name} PROPERTIES BUILD_RPATH ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}) # gtest_discover_tests is recommended over gtest_add_tests, but has some issues in Windows. @@ -48,6 +68,7 @@ macro(mujoco_test name) if(_ARGS_PROPERTIES) set_tests_properties(${testList} PROPERTIES ${_ARGS_PROPERTIES}) endif() + endmacro() add_library(fixture STATIC fixture.h fixture.cc) @@ -67,13 +88,10 @@ target_link_libraries( target_include_directories(fixture PRIVATE ${mujoco_SOURCE_DIR}/include gmock) mujoco_test(fixture_test) -target_link_libraries(fixture_test fixture gmock) mujoco_test(header_test) -target_link_libraries(header_test fixture gmock) mujoco_test(pipeline_test) -target_link_libraries(pipeline_test fixture gmock) add_subdirectory(benchmark) add_subdirectory(engine) diff --git a/test/benchmark/CMakeLists.txt b/test/benchmark/CMakeLists.txt index dea826ff..658e0639 100644 --- a/test/benchmark/CMakeLists.txt +++ b/test/benchmark/CMakeLists.txt @@ -12,86 +12,45 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Macro for benchmarks that don't use GL (same as mujoco_api_test, but uses -# benchmark::benchmark_main). -macro(mujoco_benchmark_test name) - add_executable(${name} ${name}.cc) - target_link_libraries( - ${name} - benchmark::benchmark_main - mujoco - absl::core_headers - ) - target_include_directories(${name} PRIVATE ${MUJOCO_TEST_INCLUDE}) - # TODO(fraromano) Check RPATH settings - set_target_properties(${name} PROPERTIES BUILD_RPATH ${CMAKE_LIBRARY_OUTPUT_DIRECTORY}) - # gtest_discover_tests is recommended over gtest_add_tests, but has some issues in Windows. - gtest_add_tests( - TARGET ${name} - SOURCES ${name}.cc - WORKING_DIRECTORY ${MUJOCO_TEST_WORKING_DIR} - TEST_LIST testList - ) - if(WIN32) - set_tests_properties( - ${testList} PROPERTIES ENVIRONMENT "PATH=$;$ENV{PATH}" - ) - endif() -endmacro() - -mujoco_benchmark_test(ccd_benchmark_test) -target_link_libraries( +mujoco_test( ccd_benchmark_test - fixture - gmock - benchmark::benchmark + MAIN_TARGET benchmark::benchmark_main + ADDITIONAL_LINK_LIBRARIES benchmark::benchmark absl::core_headers ) -mujoco_benchmark_test(step_benchmark_test) -target_link_libraries( +mujoco_test( step_benchmark_test - fixture - gmock - benchmark::benchmark + MAIN_TARGET benchmark::benchmark_main + ADDITIONAL_LINK_LIBRARIES benchmark::benchmark absl::core_headers ) -mujoco_benchmark_test(thread_performance_test) -target_link_libraries( +mujoco_test( thread_performance_test - fixture - gmock - benchmark::benchmark + MAIN_TARGET benchmark::benchmark_main + ADDITIONAL_LINK_LIBRARIES benchmark::benchmark absl::core_headers ) -mujoco_benchmark_test(parse_benchmark_test) -target_link_libraries( +mujoco_test( parse_benchmark_test - fixture - gmock - benchmark::benchmark + MAIN_TARGET benchmark::benchmark_main + ADDITIONAL_LINK_LIBRARIES benchmark::benchmark absl::core_headers ) -mujoco_benchmark_test(engine_util_spatial_benchmark_test) -target_link_libraries( +mujoco_test( engine_util_spatial_benchmark_test - fixture - gmock - benchmark::benchmark + MAIN_TARGET benchmark::benchmark_main + ADDITIONAL_LINK_LIBRARIES benchmark::benchmark absl::core_headers ) -mujoco_benchmark_test(engine_core_smooth_benchmark_test) -target_link_libraries( +mujoco_test( engine_core_smooth_benchmark_test - fixture - gmock - benchmark::benchmark + MAIN_TARGET benchmark::benchmark_main + ADDITIONAL_LINK_LIBRARIES benchmark::benchmark absl::core_headers ) -mujoco_benchmark_test(engine_util_sparse_benchmark_test) -target_link_libraries( +mujoco_test( engine_util_sparse_benchmark_test - fixture - gmock - benchmark::benchmark + MAIN_TARGET benchmark::benchmark_main + ADDITIONAL_LINK_LIBRARIES benchmark::benchmark absl::core_headers ) diff --git a/test/engine/CMakeLists.txt b/test/engine/CMakeLists.txt index b562e585..625a0cbf 100644 --- a/test/engine/CMakeLists.txt +++ b/test/engine/CMakeLists.txt @@ -13,91 +13,59 @@ # limitations under the License. mujoco_test(engine_collision_box_test) -target_link_libraries(engine_collision_box_test fixture gmock) mujoco_test(engine_collision_convex_test) -target_link_libraries(engine_collision_convex_test fixture gmock) mujoco_test(engine_collision_driver_test) -target_link_libraries(engine_collision_driver_test fixture gmock) -mujoco_test(engine_collision_gjk_test) -target_link_libraries(engine_collision_gjk_test fixture gmock ccd) +mujoco_test(engine_collision_gjk_test ADDITIONAL_LINK_LIBRARIES ccd) mujoco_test(engine_core_constraint_test) -target_link_libraries(engine_core_constraint_test fixture gmock) -mujoco_test(engine_core_smooth_test) -target_link_libraries(engine_core_smooth_test fixture gmock absl::span) +mujoco_test(engine_core_smooth_test ADDITIONAL_LINK_LIBRARIES absl::span) mujoco_test(engine_derivative_test) -target_link_libraries(engine_derivative_test fixture gmock) mujoco_test(engine_forward_test) -target_link_libraries(engine_forward_test fixture gmock) mujoco_test(engine_inverse_test) -target_link_libraries(engine_inverse_test fixture gmock) mujoco_test(engine_island_test) -target_link_libraries(engine_island_test fixture gmock) -mujoco_test(engine_io_test) -target_link_libraries( - engine_io_test - fixture - gmock - absl::str_format -) +mujoco_test(engine_io_test ADDITIONAL_LINK_LIBRARIES absl::str_format) mujoco_test( engine_plugin_test + ADDITIONAL_LINK_LIBRARIES + absl::str_format PROPERTIES ENVIRONMENT "MUJOCO_PLUGIN_DIR=$" ) -target_link_libraries( - engine_plugin_test - fixture - gmock - absl::str_format -) mujoco_test(engine_passive_test) -target_link_libraries(engine_passive_test fixture gmock) mujoco_test(engine_print_test) -target_link_libraries(engine_print_test fixture gmock) mujoco_test(engine_ray_test) -target_link_libraries(engine_ray_test fixture gmock) mujoco_test(engine_sensor_test) -target_link_libraries(engine_sensor_test fixture gmock) mujoco_test(engine_solver_test) -target_link_libraries(engine_solver_test fixture gmock) mujoco_test(engine_sort_test) -target_link_libraries(engine_sort_test fixture gmock) mujoco_test(engine_support_test) -target_link_libraries(engine_support_test fixture gmock) mujoco_test(engine_thread_test) -target_link_libraries(engine_thread_test fixture gmock) mujoco_test(engine_util_blas_test) -target_link_libraries(engine_util_blas_test fixture gmock) mujoco_test(engine_util_errmem_test) -target_link_libraries(engine_util_errmem_test fixture gmock) mujoco_test(engine_util_solve_test) -target_link_libraries(engine_util_solve_test fixture gmock) mujoco_test(engine_util_spatial_test) -target_link_libraries(engine_util_spatial_test fixture gmock) mujoco_test( engine_vis_state_test @@ -105,4 +73,3 @@ mujoco_test( ENVIRONMENT "MUJOCO_PLUGIN_DIR=$" ) -target_link_libraries(engine_vis_state_test fixture gmock) diff --git a/test/plugin/actuator/CMakeLists.txt b/test/plugin/actuator/CMakeLists.txt index ac795625..b8e0f10b 100644 --- a/test/plugin/actuator/CMakeLists.txt +++ b/test/plugin/actuator/CMakeLists.txt @@ -14,8 +14,11 @@ mujoco_test( pid_test + ADDITIONAL_LINK_LIBRARIES + absl::cleanup + absl::strings PROPERTIES ENVIRONMENT "MUJOCO_PLUGIN_DIR=$" ) -target_link_libraries(pid_test fixture gmock absl::cleanup absl::strings) +target_link_libraries(pid_test fixture gmock ) diff --git a/test/plugin/elasticity/CMakeLists.txt b/test/plugin/elasticity/CMakeLists.txt index 008ae8be..93883937 100644 --- a/test/plugin/elasticity/CMakeLists.txt +++ b/test/plugin/elasticity/CMakeLists.txt @@ -18,4 +18,3 @@ mujoco_test( ENVIRONMENT "MUJOCO_PLUGIN_DIR=$" ) -target_link_libraries(elasticity_test fixture gmock) diff --git a/test/plugin/sensor/CMakeLists.txt b/test/plugin/sensor/CMakeLists.txt index 4adcba29..e634d16c 100644 --- a/test/plugin/sensor/CMakeLists.txt +++ b/test/plugin/sensor/CMakeLists.txt @@ -18,4 +18,3 @@ mujoco_test( ENVIRONMENT "MUJOCO_PLUGIN_DIR=$" ) -target_link_libraries(sensor_test fixture gmock) diff --git a/test/thread/CMakeLists.txt b/test/thread/CMakeLists.txt index 920bea0d..ada7b7ef 100644 --- a/test/thread/CMakeLists.txt +++ b/test/thread/CMakeLists.txt @@ -13,7 +13,5 @@ # limitations under the License. mujoco_test(thread_pool_test) -target_link_libraries(thread_pool_test fixture gmock) mujoco_test(thread_queue_test) -target_link_libraries(thread_queue_test fixture gmock) diff --git a/test/user/CMakeLists.txt b/test/user/CMakeLists.txt index ac4dc7b3..c176c3bd 100644 --- a/test/user/CMakeLists.txt +++ b/test/user/CMakeLists.txt @@ -12,11 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -mujoco_test(user_model_test) -target_link_libraries(user_model_test fixture gmock absl::str_format) +mujoco_test(user_model_test ADDITIONAL_LINK_LIBRARIES absl::str_format) mujoco_test(user_objects_test) -target_link_libraries(user_objects_test fixture gmock) mujoco_test( user_api_test @@ -24,24 +22,13 @@ mujoco_test( ENVIRONMENT "MUJOCO_PLUGIN_DIR=$" ) -target_link_libraries(user_api_test fixture gmock) mujoco_test(user_flex_test) -target_link_libraries(user_flex_test fixture gmock) -mujoco_test(user_mesh_test) -target_link_libraries( - user_mesh_test - fixture - gmock - absl::str_format -) +mujoco_test(user_mesh_test ADDITIONAL_LINK_LIBRARIES absl::str_format) mujoco_test(user_composite_test) -target_link_libraries(user_composite_test fixture gmock) mujoco_test(user_resource_test) -target_link_libraries(user_resource_test fixture gmock) mujoco_test(user_vfs_test) -target_link_libraries(user_vfs_test fixture gmock) diff --git a/test/xml/CMakeLists.txt b/test/xml/CMakeLists.txt index 537a991f..91753174 100644 --- a/test/xml/CMakeLists.txt +++ b/test/xml/CMakeLists.txt @@ -13,20 +13,14 @@ # limitations under the License. mujoco_test(xml_api_test) -target_link_libraries(xml_api_test fixture gmock) mujoco_test(xml_native_reader_test) -target_link_libraries(xml_native_reader_test fixture gmock) mujoco_test( xml_native_writer_test + ADDITIONAL_LINK_LIBRARIES + absl::flat_hash_set PROPERTIES ENVIRONMENT "MUJOCO_PLUGIN_DIR=$" ) -target_link_libraries( - xml_native_writer_test - fixture - gmock - absl::flat_hash_set -)