Refactored test filtering and sorting

include/catch_runner.hpp

@@ -64,36 +64,22 @@ namespace Catch {
         if( !testSpec.hasFilters() )
             testSpec = TestSpecParser( ITagAliasRegistry::get() ).parse( "~[.]" ).testSpec(); // All not hidden tests
 
-        std::vector<TestCase> testCases;
+        std::vector<TestCase> const& allTestCases = getAllTestCasesSorted( *config );
-        getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, *config, testCases );
+        for( std::vector<TestCase>::const_iterator it = allTestCases.begin(), itEnd = allTestCases.end();
-
-        std::set<TestCase> testsAlreadyRun;
-        for( std::vector<TestCase>::const_iterator it = testCases.begin(), itEnd = testCases.end();
                 it != itEnd;
                 ++it ) {
-            if( testsAlreadyRun.find( *it ) == testsAlreadyRun.end() ) {
+            if( !context.aborting() && matchTest( *it, testSpec, *config ) )
-
-                if( context.aborting() )
-                    break;
-
                 totals += context.runTest( *it );
-                testsAlreadyRun.insert( *it );
+            else
-            }
+                reporter->skipTest( *it );
         }
-        std::vector<TestCase> skippedTestCases;
-        getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, *config, skippedTestCases, true );
-        
-        for( std::vector<TestCase>::const_iterator it = skippedTestCases.begin(), itEnd = skippedTestCases.end();
-                it != itEnd;
-                ++it )
-            reporter->skipTest( *it );
 
         context.testGroupEnded( config->name(), totals, 1, 1 );
         return totals;
     }
     
