From 46a2eb030ffb5eea25a6f26c82b11e1667b407d0 Mon Sep 17 00:00:00 2001 From: brawner Date: Thu, 11 Jun 2020 14:31:28 -0700 Subject: [PATCH] Increase coverage with a graveyard behavior test and unmanaged instance test (#159) --- QUALITY_DECLARATION.md | 4 ++- test/CMakeLists.txt | 17 ++++++++- test/global_plugins.cpp | 79 +++++++++++++++++++++++++++++++++++++++++ test/utest.cpp | 72 +++++++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 test/global_plugins.cpp diff --git a/QUALITY_DECLARATION.md b/QUALITY_DECLARATION.md index daccc738..a1b1058c 100644 --- a/QUALITY_DECLARATION.md +++ b/QUALITY_DECLARATION.md @@ -114,7 +114,9 @@ This includes: Changes are required to make a best effort to keep or increase coverage before being accepted, but decreases are allowed if properly justified and accepted by reviewers. -Current coverage statistics can be viewed [here](https://ci.ros2.org/job/ci_linux_coverage/85/cobertura/src_ros_class_loader_include_class_loader/) and [here](https://ci.ros2.org/job/ci_linux_coverage/85/cobertura/src_ros_class_loader_include_class_loader/). This package does not yet meet the 95% coverage guideline, but it is currently above 90%. +This package has testing coverage of at least 95%. +Current coverage statistics can be viewed [here](https://ci.ros2.org/job/nightly_linux_coverage/lastSuccessfulBuild/cobertura/). +A description of how coverage statistics are calculated is summarized in the [ROS 2 On-boarding Guide](https://index.ros.org/doc/ros2/Contributing/ROS-2-On-boarding-Guide/#note-on-coverage-runs). ### Performance [4.iv] diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 2f989fc7..50f0b67a 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -26,6 +26,20 @@ endif() target_link_libraries(${PROJECT_NAME}_TestPlugins2 ${PROJECT_NAME}) class_loader_hide_library_symbols(${PROJECT_NAME}_TestPlugins2) +# These plugins are loaded using dlopen() with RTLD_GLOBAL in utest and may cause side effects +# in other tests if they are used elsewhere. +add_library(${PROJECT_NAME}_TestGlobalPlugins EXCLUDE_FROM_ALL SHARED global_plugins.cpp) +if(ament_cmake_FOUND) + target_include_directories(${PROJECT_NAME}_TestGlobalPlugins + PUBLIC "../include") + ament_target_dependencies(${PROJECT_NAME}_TestGlobalPlugins "console_bridge") +else() + target_include_directories(${PROJECT_NAME}_TestGlobalPlugins + PUBLIC "../include" ${console_bridge_INCLUDE_DIRS}) +endif() +target_link_libraries(${PROJECT_NAME}_TestGlobalPlugins ${PROJECT_NAME}) +class_loader_hide_library_symbols(${PROJECT_NAME}_TestGlobalPlugins) + if(WIN32) set(append_library_dirs "$;$") else() @@ -52,7 +66,8 @@ if(TARGET ${PROJECT_NAME}_utest) endif() add_dependencies(${PROJECT_NAME}_utest ${PROJECT_NAME}_TestPlugins1 - ${PROJECT_NAME}_TestPlugins2) + ${PROJECT_NAME}_TestPlugins2 + ${PROJECT_NAME}_TestGlobalPlugins) endif() ament_add_gtest(${PROJECT_NAME}_unique_ptr_test unique_ptr_test.cpp diff --git a/test/global_plugins.cpp b/test/global_plugins.cpp new file mode 100644 index 00000000..47cdd600 --- /dev/null +++ b/test/global_plugins.cpp @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2012, Willow Garage, Inc. + * All rights reserved. + * Copyright (c) 2020, Open Source Robotics Foundation, Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of the Willow Garage, Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * 2020, Copied from plugins1.cpp and plugins2.cpp and changing class names + */ + +#include + +#include "class_loader/class_loader.hpp" + +#include "./base.hpp" + +class Kangaroo : public Base +{ +public: + void saySomething() + { + printf("[Angry growl]\n"); + } +}; + +class Panda : public Base +{ +public: + void saySomething() + { + printf("[Excited squeaks!!!]\n"); + } +}; + +class Hyena : public Base +{ +public: + void saySomething() + { + printf("[Cackling laugh]\n"); + } +}; + +class Alpaca : public Base +{ +public: + void saySomething() + { + printf("hhhaaaaaaaaaa\n"); + } +}; + + +CLASS_LOADER_REGISTER_CLASS(Kangaroo, Base) +CLASS_LOADER_REGISTER_CLASS(Panda, Base) +CLASS_LOADER_REGISTER_CLASS(Hyena, Base) +CLASS_LOADER_REGISTER_CLASS(Alpaca, Base) diff --git a/test/utest.cpp b/test/utest.cpp index 144ad2fe..6a0870b5 100644 --- a/test/utest.cpp +++ b/test/utest.cpp @@ -29,6 +29,11 @@ #include #include + +#ifndef _WIN32 +#include +#endif + #include #include #include @@ -45,6 +50,11 @@ const std::string LIBRARY_1 = class_loader::systemLibraryFormat("class_loader_TestPlugins1"); // NOLINT const std::string LIBRARY_2 = class_loader::systemLibraryFormat("class_loader_TestPlugins2"); // NOLINT +// These are loaded with dlopen() and RTLD_GLOBAL in loadUnloadLoadFromGraveyard and may cause +// unexpected side-effects if used elsewhere +const std::string GLOBAL_PLUGINS = // NOLINT + class_loader::systemLibraryFormat("class_loader_TestGlobalPlugins"); + TEST(ClassLoaderTest, basicLoad) { try { class_loader::ClassLoader loader1(LIBRARY_1, false); @@ -57,6 +67,21 @@ TEST(ClassLoaderTest, basicLoad) { SUCCEED(); } +// Requires separate namespace so static variables are isolated +TEST(ClassLoaderUnmanagedTest, basicLoadUnmanaged) { + try { + class_loader::ClassLoader loader1(LIBRARY_1, false); + Base * unmanaged_instance = loader1.createUnmanagedInstance("Dog"); + ASSERT_NE(unmanaged_instance, nullptr); + unmanaged_instance->saySomething(); + delete unmanaged_instance; + } catch (class_loader::ClassLoaderException & e) { + FAIL() << "ClassLoaderException: " << e.what() << "\n"; + } + + SUCCEED(); +} + TEST(ClassLoaderUniquePtrTest, basicLoadFailures) { class_loader::ClassLoader loader1(LIBRARY_1, false); class_loader::ClassLoader loader2("", false); @@ -337,6 +362,53 @@ TEST(MultiClassLoaderTest, noWarningOnLazyLoad) { SUCCEED(); } +#ifndef _WIN32 +// Not run on Windows because this tests dlopen-specific behavior + +// This is a different class name so that static variables in ClassLoader are isolated +TEST(ClassLoaderGraveyardTest, loadUnloadLoadFromGraveyard) { + // This first load/unload adds the plugin to the graveyard + try { + class_loader::ClassLoader loader(GLOBAL_PLUGINS, false); + loader.createInstance("Kangaroo")->saySomething(); + loader.unloadLibrary(); + } catch (class_loader::ClassLoaderException & e) { + FAIL() << "ClassLoaderException: " << e.what() << "\n"; + } + + // Not all platforms use RTLD_GLOBAL as a default, and rcutils doesn't explicitly choose either. + // In order to invoke graveyard behavior, this needs to be loaded first for global relocation. + + void * handle = dlopen(GLOBAL_PLUGINS.c_str(), RTLD_NOW | RTLD_GLOBAL); + ASSERT_NE(handle, nullptr); + + // This load will cause system to use globally relocatable library. + // For testing purposes, this will cause ClassLoader to revive the library from the graveyard. + try { + class_loader::ClassLoader loader(GLOBAL_PLUGINS, false); + loader.createInstance("Panda")->saySomething(); + loader.unloadLibrary(); + + loader.loadLibrary(); + loader.createInstance("Hyena")->saySomething(); + loader.unloadLibrary(); + } catch (class_loader::ClassLoaderException & e) { + FAIL() << "ClassLoaderException: " << e.what() << "\n"; + } + + dlclose(handle); + // With all libraries closed, this should act like a normal load/unload. + try { + class_loader::ClassLoader loader(GLOBAL_PLUGINS, false); + loader.createInstance("Alpaca")->saySomething(); + loader.unloadLibrary(); + } catch (class_loader::ClassLoaderException & e) { + FAIL() << "ClassLoaderException: " << e.what() << "\n"; + } +} + +#endif // ifndef _WIN32 + // Run all the tests that were declared with TEST() int main(int argc, char ** argv) {