From ad9c2cceda9cd893c0f754e49f7fca6e417ee95f Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Sun, 29 Sep 2024 14:21:49 -0300
Subject: [PATCH] test, bench: specialize working directory name

Since G_TEST_GET_FULL_NAME is not initialized in the benchmark framework,
benchmarks using the unit test setup run in the same directory without
any clear distinction between them.
This poses an extra complication for locating any specific benchmark
directory during a failure.

In master, unit tests and benchmarks run in the following path:
/<OS_tmp_dir>/test_common bitcoin/<random_uint256>/

After this commit, unit tests and benchmarks are contained within its
own directory:
/<OS_tmp_dir>/test_common bitcoin/<test_name>/<time_in_nanoseconds>/

This makes it easier to find any benchmark run when a failure occurs.
---
 src/bench/bench.cpp            | 12 +++++++++++-
 src/test/util/setup_common.cpp | 11 ++++-------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp
index a2dbb118885..d6913a05f2a 100644
--- a/src/bench/bench.cpp
+++ b/src/bench/bench.cpp
@@ -29,7 +29,16 @@ const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
 
 const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS{};
 
-const std::function<std::string()> G_TEST_GET_FULL_NAME{};
+/**
+ * Retrieve the name of the currently in-use benchmark.
+ * This is applicable only to benchmarks that utilize the unit test
+ * framework context setup (e.g. ones using 'MakeNoLogFileContext<TestingSetup>()').
+ * It places the datadir of each benchmark run within their respective benchmark name.
+ */
+static std::string g_running_benchmark_name;
+const std::function<std::string()> G_TEST_GET_FULL_NAME = []() {
+    return g_running_benchmark_name;
+};
 
 namespace {
 
@@ -117,6 +126,7 @@ void BenchRunner::RunAll(const Args& args)
             bench.output(nullptr);
         }
         bench.name(name);
+        g_running_benchmark_name = name;
         if (args.min_time > 0ms) {
             // convert to nanos before dividing to reduce rounding errors
             std::chrono::nanoseconds min_time_ns = args.min_time;
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 2de282dbf6d..a22d34c075f 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -75,8 +75,6 @@ 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;
 
 struct NetworkSetup
 {
@@ -139,10 +137,10 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)
     // data directories use a random name that doesn't overlap with other tests.
     SeedRandomForTest(SeedRand::FIXED_SEED);
 
+    const std::string test_name{G_TEST_GET_FULL_NAME ? G_TEST_GET_FULL_NAME() : ""};
     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_DIR_PATH_ELEMENT / rand_str;
+        const auto now{TicksSinceEpoch<std::chrono::nanoseconds>(SystemClock::now())};
+        m_path_root = fs::temp_directory_path() / TEST_DIR_PATH_ELEMENT / test_name / util::ToString(now);
         TryCreateDirectories(m_path_root);
     } else {
         // Custom data directory
@@ -151,8 +149,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)
         if (root_dir.empty()) ExitFailure("-testdatadir argument is empty, please specify a path");
 
         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_DIR_PATH_ELEMENT / fs::PathFromString(test_path);
+        m_path_lock = root_dir / TEST_DIR_PATH_ELEMENT / fs::PathFromString(test_name);
         m_path_root = m_path_lock / "datadir";
 
         // Try to obtain the lock; if unsuccessful don't disturb the existing test.