Merge bitcoin/bitcoin#26564: test: test_bitcoin: allow -testdatadir=<datadir>

d27e2d87b95b7982c05b4c88e463cc9626ab9f0a test: test_bitcoin: allow -testdatadir=<datadir> (Larry Ruane)

Pull request description:

  This backward-compatible change would help with code review, testing, and debugging. When `test_bitcoin` runs, it creates a working or data directory within `/tmp/test_common_Bitcoin\ Core/`, named as a long random (hex) string.

  This small patch does three things:

  - If the (new) argument `-testdatadir=<datadir>` is given, use `<datadir>/test_temp/<test-name>/datadir` as the working directory
  - When the test starts, remove `<datadir>/test_temp/<test-name>/datadir` if it exists from an earlier run (currently, it's presumed not to exist due to the long random string)
  - Don't delete the working directory at the end of the test if a custom data directory is being used

  Example usage, which will remove, create, use `/somewhere/test_temp/getarg_tests/boolarg`, and leave it afterward:
  ```
  $ test_bitcoin --run_test=getarg_tests/boolarg -- -testdatadir=/somewhere
  Running 1 test case...
  Test directory (will not be deleted): "/somewhere/test_temp/getarg_tests/boolarg/datadir"

  *** No errors detected
  $ ls -l /somewhere/test_temp/getarg_tests/boolarg/datadir
  total 8
  drwxrwxr-x 2 larry larry 4096 Feb 22 10:28 blocks
  -rw-rw-r-- 1 larry larry 1273 Feb 22 10:28 debug.log
  ```
  (A relative pathname also works.)

  This change affects only `test_bitcoin`; it could also be applied to `test_bitcoin-qt` but that's slightly more involved so I'm skipping that for now.

  The rationale for this change is that, when running the test using the debugger, it's often useful to watch `debug.log` as the test runs and inspect some of the other files (I've looked at the generated `blknnnn.dat` files for example). Currently, that requires figuring out where the test's working directory is since it changes on every test run. Tests can be run with `-printtoconsole=1` to show debug logging to the terminal, but it's nice to keep `debug.log` continuously open in an editor, for example.

  Even if not using a debugger, it's sometimes helpful to see `debug.log` and other artifacts after the test completes.

  Similar functionality is already possible with the functional tests using the `--tmpdir=` and `--nocleanup` arguments.

ACKs for top commit:
  davidgumberg:
    ACK https://github.com/bitcoin/bitcoin/pull/26564/commits/d27e2d87b95b7982c05b4c88e463cc9626ab9f0a
  tdb3:
    re-ACK for d27e2d87b95b7982c05b4c88e463cc9626ab9f0a
  achow101:
    ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a
  cbergqvist:
    ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a! (Already did some testing with `fs::remove()` to make sure it was compatible with the `util::Lock/UnlockDirectory` implementation).
  marcofleon:
    ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a. I ran all the tests with my previous open file limit and no errors were detected. Also ran some individual tests with no, relative, and absolute paths and everything looks good.
  furszy:
    ACK d27e2d8

Tree-SHA512: a8f535f34a48b6699cb440f97f5562ec643f3bfba4ea685768980b871fc8b6e1135f70fc05dbe19aa2c8bacb1ddeaff212d63473605a7422ff76332b3a6b1f68

1 files changed, 57 insertions, 6 deletions

diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index d26a440074..2c18184261 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -52,6 +52,7 @@
 #include <txmempool.h>
 #include <util/chaintype.h>
 #include <util/check.h>
+#include <util/fs_helpers.h>
 #include <util/rbf.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -100,9 +101,22 @@ struct NetworkSetup
 };
 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 / "")),
+                   ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
+}
+
+/** Test setup failure */
+static void ExitFailure(std::string_view str_err)
+{
+    std::cerr << str_err << std::endl;
+    exit(EXIT_FAILURE);
+}
+
 BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vector<const char*>& extra_args)
-    : m_path_root{fs::temp_directory_path() / "test_common_" PACKAGE_NAME / g_insecure_rand_ctx_temp_path.rand256().ToString()},
-      m_args{}
+    : m_args{}
 {
     m_node.shutdown = &m_interrupt;
     m_node.args = &gArgs;
@@ -123,18 +137,49 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
         arguments = Cat(arguments, G_TEST_COMMAND_LINE_ARGUMENTS());
     }
     util::ThreadRename("test");
-    fs::create_directories(m_path_root);
-    m_args.ForceSetArg("-datadir", fs::PathToString(m_path_root));
-    gArgs.ForceSetArg("-datadir", fs::PathToString(m_path_root));
     gArgs.ClearPathCache();
     {
         SetupServerArgs(*m_node.args);
+        SetupUnitTestArgs(*m_node.args);
         std::string error;
         if (!m_node.args->ParseParameters(arguments.size(), arguments.data(), error)) {
             m_node.args->ClearArgs();
             throw std::runtime_error{error};
         }
     }
+
+    if (!m_node.args->IsArgSet("-testdatadir")) {
+        // By default, the data directory has a random name
+        const auto rand_str{g_insecure_rand_ctx_temp_path.rand256().ToString()};
+        m_path_root = fs::temp_directory_path() / "test_common_" PACKAGE_NAME / rand_str;
+        TryCreateDirectories(m_path_root);
+    } else {
+        // Custom data directory
+        m_has_custom_datadir = true;
+        fs::path root_dir{m_node.args->GetPathArg("-testdatadir")};
+        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_common_" PACKAGE_NAME / fs::PathFromString(test_path);
+        m_path_root = m_path_lock / "datadir";
+
+        // Try to obtain the lock; if unsuccessful don't disturb the existing test.
+        TryCreateDirectories(m_path_lock);
+        if (util::LockDirectory(m_path_lock, ".lock", /*probe_only=*/false) != util::LockResult::Success) {
+            ExitFailure("Cannot obtain a lock on test data lock directory " + fs::PathToString(m_path_lock) + '\n' + "The test executable is probably already running.");
+        }
+
+        // Always start with a fresh data directory; this doesn't delete the .lock file located one level above.
+        fs::remove_all(m_path_root);
+        if (!TryCreateDirectories(m_path_root)) ExitFailure("Cannot create test data directory");
+
+        // Print the test directory name if custom.
+        std::cout << "Test directory (will not be deleted): " << m_path_root << std::endl;
+    }
+    m_args.ForceSetArg("-datadir", fs::PathToString(m_path_root));
+    gArgs.ForceSetArg("-datadir", fs::PathToString(m_path_root));
+
     SelectParams(chainType);
     SeedInsecureRand();
     if (G_TEST_LOG_FUN) LogInstance().PushBackCallback(G_TEST_LOG_FUN);
@@ -162,7 +207,13 @@ BasicTestingSetup::~BasicTestingSetup()
     m_node.kernel.reset();
     SetMockTime(0s); // Reset mocktime for following tests
     LogInstance().DisconnectTestLogger();
-    fs::remove_all(m_path_root);
+    if (m_has_custom_datadir) {
+        // Only remove the lock file, preserve the data directory.
+        UnlockDirectory(m_path_lock, ".lock");
+        fs::remove(m_path_lock / ".lock");
+    } else {
+        fs::remove_all(m_path_root);
+    }
     gArgs.ClearArgs();
 }