feat(cli): support generator index lists and ranges (#3071)

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

src/catch2/internal/catch_commandline.cpp

@@ -14,6 +14,7 @@
 #include <catch2/internal/catch_reporter_registry.hpp>
 #include <catch2/internal/catch_console_colour.hpp>
 #include <catch2/internal/catch_parse_numbers.hpp>
+#include <catch2/internal/catch_generator_filter_parser.hpp>
 #include <catch2/internal/catch_reporter_spec_parser.hpp>
 
 #include <fstream>
@@ -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;

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 <catch2/internal/catch_generator_filter_parser.hpp>
+
+#include <catch2/internal/catch_parse_numbers.hpp>
+#include <catch2/internal/catch_string_manip.hpp>
+
+#include <string>
+
+namespace Catch {
+
+    namespace {
+
+        GeneratorFilterParseResult error( std::string message ) {
+            GeneratorFilterParseResult result;
+            result.type = GeneratorFilterParseResult::Type::Error;
+            result.errorMessage = CATCH_MOVE( message );
+            return result;
+        }
+
+        Optional<unsigned int> 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

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 <catch2/internal/catch_optional.hpp>
+
+#include <string>
+#include <vector>
+
+namespace Catch {
+
+    struct GeneratorFilterParseResult {
+        enum class Type { Ok, Error };
+        Type type = Type::Error;
+        std::vector<unsigned int> 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

src/catch2/internal/catch_run_context.cpp

@@ -21,6 +21,7 @@
 #include <catch2/catch_timer.hpp>
 #include <catch2/internal/catch_output_redirect.hpp>
 #include <catch2/internal/catch_assertion_handler.hpp>
+#include <catch2/internal/catch_generator_filter_parser.hpp>
 #include <catch2/internal/catch_path_filter.hpp>
 #include <catch2/internal/catch_test_failure_exception.hpp>
 #include <catch2/internal/catch_thread_local.hpp>
@@ -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<std::size_t> 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;
                     }

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

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" } ) );
+    }
 }

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 <catch2/catch_test_macros.hpp>
+#include <catch2/internal/catch_generator_filter_parser.hpp>
+
+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<unsigned int>{ 42 } );
+    }
+
+    SECTION( "comma-separated indices" ) {
+        auto const result = parseGeneratorIndexSpec( "1,3,4" );
+        REQUIRE( result.type == GeneratorFilterParseResult::Type::Ok );
+        REQUIRE( result.indices == ( std::vector<unsigned int>{ 1, 3, 4 } ) );
+    }
+
+    SECTION( "closed range" ) {
+        auto const result = parseGeneratorIndexSpec( "1-3" );
+        REQUIRE( result.type == GeneratorFilterParseResult::Type::Ok );
+        REQUIRE( result.indices == ( std::vector<unsigned int>{ 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<unsigned int>{ 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 );
+    }
+}

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',