diff --git a/src/catch2/internal/catch_commandline.cpp b/src/catch2/internal/catch_commandline.cpp index 5f8a44d566..3f504ed6de 100644 --- a/src/catch2/internal/catch_commandline.cpp +++ b/src/catch2/internal/catch_commandline.cpp @@ -150,15 +150,42 @@ namespace Catch { return ParserResult::ok( ParseResultType::Matched ); }; auto const setShardCount = [&]( std::string const& shardCount ) { - auto result = Clara::Detail::convertInto( shardCount, config.shardCount ); + CATCH_TRY{ + std::size_t parsedTo = 0; + int64_t parsedCount = std::stoll(shardCount, &parsedTo, 0); + if (parsedTo != shardCount.size()) { + return ParserResult::runtimeError("Could not parse '" + shardCount + "' as shard count"); + } + if (parsedCount <= 0) { + return ParserResult::runtimeError("Shard count must be a positive number"); + } - if (config.shardCount == 0) { - return ParserResult::runtimeError( "The shard count must be greater than 0" ); - } else { - return result; + config.shardCount = static_cast(parsedCount); + return ParserResult::ok(ParseResultType::Matched); + } CATCH_CATCH_ANON(std::exception const&) { + return ParserResult::runtimeError("Could not parse '" + shardCount + "' as shard count"); } }; + auto const setShardIndex = [&](std::string const& shardIndex) { + CATCH_TRY{ + std::size_t parsedTo = 0; + int64_t parsedIndex = std::stoll(shardIndex, &parsedTo, 0); + if (parsedTo != shardIndex.size()) { + return ParserResult::runtimeError("Could not parse '" + shardIndex + "' as shard index"); + } + if (parsedIndex <= 0) { + return ParserResult::runtimeError("Shard index must be a non-negative number"); + } + + config.shardIndex = static_cast(parsedIndex); + return ParserResult::ok(ParseResultType::Matched); + } CATCH_CATCH_ANON(std::exception const&) { + return ParserResult::runtimeError("Could not parse '" + shardIndex + "' as shard index"); + } + }; + + auto cli = ExeName( config.processName ) | Help( config.showHelp ) @@ -252,7 +279,7 @@ namespace Catch { | Opt( setShardCount, "shard count" ) ["--shard-count"] ( "split the tests to execute into this many groups" ) - | Opt( config.shardIndex, "shard index" ) + | Opt( setShardIndex, "shard index" ) ["--shard-index"] ( "index of the group of tests to execute (see --shard-count)" ) | Arg( config.testsOrTags, "test name|pattern|tags" ) diff --git a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp index 5b58bdec14..c552363a76 100644 --- a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp @@ -569,27 +569,47 @@ TEST_CASE( "Process can be configured on command line", "[config][command-line]" REQUIRE(config.benchmarkWarmupTime == 10); } } +} - SECTION("Sharding options") { - SECTION("shard-count") { - CHECK(cli.parse({ "test", "--shard-count=8"})); +TEST_CASE("Parsing sharding-related cli flags", "[sharding]") { + using namespace Catch::Matchers; - REQUIRE(config.shardCount == 8); - } + Catch::ConfigData config; + auto cli = Catch::makeCommandLineParser(config); - SECTION("shard-index") { - CHECK(cli.parse({ "test", "--shard-index=2"})); + SECTION("shard-count") { + CHECK(cli.parse({ "test", "--shard-count=8" })); - REQUIRE(config.shardIndex == 2); - } + REQUIRE(config.shardCount == 8); + } - SECTION("Zero shard-count") { - auto result = cli.parse({ "test", "--shard-count=0"}); + SECTION("Negative shard count reports error") { + auto result = cli.parse({ "test", "--shard-count=-1" }); - CHECK( !result ); - CHECK_THAT( result.errorMessage(), ContainsSubstring( "The shard count must be greater than 0" ) ); - } + CHECK_FALSE(result); + REQUIRE_THAT(result.errorMessage(), ContainsSubstring("Shard count must be a positive number")); } + + SECTION("Zero shard count reports error") { + auto result = cli.parse({ "test", "--shard-count=0" }); + + CHECK_FALSE(result); + REQUIRE_THAT(result.errorMessage(), ContainsSubstring("Shard count must be a positive number")); + } + + SECTION("shard-index") { + CHECK(cli.parse({ "test", "--shard-index=2" })); + + REQUIRE(config.shardIndex == 2); + } + + SECTION("Negative shard index reports error") { + auto result = cli.parse({ "test", "--shard-index=-12" }); + + CHECK_FALSE(result); + REQUIRE_THAT(result.errorMessage(), ContainsSubstring("Shard index must be a non-negative number")); + } + } TEST_CASE("Test with special, characters \"in name", "[cli][regression]") {