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

@@ -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;

src/catch2/catch_session.cpp

@@ -24,6 +24,8 @@
 #include <catch2/internal/catch_move_and_forward.hpp>
 #include <catch2/internal/catch_stdstreams.hpp>
 #include <catch2/internal/catch_istream.hpp>
+#include <catch2/internal/catch_test_case_registry_impl.hpp>
+
 
 #include <algorithm>
 #include <cassert>

src/catch2/catch_test_spec.cpp

@@ -8,6 +8,7 @@
 #include <catch2/catch_test_spec.hpp>
 #include <catch2/internal/catch_reusable_string_stream.hpp>
 #include <catch2/internal/catch_string_manip.hpp>
+#include <catch2/internal/catch_test_case_registry_impl.hpp>
 #include <catch2/catch_test_case_info.hpp>
 
 #include <algorithm>

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;
 

src/catch2/interfaces/catch_interfaces_testcase.cpp

@@ -10,5 +10,4 @@
 
 namespace Catch {
     ITestInvoker::~ITestInvoker() = default;
-    ITestCaseRegistry::~ITestCaseRegistry() = default;
 }

src/catch2/interfaces/catch_interfaces_testcase.hpp

@@ -8,36 +8,14 @@
 #ifndef CATCH_INTERFACES_TESTCASE_HPP_INCLUDED
 #define CATCH_INTERFACES_TESTCASE_HPP_INCLUDED
 
-#include <vector>
-
 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<TestCaseInfo* > const& getAllInfos() const = 0;
-        virtual std::vector<TestCaseHandle> const& getAllTests() const = 0;
-        virtual std::vector<TestCaseHandle> 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<TestCaseHandle> filterTests( std::vector<TestCaseHandle> const& testCases, TestSpec const& testSpec, IConfig const& config );
-    std::vector<TestCaseHandle> const& getAllTestCasesSorted( IConfig const& config );
-
 }
 
 #endif // CATCH_INTERFACES_TESTCASE_HPP_INCLUDED

src/catch2/internal/catch_list.cpp

@@ -14,7 +14,7 @@
 #include <catch2/internal/catch_reporter_registry.hpp>
 #include <catch2/internal/catch_move_and_forward.hpp>
 #include <catch2/internal/catch_case_insensitive_comparisons.hpp>
