Merge bitcoin/bitcoin#30748: test: Pin and document TEST_DIR_PATH_ELEMENT, SeedRand::FIXED_SEED

fa84f9d test: Pin and document TEST_DIR_PATH_ELEMENT (MarcoFalke)
2222f7a test: Rename SeedRand::SEED to FIXED_SEED for clarity (MarcoFalke)

Pull request description:

  Two small test changes:

  * A refactor to update the name and documentation around `SeedRand::FIXED_SEED`.
  * A change to extract and document `TEST_DIR_PATH_ELEMENT`, and to change its value to better match the `TMPDIR_PREFIX` in functional tests. The value previously included `PACKAGE_NAME`, which is cute, but doesn't explain why it was used (to include a space). So just use `test_common bitcoin` to achieve the same with less effort.

ACKs for top commit:
  hodlinator:
    ACK fa84f9d
  ryanofsky:
    Code review ACK fa84f9d

Tree-SHA512: eb35d6598bb08f9b996e3a4762d8f26b2441c0ca00780798e473015af735dfc9997120895a922b94d4b6ada45adadba4a686e9cf9c285ddf688848e764c64840

Author: fanquake

src/test/README.md

@@ -63,7 +63,7 @@ build/src/test/test_bitcoin --run_test=getarg_tests/doubledash
 ```
 
 `test_bitcoin` creates a temporary working (data) directory with a randomly
-generated pathname within `test_common_Bitcoin Core/`, which in turn is within
+generated pathname within `test_common bitcoin/`, which in turn is within
 the system's temporary directory (see
 [`temp_directory_path`](https://en.cppreference.com/w/cpp/filesystem/temp_directory_path)).
 This data directory looks like a simplified form of the standard `bitcoind` data
@@ -73,7 +73,7 @@ have a `debug.log` file, for example.
 The location of the temporary data directory can be specified with the
 `-testdatadir` option. This can make debugging easier. The directory
 path used is the argument path appended with
-`/test_common_Bitcoin Core/<test-name>/datadir`.
+`/test_common bitcoin/<test-name>/datadir`.
 The directory path is created if necessary.
 Specifying this argument also causes the data directory
 not to be removed after the last test. This is useful for looking at
@@ -83,11 +83,11 @@ so no leftover state is used.)
 
 ```bash
 $ build/src/test/test_bitcoin --run_test=getarg_tests/doubledash -- -testdatadir=/somewhere/mydatadir
-Test directory (will not be deleted): "/somewhere/mydatadir/test_common_Bitcoin Core/getarg_tests/doubledash/datadir"
+Test directory (will not be deleted): "/somewhere/mydatadir/test_common bitcoin/getarg_tests/doubledash/datadir"
 Running 1 test case...
 
 *** No errors detected
-$ ls -l '/somewhere/mydatadir/test_common_Bitcoin Core/getarg_tests/doubledash/datadir'
+$ ls -l '/somewhere/mydatadir/test_common bitcoin/getarg_tests/doubledash/datadir'
 total 8
 drwxrwxr-x 2 admin admin 4096 Nov 27 22:45 blocks
 -rw-rw-r-- 1 admin admin 1003 Nov 27 22:45 debug.log

src/test/util/random.cpp

@@ -36,7 +36,7 @@ void SeedRandomStateForTest(SeedRand seedtype)
         return GetRandHash();
     }();
 
-    const uint256& seed{seedtype == SeedRand::SEED ? ctx_seed : uint256::ZERO};
+    const uint256& seed{seedtype == SeedRand::FIXED_SEED ? ctx_seed : uint256::ZERO};
     LogInfo("Setting random seed for current tests to %s=%s\n", RANDOM_CTX_SEED, seed.GetHex());
     MakeRandDeterministicDANGEROUS(seed);
 }

src/test/util/random.h

@@ -12,8 +12,16 @@
 #include <cstdint>
 
 enum class SeedRand {
-    ZEROS, //!< Seed with a compile time constant of zeros
-    SEED,  //!< Use (and report) random seed from environment, or a (truly) random one.
+    /**
+     * Seed with a compile time constant of zeros.
+     */
+    ZEROS,
+    /**
+     * Seed with a fixed value that never changes over the lifetime of this
+     * process. The seed is read from the RANDOM_CTX_SEED environment variable
+     * if set, otherwise generated randomly once, saved, and reused.
+     */
+    FIXED_SEED,
 };
 
 /** Seed the global RNG state for testing and log the seed value. This affects all randomness, except GetStrongRandBytes(). */

src/test/util/setup_common.cpp

@@ -2,8 +2,6 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <config/bitcoin-config.h> // IWYU pragma: keep
-
 #include <test/util/setup_common.h>
 
 #include <addrman.h>
@@ -76,6 +74,7 @@ using node::VerifyLoadedChainstate;
 
 const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
 
+constexpr inline auto TEST_DIR_PATH_ELEMENT{"test_common bitcoin"}; // Includes a space to catch possible path escape issues.
 /** Random context to get unique temp data dirs. Separate from m_rng, which can be seeded from a const env var */
 static FastRandomContext g_rng_temp_path;
 
@@ -109,7 +108,7 @@ static NetworkSetup g_networksetup_instance;
 /** Register test-only arguments */
 static void SetupUnitTestArgs(ArgsManager& argsman)
 {
-    argsman.AddArg("-testdatadir", strprintf("Custom data directory (default: %s<random_string>)", fs::PathToString(fs::temp_directory_path() / "test_common_" PACKAGE_NAME / "")),
+    argsman.AddArg("-testdatadir", strprintf("Custom data directory (default: %s<random_string>)", fs::PathToString(fs::temp_directory_path() / TEST_DIR_PATH_ELEMENT / "")),
                    ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
 }
 
@@ -155,12 +154,12 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)
 
     // Use randomly chosen seed for deterministic PRNG, so that (by default) test
     // data directories use a random name that doesn't overlap with other tests.
-    SeedRandomForTest(SeedRand::SEED);
+    SeedRandomForTest(SeedRand::FIXED_SEED);
 
     if (!m_node.args->IsArgSet("-testdatadir")) {
         // By default, the data directory has a random name
         const auto rand_str{g_rng_temp_path.rand256().ToString()};
-        m_path_root = fs::temp_directory_path() / "test_common_" PACKAGE_NAME / rand_str;
+        m_path_root = fs::temp_directory_path() / TEST_DIR_PATH_ELEMENT / rand_str;
         TryCreateDirectories(m_path_root);
     } else {
         // Custom data directory
@@ -170,7 +169,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)
 
         root_dir = fs::absolute(root_dir);
         const std::string test_path{G_TEST_GET_FULL_NAME ? G_TEST_GET_FULL_NAME() : ""};
-        m_path_lock = root_dir / "test_common_" PACKAGE_NAME / fs::PathFromString(test_path);
+        m_path_lock = root_dir / TEST_DIR_PATH_ELEMENT / fs::PathFromString(test_path);
         m_path_root = m_path_lock / "datadir";
 
         // Try to obtain the lock; if unsuccessful don't disturb the existing test.

src/test/util/setup_common.h

@@ -68,7 +68,7 @@ struct BasicTestingSetup {
 
     FastRandomContext m_rng;
     /** Seed the global RNG state and m_rng for testing and log the seed value. This affects all randomness, except GetStrongRandBytes(). */
-    void SeedRandomForTest(SeedRand seed = SeedRand::SEED)
+    void SeedRandomForTest(SeedRand seed)
     {
         SeedRandomStateForTest(seed);
         m_rng.Reseed(GetRandHash());