From ef3a728265292e6a1dbec727b0099d3d8b3f6420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sat, 22 Aug 2026 23:28:11 +0200 Subject: [PATCH] Don't store extra copy of all `TestCaseInfo*` in `TestRegistry` We used to store a full vector of "views" of the registered test infos, so that it can be retrieved externally. However, the only user of that interface left was `Session`, specifically `Session::applyFilenameAsTags`. By moving the adding of the filename tags into the `ITestCaseRegistry` interface, we avoid storing the vector and make the initial test registration faster. The new code is ugly and shows a design/layering issue, but no worse than the previous approach did. --- src/catch2/catch_registry_hub.cpp | 4 ++++ src/catch2/catch_session.cpp | 4 +--- .../interfaces/catch_interfaces_registry_hub.hpp | 2 ++ src/catch2/interfaces/catch_interfaces_testcase.hpp | 3 +-- .../internal/catch_test_case_registry_impl.cpp | 12 ++++++------ .../internal/catch_test_case_registry_impl.hpp | 11 +++++------ 6 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/catch2/catch_registry_hub.cpp b/src/catch2/catch_registry_hub.cpp index 25aecdde..165d74a2 100644 --- a/src/catch2/catch_registry_hub.cpp +++ b/src/catch2/catch_registry_hub.cpp @@ -72,6 +72,10 @@ namespace Catch { #endif } + ITestCaseRegistry& getMutableTestCaseRegistry() override { + return m_testCaseRegistry; + } + private: TestRegistry m_testCaseRegistry; ReporterRegistry m_reporterRegistry; diff --git a/src/catch2/catch_session.cpp b/src/catch2/catch_session.cpp index 5e9c5c19..bf14d23b 100644 --- a/src/catch2/catch_session.cpp +++ b/src/catch2/catch_session.cpp @@ -140,9 +140,7 @@ namespace Catch { }; void applyFilenamesAsTags() { - for (auto const& testInfo : getRegistryHub().getTestCaseRegistry().getAllInfos()) { - testInfo->addFilenameTag(); - } + getMutableRegistryHub().getMutableTestCaseRegistry().enableFilenameTags(); } // Creates empty file at path. The path must be writable, we do not diff --git a/src/catch2/interfaces/catch_interfaces_registry_hub.hpp b/src/catch2/interfaces/catch_interfaces_registry_hub.hpp index 3695a192..072b48bf 100644 --- a/src/catch2/interfaces/catch_interfaces_registry_hub.hpp +++ b/src/catch2/interfaces/catch_interfaces_registry_hub.hpp @@ -53,6 +53,8 @@ namespace Catch { virtual void registerTranslator( Detail::unique_ptr&& translator ) = 0; virtual void registerTagAlias( std::string const& alias, std::string const& tag, SourceLineInfo const& lineInfo ) = 0; virtual void registerStartupException() noexcept = 0; + + virtual ITestCaseRegistry& getMutableTestCaseRegistry() = 0; }; IRegistryHub const& getRegistryHub(); diff --git a/src/catch2/interfaces/catch_interfaces_testcase.hpp b/src/catch2/interfaces/catch_interfaces_testcase.hpp index daee8482..67c4371a 100644 --- a/src/catch2/interfaces/catch_interfaces_testcase.hpp +++ b/src/catch2/interfaces/catch_interfaces_testcase.hpp @@ -19,8 +19,7 @@ namespace Catch { class ITestCaseRegistry { public: virtual ~ITestCaseRegistry(); // = default - // TODO: this exists only for adding filenames to test cases -- let's expose this in a saner way later - virtual std::vector const& getAllInfos() const = 0; + virtual void enableFilenameTags() = 0; virtual std::vector const& getAllTests() const = 0; virtual std::vector const& getAllTestsSorted( IConfig const& config ) const = 0; }; diff --git a/src/catch2/internal/catch_test_case_registry_impl.cpp b/src/catch2/internal/catch_test_case_registry_impl.cpp index d23056bc..e0d7c139 100644 --- a/src/catch2/internal/catch_test_case_registry_impl.cpp +++ b/src/catch2/internal/catch_test_case_registry_impl.cpp @@ -131,21 +131,21 @@ namespace Catch { // We pre-reserve some reasonable number of tests to avoid the // initial geometric growth churning during test registration. m_handles.reserve( kInitialTestCount ); - m_viewed_test_infos.reserve( kInitialTestCount ); - m_owned_test_infos.reserve( kInitialTestCount ); + m_test_infos.reserve( kInitialTestCount ); m_invokers.reserve( kInitialTestCount ); } TestRegistry::~TestRegistry() = default; void TestRegistry::registerTest(Detail::unique_ptr testInfo, Detail::unique_ptr testInvoker) { m_handles.emplace_back(testInfo.get(), testInvoker.get()); - m_viewed_test_infos.push_back(testInfo.get()); - m_owned_test_infos.push_back(CATCH_MOVE(testInfo)); + m_test_infos.push_back(CATCH_MOVE(testInfo)); m_invokers.push_back(CATCH_MOVE(testInvoker)); } - std::vector const& TestRegistry::getAllInfos() const { - return m_viewed_test_infos; + void TestRegistry::enableFilenameTags() { + for (auto& info : m_test_infos) { + info->addFilenameTag(); + } } std::vector const& TestRegistry::getAllTests() const { diff --git a/src/catch2/internal/catch_test_case_registry_impl.hpp b/src/catch2/internal/catch_test_case_registry_impl.hpp index 1f883109..3a9dc30c 100644 --- a/src/catch2/internal/catch_test_case_registry_impl.hpp +++ b/src/catch2/internal/catch_test_case_registry_impl.hpp @@ -31,8 +31,8 @@ namespace Catch { class TestRegistry final : public ITestCaseRegistry { public: void registerTest( Detail::unique_ptr testInfo, Detail::unique_ptr testInvoker ); + void enableFilenameTags() override; - std::vector const& getAllInfos() const override; std::vector const& getAllTests() const override; std::vector const& getAllTestsSorted( IConfig const& config ) const override; @@ -40,12 +40,11 @@ namespace Catch { ~TestRegistry() override; // = default private: - std::vector> m_owned_test_infos; - // Keeps a materialized vector for `getAllInfos`. - // We should get rid of that eventually (see interface note) - std::vector m_viewed_test_infos; - + // Owns the test infos for handles + std::vector> m_test_infos; + // Owns the test invokers for handles std::vector> m_invokers; + std::vector m_handles; mutable TestRunOrder m_currentSortOrder = TestRunOrder::Declared; mutable std::vector m_sortedFunctions;