diff --git a/src/catch2/catch_config.cpp b/src/catch2/catch_config.cpp index 86b9b8dd..e71ee353 100644 --- a/src/catch2/catch_config.cpp +++ b/src/catch2/catch_config.cpp @@ -209,6 +209,7 @@ namespace Catch { double Config::minDuration() const { return m_data.minDuration; } TestRunOrder Config::runOrder() const { return m_data.runOrder; } uint32_t Config::rngSeed() const { return m_data.rngSeed; } + bool Config::rngSeedWasFixed() const { return m_data.rngSeedWasFixed; } unsigned int Config::shardCount() const { return m_data.shardCount; } unsigned int Config::shardIndex() const { return m_data.shardIndex; } ColourMode Config::defaultColourMode() const { return m_data.defaultColourMode; } @@ -273,6 +274,7 @@ namespace Catch { << bazelRandomSeed << "') as proper seed.\n"; } else { m_data.rngSeed = *parsedSeed; + m_data.rngSeedWasFixed = true; } } } diff --git a/src/catch2/catch_config.hpp b/src/catch2/catch_config.hpp index 429c7156..3d0c1e96 100644 --- a/src/catch2/catch_config.hpp +++ b/src/catch2/catch_config.hpp @@ -63,6 +63,7 @@ namespace Catch { int abortAfter = -1; uint32_t rngSeed = generateRandomSeed(GenerateFrom::Default); + bool rngSeedWasFixed = false; unsigned int shardCount = 1; unsigned int shardIndex = 0; @@ -134,6 +135,7 @@ namespace Catch { double minDuration() const override; TestRunOrder runOrder() const override; uint32_t rngSeed() const override; + bool rngSeedWasFixed() const; unsigned int shardCount() const override; unsigned int shardIndex() const override; ColourMode defaultColourMode() const override; diff --git a/src/catch2/catch_session.cpp b/src/catch2/catch_session.cpp index 97def8dc..5e9c5c19 100644 --- a/src/catch2/catch_session.cpp +++ b/src/catch2/catch_session.cpp @@ -348,6 +348,19 @@ namespace Catch { CATCH_TRY { config(); // Force config to be constructed + if ( m_config->shardCount() > 1 && + m_config->runOrder() == TestRunOrder::Randomized && + !m_config->rngSeedWasFixed() ) { + Catch::cerr() + << "Warning: using sharding (--shard-count) with random " + "order (--order rand, the default) and without a fixed " + "numeric --rng-seed does not guarantee disjoint coverage " + "between shard invocations. Pass the same numeric " + "--rng-seed to every shard, or use --order decl or " + "--order lex instead.\n" + << std::flush; + } + // We need to retrieve potential Bazel config with the full Config // constructor, so we have to create the guard file after it is created. setUpGuardFile( m_config->getExitGuardFilePath() ); diff --git a/src/catch2/internal/catch_commandline.cpp b/src/catch2/internal/catch_commandline.cpp index 4ce42397..22948c27 100644 --- a/src/catch2/internal/catch_commandline.cpp +++ b/src/catch2/internal/catch_commandline.cpp @@ -75,9 +75,11 @@ namespace Catch { auto const setRngSeed = [&]( std::string const& seed ) { if( seed == "time" ) { config.rngSeed = generateRandomSeed(GenerateFrom::Time); + config.rngSeedWasFixed = false; return ParserResult::ok(ParseResultType::Matched); } else if (seed == "random-device") { config.rngSeed = generateRandomSeed(GenerateFrom::RandomDevice); + config.rngSeedWasFixed = false; return ParserResult::ok(ParseResultType::Matched); } @@ -88,6 +90,7 @@ namespace Catch { return ParserResult::runtimeError( "Could not parse '" + seed + "' as seed" ); } config.rngSeed = *parsedSeed; + config.rngSeedWasFixed = true; return ParserResult::ok( ParseResultType::Matched ); }; auto const setDefaultColourMode = [&]( std::string const& colourMode ) { diff --git a/tests/ExtraTests/CMakeLists.txt b/tests/ExtraTests/CMakeLists.txt index 21075a5b..a7b78748 100644 --- a/tests/ExtraTests/CMakeLists.txt +++ b/tests/ExtraTests/CMakeLists.txt @@ -21,6 +21,37 @@ set_tests_properties(TestSharding::OverlyLargeShardIndex PASS_REGULAR_EXPRESSION "The shard count \\(5\\) must be greater than the shard index \\(5\\)" ) +set(CATCH_SHARDING_WARNING_REGEX "Warning: using sharding .* with random order") + +add_test( + NAME TestSharding::WarningOnRandomOrderWithoutFixedSeed + COMMAND $ --shard-index 0 --shard-count 2 --list-tests +) +set_tests_properties(TestSharding::WarningOnRandomOrderWithoutFixedSeed + PROPERTIES + PASS_REGULAR_EXPRESSION "${CATCH_SHARDING_WARNING_REGEX}" +) + +add_test( + NAME TestSharding::NoWarningOnRandomOrderWithFixedSeed + COMMAND $ --shard-index 0 --shard-count 2 --rng-seed 12345 --list-tests +) +set_tests_properties(TestSharding::NoWarningOnRandomOrderWithFixedSeed + PROPERTIES + FAIL_REGULAR_EXPRESSION "${CATCH_SHARDING_WARNING_REGEX}" +) + +foreach(shardOrder decl lex) + add_test( + NAME TestSharding::NoWarningOn${shardOrder}OrderWithoutFixedSeed + COMMAND $ --shard-index 0 --shard-count 2 --order ${shardOrder} --list-tests + ) + set_tests_properties(TestSharding::NoWarningOn${shardOrder}OrderWithoutFixedSeed + PROPERTIES + FAIL_REGULAR_EXPRESSION "${CATCH_SHARDING_WARNING_REGEX}" + ) +endforeach() + # The MinDuration reporting tests do not need separate compilation, but # they have non-trivial execution time, so they are categorized as # extra tests, so that they are run less. diff --git a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp index 0717ae2e..71ddc6fd 100644 --- a/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp +++ b/tests/SelfTest/IntrospectiveTests/CmdLine.tests.cpp @@ -473,15 +473,34 @@ TEST_CASE( "Parse rng seed in different formats", "[approvals][cli][rng-seed]" ) CAPTURE( seed_string ); auto result = cli.parse( { "tests", "--rng-seed", seed_string } ); - REQUIRE( result ); - REQUIRE( config.rngSeed == seed_value ); + + Catch::Config cfg{config}; + REQUIRE( cfg.rngSeed() == seed_value ); + REQUIRE( cfg.rngSeedWasFixed() ); + } + SECTION( "time seed is not considered fixed" ) { + auto result = cli.parse( { "tests", "--rng-seed", "time" } ); + REQUIRE( result ); + + Catch::Config cfg{config}; + REQUIRE_FALSE( cfg.rngSeedWasFixed() ); + } + SECTION( "random-device seed is not considered fixed" ) { + auto result = cli.parse( { "tests", "--rng-seed", "random-device" } ); + REQUIRE( result ); + + Catch::Config cfg{config}; + REQUIRE_FALSE( cfg.rngSeedWasFixed() ); } SECTION( "Error cases" ) { auto seed_string = GENERATE( "0xSEED", "999999999999", "08888", "BEEF", "123 456" ); CAPTURE( seed_string ); REQUIRE_FALSE( cli.parse( { "tests", "--rng-seed", seed_string } ) ); + + Catch::Config cfg{config}; + REQUIRE_FALSE( cfg.rngSeedWasFixed() ); } }