Refactor implementation of hashed random order test sorting

Also removed the iterative checking that seeds 1-100 do not create
the same output, because it used too much runtime.

include/internal/catch_test_case_registry_impl.cpp

@@ -15,27 +15,78 @@
 #include "catch_string_manip.h"
 #include "catch_test_case_info.h"
 
+#include <algorithm>
 #include <sstream>
 
 namespace Catch {
 
+    namespace {
+        struct TestHasher {
+            explicit TestHasher(Catch::SimplePcg32& rng) {
+                basis = rng();
+                basis <<= 32;
+                basis |= rng();
+            }
+
+            uint64_t basis;
+
+            uint64_t operator()(TestCase const& t) const {
+                // Modified FNV-1a hash
+                static constexpr uint64_t prime = 1099511628211;
+                uint64_t hash = basis;
+                for (const char c : t.name) {
+                    hash ^= c;
+                    hash *= prime;
+                }
+                return hash;
+            }
+        };
+    } // end unnamed namespace
+
+
     std::vector<TestCase> sortTests( IConfig const& config, std::vector<TestCase> const& unsortedTestCases ) {
-
-        std::vector<TestCase> sorted = unsortedTestCases;
-
         switch( config.runOrder() ) {
-            case RunTests::InLexicographicalOrder:
-                std::sort( sorted.begin(), sorted.end() );
-                break;
-            case RunTests::InRandomOrder:
-                seedRng( config );
-                std::shuffle( sorted.begin(), sorted.end(), rng() );
-                break;
             case RunTests::InDeclarationOrder:
                 // already in declaration order
                 break;
+
+            case RunTests::InLexicographicalOrder: {
+                std::vector<TestCase> sorted = unsortedTestCases;
+                std::sort( sorted.begin(), sorted.end() );
+                return sorted;
+            }
+
+            case RunTests::InRandomOrder: {
+                seedRng( config );
+                TestHasher h( rng() );
+
+                using hashedTest = std::pair<uint64_t, TestCase const*>;
+                std::vector<hashedTest> indexed_tests;
+                indexed_tests.reserve( unsortedTestCases.size() );
+
+                for (auto const& testCase : unsortedTestCases) {
+                    indexed_tests.emplace_back(h(testCase), &testCase);
+                }
+
+                std::sort(indexed_tests.begin(), indexed_tests.end(),
+                          [](hashedTest const& lhs, hashedTest const& rhs) {
+                          if (lhs.first == rhs.first) {
+                              return lhs.second->name < rhs.second->name;
+                          }
+                          return lhs.first < rhs.first;
+                });
+
+                std::vector<TestCase> sorted;
+                sorted.reserve( indexed_tests.size() );
+
+                for (auto const& hashed : indexed_tests) {
+                    sorted.emplace_back(*hashed.second);
+                }
+
+                return sorted;
+            }
         }
-        return sorted;
+        return unsortedTestCases;
     }
 
     bool isThrowSafe( TestCase const& testCase, IConfig const& config ) {

projects/CMakeLists.txt

@@ -451,6 +451,8 @@ set_tests_properties(TestsInFile::InvalidTestNames-1 PROPERTIES PASS_REGULAR_EXP
 add_test(NAME TestsInFile::InvalidTestNames-2 COMMAND $<TARGET_FILE:SelfTest> "-f ${CATCH_DIR}/projects/SelfTest/Misc/invalid-test-names.input")
 set_tests_properties(TestsInFile::InvalidTestNames-2 PROPERTIES PASS_REGULAR_EXPRESSION "No tests ran")
 
+add_test(NAME RandomTestOrdering COMMAND ${PYTHON_EXECUTABLE}
+  ${CATCH_DIR}/projects/TestScripts/testRandomOrder.py $<TARGET_FILE:SelfTest>)
 
 if (CATCH_USE_VALGRIND)
     add_test(NAME ValgrindRunTests COMMAND valgrind --leak-check=full --error-exitcode=1 $<TARGET_FILE:SelfTest>)

projects/TestScripts/testRandomOrder.py

@@ -0,0 +1,59 @@
+#!/usr/bin/env python3
+
+"""
+This test script verifies that the random ordering of tests inside
+Catch2 is invariant in regards to subsetting. This is done by running
+the binary 3 times, once with all tests selected, and twice with smaller
+subsets of tests selected, and verifying that the selected tests are in
+the same relative order.
+"""
+
+import subprocess
+import sys
+import random
+
+def list_tests(self_test_exe, tags, rng_seed):
+    cmd = [self_test_exe, '--list-test-names-only', '--order', 'rand',
+            '--rng-seed', str(rng_seed)]
+    tags_arg = ','.join('[{}]'.format(t) for t in tags)
+    if tags_arg:
+        cmd.append(tags_arg + '~[.]')
+    process = subprocess.Popen(
+            cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    stdout, stderr = process.communicate()
+    if stderr:
+        raise RuntimeError("Unexpected error output:\n" + process.stderr)
+    result = stdout.split(b'\n')
+    result = [s for s in result if s]
+    if len(result) < 2:
+        raise RuntimeError("Unexpectedly few tests listed (got {})".format(
+            len(result)))
+    return result
+
+def check_is_sublist_of(shorter, longer):
+    assert len(shorter) < len(longer)
+    assert len(set(longer)) == len(longer)
+
+    indexes_in_longer = {s: i for i, s in enumerate(longer)}
+    for s1, s2 in zip(shorter, shorter[1:]):
+        assert indexes_in_longer[s1] < indexes_in_longer[s2], (
+                '{} comes before {} in longer list.\n'
+                'Longer: {}\nShorter: {}'.format(s2, s1, longer, shorter))
+
+def main():
+    self_test_exe, = sys.argv[1:]
+
+    # We want a random seed for the test, but want to avoid 0,
+    # because it has special meaning
+    seed = random.randint(1, 2 ** 32 - 1)
+
+    list_one_tag = list_tests(self_test_exe, ['generators'], seed)
+    list_two_tags = list_tests(self_test_exe, ['generators', 'matchers'], seed)
+    list_all = list_tests(self_test_exe, [], seed)
+
+    # First, verify that restricting to a subset yields the same order
+    check_is_sublist_of(list_two_tags, list_all)
+    check_is_sublist_of(list_one_tag, list_two_tags)
+
+if __name__ == '__main__':
+    sys.exit(main())