Skip to content

Commit

Permalink
Refactor parsing of shard index/count in cmdline handling
Browse files Browse the repository at this point in the history 
This worsens the message for negative numbers a bit, but
simplifies the code enough that this is still a win.
  • Loading branch information
@horenmar
horenmar committed Oct 22, 2022
1 parent f1361ef commit a43f679
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 76 deletions.
46 changes: 16 additions & 30 deletions src/catch2/internal/catch_commandline.cpp
View file
Expand Up @@ -7,7 +7,6 @@
// SPDX-License-Identifier: BSL-1.0
#include <catch2/internal/catch_commandline.hpp>

#include <catch2/internal/catch_compiler_capabilities.hpp>
#include <catch2/catch_config.hpp>
#include <catch2/internal/catch_string_manip.hpp>
#include <catch2/interfaces/catch_interfaces_registry_hub.hpp>
Expand Down Expand Up @@ -177,42 +176,29 @@ namespace Catch {
return ParserResult::ok( ParseResultType::Matched );
};
auto const setShardCount = [&]( std::string const& 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");
}

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 parsedCount = parseUInt( shardCount );
if ( !parsedCount ) {
return ParserResult::runtimeError(
"Could not parse '" + shardCount + "' as shard count" );
}
if ( *parsedCount == 0 ) {
return ParserResult::runtimeError(
"Shard count must be positive" );
}
config.shardCount = *parsedCount;
return ParserResult::ok( ParseResultType::Matched );
};

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 parsedIndex = parseUInt( shardIndex );
if ( !parsedIndex ) {
return ParserResult::runtimeError(
"Could not parse '" + shardIndex + "' as shard index" );
}
config.shardIndex = *parsedIndex;
return ParserResult::ok( ParseResultType::Matched );
};


auto cli
= ExeName( config.processName )
| Help( config.showHelp )
Expand Down
6 changes: 3 additions & 3 deletions tests/SelfTest/Baselines/compact.sw.approved.txt
View file
Expand Up @@ -1287,13 +1287,13 @@ TestSpecParser.tests.cpp:<line number>: passed: spec.matches( testCase ) for: tr
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.errorMessage(), ContainsSubstring( "Could not parse '-1' as shard count" ) for: "Could not parse '-1' as shard count" contains: "Could not parse '-1' as shard count"
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.errorMessage(), ContainsSubstring( "Shard count must be positive" ) for: "Shard count must be positive" contains: "Shard count must be positive"
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: result.errorMessage(), ContainsSubstring( "Could not parse '-12' as shard index" ) for: "Could not parse '-12' as shard index" contains: "Could not parse '-12' as shard index"
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
TestSpecParser.tests.cpp:<line number>: passed: spec.hasFilters() for: true with 1 message: 'tagString := "[tag with spaces]"'
Expand Down
6 changes: 3 additions & 3 deletions tests/SelfTest/Baselines/compact.sw.multi.approved.txt
View file
Expand Up @@ -1285,13 +1285,13 @@ TestSpecParser.tests.cpp:<line number>: passed: spec.matches( testCase ) for: tr
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.errorMessage(), ContainsSubstring( "Could not parse '-1' as shard count" ) for: "Could not parse '-1' as shard count" contains: "Could not parse '-1' as shard count"
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.errorMessage(), ContainsSubstring( "Shard count must be positive" ) for: "Shard count must be positive" contains: "Shard count must be positive"
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: result.errorMessage(), ContainsSubstring( "Could not parse '-12' as shard index" ) for: "Could not parse '-12' as shard index" contains: "Could not parse '-12' as shard index"
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
TestSpecParser.tests.cpp:<line number>: passed: spec.hasFilters() for: true with 1 message: 'tagString := "[tag with spaces]"'
Expand Down
17 changes: 8 additions & 9 deletions tests/SelfTest/Baselines/console.sw.approved.txt
View file
Expand Up @@ -9175,10 +9175,10 @@ with expansion:
!{?}

CmdLine.tests.cpp:<line number>: PASSED:
REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Shard count must be a positive number") )
REQUIRE_THAT( result.errorMessage(), ContainsSubstring( "Could not parse '-1' as shard count" ) )
with expansion:
"Shard count must be a positive number" contains: "Shard count must be a
positive number"
"Could not parse '-1' as shard count" contains: "Could not parse '-1' as
shard count"

-------------------------------------------------------------------------------
Parsing sharding-related cli flags
Expand All @@ -9193,10 +9193,9 @@ with expansion:
!{?}

CmdLine.tests.cpp:<line number>: PASSED:
REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Shard count must be a positive number") )
REQUIRE_THAT( result.errorMessage(), ContainsSubstring( "Shard count must be positive" ) )
with expansion:
"Shard count must be a positive number" contains: "Shard count must be a
positive number"
"Shard count must be positive" contains: "Shard count must be positive"

