Improve shardIndex/Count cli argument parsing

catchorg · Oct 27, 2021 · 60442bb · 60442bb
1 parent fd39f65
commit 60442bb
Show file tree

Hide file tree

Showing 11 changed files with 354 additions and 173 deletions.
diff --git 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<unsigned int>(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<unsigned int>(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/Baselines/automake.sw.approved.txt b/tests/SelfTest/Baselines/automake.sw.approved.txt
@@ -177,6 +177,7 @@ Nor would this
 :test-result: FAIL Output from all sections is reported
 :test-result: PASS Overloaded comma or address-of operators are not used
 :test-result: PASS Parse test names and tags
+:test-result: PASS Parsing sharding-related cli flags
 :test-result: PASS Pointers can be compared to null
 :test-result: PASS Precision of floating point stringification can be set
 :test-result: PASS Predicate matcher can accept const char*

diff --git a/tests/SelfTest/Baselines/compact.sw.approved.txt b/tests/SelfTest/Baselines/compact.sw.approved.txt
@@ -1178,6 +1178,18 @@ CmdLine.tests.cpp:<line number>: passed: !(spec.matches(*fakeTestCase("hidden an
 CmdLine.tests.cpp:<line number>: passed: !(spec.matches(*fakeTestCase("only foo", "[foo]"))) for: !false
 CmdLine.tests.cpp:<line number>: passed: !(spec.matches(*fakeTestCase("only hidden", "[.]"))) for: !false
 CmdLine.tests.cpp:<line number>: passed: spec.matches(*fakeTestCase("neither foo nor hidden", "[bar]")) for: true
+CmdLine.tests.cpp:<line number>: passed: cli.parse({ "test", "--shard-count=8" }) for: {?}
+CmdLine.tests.cpp:<line number>: passed: config.shardCount == 8 for: 8 == 8
+CmdLine.tests.cpp:<line number>: passed: !(result) for: !{?}
+CmdLine.tests.cpp:<line number>: passed: result.errorMessage(), ContainsSubstring("Shard count must be a positive number") for: "Shard count must be a positive number" contains: "Shard count must be a positive number"
+CmdLine.tests.cpp:<line number>: passed: !(result) for: !{?}
+CmdLine.tests.cpp:<line number>: passed: result.errorMessage(), ContainsSubstring("Shard count must be a positive number") for: "Shard count must be a positive number" contains: "Shard count must be a positive number"
+CmdLine.tests.cpp:<line number>: passed: cli.parse({ "test", "--shard-index=2" }) for: {?}
+CmdLine.tests.cpp:<line number>: passed: config.shardIndex == 2 for: 2 == 2
+CmdLine.tests.cpp:<line number>: passed: !(result) for: !{?}
+CmdLine.tests.cpp:<line number>: passed: result.errorMessage(), ContainsSubstring("Shard index must be a non-negative number") for: "Shard index must be a non-negative number" contains: "Shard index must be a non-negative number"
+CmdLine.tests.cpp:<line number>: passed: cli.parse({ "test", "--shard-index=0" }) for: {?}
+CmdLine.tests.cpp:<line number>: passed: config.shardIndex == 0 for: 0 == 0
 Condition.tests.cpp:<line number>: passed: p == 0 for: 0 == 0
 Condition.tests.cpp:<line number>: passed: p == pNULL for: 0 == 0
 Condition.tests.cpp:<line number>: passed: p != 0 for: 0x<hex digits> != 0
@@ -1273,12 +1285,6 @@ CmdLine.tests.cpp:<line number>: passed: cli.parse({ "test", "--benchmark-no-ana
 CmdLine.tests.cpp:<line number>: passed: config.benchmarkNoAnalysis for: true
 CmdLine.tests.cpp:<line number>: passed: cli.parse({ "test", "--benchmark-warmup-time=10" }) for: {?}
 CmdLine.tests.cpp:<line number>: passed: config.benchmarkWarmupTime == 10 for: 10 == 10
-CmdLine.tests.cpp:<line number>: passed: cli.parse({ "test", "--shard-count=8"}) for: {?}
-CmdLine.tests.cpp:<line number>: passed: config.shardCount == 8 for: 8 == 8
-CmdLine.tests.cpp:<line number>: passed: cli.parse({ "test", "--shard-index=2"}) for: {?}
-CmdLine.tests.cpp:<line number>: passed: config.shardIndex == 2 for: 2 == 2
-CmdLine.tests.cpp:<line number>: passed: !result for: true
-CmdLine.tests.cpp:<line number>: passed: result.errorMessage(), ContainsSubstring( "The shard count must be greater than 0" ) for: "The shard count must be greater than 0" contains: "The shard count must be greater than 0"
 Misc.tests.cpp:<line number>: passed: std::tuple_size<TestType>::value >= 1 for: 3 >= 1
 Misc.tests.cpp:<line number>: passed: std::tuple_size<TestType>::value >= 1 for: 2 >= 1
 Misc.tests.cpp:<line number>: passed: std::tuple_size<TestType>::value >= 1 for: 1 >= 1

diff --git a/tests/SelfTest/Baselines/console.std.approved.txt b/tests/SelfTest/Baselines/console.std.approved.txt
@@ -1426,6 +1426,6 @@ due to unexpected exception with message:
   Why would you throw a std::string?
 
 ===============================================================================
-test cases:  373 |  296 passed |  70 failed |  7 failed as expected
-assertions: 2121 | 1965 passed | 129 failed | 27 failed as expected
+test cases:  374 |  297 passed |  70 failed |  7 failed as expected
+assertions: 2127 | 1971 passed | 129 failed | 27 failed as expected
 
diff --git a/tests/SelfTest/Baselines/console.sw.approved.txt b/tests/SelfTest/Baselines/console.sw.approved.txt
@@ -8602,6 +8602,111 @@ CmdLine.tests.cpp:<line number>: PASSED:
 with expansion:
   true
 
+-------------------------------------------------------------------------------
+Parsing sharding-related cli flags
+  shard-count
+-------------------------------------------------------------------------------
+CmdLine.tests.cpp:<line number>
+...............................................................................
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK( cli.parse({ "test", "--shard-count=8" }) )
+with expansion:
+  {?}
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  REQUIRE( config.shardCount == 8 )
+with expansion:
+  8 == 8
+
+-------------------------------------------------------------------------------
+Parsing sharding-related cli flags
+  Negative shard count reports error
+-------------------------------------------------------------------------------
+CmdLine.tests.cpp:<line number>
+...............................................................................
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK_FALSE( result )
+with expansion:
+  !{?}
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Shard count must be a positive number") )
+with expansion:
+  "Shard count must be a positive number" contains: "Shard count must be a
+  positive number"
+
+-------------------------------------------------------------------------------
+Parsing sharding-related cli flags
+  Zero shard count reports error
+-------------------------------------------------------------------------------
+CmdLine.tests.cpp:<line number>
+...............................................................................
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK_FALSE( result )
+with expansion:
+  !{?}
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Shard count must be a positive number") )
+with expansion:
+  "Shard count must be a positive number" contains: "Shard count must be a
+  positive number"
+
+-------------------------------------------------------------------------------
+Parsing sharding-related cli flags
+  shard-index
+-------------------------------------------------------------------------------
+CmdLine.tests.cpp:<line number>
+...............................................................................
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK( cli.parse({ "test", "--shard-index=2" }) )
+with expansion:
+  {?}
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  REQUIRE( config.shardIndex == 2 )
+with expansion:
+  2 == 2
+
+-------------------------------------------------------------------------------
+Parsing sharding-related cli flags
+  Negative shard index reports error
+-------------------------------------------------------------------------------
+CmdLine.tests.cpp:<line number>
+...............................................................................
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK_FALSE( result )
+with expansion:
+  !{?}
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Shard index must be a non-negative number") )
+with expansion:
+  "Shard index must be a non-negative number" contains: "Shard index must be a
+  non-negative number"
+
+-------------------------------------------------------------------------------
+Parsing sharding-related cli flags
+  Shard index 0 is accepted
+-------------------------------------------------------------------------------
+CmdLine.tests.cpp:<line number>
+...............................................................................
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  CHECK( cli.parse({ "test", "--shard-index=0" }) )
+with expansion:
+  {?}
+
+CmdLine.tests.cpp:<line number>: PASSED:
+  REQUIRE( config.shardIndex == 0 )
+with expansion:
+  0 == 0
+
 -------------------------------------------------------------------------------
 Pointers can be compared to null
 -------------------------------------------------------------------------------
@@ -9390,61 +9495,6 @@ CmdLine.tests.cpp:<line number>: PASSED:
 with expansion:
   10 == 10
 
--------------------------------------------------------------------------------
-Process can be configured on command line
-  Sharding options
-  shard-count
--------------------------------------------------------------------------------
-CmdLine.tests.cpp:<line number>
-...............................................................................
-
-CmdLine.tests.cpp:<line number>: PASSED:
-  CHECK( cli.parse({ "test", "--shard-count=8"}) )
-with expansion:
-  {?}
-
-CmdLine.tests.cpp:<line number>: PASSED:
-  REQUIRE( config.shardCount == 8 )
-with expansion:
-  8 == 8
-
--------------------------------------------------------------------------------
-Process can be configured on command line
-  Sharding options
-  shard-index
--------------------------------------------------------------------------------
-CmdLine.tests.cpp:<line number>
-...............................................................................
-
-CmdLine.tests.cpp:<line number>: PASSED:
-  CHECK( cli.parse({ "test", "--shard-index=2"}) )
-with expansion:
-  {?}
-
-CmdLine.tests.cpp:<line number>: PASSED:
-  REQUIRE( config.shardIndex == 2 )
-with expansion:
-  2 == 2
-
--------------------------------------------------------------------------------
-Process can be configured on command line
-  Sharding options
-  Zero shard-count
--------------------------------------------------------------------------------
-CmdLine.tests.cpp:<line number>
-...............................................................................
-
-CmdLine.tests.cpp:<line number>: PASSED:
-  CHECK( !result )
-with expansion:
-  true
-
-CmdLine.tests.cpp:<line number>: PASSED:
-  CHECK_THAT( result.errorMessage(), ContainsSubstring( "The shard count must be greater than 0" ) )
-with expansion:
-  "The shard count must be greater than 0" contains: "The shard count must be
-  greater than 0"
-
 -------------------------------------------------------------------------------
 Product with differing arities - std::tuple<int, double, float>
 -------------------------------------------------------------------------------
@@ -17094,6 +17144,6 @@ Misc.tests.cpp:<line number>
 Misc.tests.cpp:<line number>: PASSED:
 
 ===============================================================================
-test cases:  373 |  280 passed |  86 failed |  7 failed as expected
-assertions: 2138 | 1965 passed | 146 failed | 27 failed as expected
+test cases:  374 |  281 passed |  86 failed |  7 failed as expected
+assertions: 2144 | 1971 passed | 146 failed | 27 failed as expected
 
diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/tests/SelfTest/Baselines/junit.sw.approved.txt
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <testsuitesloose text artifact
 >
-  <testsuite name="<exe-name>" errors="17" failures="129" tests="2138" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
+  <testsuite name="<exe-name>" errors="17" failures="129" tests="2144" hostname="tbd" time="{duration}" timestamp="{iso8601-timestamp}">
     <properties>
       <property name="random-seed" value="1"/>
       <property name="filters" value="~[!nonportable]~[!benchmark]~[approvals] *"/>
@@ -1060,6 +1060,12 @@ Message.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="Parse test names and tags/Leading and trailing spaces in test name" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Parse test names and tags/Shortened hide tags are split apart when parsing" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Parse test names and tags/Shortened hide tags also properly handle exclusion" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Parsing sharding-related cli flags/shard-count" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Parsing sharding-related cli flags/Negative shard count reports error" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Parsing sharding-related cli flags/Zero shard count reports error" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Parsing sharding-related cli flags/shard-index" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Parsing sharding-related cli flags/Negative shard index reports error" time="{duration}" status="run"/>
+    <testcase classname="<exe-name>.global" name="Parsing sharding-related cli flags/Shard index 0 is accepted" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Pointers can be compared to null" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Precision of floating point stringification can be set/Floats" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Precision of floating point stringification can be set/Double" time="{duration}" status="run"/>
@@ -1096,9 +1102,6 @@ Message.tests.cpp:<line number>
     <testcase classname="<exe-name>.global" name="Process can be configured on command line/Benchmark options/confidence-interval" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Process can be configured on command line/Benchmark options/no-analysis" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Process can be configured on command line/Benchmark options/warmup-time" time="{duration}" status="run"/>
-    <testcase classname="<exe-name>.global" name="Process can be configured on command line/Sharding options/shard-count" time="{duration}" status="run"/>
-    <testcase classname="<exe-name>.global" name="Process can be configured on command line/Sharding options/shard-index" time="{duration}" status="run"/>
-    <testcase classname="<exe-name>.global" name="Process can be configured on command line/Sharding options/Zero shard-count" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Product with differing arities - std::tuple&lt;int, double, float>" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Product with differing arities - std::tuple&lt;int, double>" time="{duration}" status="run"/>
     <testcase classname="<exe-name>.global" name="Product with differing arities - std::tuple&lt;int>" time="{duration}" status="run"/>

diff --git a/tests/SelfTest/Baselines/sonarqube.sw.approved.txt b/tests/SelfTest/Baselines/sonarqube.sw.approved.txt
@@ -43,6 +43,12 @@
     <testCase name="Parse test names and tags/Leading and trailing spaces in test name" duration="{duration}"/>
     <testCase name="Parse test names and tags/Shortened hide tags are split apart when parsing" duration="{duration}"/>
     <testCase name="Parse test names and tags/Shortened hide tags also properly handle exclusion" duration="{duration}"/>
+    <testCase name="Parsing sharding-related cli flags/shard-count" duration="{duration}"/>
+    <testCase name="Parsing sharding-related cli flags/Negative shard count reports error" duration="{duration}"/>
+    <testCase name="Parsing sharding-related cli flags/Zero shard count reports error" duration="{duration}"/>
+    <testCase name="Parsing sharding-related cli flags/shard-index" duration="{duration}"/>
+    <testCase name="Parsing sharding-related cli flags/Negative shard index reports error" duration="{duration}"/>
+    <testCase name="Parsing sharding-related cli flags/Shard index 0 is accepted" duration="{duration}"/>
     <testCase name="Process can be configured on command line/empty args don't cause a crash" duration="{duration}"/>
     <testCase name="Process can be configured on command line/default - no arguments" duration="{duration}"/>
     <testCase name="Process can be configured on command line/test lists/Specify one test case using" duration="{duration}"/>
@@ -75,9 +81,6 @@
     <testCase name="Process can be configured on command line/Benchmark options/confidence-interval" duration="{duration}"/>
     <testCase name="Process can be configured on command line/Benchmark options/no-analysis" duration="{duration}"/>
     <testCase name="Process can be configured on command line/Benchmark options/warmup-time" duration="{duration}"/>
-    <testCase name="Process can be configured on command line/Sharding options/shard-count" duration="{duration}"/>
-    <testCase name="Process can be configured on command line/Sharding options/shard-index" duration="{duration}"/>
-    <testCase name="Process can be configured on command line/Sharding options/Zero shard-count" duration="{duration}"/>
     <testCase name="Test with special, characters &quot;in name" duration="{duration}"/>
   </file>
   <file path="tests/<exe-name>/IntrospectiveTests/FloatingPoint.tests.cpp">