Skip to content

Commit

Permalink
test, bench: specialize working directory name
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
furszy committed Nov 11, 2024
1 parent 2c90f8e commit ad9c2cc
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 8 deletions.
12 changes: 11 additions & 1 deletion src/bench/bench.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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;
Expand Down
11 changes: 4 additions & 7 deletions src/test/util/setup_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down

0 comments on commit ad9c2cc

Please sign in to comment.