From c06e1909aecc5d628151446c84c7978eb5e8ce3e Mon Sep 17 00:00:00 2001 From: Phil Nash Date: Tue, 4 Aug 2015 23:11:56 +0100 Subject: [PATCH] Refactored test filtering and sorting --- include/catch_runner.hpp | 34 ++--- include/internal/catch_interfaces_testcase.h | 8 +- include/internal/catch_list.hpp | 9 +- .../catch_test_case_registry_impl.hpp | 142 ++++++++++-------- 4 files changed, 99 insertions(+), 94 deletions(-) diff --git a/include/catch_runner.hpp b/include/catch_runner.hpp index 3e08884d..2b5ca8fe 100644 --- a/include/catch_runner.hpp +++ b/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 testCases; - getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, *config, testCases ); - - std::set testsAlreadyRun; - for( std::vector::const_iterator it = testCases.begin(), itEnd = testCases.end(); + std::vector const& allTestCases = getAllTestCasesSorted( *config ); + for( std::vector::const_iterator it = allTestCases.begin(), itEnd = allTestCases.end(); it != itEnd; ++it ) { - if( testsAlreadyRun.find( *it ) == testsAlreadyRun.end() ) { - - if( context.aborting() ) - break; - + if( !context.aborting() && matchTest( *it, testSpec, *config ) ) totals += context.runTest( *it ); - testsAlreadyRun.insert( *it ); - } + else + reporter->skipTest( *it ); } - std::vector skippedTestCases; - getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, *config, skippedTestCases, true ); - - for( std::vector::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() { - std::vector const& tests = getRegistryHub().getTestCaseRegistry().getAllTests(); + void applyFilenamesAsTags( IConfig const& config ) { + std::vector const& tests = getAllTestCasesSorted( config ); for(std::size_t i = 0; i < tests.size(); ++i ) { TestCase& test = const_cast( tests[i] ); std::set 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 listed = list( config() ) ) return static_cast( *listed ); diff --git a/include/internal/catch_interfaces_testcase.h b/include/internal/catch_interfaces_testcase.h index a23ff9f4..3ae412c5 100644 --- a/include/internal/catch_interfaces_testcase.h +++ b/include/internal/catch_interfaces_testcase.h @@ -28,9 +28,13 @@ namespace Catch { struct ITestCaseRegistry { virtual ~ITestCaseRegistry(); virtual std::vector const& getAllTests() const = 0; - virtual void getFilteredTests( TestSpec const& testSpec, IConfig const& config, std::vector& matchingTestCases, bool negated = false ) const = 0; - + virtual std::vector const& getAllTestsSorted( IConfig const& config ) const = 0; }; + + bool matchTest( TestCase 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 // TWOBLUECUBES_CATCH_INTERFACES_TESTCASE_H_INCLUDED diff --git a/include/internal/catch_list.hpp b/include/internal/catch_list.hpp index 1c510ef8..4c87ccf4 100644 --- a/include/internal/catch_list.hpp +++ b/include/internal/catch_list.hpp @@ -34,8 +34,7 @@ namespace Catch { nameAttr.setInitialIndent( 2 ).setIndent( 4 ); tagsAttr.setIndent( 6 ); - std::vector matchedTestCases; - getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, config, matchedTestCases ); + std::vector matchedTestCases = filterTests( getAllTestCasesSorted( config ), testSpec, config ); for( std::vector::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 matchedTestCases; - getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, config, matchedTestCases ); + std::vector matchedTestCases = filterTests( getAllTestCasesSorted( config ), testSpec, config ); for( std::vector::const_iterator it = matchedTestCases.begin(), itEnd = matchedTestCases.end(); it != itEnd; ++it ) { @@ -104,8 +102,7 @@ namespace Catch { std::map tagCounts; - std::vector matchedTestCases; - getRegistryHub().getTestCaseRegistry().getFilteredTests( testSpec, config, matchedTestCases ); + std::vector matchedTestCases = filterTests( getAllTestCasesSorted( config ), testSpec, config ); for( std::vector::const_iterator it = matchedTestCases.begin(), itEnd = matchedTestCases.end(); it != itEnd; ++it ) { diff --git a/include/internal/catch_test_case_registry_impl.hpp b/include/internal/catch_test_case_registry_impl.hpp index 41170052..ac789a38 100644 --- a/include/internal/catch_test_case_registry_impl.hpp +++ b/include/internal/catch_test_case_registry_impl.hpp @@ -21,14 +21,75 @@ namespace Catch { - class TestRegistry : public ITestCaseRegistry { - struct LexSort { - bool operator() (TestCase i,TestCase j) const { return (i sortTests( IConfig const& config, std::vector const& unsortedTestCases ) { + + std::vector 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 const& functions ) { + std::set seenFunctions; + for( std::vector::const_iterator it = functions.begin(), itEnd = functions.end(); + it != itEnd; + ++it ) { + std::pair::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 filterTests( std::vector const& testCases, TestSpec const& testSpec, IConfig const& config ) { + std::vector filtered; + std::copy_if( testCases.begin(), testCases.end(), std::back_inserter( filtered ), TestMatcher( testSpec, config.allowThrows() ) ); + return filtered; + } + std::vector 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() ) ); } - - 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); - } + m_functions.push_back( testCase ); } virtual std::vector const& getAllTests() const { - return m_functionsInOrder; + return m_functions; } + virtual std::vector const& getAllTestsSorted( IConfig const& config ) const { + if( m_sortedFunctions.empty() ) + enforceNoDuplicateTestCases( m_functions ); - virtual std::vector const& getAllNonHiddenTests() const { - return m_nonHiddenFunctions; - } - - virtual void getFilteredTests( TestSpec const& testSpec, IConfig const& config, std::vector& matchingTestCases, bool negated = false ) const { - - for( std::vector::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 ); + if( m_currentSortOrder != config.runOrder() || m_sortedFunctions.empty() ) { + m_sortedFunctions = sortTests( config, m_functions ); + m_currentSortOrder = config.runOrder(); } - sortTests( config, matchingTestCases ); + return m_sortedFunctions; } private: - - static void sortTests( IConfig const& config, std::vector& matchingTestCases ) { - - 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 m_functions; - std::vector m_functionsInOrder; - std::vector m_nonHiddenFunctions; + std::vector m_functions; + mutable RunTests::InWhatOrder m_currentSortOrder; + mutable std::vector m_sortedFunctions; size_t m_unnamedCount; std::ios_base::Init m_ostreamInit; // Forces cout/ cerr to be initialised };