diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0ed64ebf88..61b5fda517 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -108,6 +108,7 @@ set(IMPL_HEADERS ${SOURCES_DIR}/internal/catch_optional.hpp ${SOURCES_DIR}/internal/catch_output_redirect.hpp ${SOURCES_DIR}/internal/catch_parse_numbers.hpp + ${SOURCES_DIR}/internal/catch_generator_filter_parser.hpp ${SOURCES_DIR}/internal/catch_path_filter.hpp ${SOURCES_DIR}/internal/catch_platform.hpp ${SOURCES_DIR}/internal/catch_polyfills.hpp @@ -195,6 +196,7 @@ set(IMPL_SOURCES ${SOURCES_DIR}/internal/catch_message_info.cpp ${SOURCES_DIR}/internal/catch_output_redirect.cpp ${SOURCES_DIR}/internal/catch_parse_numbers.cpp + ${SOURCES_DIR}/internal/catch_generator_filter_parser.cpp ${SOURCES_DIR}/internal/catch_polyfills.cpp ${SOURCES_DIR}/internal/catch_random_number_generator.cpp ${SOURCES_DIR}/internal/catch_random_seed_generation.cpp diff --git a/src/catch2/internal/catch_commandline.cpp b/src/catch2/internal/catch_commandline.cpp index 22948c279f..53ca309668 100644 --- a/src/catch2/internal/catch_commandline.cpp +++ b/src/catch2/internal/catch_commandline.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -225,12 +226,9 @@ namespace Catch { }; auto const setGeneratorFilter = [&]( std::string const& generatorFilter ) { if (generatorFilter != "*") { - // TODO: avoid re-parsing the index? - auto parsedIndex = parseUInt( generatorFilter ); - if ( !parsedIndex ) { - return ParserResult::runtimeError( "Could not parse '" + - generatorFilter + - "' as generator index" ); + auto parsedSpec = parseGeneratorIndexSpec( trim( generatorFilter ) ); + if ( parsedSpec.type == GeneratorFilterParseResult::Type::Error ) { + return ParserResult::runtimeError( parsedSpec.errorMessage ); } } config.useNewPathFilteringBehaviour = true; diff --git a/src/catch2/internal/catch_generator_filter_parser.cpp b/src/catch2/internal/catch_generator_filter_parser.cpp new file mode 100644 index 0000000000..f6498509d0 --- /dev/null +++ b/src/catch2/internal/catch_generator_filter_parser.cpp @@ -0,0 +1,135 @@ + +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 +#include + +#include +#include + +#include + +namespace Catch { + + namespace { + + GeneratorFilterParseResult error( std::string message ) { + GeneratorFilterParseResult result; + result.type = GeneratorFilterParseResult::Type::Error; + result.errorMessage = CATCH_MOVE( message ); + return result; + } + + Optional parseSingleIndex( std::string const& token ) { + if ( token.empty() ) { return {}; } + return parseUInt( token ); + } + + GeneratorFilterParseResult parseRangeToken( std::string const& token ) { + auto const dashPos = token.find( '-' ); + if ( dashPos == std::string::npos ) { + auto parsed = parseSingleIndex( token ); + if ( !parsed ) { + return error( "Could not parse '" + token + + "' as generator index" ); + } + GeneratorFilterParseResult result; + result.type = GeneratorFilterParseResult::Type::Ok; + result.indices.push_back( *parsed ); + return result; + } + + if ( dashPos == 0 || dashPos + 1 == token.size() ) { + return error( "Could not parse '" + token + + "' as generator index range" ); + } + + auto const startToken = token.substr( 0, dashPos ); + auto const endToken = token.substr( dashPos + 1 ); + if ( endToken.find( '-' ) != std::string::npos ) { + return error( "Could not parse '" + token + + "' as generator index range" ); + } + + auto const start = parseSingleIndex( startToken ); + auto const end = parseSingleIndex( endToken ); + if ( !start || !end ) { + return error( "Could not parse '" + token + + "' as generator index range" ); + } + if ( *start > *end ) { + return error( "Generator index range '" + token + + "' must be monotonically increasing" ); + } + + GeneratorFilterParseResult result; + result.type = GeneratorFilterParseResult::Type::Ok; + for ( unsigned int index = *start; index <= *end; ++index ) { + result.indices.push_back( index ); + } + return result; + } + + } // namespace + + GeneratorFilterParseResult parseGeneratorIndexSpec( std::string const& input ) { + auto const trimmed = trim( input ); + if ( trimmed.empty() ) { + return error( "Generator index spec cannot be empty" ); + } + + GeneratorFilterParseResult result; + result.type = GeneratorFilterParseResult::Type::Ok; + + std::string token; + for ( char ch : trimmed ) { + if ( ch == ',' ) { + if ( token.empty() ) { + return error( "Could not parse '" + trimmed + + "' as generator index spec" ); + } + auto parsedToken = parseRangeToken( token ); + if ( parsedToken.type == GeneratorFilterParseResult::Type::Error ) { + return parsedToken; + } + result.indices.insert( result.indices.end(), + parsedToken.indices.begin(), + parsedToken.indices.end() ); + token.clear(); + } else { + token.push_back( ch ); + } + } + + if ( token.empty() ) { + return error( "Could not parse '" + trimmed + + "' as generator index spec" ); + } + + auto parsedToken = parseRangeToken( token ); + if ( parsedToken.type == GeneratorFilterParseResult::Type::Error ) { + return parsedToken; + } + result.indices.insert( result.indices.end(), + parsedToken.indices.begin(), + parsedToken.indices.end() ); + + if ( result.indices.empty() ) { + return error( "Could not parse '" + trimmed + + "' as generator index spec" ); + } + + for ( std::size_t i = 1; i < result.indices.size(); ++i ) { + if ( result.indices[i] <= result.indices[i - 1] ) { + return error( "Generator index spec '" + trimmed + + "' must be monotonically increasing" ); + } + } + + return result; + } + +} // namespace Catch diff --git a/src/catch2/internal/catch_generator_filter_parser.hpp b/src/catch2/internal/catch_generator_filter_parser.hpp new file mode 100644 index 0000000000..610461dca3 --- /dev/null +++ b/src/catch2/internal/catch_generator_filter_parser.hpp @@ -0,0 +1,35 @@ + +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 +#ifndef CATCH_GENERATOR_FILTER_PARSER_HPP_INCLUDED +#define CATCH_GENERATOR_FILTER_PARSER_HPP_INCLUDED + +#include + +#include +#include + +namespace Catch { + + struct GeneratorFilterParseResult { + enum class Type { Ok, Error }; + Type type = Type::Error; + std::vector indices; + std::string errorMessage; + }; + + /** + * Parses generator index filter specs such as `1,3,6-10`. + * + * Indices must be monotonically increasing with no duplicates or + * overlapping ranges. + */ + GeneratorFilterParseResult parseGeneratorIndexSpec( std::string const& input ); + +} // end namespace Catch + +#endif // CATCH_GENERATOR_FILTER_PARSER_HPP_INCLUDED diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp index 18e01f75a7..a9766a89d2 100644 --- a/src/catch2/internal/catch_run_context.cpp +++ b/src/catch2/internal/catch_run_context.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,8 @@ namespace Catch { // Filtered generator has moved to specific index due to // a filter, it needs special handling of `countedNext()` bool m_isFiltered = false; + std::vector m_filterIndices; + std::size_t m_filterIndexPos = 0; GeneratorTracker( TestCaseTracking::NameAndLocation&& nameAndLocation, @@ -74,12 +77,16 @@ namespace Catch { // used for filtering sections below the generator, but // not the generator itself. if ( filter.filter != "*" ) { + auto parsedSpec = + parseGeneratorIndexSpec( filter.filter ); + assert( parsedSpec.type == + GeneratorFilterParseResult::Type::Ok ); + m_filterIndices.assign( parsedSpec.indices.begin(), + parsedSpec.indices.end() ); m_isFiltered = true; - // TBD: We assume that the filter was validated as - // number during parsing. We should pass it - // as number from the CLI parser. - size_t targetIndex = std::stoul( filter.filter ); - m_generator->skipToNthElement( targetIndex ); + m_filterIndexPos = 0; + m_generator->skipToNthElement( + m_filterIndices[m_filterIndexPos] ); } } } @@ -183,10 +190,19 @@ namespace Catch { // value, but we do not want to invoke the side-effect if // this generator is still waiting for any child to start. assert( m_generator && "Tracker without generator" ); + bool advancedFilteredGenerator = false; + if ( m_runState == CompletedSuccessfully && m_isFiltered && + m_filterIndexPos + 1 < m_filterIndices.size() ) { + ++m_filterIndexPos; + m_generator->skipToNthElement( + m_filterIndices[m_filterIndexPos] ); + advancedFilteredGenerator = true; + } if ( should_wait_for_child || ( m_runState == CompletedSuccessfully - && !m_isFiltered // filtered generators cannot meaningfully move forward, as they would get past the filter - && m_generator->countedNext() ) ) { + && ( ( !m_isFiltered && + m_generator->countedNext() ) || + advancedFilteredGenerator ) ) ) { m_children.clear(); m_runState = Executing; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5a74a9d06e..d45b4cb514 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -78,6 +78,7 @@ set(TEST_SOURCES ${SELF_TEST_DIR}/IntrospectiveTests/Details.tests.cpp ${SELF_TEST_DIR}/IntrospectiveTests/FloatingPoint.tests.cpp ${SELF_TEST_DIR}/IntrospectiveTests/GeneratorsImpl.tests.cpp + ${SELF_TEST_DIR}/IntrospectiveTests/GeneratorFilterParser.tests.cpp ${SELF_TEST_DIR}/IntrospectiveTests/Integer.tests.cpp ${SELF_TEST_DIR}/IntrospectiveTests/InternalBenchmark.tests.cpp ${SELF_TEST_DIR}/IntrospectiveTests/Json.tests.cpp diff --git a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp index 71ddc6fd2c..995b0934b6 100644 --- a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp @@ -579,4 +579,42 @@ TEST_CASE( "Parsing path filter specs", REQUIRE( config.pathFilters[0] == PathFilter(PathFilter::For::Section, "untrimmed" ) ); REQUIRE( config.pathFilters[1] == PathFilter(PathFilter::For::Generator, "42" ) ); } + SECTION( "Generator specs accept comma-separated indices" ) { + auto result = cli.parse( { "tests", "-g", "1,3,4" } ); + REQUIRE( result ); + REQUIRE( config.pathFilters[0] == + PathFilter( PathFilter::For::Generator, "1,3,4" ) ); + } + SECTION( "Generator specs accept closed ranges" ) { + auto result = cli.parse( { "tests", "-g", "1-3" } ); + REQUIRE( result ); + REQUIRE( config.pathFilters[0] == + PathFilter( PathFilter::For::Generator, "1-3" ) ); + } + SECTION( "Generator specs accept combined lists and ranges" ) { + auto result = cli.parse( { "tests", "-g", "1,3,6-10" } ); + REQUIRE( result ); + REQUIRE( config.pathFilters[0] == + PathFilter( PathFilter::For::Generator, "1,3,6-10" ) ); + } + SECTION( "Generator specs reject overlapping ranges" ) { + auto result = cli.parse( { "tests", "-g", "1-3,2-4" } ); + REQUIRE_FALSE( result ); + } + SECTION( "Generator specs reject unsorted indices" ) { + auto result = cli.parse( { "tests", "-g", "3,2,1" } ); + REQUIRE_FALSE( result ); + } + SECTION( "Generator specs reject decreasing ranges" ) { + auto result = cli.parse( { "tests", "-g", "3-1" } ); + REQUIRE_FALSE( result ); + } + SECTION( "Generator specs reject duplicate indices" ) { + auto result = cli.parse( { "tests", "-g", "1,2,2,3" } ); + REQUIRE_FALSE( result ); + } + SECTION( "Generator specs reject syntactically invalid ranges" ) { + REQUIRE_FALSE( cli.parse( { "tests", "-g", "1--2" } ) ); + REQUIRE_FALSE( cli.parse( { "tests", "-g", "1-2-3" } ) ); + } } diff --git a/tests/SelfTest/IntrospectiveTests/GeneratorFilterParser.tests.cpp b/tests/SelfTest/IntrospectiveTests/GeneratorFilterParser.tests.cpp new file mode 100644 index 0000000000..597c24b1b5 --- /dev/null +++ b/tests/SelfTest/IntrospectiveTests/GeneratorFilterParser.tests.cpp @@ -0,0 +1,44 @@ + +// Copyright Catch2 Authors +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// SPDX-License-Identifier: BSL-1.0 +#include +#include + +TEST_CASE( "parseGeneratorIndexSpec", "[generators][cli]" ) { + using Catch::GeneratorFilterParseResult; + using Catch::parseGeneratorIndexSpec; + + SECTION( "single index" ) { + auto const result = parseGeneratorIndexSpec( "42" ); + REQUIRE( result.type == GeneratorFilterParseResult::Type::Ok ); + REQUIRE( result.indices == std::vector{ 42 } ); + } + + SECTION( "comma-separated indices" ) { + auto const result = parseGeneratorIndexSpec( "1,3,4" ); + REQUIRE( result.type == GeneratorFilterParseResult::Type::Ok ); + REQUIRE( result.indices == ( std::vector{ 1, 3, 4 } ) ); + } + + SECTION( "closed range" ) { + auto const result = parseGeneratorIndexSpec( "1-3" ); + REQUIRE( result.type == GeneratorFilterParseResult::Type::Ok ); + REQUIRE( result.indices == ( std::vector{ 1, 2, 3 } ) ); + } + + SECTION( "combined list and ranges" ) { + auto const result = parseGeneratorIndexSpec( "1,3,6-10" ); + REQUIRE( result.type == GeneratorFilterParseResult::Type::Ok ); + REQUIRE( result.indices == + ( std::vector{ 1, 3, 6, 7, 8, 9, 10 } ) ); + } + + SECTION( "rejects overlapping ranges" ) { + auto const result = parseGeneratorIndexSpec( "1-3,2-4" ); + REQUIRE( result.type == GeneratorFilterParseResult::Type::Error ); + } +} diff --git a/tests/meson.build b/tests/meson.build index 0e44eaab44..007f8e6493 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -17,6 +17,7 @@ self_test_sources = files( 'SelfTest/IntrospectiveTests/Details.tests.cpp', 'SelfTest/IntrospectiveTests/FloatingPoint.tests.cpp', 'SelfTest/IntrospectiveTests/GeneratorsImpl.tests.cpp', + 'SelfTest/IntrospectiveTests/GeneratorFilterParser.tests.cpp', 'SelfTest/IntrospectiveTests/Integer.tests.cpp', 'SelfTest/IntrospectiveTests/InternalBenchmark.tests.cpp', 'SelfTest/IntrospectiveTests/Parse.tests.cpp',