-------------------------------------------------------------------------------
Parsing sharding-related cli flags
Expand Down Expand Up @@ -9228,10 +9227,10 @@ with expansion:
!{?}

CmdLine.tests.cpp:<line number>: PASSED:
REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Shard index must be a non-negative number") )
REQUIRE_THAT( result.errorMessage(), ContainsSubstring( "Could not parse '-12' as shard index" ) )
with expansion:
"Shard index must be a non-negative number" contains: "Shard index must be a
non-negative number"
"Could not parse '-12' as shard index" contains: "Could not parse '-12' as
shard index"

-------------------------------------------------------------------------------
Parsing sharding-related cli flags
Expand Down
17 changes: 8 additions & 9 deletions tests/SelfTest/Baselines/console.sw.multi.approved.txt
View file
Expand Up @@ -9173,10 +9173,10 @@ with expansion:
!{?}

CmdLine.tests.cpp:<line number>: PASSED:
REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Shard count must be a positive number") )
REQUIRE_THAT( result.errorMessage(), ContainsSubstring( "Could not parse '-1' as shard count" ) )
with expansion:
"Shard count must be a positive number" contains: "Shard count must be a
positive number"
"Could not parse '-1' as shard count" contains: "Could not parse '-1' as
shard count"

-------------------------------------------------------------------------------
Parsing sharding-related cli flags
Expand All @@ -9191,10 +9191,9 @@ with expansion:
!{?}

CmdLine.tests.cpp:<line number>: PASSED:
REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Shard count must be a positive number") )
REQUIRE_THAT( result.errorMessage(), ContainsSubstring( "Shard count must be positive" ) )
with expansion:
"Shard count must be a positive number" contains: "Shard count must be a
positive number"
"Shard count must be positive" contains: "Shard count must be positive"

-------------------------------------------------------------------------------
Parsing sharding-related cli flags
Expand Down Expand Up @@ -9226,10 +9225,10 @@ with expansion:
!{?}

CmdLine.tests.cpp:<line number>: PASSED:
REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Shard index must be a non-negative number") )
REQUIRE_THAT( result.errorMessage(), ContainsSubstring( "Could not parse '-12' as shard index" ) )
with expansion:
"Shard index must be a non-negative number" contains: "Shard index must be a
non-negative number"
"Could not parse '-12' as shard index" contains: "Could not parse '-12' as
shard index"

