From 4a496db278c9f2af7270ed27116b7282e1820769 Mon Sep 17 00:00:00 2001 From: Jaroslav Rohel Date: Tue, 10 Dec 2024 22:50:48 +0100 Subject: [PATCH] Unit tests: Fix problematic nasty hack in run_tests I found a nasty hack in the code. A pointer to the `CPPUnit::TestCaller` class was cast to a pointer to `HackTestCaller` using `reinterpret_cast`. The hack was used to make private members accessible in the original class for `LogCaptureListener`. Additionally, `BaseTestCase` has been modified in the `LogCaptureListener` class. The code has been rewritten to not need this hack. And the `BaseTestCase` modification was moved to the `BaseTestCase::setUp` method. Solves: Start 1: test_libdnf 1/3 Test #1: test_libdnf ......................***Failed 4.64 sec /dnf5/test/libdnf5/run_tests.cpp:69:53: runtime error: member access within address 0x50800001a7a0 which does not point to an object of type 'HackTestCaller' 0x50800001a7a0: note: object is of type 'CppUnit::TestCaller' 00 00 00 00 a0 7d c1 01 00 00 00 00 30 7e c1 01 00 00 00 00 90 c3 01 ^~~~~~~~~~~~~~~~~~~~~~~ vptr for 'CppUnit::TestCaller' 00 40 50 00 00 29 00 00 00 ================================================================= ==441509==ERROR: LeakSanitizer: detected memory leaks Direct leak of 128 byte(s) in 2 object(s) allocated from: #0 0x7f19b32c17d8 in realloc.part.0 (/lib64/libasan.so.8+0xc17d8) (BuildId: 5294bd2731fcae07af92dfea7808576c57d53bc9) #1 0x7f19b261e242 in d_growable_string_callback_adapter (/lib64/libstdc++.so.6+0x1e242) SUMMARY: AddressSanitizer: 128 byte(s) leaked in 2 allocation(s). --- test/libdnf5/run_tests.cpp | 41 +++++++---------------------- test/shared/base_test_case.cpp | 4 +++ test/shared/logger_redirector.hpp | 43 +++++++++++++++++++++++++++++++ test/shared/test_logger.cpp | 23 +++++++++++++++++ test/shared/test_logger.hpp | 30 +++++++++++++++++++++ 5 files changed, 110 insertions(+), 31 deletions(-) create mode 100644 test/shared/logger_redirector.hpp create mode 100644 test/shared/test_logger.cpp create mode 100644 test/shared/test_logger.hpp diff --git a/test/libdnf5/run_tests.cpp b/test/libdnf5/run_tests.cpp index 54433a137..e44469481 100644 --- a/test/libdnf5/run_tests.cpp +++ b/test/libdnf5/run_tests.cpp @@ -19,6 +19,7 @@ along with libdnf. If not, see . #include "../shared/base_test_case.hpp" +#include "../shared/test_logger.hpp" #include #include @@ -35,19 +36,6 @@ along with libdnf. If not, see . #include -// HACK: CppUnit doesn't give access to the actual test case it is running. The -// given pointer is an instance of CppUnit::TestCaller, which has the test case -// itself as a private member with no getter. -// -// Here we mimic the structure of CppUnit::TestCaller and make the pointer -// accessible. -class HackTestCaller : CppUnit::TestCase { -public: - bool m_ownFixture; // unused - CppUnit::TestCase * m_fixture; // our test case class -}; - - class TimingListener : public CppUnit::TestListener { public: void startTest(CppUnit::Test *) override { start = std::chrono::high_resolution_clock::now(); } @@ -62,28 +50,19 @@ class TimingListener : public CppUnit::TestListener { std::chrono::high_resolution_clock::time_point start = std::chrono::high_resolution_clock::from_time_t(0); }; + class LogCaptureListener : public CppUnit::TestListener { public: - void startTest(CppUnit::Test * t) override { - auto * f = reinterpret_cast(t); - auto * tc = dynamic_cast(f->m_fixture); - - if (tc) { - tc->base.get_logger()->add_logger(std::make_unique(10000, 256)); - } + void startTest(CppUnit::Test *) override { + // Global test_logger is used. Clear it befor starting new test. + test_logger.clear(); } - void addFailure(const CppUnit::TestFailure & failure) override { - auto * f = reinterpret_cast(failure.failedTest()); - auto * tc = dynamic_cast(f->m_fixture); - - if (tc) { - std::cout << std::endl << "Dnf log:" << std::endl; - libdnf5::StdCStreamLogger cout_logger(std::cout); - dynamic_cast(tc->base.get_logger()->get_logger(0)) - ->write_to_logger(cout_logger); - std::cout << std::endl << std::flush; - } + void addFailure(const CppUnit::TestFailure &) override { + std::cout << std::endl << "Dnf log:" << std::endl; + libdnf5::StdCStreamLogger cout_logger(std::cout); + test_logger.write_to_logger(cout_logger); + std::cout << std::endl; } }; diff --git a/test/shared/base_test_case.cpp b/test/shared/base_test_case.cpp index d3d17d327..0cfdd9b2e 100644 --- a/test/shared/base_test_case.cpp +++ b/test/shared/base_test_case.cpp @@ -21,7 +21,9 @@ 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" @@ -222,6 +224,8 @@ libdnf5::rpm::Package BaseTestCase::first_query_pkg(libdnf5::rpm::PackageQuery & void BaseTestCase::setUp() { TestCaseFixture::setUp(); + base.get_logger()->add_logger(std::make_unique(test_logger)); + // TODO we could use get_preconfigured_base() for this now, but that would // need changing the `base` member to a unique_ptr temp_dir = std::make_unique("libdnf5_unittest"); diff --git a/test/shared/logger_redirector.hpp b/test/shared/logger_redirector.hpp new file mode 100644 index 000000000..461e0a7fc --- /dev/null +++ b/test/shared/logger_redirector.hpp @@ -0,0 +1,43 @@ +/* +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_LOGGER_REDIRECTOR_HPP +#define TEST_LIBDNF5_LOGGER_REDIRECTOR_HPP + +#include + +// LoggerRedirector is used in tests to route logging from libdnf5::Base to the global logger. +class LoggerRedirector : public libdnf5::Logger { +public: + LoggerRedirector(libdnf5::Logger & target_logger) : target_logger{target_logger} {} + + void write( + const std::chrono::time_point & time, + pid_t pid, + Level level, + const std::string & message) noexcept override { + target_logger.write(time, pid, level, message); + } + +private: + libdnf5::Logger & target_logger; +}; + +#endif // TEST_LIBDNF5_LOGGER_REDIRECTOR_HPP diff --git a/test/shared/test_logger.cpp b/test/shared/test_logger.cpp new file mode 100644 index 000000000..736d96f39 --- /dev/null +++ b/test/shared/test_logger.cpp @@ -0,0 +1,23 @@ +/* +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 "test_logger.hpp" + +libdnf5::MemoryBufferLogger test_logger(10000, 256); diff --git a/test/shared/test_logger.hpp b/test/shared/test_logger.hpp new file mode 100644 index 000000000..9d69f7a85 --- /dev/null +++ b/test/shared/test_logger.hpp @@ -0,0 +1,30 @@ +/* +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_TEST_LOGGER_HPP +#define TEST_LIBDNF5_TEST_LOGGER_HPP + +#include + +// Global logger used in many tests. +// Logging from libdnf5::Base is routed to it using LoggerRedirector. +extern libdnf5::MemoryBufferLogger test_logger; + +#endif // TEST_LIBDNF5_TEST_LOGGER_HPP