From b66b89374e60fa2fb725814acefbfc987ee5ff5e Mon Sep 17 00:00:00 2001 From: qerased <55762598+qerased@users.noreply.github.com> Date: Sat, 3 Jan 2026 01:15:13 +0300 Subject: [PATCH] Add validation for --benchmark-samples to prevent crash with zero value (#3056) * Add validation for --benchmark-samples to prevent crash with zero samples * Add automated test for benchmark samples validation * Update baselines for benchmark-samples validation * Add missing test for benchmark samples validation --------- Co-authored-by: Chan Aung --- src/catch2/internal/catch_commandline.cpp | 15 +++++- .../Baselines/compact.sw.approved.txt | 6 ++- .../Baselines/compact.sw.multi.approved.txt | 6 ++- .../Baselines/console.std.approved.txt | 2 +- .../Baselines/console.sw.approved.txt | 40 +++++++++++++++- .../Baselines/console.sw.multi.approved.txt | 40 +++++++++++++++- .../SelfTest/Baselines/junit.sw.approved.txt | 4 +- .../Baselines/junit.sw.multi.approved.txt | 4 +- .../Baselines/sonarqube.sw.approved.txt | 2 + .../Baselines/sonarqube.sw.multi.approved.txt | 2 + tests/SelfTest/Baselines/tap.sw.approved.txt | 10 +++- .../Baselines/tap.sw.multi.approved.txt | 10 +++- tests/SelfTest/Baselines/xml.sw.approved.txt | 46 ++++++++++++++++++- .../Baselines/xml.sw.multi.approved.txt | 46 ++++++++++++++++++- .../IntrospectiveTests/CmdLine.tests.cpp | 17 +++++++ 15 files changed, 238 insertions(+), 12 deletions(-) diff --git a/src/catch2/internal/catch_commandline.cpp b/src/catch2/internal/catch_commandline.cpp index 88454c0a..cf9cbf6d 100644 --- a/src/catch2/internal/catch_commandline.cpp +++ b/src/catch2/internal/catch_commandline.cpp @@ -189,6 +189,19 @@ namespace Catch { config.shardCount = *parsedCount; return ParserResult::ok( ParseResultType::Matched ); }; + auto const setBenchmarkSamples = [&]( std::string const& samples ) { + auto parsedSamples = parseUInt( samples ); + if ( !parsedSamples ) { + return ParserResult::runtimeError( + "Could not parse '" + samples + "' as benchmark samples" ); + } + if ( *parsedSamples == 0 ) { + return ParserResult::runtimeError( + "Benchmark samples must be greater than 0" ); + } + config.benchmarkSamples = *parsedSamples; + return ParserResult::ok( ParseResultType::Matched ); + }; auto const setShardIndex = [&](std::string const& shardIndex) { auto parsedIndex = parseUInt( shardIndex ); @@ -281,7 +294,7 @@ namespace Catch { | Opt( config.skipBenchmarks) ["--skip-benchmarks"] ( "disable running benchmarks") - | Opt( config.benchmarkSamples, "samples" ) + | Opt( setBenchmarkSamples, "samples" ) ["--benchmark-samples"] ( "number of samples to collect (default: 100)" ) | Opt( config.benchmarkResamples, "resamples" ) diff --git a/tests/SelfTest/Baselines/compact.sw.approved.txt b/tests/SelfTest/Baselines/compact.sw.approved.txt index 37c2624f..29a1017c 100644 --- a/tests/SelfTest/Baselines/compact.sw.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.approved.txt @@ -1503,6 +1503,10 @@ CmdLine.tests.cpp:: passed: !result for: true CmdLine.tests.cpp:: passed: result.errorMessage(), ContainsSubstring( "colour mode must be one of" ) for: "colour mode must be one of: default, ansi, win32, or none. 'wrong' is not recognised" contains: "colour mode must be one of" CmdLine.tests.cpp:: passed: cli.parse({ "test", "--benchmark-samples=200" }) for: {?} CmdLine.tests.cpp:: passed: config.benchmarkSamples == 200 for: 200 == 200 +CmdLine.tests.cpp:: passed: !(result) for: !{?} +CmdLine.tests.cpp:: passed: result.errorMessage(), ContainsSubstring("Benchmark samples must be greater than 0") for: "Benchmark samples must be greater than 0" contains: "Benchmark samples must be greater than 0" +CmdLine.tests.cpp:: passed: !(result) for: !{?} +CmdLine.tests.cpp:: passed: result.errorMessage(), ContainsSubstring("Could not parse 'abc' as benchmark samples") for: "Could not parse 'abc' as benchmark samples" contains: "Could not parse 'abc' as benchmark samples" CmdLine.tests.cpp:: passed: cli.parse({ "test", "--benchmark-resamples=20000" }) for: {?} CmdLine.tests.cpp:: passed: config.benchmarkResamples == 20000 for: 20000 (0x) == 20000 (0x) CmdLine.tests.cpp:: passed: cli.parse({ "test", "--benchmark-confidence-interval=0.99" }) for: {?} @@ -2891,6 +2895,6 @@ InternalBenchmark.tests.cpp:: passed: q3 == 23. for: 23.0 == 23.0 Misc.tests.cpp:: passed: Misc.tests.cpp:: passed: test cases: 436 | 317 passed | 95 failed | 6 skipped | 18 failed as expected -assertions: 2305 | 2106 passed | 157 failed | 42 failed as expected +assertions: 2309 | 2110 passed | 157 failed | 42 failed as expected diff --git a/tests/SelfTest/Baselines/compact.sw.multi.approved.txt b/tests/SelfTest/Baselines/compact.sw.multi.approved.txt index b103d28f..5e9a32c7 100644 --- a/tests/SelfTest/Baselines/compact.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/compact.sw.multi.approved.txt @@ -1501,6 +1501,10 @@ CmdLine.tests.cpp:: passed: !result for: true CmdLine.tests.cpp:: passed: result.errorMessage(), ContainsSubstring( "colour mode must be one of" ) for: "colour mode must be one of: default, ansi, win32, or none. 'wrong' is not recognised" contains: "colour mode must be one of" CmdLine.tests.cpp:: passed: cli.parse({ "test", "--benchmark-samples=200" }) for: {?} CmdLine.tests.cpp:: passed: config.benchmarkSamples == 200 for: 200 == 200 +CmdLine.tests.cpp:: passed: !(result) for: !{?} +CmdLine.tests.cpp:: passed: result.errorMessage(), ContainsSubstring("Benchmark samples must be greater than 0") for: "Benchmark samples must be greater than 0" contains: "Benchmark samples must be greater than 0" +CmdLine.tests.cpp:: passed: !(result) for: !{?} +CmdLine.tests.cpp:: passed: result.errorMessage(), ContainsSubstring("Could not parse 'abc' as benchmark samples") for: "Could not parse 'abc' as benchmark samples" contains: "Could not parse 'abc' as benchmark samples" CmdLine.tests.cpp:: passed: cli.parse({ "test", "--benchmark-resamples=20000" }) for: {?} CmdLine.tests.cpp:: passed: config.benchmarkResamples == 20000 for: 20000 (0x) == 20000 (0x) CmdLine.tests.cpp:: passed: cli.parse({ "test", "--benchmark-confidence-interval=0.99" }) for: {?} @@ -2880,6 +2884,6 @@ InternalBenchmark.tests.cpp:: passed: q3 == 23. for: 23.0 == 23.0 Misc.tests.cpp:: passed: Misc.tests.cpp:: passed: test cases: 436 | 317 passed | 95 failed | 6 skipped | 18 failed as expected -assertions: 2305 | 2106 passed | 157 failed | 42 failed as expected +assertions: 2309 | 2110 passed | 157 failed | 42 failed as expected diff --git a/tests/SelfTest/Baselines/console.std.approved.txt b/tests/SelfTest/Baselines/console.std.approved.txt index eefaa889..8c7f79da 100644 --- a/tests/SelfTest/Baselines/console.std.approved.txt +++ b/tests/SelfTest/Baselines/console.std.approved.txt @@ -1731,5 +1731,5 @@ due to unexpected exception with message: =============================================================================== test cases: 436 | 335 passed | 76 failed | 7 skipped | 18 failed as expected -assertions: 2284 | 2106 passed | 136 failed | 42 failed as expected +assertions: 2288 | 2110 passed | 136 failed | 42 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.approved.txt b/tests/SelfTest/Baselines/console.sw.approved.txt index 5b82e70e..d6e61ff6 100644 --- a/tests/SelfTest/Baselines/console.sw.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.approved.txt @@ -9987,6 +9987,44 @@ CmdLine.tests.cpp:: PASSED: with expansion: 200 == 200 +------------------------------------------------------------------------------- +Process can be configured on command line + Benchmark options + samples must be greater than zero +------------------------------------------------------------------------------- +CmdLine.tests.cpp: +............................................................................... + +CmdLine.tests.cpp:: PASSED: + CHECK_FALSE( result ) +with expansion: + !{?} + +CmdLine.tests.cpp:: PASSED: + REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Benchmark samples must be greater than 0") ) +with expansion: + "Benchmark samples must be greater than 0" contains: "Benchmark samples must + be greater than 0" + +------------------------------------------------------------------------------- +Process can be configured on command line + Benchmark options + samples must be parseable +------------------------------------------------------------------------------- +CmdLine.tests.cpp: +............................................................................... + +CmdLine.tests.cpp:: PASSED: + CHECK_FALSE( result ) +with expansion: + !{?} + +CmdLine.tests.cpp:: PASSED: + REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Could not parse 'abc' as benchmark samples") ) +with expansion: + "Could not parse 'abc' as benchmark samples" contains: "Could not parse 'abc' + as benchmark samples" + ------------------------------------------------------------------------------- Process can be configured on command line Benchmark options @@ -19312,5 +19350,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 436 | 317 passed | 95 failed | 6 skipped | 18 failed as expected -assertions: 2305 | 2106 passed | 157 failed | 42 failed as expected +assertions: 2309 | 2110 passed | 157 failed | 42 failed as expected diff --git a/tests/SelfTest/Baselines/console.sw.multi.approved.txt b/tests/SelfTest/Baselines/console.sw.multi.approved.txt index 61873e2f..9bd865da 100644 --- a/tests/SelfTest/Baselines/console.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/console.sw.multi.approved.txt @@ -9985,6 +9985,44 @@ CmdLine.tests.cpp:: PASSED: with expansion: 200 == 200 +------------------------------------------------------------------------------- +Process can be configured on command line + Benchmark options + samples must be greater than zero +------------------------------------------------------------------------------- +CmdLine.tests.cpp: +............................................................................... + +CmdLine.tests.cpp:: PASSED: + CHECK_FALSE( result ) +with expansion: + !{?} + +CmdLine.tests.cpp:: PASSED: + REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Benchmark samples must be greater than 0") ) +with expansion: + "Benchmark samples must be greater than 0" contains: "Benchmark samples must + be greater than 0" + +------------------------------------------------------------------------------- +Process can be configured on command line + Benchmark options + samples must be parseable +------------------------------------------------------------------------------- +CmdLine.tests.cpp: +............................................................................... + +CmdLine.tests.cpp:: PASSED: + CHECK_FALSE( result ) +with expansion: + !{?} + +CmdLine.tests.cpp:: PASSED: + REQUIRE_THAT( result.errorMessage(), ContainsSubstring("Could not parse 'abc' as benchmark samples") ) +with expansion: + "Could not parse 'abc' as benchmark samples" contains: "Could not parse 'abc' + as benchmark samples" + ------------------------------------------------------------------------------- Process can be configured on command line Benchmark options @@ -19301,5 +19339,5 @@ Misc.tests.cpp:: PASSED: =============================================================================== test cases: 436 | 317 passed | 95 failed | 6 skipped | 18 failed as expected -assertions: 2305 | 2106 passed | 157 failed | 42 failed as expected +assertions: 2309 | 2110 passed | 157 failed | 42 failed as expected diff --git a/tests/SelfTest/Baselines/junit.sw.approved.txt b/tests/SelfTest/Baselines/junit.sw.approved.txt index ea99ab45..24d16a96 100644 --- a/tests/SelfTest/Baselines/junit.sw.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + @@ -1325,6 +1325,8 @@ at Message.tests.cpp: + + diff --git a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt index 07e402c7..21e6a885 100644 --- a/tests/SelfTest/Baselines/junit.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/junit.sw.multi.approved.txt @@ -1,6 +1,6 @@ - + @@ -1324,6 +1324,8 @@ at Message.tests.cpp: + + diff --git a/tests/SelfTest/Baselines/sonarqube.sw.approved.txt b/tests/SelfTest/Baselines/sonarqube.sw.approved.txt index 881851c3..184bdf71 100644 --- a/tests/SelfTest/Baselines/sonarqube.sw.approved.txt +++ b/tests/SelfTest/Baselines/sonarqube.sw.approved.txt @@ -119,6 +119,8 @@ at AssertionHandler.tests.cpp: + + diff --git a/tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt b/tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt index 5a64789d..c7dc1ce0 100644 --- a/tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/sonarqube.sw.multi.approved.txt @@ -118,6 +118,8 @@ at AssertionHandler.tests.cpp: + + diff --git a/tests/SelfTest/Baselines/tap.sw.approved.txt b/tests/SelfTest/Baselines/tap.sw.approved.txt index 415f9964..c9498ec5 100644 --- a/tests/SelfTest/Baselines/tap.sw.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.approved.txt @@ -2485,6 +2485,14 @@ ok {test-number} - cli.parse({ "test", "--benchmark-samples=200" }) for: {?} # Process can be configured on command line ok {test-number} - config.benchmarkSamples == 200 for: 200 == 200 # Process can be configured on command line +ok {test-number} - !(result) for: !{?} +# Process can be configured on command line +ok {test-number} - result.errorMessage(), ContainsSubstring("Benchmark samples must be greater than 0") for: "Benchmark samples must be greater than 0" contains: "Benchmark samples must be greater than 0" +# Process can be configured on command line +ok {test-number} - !(result) for: !{?} +# Process can be configured on command line +ok {test-number} - result.errorMessage(), ContainsSubstring("Could not parse 'abc' as benchmark samples") for: "Could not parse 'abc' as benchmark samples" contains: "Could not parse 'abc' as benchmark samples" +# Process can be configured on command line ok {test-number} - cli.parse({ "test", "--benchmark-resamples=20000" }) for: {?} # Process can be configured on command line ok {test-number} - config.benchmarkResamples == 20000 for: 20000 (0x) == 20000 (0x) @@ -4631,5 +4639,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2317 +1..2321 diff --git a/tests/SelfTest/Baselines/tap.sw.multi.approved.txt b/tests/SelfTest/Baselines/tap.sw.multi.approved.txt index 63ea617b..4d86c1c1 100644 --- a/tests/SelfTest/Baselines/tap.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/tap.sw.multi.approved.txt @@ -2483,6 +2483,14 @@ ok {test-number} - cli.parse({ "test", "--benchmark-samples=200" }) for: {?} # Process can be configured on command line ok {test-number} - config.benchmarkSamples == 200 for: 200 == 200 # Process can be configured on command line +ok {test-number} - !(result) for: !{?} +# Process can be configured on command line +ok {test-number} - result.errorMessage(), ContainsSubstring("Benchmark samples must be greater than 0") for: "Benchmark samples must be greater than 0" contains: "Benchmark samples must be greater than 0" +# Process can be configured on command line +ok {test-number} - !(result) for: !{?} +# Process can be configured on command line +ok {test-number} - result.errorMessage(), ContainsSubstring("Could not parse 'abc' as benchmark samples") for: "Could not parse 'abc' as benchmark samples" contains: "Could not parse 'abc' as benchmark samples" +# Process can be configured on command line ok {test-number} - cli.parse({ "test", "--benchmark-resamples=20000" }) for: {?} # Process can be configured on command line ok {test-number} - config.benchmarkResamples == 20000 for: 20000 (0x) == 20000 (0x) @@ -4620,5 +4628,5 @@ ok {test-number} - q3 == 23. for: 23.0 == 23.0 ok {test-number} - # xmlentitycheck ok {test-number} - -1..2317 +1..2321 diff --git a/tests/SelfTest/Baselines/xml.sw.approved.txt b/tests/SelfTest/Baselines/xml.sw.approved.txt index 3536950e..d2fea72f 100644 --- a/tests/SelfTest/Baselines/xml.sw.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.approved.txt @@ -11987,6 +11987,50 @@ Approx( 1.21999999999999997 ) +
+
+ + + !(result) + + + !{?} + + + + + result.errorMessage(), ContainsSubstring("Benchmark samples must be greater than 0") + + + "Benchmark samples must be greater than 0" contains: "Benchmark samples must be greater than 0" + + + +
+ +
+
+
+ + + !(result) + + + !{?} + + + + + result.errorMessage(), ContainsSubstring("Could not parse 'abc' as benchmark samples") + + + "Could not parse 'abc' as benchmark samples" contains: "Could not parse 'abc' as benchmark samples" + + + +
+ +
@@ -22349,6 +22393,6 @@ Approx( -1.95996398454005449 )
- + diff --git a/tests/SelfTest/Baselines/xml.sw.multi.approved.txt b/tests/SelfTest/Baselines/xml.sw.multi.approved.txt index 5242f5c9..43afc154 100644 --- a/tests/SelfTest/Baselines/xml.sw.multi.approved.txt +++ b/tests/SelfTest/Baselines/xml.sw.multi.approved.txt @@ -11987,6 +11987,50 @@ Approx( 1.21999999999999997 )
+
+
+ + + !(result) + + + !{?} + + + + + result.errorMessage(), ContainsSubstring("Benchmark samples must be greater than 0") + + + "Benchmark samples must be greater than 0" contains: "Benchmark samples must be greater than 0" + + + +
+ +
+
+
+ + + !(result) + + + !{?} + + + + + result.errorMessage(), ContainsSubstring("Could not parse 'abc' as benchmark samples") + + + "Could not parse 'abc' as benchmark samples" contains: "Could not parse 'abc' as benchmark samples" + + + +
+ +
@@ -22348,6 +22392,6 @@ Approx( -1.95996398454005449 )
- + diff --git a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp index 404bad27..2cb7cae2 100644 --- a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp @@ -301,6 +301,23 @@ TEST_CASE( "Process can be configured on command line", "[config][command-line]" REQUIRE(config.benchmarkSamples == 200); } + SECTION("samples must be greater than zero"){ + auto result = cli.parse({"test", "--benchmark-samples=0"}); + + CHECK_FALSE(result); + REQUIRE_THAT( + result.errorMessage(), + ContainsSubstring("Benchmark samples must be greater than 0")); + } + + SECTION("samples must be parseable") { + auto result = cli.parse({"test", "--benchmark-samples=abc"}); + + CHECK_FALSE(result); + REQUIRE_THAT( + result.errorMessage(), + ContainsSubstring("Could not parse 'abc' as benchmark samples")); + } SECTION("resamples") { CHECK(cli.parse({ "test", "--benchmark-resamples=20000" }));