From e51e58f430a1cc1354e54424d5035fd077216d99 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Wed, 8 Jan 2025 11:34:38 +0100 Subject: [PATCH] Unit tests: Fix One Definition Rule violation caused by overlinking The libdnf5 library unit tests use the private methods of the tested libdnf5 library. The `BaseTestCase` class used as a parent of many test scenarios, not only libdnf5 but also libdnf5-cli and other unit tests, provides the `add_system_pkg` method, which also uses private methods of the libdnf5 library. Because the private methods have hidden symbols, they are not exported by the libdnf5 shared library. To access the symbols of the private methods, unit tests link libdnf5 statically instead of using the libdnf5 shared library. This is fine for unit tests of the libdnf5 library. The problem is with unit tests of other components (e.g. libdnf-cli) that link the shared libdnf5 library. Due to this, in these tests the libdnf5 library was linked statically because of used class `BaseTestCase` and at the same time the shared libdnf5 library was linked. The problem was solved by moving the `add_system_pkg` method from the `BaseTestCase` class to the newly created inherited `LibdnfPrivateTestCase` class, which is only used in the libdnf5 library unit tests. Thus, only in these tests must libdnf5 be linked statically. The tests of the other components still use the `BaseTestClass` class, which now uses only public (API) functions from the libdnf5 library. These tests now only link the shared version of the libdnf5 library. --- test/libdnf5-cli/CMakeLists.txt | 2 +- test/libdnf5/CMakeLists.txt | 2 +- test/libdnf5/base/test_goal.hpp | 4 +- test/libdnf5/libdnf_private_test_case.cpp | 52 +++++++++++++++++++++++ test/libdnf5/libdnf_private_test_case.hpp | 37 ++++++++++++++++ test/shared/CMakeLists.txt | 2 +- test/shared/base_test_case.cpp | 37 ++-------------- test/shared/base_test_case.hpp | 9 ++-- 8 files changed, 102 insertions(+), 43 deletions(-) create mode 100644 test/libdnf5/libdnf_private_test_case.cpp create mode 100644 test/libdnf5/libdnf_private_test_case.hpp diff --git a/test/libdnf5-cli/CMakeLists.txt b/test/libdnf5-cli/CMakeLists.txt index e37cbfa51..18d99d642 100644 --- a/test/libdnf5-cli/CMakeLists.txt +++ b/test/libdnf5-cli/CMakeLists.txt @@ -16,7 +16,7 @@ include_directories(${PROJECT_SOURCE_DIR}/libdnf5) add_executable(run_tests_cli ${TEST_LIBDNF5_CLI_SOURCES}) target_link_directories(run_tests_cli PRIVATE ${CMAKE_BINARY_DIR}/libdnf5) -target_link_libraries(run_tests_cli PRIVATE stdc++ libdnf5_static libdnf5-cli cppunit test_shared) +target_link_libraries(run_tests_cli PRIVATE stdc++ libdnf5 libdnf5-cli test_shared) add_test(NAME test_libdnf_cli COMMAND run_tests_cli) diff --git a/test/libdnf5/CMakeLists.txt b/test/libdnf5/CMakeLists.txt index f966e96f5..478b17d75 100644 --- a/test/libdnf5/CMakeLists.txt +++ b/test/libdnf5/CMakeLists.txt @@ -16,7 +16,7 @@ target_compile_options(run_tests PRIVATE "-Wno-self-assign-overloaded") target_compile_options(run_tests PRIVATE "-Wno-self-move") target_link_directories(run_tests PRIVATE ${CMAKE_BINARY_DIR}/libdnf5) -target_link_libraries(run_tests PRIVATE stdc++ libdnf5_static cppunit test_shared) +target_link_libraries(run_tests PRIVATE stdc++ libdnf5_static test_shared) pkg_check_modules(JSONC REQUIRED json-c) include_directories(${JSONC_INCLUDE_DIRS}) diff --git a/test/libdnf5/base/test_goal.hpp b/test/libdnf5/base/test_goal.hpp index 5567b316a..9a7682006 100644 --- a/test/libdnf5/base/test_goal.hpp +++ b/test/libdnf5/base/test_goal.hpp @@ -22,12 +22,12 @@ along with libdnf. If not, see . #define TEST_LIBDNF5_BASE_GOAL_HPP -#include "../shared/base_test_case.hpp" +#include "libdnf_private_test_case.hpp" #include -class BaseGoalTest : public BaseTestCase { +class BaseGoalTest : public LibdnfPrivateTestCase { CPPUNIT_TEST_SUITE(BaseGoalTest); #ifndef WITH_PERFORMANCE_TESTS diff --git a/test/libdnf5/libdnf_private_test_case.cpp b/test/libdnf5/libdnf_private_test_case.cpp new file mode 100644 index 000000000..68b8dda0b --- /dev/null +++ b/test/libdnf5/libdnf_private_test_case.cpp @@ -0,0 +1,52 @@ +/* +Copyright Contributors to the libdnf project. + +This file is part of libdnf: https://github.com/rpm-software-management/libdnf/ + +Libdnf is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 2 of the License, or +(at your option) any later version. + +Libdnf is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with libdnf. If not, see . +*/ + + +#include "libdnf_private_test_case.hpp" + +#include "../shared/private_accessor.hpp" +#include "base/base_impl.hpp" +#include "utils/string.hpp" + +#include + + +namespace { + +// Accessor of private Base::p_impl, see private_accessor.hpp +create_private_getter_template; +create_getter(priv_impl, &libdnf5::Base::p_impl); +create_getter(add_rpm_package, &libdnf5::repo::Repo::add_rpm_package); + +} // namespace + +libdnf5::rpm::Package LibdnfPrivateTestCase::add_system_pkg( + const std::string & relative_path, libdnf5::transaction::TransactionItemReason reason) { + // parse out the NA from the package path to set the reason for the installed package + auto filename_toks = libdnf5::utils::string::split(relative_path, "/"); + auto basename_toks = libdnf5::utils::string::rsplit(filename_toks.back(), ".", 2); + auto nevras = libdnf5::rpm::Nevra::parse(basename_toks.front()); + CPPUNIT_ASSERT_MESSAGE("Couldn't parse NEVRA from package path: \"" + relative_path + "\"", !nevras.empty()); + auto na = nevras[0].get_name() + "." + nevras[0].get_arch(); + + (base.*get(priv_impl()))->get_system_state().set_package_reason(na, reason); + + return (*(repo_sack->get_system_repo()).*get(add_rpm_package{}))( + PROJECT_BINARY_DIR "/test/data/" + relative_path, false); +} diff --git a/test/libdnf5/libdnf_private_test_case.hpp b/test/libdnf5/libdnf_private_test_case.hpp new file mode 100644 index 000000000..8de2ab5c1 --- /dev/null +++ b/test/libdnf5/libdnf_private_test_case.hpp @@ -0,0 +1,37 @@ +/* +Copyright Contributors to the libdnf project. + +This file is part of libdnf: https://github.com/rpm-software-management/libdnf/ + +Libdnf is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 2 of the License, or +(at your option) any later version. + +Libdnf is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with libdnf. If not, see . +*/ + + +#ifndef TEST_LIBDNF5_LIBDNF_PRIVATE_TEST_CASE_HPP +#define TEST_LIBDNF5_LIBDNF_PRIVATE_TEST_CASE_HPP + +#include "../shared/base_test_case.hpp" + +#include +#include + +#include + +class LibdnfPrivateTestCase : public BaseTestCase { +public: + libdnf5::rpm::Package add_system_pkg( + const std::string & relative_path, libdnf5::transaction::TransactionItemReason reason); +}; + +#endif // TEST_LIBDNF5_LIBDNF_PRIVATE_TEST_CASE_HPP diff --git a/test/shared/CMakeLists.txt b/test/shared/CMakeLists.txt index 1b0b0ec9f..8cab1ae7e 100644 --- a/test/shared/CMakeLists.txt +++ b/test/shared/CMakeLists.txt @@ -10,4 +10,4 @@ include_directories(${PROJECT_SOURCE_DIR}/libdnf5) add_library(test_shared OBJECT ${TEST_SHARED_SOURCES}) target_link_directories(test_shared PUBLIC ${CMAKE_BINARY_DIR}/libdnf5) -target_link_libraries(test_shared PUBLIC stdc++ libdnf5_static cppunit) +target_link_libraries(test_shared PUBLIC cppunit) diff --git a/test/shared/base_test_case.cpp b/test/shared/base_test_case.cpp index 0cfdd9b2e..1c69a27ae 100644 --- a/test/shared/base_test_case.cpp +++ b/test/shared/base_test_case.cpp @@ -20,26 +20,19 @@ along with libdnf. If not, see . #include "base_test_case.hpp" -#include "base/base_impl.hpp" #include "logger_redirector.hpp" -#include "private_accessor.hpp" #include "test_logger.hpp" #include "utils.hpp" -#include "utils/string.hpp" -#include +#include "libdnf5/utils/fs/temp.hpp" + #include -#include #include #include -#include #include #include -#include - - -using fmt::format; +#include libdnf5::repo::RepoWeakPtr BaseTestCase::add_repo( @@ -178,30 +171,6 @@ libdnf5::rpm::Package BaseTestCase::get_pkg_i(const std::string & nevra, size_t return *it; } -namespace { - -// Accessor of private Base::p_impl, see private_accessor.hpp -create_private_getter_template; -create_getter(priv_impl, &libdnf5::Base::p_impl); -create_getter(add_rpm_package, &libdnf5::repo::Repo::add_rpm_package); - -} // namespace - -libdnf5::rpm::Package BaseTestCase::add_system_pkg( - const std::string & relative_path, libdnf5::transaction::TransactionItemReason reason) { - // parse out the NA from the package path to set the reason for the installed package - auto filename_toks = libdnf5::utils::string::split(relative_path, "/"); - auto basename_toks = libdnf5::utils::string::rsplit(filename_toks.back(), ".", 2); - auto nevras = libdnf5::rpm::Nevra::parse(basename_toks.front()); - CPPUNIT_ASSERT_MESSAGE("Couldn't parse NEVRA from package path: \"" + relative_path + "\"", !nevras.empty()); - auto na = nevras[0].get_name() + "." + nevras[0].get_arch(); - - (base.*get(priv_impl()))->get_system_state().set_package_reason(na, reason); - - return (*(repo_sack->get_system_repo()).*get(add_rpm_package{}))( - PROJECT_BINARY_DIR "/test/data/" + relative_path, false); -} - libdnf5::rpm::Package BaseTestCase::add_cmdline_pkg(const std::string & relative_path) { std::string path = PROJECT_BINARY_DIR "/test/data/" + relative_path; diff --git a/test/shared/base_test_case.hpp b/test/shared/base_test_case.hpp index eebc5aa4a..18be125d0 100644 --- a/test/shared/base_test_case.hpp +++ b/test/shared/base_test_case.hpp @@ -23,10 +23,13 @@ along with libdnf. If not, see . #include "test_case_fixture.hpp" -#include "libdnf5/utils/fs/temp.hpp" - +#include #include +#include +#include +#include #include +#include #include #include @@ -67,8 +70,6 @@ class BaseTestCase : public TestCaseFixture { } libdnf5::rpm::Package get_pkg_i(const std::string & nevra, size_t index); - libdnf5::rpm::Package add_system_pkg( - const std::string & relative_path, libdnf5::transaction::TransactionItemReason reason); libdnf5::rpm::Package add_cmdline_pkg(const std::string & relative_path); libdnf5::Base base;