Emit warning when using --shard-count (> 1) with --order rand

Author: mattyrazz7

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;
             }
         }
     }

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;

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() );

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 ) {

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 $<TARGET_FILE:SelfTest> --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 $<TARGET_FILE:SelfTest> --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 $<TARGET_FILE:SelfTest> --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.

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 );
 
+        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 );
-        REQUIRE( config.rngSeed == seed_value );
+
+        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() );
     }
 }