Skip to content

Commit

Permalink
Unit tests: Fix One Definition Rule violation caused by overlinking
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jrohel committed Jan 9, 2025
1 parent 9a5cba8 commit e51e58f
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 43 deletions.
2 changes: 1 addition & 1 deletion test/libdnf5-cli/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion test/libdnf5/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
4 changes: 2 additions & 2 deletions test/libdnf5/base/test_goal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.
#define TEST_LIBDNF5_BASE_GOAL_HPP


#include "../shared/base_test_case.hpp"
#include "libdnf_private_test_case.hpp"

#include <cppunit/extensions/HelperMacros.h>


class BaseGoalTest : public BaseTestCase {
class BaseGoalTest : public LibdnfPrivateTestCase {
CPPUNIT_TEST_SUITE(BaseGoalTest);

#ifndef WITH_PERFORMANCE_TESTS
Expand Down
52 changes: 52 additions & 0 deletions test/libdnf5/libdnf_private_test_case.cpp
Original file line number Diff line number Diff line change
@@ -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 <https://www.gnu.org/licenses/>.
*/


#include "libdnf_private_test_case.hpp"

#include "../shared/private_accessor.hpp"
#include "base/base_impl.hpp"
#include "utils/string.hpp"

#include <libdnf5/rpm/nevra.hpp>


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);
}
37 changes: 37 additions & 0 deletions test/libdnf5/libdnf_private_test_case.hpp
Original file line number Diff line number Diff line change
@@ -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 <https://www.gnu.org/licenses/>.
*/


#ifndef TEST_LIBDNF5_LIBDNF_PRIVATE_TEST_CASE_HPP
#define TEST_LIBDNF5_LIBDNF_PRIVATE_TEST_CASE_HPP

#include "../shared/base_test_case.hpp"

#include <libdnf5/rpm/package.hpp>
#include <libdnf5/transaction/transaction_item_reason.hpp>

#include <string>

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
2 changes: 1 addition & 1 deletion test/shared/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
37 changes: 3 additions & 34 deletions test/shared/base_test_case.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,19 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

#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 <libdnf5/comps/environment/environment.hpp>
#include "libdnf5/utils/fs/temp.hpp"

#include <libdnf5/comps/environment/query.hpp>
#include <libdnf5/comps/group/group.hpp>
#include <libdnf5/comps/group/query.hpp>
#include <libdnf5/conf/const.hpp>
#include <libdnf5/rpm/nevra.hpp>
#include <libdnf5/rpm/package_query.hpp>

#include <filesystem>
#include <map>


using fmt::format;
#include <set>


libdnf5::repo::RepoWeakPtr BaseTestCase::add_repo(
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 5 additions & 4 deletions test/shared/base_test_case.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ along with libdnf. If not, see <https://www.gnu.org/licenses/>.

#include "test_case_fixture.hpp"

#include "libdnf5/utils/fs/temp.hpp"

#include <libdnf5/advisory/advisory.hpp>
#include <libdnf5/base/base.hpp>
#include <libdnf5/comps/environment/environment.hpp>
#include <libdnf5/comps/group/group.hpp>
#include <libdnf5/repo/repo_query.hpp>
#include <libdnf5/repo/repo_sack.hpp>
#include <libdnf5/repo/repo_weak.hpp>
#include <libdnf5/rpm/package.hpp>
#include <libdnf5/rpm/package_sack.hpp>

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit e51e58f

Please sign in to comment.