-    void applyFilenamesAsTags() {
+    void applyFilenamesAsTags( IConfig const& config ) {
-        std::vector<TestCase> const& tests = getRegistryHub().getTestCaseRegistry().getAllTests();
+        std::vector<TestCase> const& tests = getAllTestCasesSorted( config );
         for(std::size_t i = 0; i < tests.size(); ++i ) {
             TestCase& test = const_cast<TestCase&>( tests[i] );
             std::set<std::string> tags = test.tags;
@@ -181,12 +167,12 @@ namespace Catch {
             try
             {
                 config(); // Force config to be constructed
-
-                if( m_configData.filenamesAsTags )
-                    applyFilenamesAsTags();
                 
                 seedRng( *m_config );
 
+                if( m_configData.filenamesAsTags )
+                    applyFilenamesAsTags( *m_config );
+                
                 // Handle list request
                 if( Option<std::size_t> listed = list( config() ) )
                     return static_cast<int>( *listed );

include/internal/catch_interfaces_testcase.h

@@ -28,9 +28,13 @@ namespace Catch {
     struct ITestCaseRegistry {
         virtual ~ITestCaseRegistry();
         virtual std::vector<TestCase> const& getAllTests() const = 0;
-        virtual void getFilteredTests( TestSpec const& testSpec, IConfig const& config, std::vector<TestCase>& matchingTestCases, bool negated = false ) const = 0;
+        virtual std::vector<TestCase> const& getAllTestsSorted( IConfig const& config ) const = 0;
-
     };
+
+    bool matchTest( TestCase const& testCase, TestSpec const& testSpec, IConfig const& config );
+    std::vector<TestCase> filterTests( std::vector<TestCase> const& testCases, TestSpec const& testSpec, IConfig const& config );
+    std::vector<TestCase> const& getAllTestCasesSorted( IConfig const& config );
+    
 }
 
 #endif // TWOBLUECUBES_CATCH_INTERFACES_TESTCASE_H_INCLUDED

include/internal/catch_list.hpp

@@ -34,8 +34,7 @@ namespace Catch {
         nameAttr.setInitialIndent( 2 ).setIndent( 4 );
         tagsAttr.setIndent( 6 );
 
-        std::vector<TestCase> matchedTestCases;
+        std::vector<TestCase> matchedTestCases = filterTests( getAllTestCasesSorted( config ), testSpec, config );
-        getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, config, matchedTestCases );
         for( std::vector<TestCase>::const_iterator it = matchedTestCases.begin(), itEnd = matchedTestCases.end();
                 it != itEnd;
                 ++it ) {
@@ -63,8 +62,7 @@ namespace Catch {
         if( !config.testSpec().hasFilters() )
             testSpec = TestSpecParser( ITagAliasRegistry::get() ).parse( "*" ).testSpec();
         std::size_t matchedTests = 0;
-        std::vector<TestCase> matchedTestCases;
+        std::vector<TestCase> matchedTestCases = filterTests( getAllTestCasesSorted( config ), testSpec, config );
-        getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, config, matchedTestCases );
         for( std::vector<TestCase>::const_iterator it = matchedTestCases.begin(), itEnd = matchedTestCases.end();
                 it != itEnd;
                 ++it ) {
@@ -104,8 +102,7 @@ namespace Catch {
 
         std::map<std::string, TagInfo> tagCounts;
 
-        std::vector<TestCase> matchedTestCases;
+        std::vector<TestCase> matchedTestCases = filterTests( getAllTestCasesSorted( config ), testSpec, config );
-        getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, config, matchedTestCases );
         for( std::vector<TestCase>::const_iterator it = matchedTestCases.begin(), itEnd = matchedTestCases.end();
                 it != itEnd;
                 ++it ) {

include/internal/catch_test_case_registry_impl.hpp

@@ -21,14 +21,75 @@
 
 namespace Catch {
 
-    class TestRegistry : public ITestCaseRegistry {
+    struct LexSort {
-        struct LexSort {
+        bool operator() (TestCase i,TestCase j) const { return (i<j);}
-            bool operator() (TestCase i,TestCase j) const { return (i<j);}
+    };
-        };
+    struct RandomNumberGenerator {
-        struct RandomNumberGenerator {
+        int operator()( int n ) const { return std::rand() % n; }
-            int operator()( int n ) const { return std::rand() % n; }
+    };
-        };
+    
+    inline std::vector<TestCase> sortTests( IConfig const& config, std::vector<TestCase> const& unsortedTestCases ) {
+        
+        std::vector<TestCase> sorted = unsortedTestCases;
+        
+        switch( config.runOrder() ) {
+            case RunTests::InLexicographicalOrder:
+                std::sort( sorted.begin(), sorted.end(), LexSort() );
+                break;
+            case RunTests::InRandomOrder:
+                {
+                    seedRng( config );
+                    
+                    RandomNumberGenerator rng;
+                    std::random_shuffle( sorted.begin(), sorted.end(), rng );
+                }
+                break;
+            case RunTests::InDeclarationOrder:
+                // already in declaration order
+                break;
+        }
+        return sorted;
+    }
+    bool matchTest( TestCase const& testCase, TestSpec const& testSpec, IConfig const& config ) {
+        return testSpec.matches( testCase ) && ( config.allowThrows() || !testCase.throws() );
+    }
+    struct TestMatcher {
+        TestMatcher( TestSpec const& testSpec, bool allowThrows ) : m_testSpec( testSpec ), m_allowThrows( allowThrows ) {}
 
+        bool operator()( TestCase const& testCase ) const {
+            return m_testSpec.matches( testCase ) && ( m_allowThrows || !testCase.throws() );
+        }
+        TestSpec m_testSpec;
+        bool m_allowThrows;
+    };
+
+    void enforceNoDuplicateTestCases( std::vector<TestCase> const& functions ) {
+        std::set<TestCase> seenFunctions;
+        for( std::vector<TestCase>::const_iterator it = functions.begin(), itEnd = functions.end();
+            it != itEnd;
+            ++it ) {
+            std::pair<std::set<TestCase>::const_iterator, bool> prev = seenFunctions.insert( *it );
+            if( !prev.second ){
+                Catch::cerr()
+                << Colour( Colour::Red )
+                << "error: TEST_CASE( \"" << it->name << "\" ) already defined.\n"
+                << "\tFirst seen at " << prev.first->getTestCaseInfo().lineInfo << "\n"
+                << "\tRedefined at " << it->getTestCaseInfo().lineInfo << std::endl;
+                exit(1);
+            }
+        }
+    }
+    
+    std::vector<TestCase> filterTests( std::vector<TestCase> const& testCases, TestSpec const& testSpec, IConfig const& config ) {
+        std::vector<TestCase> filtered;
+        std::copy_if( testCases.begin(), testCases.end(), std::back_inserter( filtered ), TestMatcher( testSpec, config.allowThrows() ) );
+        return filtered;
+    }
+    std::vector<TestCase> const& getAllTestCasesSorted( IConfig const& config ) {
+        return getRegistryHub().getTestCaseRegistry().getAllTestsSorted( config );
+    }
+
+    class TestRegistry : public ITestCaseRegistry {
     public:
         TestRegistry() : m_unnamedCount( 0 ) {}
         virtual ~TestRegistry();
@@ -40,70 +101,27 @@ namespace Catch {
                 oss << "Anonymous test case " << ++m_unnamedCount;
                 return registerTest( testCase.withName( oss.str() ) );
             }
-
+            m_functions.push_back( testCase );
-            if( m_functions.find( testCase ) == m_functions.end() ) {
-                m_functions.insert( testCase );
-                m_functionsInOrder.push_back( testCase );
-                if( !testCase.isHidden() )
-                    m_nonHiddenFunctions.push_back( testCase );
-            }
-            else {
-                TestCase const& prev = *m_functions.find( testCase );
-                {
-                    Colour colourGuard( Colour::Red );
-                    Catch::cerr()   << "error: TEST_CASE( \"" << name << "\" ) already defined.\n"
-                                << "\tFirst seen at " << prev.getTestCaseInfo().lineInfo << "\n"
-                                << "\tRedefined at " << testCase.getTestCaseInfo().lineInfo << std::endl;
-                }
-                exit(1);
-            }
         }
 
         virtual std::vector<TestCase> const& getAllTests() const {
-            return m_functionsInOrder;
+            return m_functions;
         }
+        virtual std::vector<TestCase> const& getAllTestsSorted( IConfig const& config ) const {
+            if( m_sortedFunctions.empty() )
+                enforceNoDuplicateTestCases( m_functions );
 
-        virtual std::vector<TestCase> const& getAllNonHiddenTests() const {
+            if(  m_currentSortOrder != config.runOrder() || m_sortedFunctions.empty() ) {
-            return m_nonHiddenFunctions;
+                m_sortedFunctions = sortTests( config, m_functions );
-        }
+                m_currentSortOrder = config.runOrder();
-
-        virtual void getFilteredTests( TestSpec const& testSpec, IConfig const& config, std::vector<TestCase>& matchingTestCases, bool negated = false ) const {
-
-            for( std::vector<TestCase>::const_iterator  it = m_functionsInOrder.begin(),
-                                                        itEnd = m_functionsInOrder.end();
-                    it != itEnd;
-                    ++it ) {
-                bool includeTest = testSpec.matches( *it ) && ( config.allowThrows() || !it->throws() );
-                if( includeTest != negated )
-                    matchingTestCases.push_back( *it );
             }
-            sortTests( config, matchingTestCases );
+            return m_sortedFunctions;
         }
 
     private:
-
+        std::vector<TestCase> m_functions;
-        static void sortTests( IConfig const& config, std::vector<TestCase>& matchingTestCases ) {
+        mutable RunTests::InWhatOrder m_currentSortOrder;
-            
+        mutable std::vector<TestCase> m_sortedFunctions;
-            switch( config.runOrder() ) {
-                case RunTests::InLexicographicalOrder:
-                    std::sort( matchingTestCases.begin(), matchingTestCases.end(), LexSort() );
-                    break;
-                case RunTests::InRandomOrder:
-                {
-                    seedRng( config );
-                    
-                    RandomNumberGenerator rng;
-                    std::random_shuffle( matchingTestCases.begin(), matchingTestCases.end(), rng );
-                }
-                    break;
-                case RunTests::InDeclarationOrder:
-                    // already in declaration order
-                    break;
-            }
-        }
-        std::set<TestCase> m_functions;
-        std::vector<TestCase> m_functionsInOrder;
-        std::vector<TestCase> m_nonHiddenFunctions;
         size_t m_unnamedCount;
         std::ios_base::Init m_ostreamInit; // Forces cout/ cerr to be initialised
     };