From 02ce0a2eecfc69b30d8e29cd210ea2fda0313ac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Tue, 14 Mar 2023 23:25:35 +0100 Subject: [PATCH] Unify ITestCaseRegistry and TestRegistry I also made a bunch of refactorings around the headers and includes to simplify the main include path. --- src/catch2/catch_registry_hub.cpp | 4 +- src/catch2/catch_session.cpp | 2 + src/catch2/catch_test_spec.cpp | 1 + .../catch_interfaces_registry_hub.hpp | 4 +- .../interfaces/catch_interfaces_testcase.cpp | 1 - .../interfaces/catch_interfaces_testcase.hpp | 22 ---- src/catch2/internal/catch_list.cpp | 2 +- .../catch_test_case_registry_impl.cpp | 103 +++++++++++------- .../catch_test_case_registry_impl.hpp | 31 ++---- 9 files changed, 82 insertions(+), 88 deletions(-) diff --git a/src/catch2/catch_registry_hub.cpp b/src/catch2/catch_registry_hub.cpp index 4db9c7e0..f47d1742 100644 --- a/src/catch2/catch_registry_hub.cpp +++ b/src/catch2/catch_registry_hub.cpp @@ -35,7 +35,7 @@ namespace Catch { ReporterRegistry const& getReporterRegistry() const override { return m_reporterRegistry; } - ITestCaseRegistry const& getTestCaseRegistry() const override { + TestCaseRegistry const& getTestCaseRegistry() const override { return m_testCaseRegistry; } ExceptionTranslatorRegistry const& getExceptionTranslatorRegistry() const override { @@ -76,7 +76,7 @@ namespace Catch { } private: - TestRegistry m_testCaseRegistry; + TestCaseRegistry m_testCaseRegistry; ReporterRegistry m_reporterRegistry; ExceptionTranslatorRegistry m_exceptionTranslatorRegistry; TagAliasRegistry m_tagAliasRegistry; diff --git a/src/catch2/catch_session.cpp b/src/catch2/catch_session.cpp index a73334f4..681a82d9 100644 --- a/src/catch2/catch_session.cpp +++ b/src/catch2/catch_session.cpp @@ -24,6 +24,8 @@ #include #include #include +#include + #include #include diff --git a/src/catch2/catch_test_spec.cpp b/src/catch2/catch_test_spec.cpp index f27ce99c..de1d32d9 100644 --- a/src/catch2/catch_test_spec.cpp +++ b/src/catch2/catch_test_spec.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include diff --git a/src/catch2/interfaces/catch_interfaces_registry_hub.hpp b/src/catch2/interfaces/catch_interfaces_registry_hub.hpp index ba9c34f7..14a935fa 100644 --- a/src/catch2/interfaces/catch_interfaces_registry_hub.hpp +++ b/src/catch2/interfaces/catch_interfaces_registry_hub.hpp @@ -16,7 +16,7 @@ namespace Catch { class TestCaseHandle; struct TestCaseInfo; - class ITestCaseRegistry; + class TestCaseRegistry; class ExceptionTranslatorRegistry; class IExceptionTranslator; class ReporterRegistry; @@ -36,7 +36,7 @@ namespace Catch { virtual ~IRegistryHub(); // = default virtual ReporterRegistry const& getReporterRegistry() const = 0; - virtual ITestCaseRegistry const& getTestCaseRegistry() const = 0; + virtual TestCaseRegistry const& getTestCaseRegistry() const = 0; virtual TagAliasRegistry const& getTagAliasRegistry() const = 0; virtual ExceptionTranslatorRegistry const& getExceptionTranslatorRegistry() const = 0; diff --git a/src/catch2/interfaces/catch_interfaces_testcase.cpp b/src/catch2/interfaces/catch_interfaces_testcase.cpp index 5e632ba8..3cd0ffd0 100644 --- a/src/catch2/interfaces/catch_interfaces_testcase.cpp +++ b/src/catch2/interfaces/catch_interfaces_testcase.cpp @@ -10,5 +10,4 @@ namespace Catch { ITestInvoker::~ITestInvoker() = default; - ITestCaseRegistry::~ITestCaseRegistry() = default; } diff --git a/src/catch2/interfaces/catch_interfaces_testcase.hpp b/src/catch2/interfaces/catch_interfaces_testcase.hpp index 78ee2021..73aaa669 100644 --- a/src/catch2/interfaces/catch_interfaces_testcase.hpp +++ b/src/catch2/interfaces/catch_interfaces_testcase.hpp @@ -8,36 +8,14 @@ #ifndef CATCH_INTERFACES_TESTCASE_HPP_INCLUDED #define CATCH_INTERFACES_TESTCASE_HPP_INCLUDED -#include - namespace Catch { - class TestSpec; - struct TestCaseInfo; - class ITestInvoker { public: virtual void invoke () const = 0; virtual ~ITestInvoker(); // = default }; - class TestCaseHandle; - class IConfig; - - 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 std::vector const& getAllTests() const = 0; - virtual std::vector const& getAllTestsSorted( IConfig const& config ) const = 0; - }; - - bool isThrowSafe( TestCaseHandle const& testCase, IConfig const& config ); - bool matchTest( TestCaseHandle const& testCase, TestSpec const& testSpec, IConfig const& config ); - std::vector filterTests( std::vector const& testCases, TestSpec const& testSpec, IConfig const& config ); - std::vector const& getAllTestCasesSorted( IConfig const& config ); - } #endif // CATCH_INTERFACES_TESTCASE_HPP_INCLUDED diff --git a/src/catch2/internal/catch_list.cpp b/src/catch2/internal/catch_list.cpp index 4eb8bb7d..87ace395 100644 --- a/src/catch2/internal/catch_list.cpp +++ b/src/catch2/internal/catch_list.cpp @@ -14,7 +14,7 @@ #include #include #include - +#include #include #include #include diff --git a/src/catch2/internal/catch_test_case_registry_impl.cpp b/src/catch2/internal/catch_test_case_registry_impl.cpp index b66469bc..e806ea6e 100644 --- a/src/catch2/internal/catch_test_case_registry_impl.cpp +++ b/src/catch2/internal/catch_test_case_registry_impl.cpp @@ -24,6 +24,54 @@ namespace Catch { + namespace { + static bool matchTest( TestCaseHandle const& testCase, + TestSpec const& testSpec, + IConfig const& config ) { + return testSpec.matches( testCase.getTestCaseInfo() ) && + isThrowSafe( testCase, config ); + } + + static void enforceNoDuplicateTestCases( + std::vector const& tests ) { + auto testInfoCmp = []( TestCaseInfo const* lhs, + TestCaseInfo const* rhs ) { + return *lhs < *rhs; + }; + std::set seenTests( + testInfoCmp ); + for ( auto const& test : tests ) { + const auto infoPtr = &test.getTestCaseInfo(); + const auto prev = seenTests.insert( infoPtr ); + CATCH_ENFORCE( prev.second, + "error: test case \"" + << infoPtr->name << "\", with tags \"" + << infoPtr->tagsAsString() + << "\" already defined.\n" + << "\tFirst seen at " + << ( *prev.first )->lineInfo << "\n" + << "\tRedefined at " << infoPtr->lineInfo ); + } + } + } // namespace + + struct TestCaseRegistry::TestCaseRegistryImpl { + std::vector> owned_test_infos; + // Keeps a materialized vector for `getAllInfos`. + // We should get rid of that eventually (see interface note) + std::vector viewed_test_infos; + + std::vector> invokers; + std::vector handles; + mutable TestRunOrder currentSortOrder = TestRunOrder::Declared; + mutable std::vector sortedFunctions; + }; + + TestCaseRegistry::TestCaseRegistry(): + m_impl( Detail::make_unique() ) {} + TestCaseRegistry ::~TestCaseRegistry() = default; + + std::vector sortTests( IConfig const& config, std::vector const& unsortedTestCases ) { switch (config.runOrder()) { case TestRunOrder::Declared: @@ -80,29 +128,6 @@ namespace Catch { return !testCase.getTestCaseInfo().throws() || config.allowThrows(); } - bool matchTest( TestCaseHandle const& testCase, TestSpec const& testSpec, IConfig const& config ) { - return testSpec.matches( testCase.getTestCaseInfo() ) && isThrowSafe( testCase, config ); - } - - void - enforceNoDuplicateTestCases( std::vector const& tests ) { - auto testInfoCmp = []( TestCaseInfo const* lhs, - TestCaseInfo const* rhs ) { - return *lhs < *rhs; - }; - std::set seenTests(testInfoCmp); - for ( auto const& test : tests ) { - const auto infoPtr = &test.getTestCaseInfo(); - const auto prev = seenTests.insert( infoPtr ); - CATCH_ENFORCE( - prev.second, - "error: test case \"" << infoPtr->name << "\", with tags \"" - << infoPtr->tagsAsString() << "\" already defined.\n" - << "\tFirst seen at " << ( *prev.first )->lineInfo << "\n" - << "\tRedefined at " << infoPtr->lineInfo ); - } - } - std::vector filterTests( std::vector const& testCases, TestSpec const& testSpec, IConfig const& config ) { std::vector filtered; filtered.reserve( testCases.size() ); @@ -118,29 +143,29 @@ namespace Catch { return getRegistryHub().getTestCaseRegistry().getAllTestsSorted( config ); } - 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_invokers.push_back(CATCH_MOVE(testInvoker)); + void TestCaseRegistry::registerTest(Detail::unique_ptr testInfo, Detail::unique_ptr testInvoker) { + m_impl->handles.emplace_back(testInfo.get(), testInvoker.get()); + m_impl->viewed_test_infos.push_back(testInfo.get()); + m_impl->owned_test_infos.push_back(CATCH_MOVE(testInfo)); + m_impl->invokers.push_back(CATCH_MOVE(testInvoker)); } - std::vector const& TestRegistry::getAllInfos() const { - return m_viewed_test_infos; + std::vector const& TestCaseRegistry::getAllInfos() const { + return m_impl->viewed_test_infos; } - std::vector const& TestRegistry::getAllTests() const { - return m_handles; + std::vector const& TestCaseRegistry::getAllTests() const { + return m_impl->handles; } - std::vector const& TestRegistry::getAllTestsSorted( IConfig const& config ) const { - if( m_sortedFunctions.empty() ) - enforceNoDuplicateTestCases( m_handles ); + std::vector const& TestCaseRegistry::getAllTestsSorted( IConfig const& config ) const { + if( m_impl->sortedFunctions.empty() ) + enforceNoDuplicateTestCases( m_impl->handles ); - if( m_currentSortOrder != config.runOrder() || m_sortedFunctions.empty() ) { - m_sortedFunctions = sortTests( config, m_handles ); - m_currentSortOrder = config.runOrder(); + if( m_impl->currentSortOrder != config.runOrder() || m_impl->sortedFunctions.empty() ) { + m_impl->sortedFunctions = sortTests( config, m_impl->handles ); + m_impl->currentSortOrder = config.runOrder(); } - return m_sortedFunctions; + return m_impl->sortedFunctions; } } // end namespace Catch diff --git a/src/catch2/internal/catch_test_case_registry_impl.hpp b/src/catch2/internal/catch_test_case_registry_impl.hpp index cc6b133f..28a91af3 100644 --- a/src/catch2/internal/catch_test_case_registry_impl.hpp +++ b/src/catch2/internal/catch_test_case_registry_impl.hpp @@ -8,8 +8,6 @@ #ifndef CATCH_TEST_CASE_REGISTRY_IMPL_HPP_INCLUDED #define CATCH_TEST_CASE_REGISTRY_IMPL_HPP_INCLUDED -#include -#include #include #include @@ -19,37 +17,28 @@ namespace Catch { class TestCaseHandle; class IConfig; class TestSpec; + class ITestInvoker; + struct TestCaseInfo; std::vector sortTests( IConfig const& config, std::vector const& unsortedTestCases ); bool isThrowSafe( TestCaseHandle const& testCase, IConfig const& config ); - bool matchTest( TestCaseHandle const& testCase, TestSpec const& testSpec, IConfig const& config ); - - void enforceNoDuplicateTestCases( std::vector const& functions ); std::vector filterTests( std::vector const& testCases, TestSpec const& testSpec, IConfig const& config ); std::vector const& getAllTestCasesSorted( IConfig const& config ); - class TestRegistry : public ITestCaseRegistry { + class TestCaseRegistry { + struct TestCaseRegistryImpl; + Detail::unique_ptr m_impl; public: - ~TestRegistry() override = default; + TestCaseRegistry(); + ~TestCaseRegistry(); // = default; void registerTest( Detail::unique_ptr testInfo, Detail::unique_ptr testInvoker ); - std::vector const& getAllInfos() const override; - std::vector const& getAllTests() const override; - std::vector const& getAllTestsSorted( IConfig const& config ) const override; - - 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; - - std::vector> m_invokers; - std::vector m_handles; - mutable TestRunOrder m_currentSortOrder = TestRunOrder::Declared; - mutable std::vector m_sortedFunctions; + std::vector const& getAllInfos() const; + std::vector const& getAllTests() const; + std::vector const& getAllTestsSorted( IConfig const& config ) const; }; ///////////////////////////////////////////////////////////////////////////