test: test_bitcoin: allow -testdatadir=<datadir>

Specifying this argument overrides the path location for test_bitcoin;
it becomes <datadir>/test_common_Bitcoin Core/<testname>/datadir. Also,
this directory isn't removed after the test completes. This can make it
easier for developers to study the results of a test (see the state of
the data directory after the test runs), and also (for example) have an
editor open on debug.log to monitor it across multiple test runs instead
of having to re-open a different pathname each time.

Example usage (note the "--" is needed):

test_bitcoin --run_test=getarg_tests/boolarg -- \
-testdatadir=/somewhere/mydatadir

This will create (if necessary) and use the data directory:

/somewhere/mydatadir/test_common_Bitcoin Core/getarg_tests/boolarg/datadir

Co-authored-by: furszy <mfurszy@protonmail.com>

src/bench/bench.cpp

@@ -23,6 +23,8 @@ 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{};
+
 namespace {
 
 void GenerateTemplateResults(const std::vector<ankerl::nanobench::Result>& benchmarkResults, const fs::path& file, const char* tpl)

src/qt/main.cpp

@@ -19,6 +19,8 @@ extern const std::function<std::string(const char*)> G_TRANSLATION_FUN = [](cons
 };
 UrlDecodeFn* const URL_DECODE = urlDecode;
 
+const std::function<std::string()> G_TEST_GET_FULL_NAME{};
+
 MAIN_FUNCTION
 {
     return GuiMain(argc, argv);

src/qt/test/test_main.cpp

@@ -48,6 +48,8 @@ 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{};
+
 // This is all you need to run all the tests
 int main(int argc, char* argv[])
 {

src/test/README.md

@@ -41,17 +41,18 @@ test_bitcoin --log_level=all --run_test=getarg_tests
 ```
 
 `log_level` controls the verbosity of the test framework, which logs when a
-test case is entered, for example. `test_bitcoin` also accepts the command
-line arguments accepted by `bitcoind`. Use `--` to separate both types of
-arguments:
+test case is entered, for example.
+
+`test_bitcoin` also accepts some of the command line arguments accepted by
+`bitcoind`. Use `--` to separate these sets of arguments:
 
 ```bash
 test_bitcoin --log_level=all --run_test=getarg_tests -- -printtoconsole=1
 ```
 
-The `-printtoconsole=1` after the two dashes redirects the debug log, which
-would normally go to a file in the test datadir
-(`BasicTestingSetup::m_path_root`), to the standard terminal output.
+The `-printtoconsole=1` after the two dashes sends debug logging, which
+normally goes only to `debug.log` within the data directory, also to the
+standard terminal output.
 
 ... or to run just the doubledash test:
 
@@ -59,7 +60,42 @@ would normally go to a file in the test datadir
 test_bitcoin --run_test=getarg_tests/doubledash
 ```
 
-Run `test_bitcoin --help` for the full list.
+`test_bitcoin` creates a temporary working (data) directory with a randomly
+generated pathname within `test_common_Bitcoin Core/`, 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
+directory. Its content will vary depending on the test, but it will always
+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`.
+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
+what the test wrote to `debug.log` after it completes, for example.
+(The directory is removed at the start of the next test run,
+so no leftover state is used.)
+
+```bash
+$ 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
+Running 1 test case...
+
+*** No errors detected
+$ ls -l '/somewhere/mydatadir/test_common_Bitcoin Core/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
+```
+
+If you run an entire test suite, such as `--run_test=getarg_tests`, or all the test suites
+(by not specifying `--run_test`), a separate directory
+will be created for each individual test.
+
+Run `test_bitcoin --help` for the full list of tests.
 
 ### Adding test cases
 

src/test/fuzz/fuzz.cpp

@@ -35,6 +35,8 @@ __AFL_FUZZ_INIT();
 
 const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
 
+const std::function<std::string()> G_TEST_GET_FULL_NAME{};
+
 /**
  * A copy of the command line arguments that start with `--`.
  * First `LLVMFuzzerInitialize()` is called, which saves the arguments to `g_args`.

src/test/main.cpp

@@ -39,3 +39,10 @@ const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS =
     }
     return args;
 };
+
+/**
+ * Retrieve the boost unit test name.
+ */
+const std::function<std::string()> G_TEST_GET_FULL_NAME = []() {
+    return boost::unit_test::framework::current_test_case().full_name();
+};

src/test/util/setup_common.cpp

@@ -49,6 +49,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>
@@ -97,9 +98,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;
@@ -120,18 +134,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);
@@ -159,7 +204,13 @@ BasicTestingSetup::~BasicTestingSetup()
     m_node.kernel.reset();
     SetMockTime(0s); // Reset mocktime for following tests
     LogInstance().DisconnectTestLogger();
+    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();
 }
 

src/test/util/setup_common.h

@@ -32,6 +32,9 @@ extern const std::function<void(const std::string&)> G_TEST_LOG_FUN;
 /** Retrieve the command line arguments. */
 extern const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS;
 
+/** Retrieve the unit test name. */
+extern const std::function<std::string()> G_TEST_GET_FULL_NAME;
+
 // Enable BOOST_CHECK_EQUAL for enum class types
 namespace std {
 template <typename T>
@@ -53,7 +56,9 @@ struct BasicTestingSetup {
     explicit BasicTestingSetup(const ChainType chainType = ChainType::MAIN, const std::vector<const char*>& extra_args = {});
     ~BasicTestingSetup();
 
-    const fs::path m_path_root;
+    fs::path m_path_root;
+    fs::path m_path_lock;
+    bool m_has_custom_datadir{false};
     ArgsManager m_args;
 };