-
+#include <catch2/internal/catch_test_case_registry_impl.hpp>
 #include <catch2/internal/catch_context.hpp>
 #include <catch2/catch_config.hpp>
 #include <catch2/catch_test_spec.hpp>

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<TestCaseHandle> const& tests ) {
+            auto testInfoCmp = []( TestCaseInfo const* lhs,
+                                   TestCaseInfo const* rhs ) {
+                return *lhs < *rhs;
+            };
+            std::set<TestCaseInfo const*, decltype( testInfoCmp )&> 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<Detail::unique_ptr<TestCaseInfo>> owned_test_infos;
+        // Keeps a materialized vector for `getAllInfos`.
+        // We should get rid of that eventually (see interface note)
+        std::vector<TestCaseInfo*> viewed_test_infos;
+
+        std::vector<Detail::unique_ptr<ITestInvoker>> invokers;
+        std::vector<TestCaseHandle> handles;
+        mutable TestRunOrder currentSortOrder = TestRunOrder::Declared;
+        mutable std::vector<TestCaseHandle> sortedFunctions;
+    };
+
+    TestCaseRegistry::TestCaseRegistry():
+        m_impl( Detail::make_unique<TestCaseRegistryImpl>() ) {}
+    TestCaseRegistry ::~TestCaseRegistry() = default;
+
+
     std::vector<TestCaseHandle> sortTests( IConfig const& config, std::vector<TestCaseHandle> 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<TestCaseHandle> const& tests ) {
-        auto testInfoCmp = []( TestCaseInfo const* lhs,
-                               TestCaseInfo const* rhs ) {
-            return *lhs < *rhs;
-        };
-        std::set<TestCaseInfo const*, decltype(testInfoCmp) &> 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<TestCaseHandle> filterTests( std::vector<TestCaseHandle> const& testCases, TestSpec const& testSpec, IConfig const& config ) {
         std::vector<TestCaseHandle> filtered;
         filtered.reserve( testCases.size() );
@@ -118,29 +143,29 @@ namespace Catch {
         return getRegistryHub().getTestCaseRegistry().getAllTestsSorted( config );
     }
 
-    void TestRegistry::registerTest(Detail::unique_ptr<TestCaseInfo> testInfo, Detail::unique_ptr<ITestInvoker> testInvoker) {
+    void TestCaseRegistry::registerTest(Detail::unique_ptr<TestCaseInfo> testInfo, Detail::unique_ptr<ITestInvoker> testInvoker) {
-        m_handles.emplace_back(testInfo.get(), testInvoker.get());
+        m_impl->handles.emplace_back(testInfo.get(), testInvoker.get());
-        m_viewed_test_infos.push_back(testInfo.get());
+        m_impl->viewed_test_infos.push_back(testInfo.get());
-        m_owned_test_infos.push_back(CATCH_MOVE(testInfo));
+        m_impl->owned_test_infos.push_back(CATCH_MOVE(testInfo));
-        m_invokers.push_back(CATCH_MOVE(testInvoker));
+        m_impl->invokers.push_back(CATCH_MOVE(testInvoker));
     }
 
-    std::vector<TestCaseInfo*> const& TestRegistry::getAllInfos() const {
+    std::vector<TestCaseInfo*> const& TestCaseRegistry::getAllInfos() const {
-        return m_viewed_test_infos;
+        return m_impl->viewed_test_infos;
     }
 
-    std::vector<TestCaseHandle> const& TestRegistry::getAllTests() const {
+    std::vector<TestCaseHandle> const& TestCaseRegistry::getAllTests() const {
-        return m_handles;
+        return m_impl->handles;
     }
-    std::vector<TestCaseHandle> const& TestRegistry::getAllTestsSorted( IConfig const& config ) const {
+    std::vector<TestCaseHandle> const& TestCaseRegistry::getAllTestsSorted( IConfig const& config ) const {
-        if( m_sortedFunctions.empty() )
+        if( m_impl->sortedFunctions.empty() )
-            enforceNoDuplicateTestCases( m_handles );
+            enforceNoDuplicateTestCases( m_impl->handles );
 
-        if(  m_currentSortOrder != config.runOrder() || m_sortedFunctions.empty() ) {
+        if(  m_impl->currentSortOrder != config.runOrder() || m_impl->sortedFunctions.empty() ) {
-            m_sortedFunctions = sortTests( config, m_handles );
+            m_impl->sortedFunctions = sortTests( config, m_impl->handles );
-            m_currentSortOrder = config.runOrder();
+            m_impl->currentSortOrder = config.runOrder();
         }
-        return m_sortedFunctions;
+        return m_impl->sortedFunctions;
     }
 
 } // end namespace Catch

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 <catch2/interfaces/catch_interfaces_testcase.hpp>
-#include <catch2/interfaces/catch_interfaces_config.hpp>
 #include <catch2/internal/catch_unique_ptr.hpp>
 
 #include <vector>
@@ -19,37 +17,28 @@ namespace Catch {
     class TestCaseHandle;
     class IConfig;
     class TestSpec;
+    class ITestInvoker;
+    struct TestCaseInfo;
 
     std::vector<TestCaseHandle> sortTests( IConfig const& config, std::vector<TestCaseHandle> 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<TestCaseHandle> const& functions );
 
     std::vector<TestCaseHandle> filterTests( std::vector<TestCaseHandle> const& testCases, TestSpec const& testSpec, IConfig const& config );
     std::vector<TestCaseHandle> const& getAllTestCasesSorted( IConfig const& config );
 
-    class TestRegistry : public ITestCaseRegistry {
+    class TestCaseRegistry {
+        struct TestCaseRegistryImpl;
+        Detail::unique_ptr<TestCaseRegistryImpl> m_impl;
     public:
-        ~TestRegistry() override = default;
+        TestCaseRegistry();
+        ~TestCaseRegistry(); // = default;
 
         void registerTest( Detail::unique_ptr<TestCaseInfo> testInfo, Detail::unique_ptr<ITestInvoker> testInvoker );
 
-        std::vector<TestCaseInfo*> const& getAllInfos() const override;
+        std::vector<TestCaseInfo*> const& getAllInfos() const;
-        std::vector<TestCaseHandle> const& getAllTests() const override;
+        std::vector<TestCaseHandle> const& getAllTests() const;
-        std::vector<TestCaseHandle> const& getAllTestsSorted( IConfig const& config ) const override;
+        std::vector<TestCaseHandle> const& getAllTestsSorted( IConfig const& config ) const;
-
-    private:
-        std::vector<Detail::unique_ptr<TestCaseInfo>> m_owned_test_infos;
-        // Keeps a materialized vector for `getAllInfos`.
-        // We should get rid of that eventually (see interface note)
-        std::vector<TestCaseInfo*> m_viewed_test_infos;
-
-        std::vector<Detail::unique_ptr<ITestInvoker>> m_invokers;
-        std::vector<TestCaseHandle> m_handles;
-        mutable TestRunOrder m_currentSortOrder = TestRunOrder::Declared;
-        mutable std::vector<TestCaseHandle> m_sortedFunctions;
     };
 
     ///////////////////////////////////////////////////////////////////////////