-------------------------------------------------------------------------------
Parsing sharding-related cli flags
Expand Down
6 changes: 3 additions & 3 deletions tests/SelfTest/Baselines/tap.sw.approved.txt
View file
Expand Up @@ -2418,19 +2418,19 @@ ok {test-number} - config.shardCount == 8 for: 8 == 8
# Parsing sharding-related cli flags
ok {test-number} - !(result) for: !{?}
# Parsing sharding-related cli flags
ok {test-number} - 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"
ok {test-number} - result.errorMessage(), ContainsSubstring( "Could not parse '-1' as shard count" ) for: "Could not parse '-1' as shard count" contains: "Could not parse '-1' as shard count"
# Parsing sharding-related cli flags
ok {test-number} - !(result) for: !{?}
# Parsing sharding-related cli flags
ok {test-number} - 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"
ok {test-number} - result.errorMessage(), ContainsSubstring( "Shard count must be positive" ) for: "Shard count must be positive" contains: "Shard count must be positive"
# Parsing sharding-related cli flags
ok {test-number} - cli.parse({ "test", "--shard-index=2" }) for: {?}
# Parsing sharding-related cli flags
ok {test-number} - config.shardIndex == 2 for: 2 == 2
# Parsing sharding-related cli flags
ok {test-number} - !(result) for: !{?}
# Parsing sharding-related cli flags
ok {test-number} - 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"
ok {test-number} - result.errorMessage(), ContainsSubstring( "Could not parse '-12' as shard index" ) for: "Could not parse '-12' as shard index" contains: "Could not parse '-12' as shard index"
# Parsing sharding-related cli flags
ok {test-number} - cli.parse({ "test", "--shard-index=0" }) for: {?}
# Parsing sharding-related cli flags
Expand Down
6 changes: 3 additions & 3 deletions tests/SelfTest/Baselines/tap.sw.multi.approved.txt
View file
Expand Up @@ -2416,19 +2416,19 @@ ok {test-number} - config.shardCount == 8 for: 8 == 8
# Parsing sharding-related cli flags
ok {test-number} - !(result) for: !{?}
# Parsing sharding-related cli flags
ok {test-number} - 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"
ok {test-number} - result.errorMessage(), ContainsSubstring( "Could not parse '-1' as shard count" ) for: "Could not parse '-1' as shard count" contains: "Could not parse '-1' as shard count"
# Parsing sharding-related cli flags
ok {test-number} - !(result) for: !{?}
# Parsing sharding-related cli flags
ok {test-number} - 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"
ok {test-number} - result.errorMessage(), ContainsSubstring( "Shard count must be positive" ) for: "Shard count must be positive" contains: "Shard count must be positive"
# Parsing sharding-related cli flags
ok {test-number} - cli.parse({ "test", "--shard-index=2" }) for: {?}
# Parsing sharding-related cli flags
ok {test-number} - config.shardIndex == 2 for: 2 == 2
# Parsing sharding-related cli flags
ok {test-number} - !(result) for: !{?}
# Parsing sharding-related cli flags
ok {test-number} - 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"
ok {test-number} - result.errorMessage(), ContainsSubstring( "Could not parse '-12' as shard index" ) for: "Could not parse '-12' as shard index" contains: "Could not parse '-12' as shard index"
# Parsing sharding-related cli flags
ok {test-number} - cli.parse({ "test", "--shard-index=0" }) for: {?}
# Parsing sharding-related cli flags
Expand Down
12 changes: 6 additions & 6 deletions tests/SelfTest/Baselines/xml.sw.approved.txt
View file
Expand Up @@ -11160,10 +11160,10 @@ C
</Expression>
<Expression success="true" type="REQUIRE_THAT" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
<Original>
result.errorMessage(), ContainsSubstring("Shard count must be a positive number")
result.errorMessage(), ContainsSubstring( "Could not parse '-1' as shard count" )
</Original>
<Expanded>
"Shard count must be a positive number" contains: "Shard count must be a positive number"
"Could not parse '-1' as shard count" contains: "Could not parse '-1' as shard count"
</Expanded>
</Expression>
<OverallResults successes="2" failures="0" expectedFailures="0"/>
Expand All @@ -11179,10 +11179,10 @@ C
</Expression>
<Expression success="true" type="REQUIRE_THAT" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
<Original>
result.errorMessage(), ContainsSubstring("Shard count must be a positive number")
result.errorMessage(), ContainsSubstring( "Shard count must be positive" )
</Original>
<Expanded>
"Shard count must be a positive number" contains: "Shard count must be a positive number"
"Shard count must be positive" contains: "Shard count must be positive"
</Expanded>
</Expression>
<OverallResults successes="2" failures="0" expectedFailures="0"/>
Expand Down Expand Up @@ -11217,10 +11217,10 @@ C
</Expression>
<Expression success="true" type="REQUIRE_THAT" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
<Original>
result.errorMessage(), ContainsSubstring("Shard index must be a non-negative number")
result.errorMessage(), ContainsSubstring( "Could not parse '-12' as shard index" )
</Original>
<Expanded>
"Shard index must be a non-negative number" contains: "Shard index must be a non-negative number"
"Could not parse '-12' as shard index" contains: "Could not parse '-12' as shard index"
</Expanded>
</Expression>
<OverallResults successes="2" failures="0" expectedFailures="0"/>
Expand Down
12 changes: 6 additions & 6 deletions tests/SelfTest/Baselines/xml.sw.multi.approved.txt
View file
Expand Up @@ -11160,10 +11160,10 @@ C
</Expression>
<Expression success="true" type="REQUIRE_THAT" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
<Original>
result.errorMessage(), ContainsSubstring("Shard count must be a positive number")
result.errorMessage(), ContainsSubstring( "Could not parse '-1' as shard count" )
</Original>
<Expanded>
"Shard count must be a positive number" contains: "Shard count must be a positive number"
"Could not parse '-1' as shard count" contains: "Could not parse '-1' as shard count"
</Expanded>
</Expression>
<OverallResults successes="2" failures="0" expectedFailures="0"/>
Expand All @@ -11179,10 +11179,10 @@ C
</Expression>
<Expression success="true" type="REQUIRE_THAT" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
<Original>
result.errorMessage(), ContainsSubstring("Shard count must be a positive number")
result.errorMessage(), ContainsSubstring( "Shard count must be positive" )
</Original>
<Expanded>
"Shard count must be a positive number" contains: "Shard count must be a positive number"
"Shard count must be positive" contains: "Shard count must be positive"
</Expanded>
</Expression>
<OverallResults successes="2" failures="0" expectedFailures="0"/>
Expand Down Expand Up @@ -11217,10 +11217,10 @@ C
</Expression>
<Expression success="true" type="REQUIRE_THAT" filename="tests/<exe-name>/IntrospectiveTests/CmdLine.tests.cpp" >
<Original>
result.errorMessage(), ContainsSubstring("Shard index must be a non-negative number")
result.errorMessage(), ContainsSubstring( "Could not parse '-12' as shard index" )
</Original>
<Expanded>
"Shard index must be a non-negative number" contains: "Shard index must be a non-negative number"
"Could not parse '-12' as shard index" contains: "Could not parse '-12' as shard index"
</Expanded>
</Expression>
<OverallResults successes="2" failures="0" expectedFailures="0"/>
Expand Down

0 comments on commit a43f679

Please sign in to comment.