From 6d6630d25102128e3ff157afd958a78ba1b3f7fe Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 25 Jun 2019 12:13:08 +0200 Subject: [PATCH] Merge #16252: test: Log to debug.log in all unit tests fabc57e07dc34c103552cb51c9277bb48ef97739 test: Log to debug.log in all tests (MarcoFalke) fa4a04a5a942d582c62773d815c7e1e9897975d0 test: use common setup in gui tests (MarcoFalke) fad3d2a624377de4b0311e6ddd446c36dafd1ddc test: Create data dir in BasicTestingSetup (MarcoFalke) Pull request description: This makes it easier to debug a frozen test or a test that failed. To debug a failed test, remove the line `fs::remove_all(m_path_root);`. The pull is done in three commits: * Create a datadir for every unit test once (and only once). This requires the `SetDataDir` function to go away. * Use the common setup in the gui unit tests. Some of those tests are testing the init sequence, so we'd have to undo some of what the testing setup did. * Log to the debug.log in all tests ACKs for top commit: laanwj: ACK fabc57e07dc34c103552cb51c9277bb48ef97739 Tree-SHA512: 73444210b88172669e2cd22c2703a1e30e105185d2d5f03decbdedcfd09c64ed208d3716c59c8bebb0e44214cee5c8095e3e995d049e1572ee98f1017e413665 --- src/logging.cpp | 8 ++++++++ src/logging.h | 2 ++ src/qt/test/apptests.cpp | 4 ++++ src/qt/test/rpcnestedtests.cpp | 1 + src/qt/test/test_main.cpp | 12 +++--------- src/test/blockfilter_index_tests.cpp | 2 -- src/test/dbwrapper_tests.cpp | 18 +++++++++--------- src/test/flatfile_tests.cpp | 8 ++++---- src/test/fs_tests.cpp | 2 +- src/test/net_tests.cpp | 2 -- src/test/setup_common.cpp | 20 +++++++++----------- src/test/setup_common.h | 7 ++----- src/test/util_tests.cpp | 4 ++-- src/wallet/test/db_tests.cpp | 12 ++++++------ src/wallet/test/init_test_fixture.cpp | 4 ++-- src/wallet/test/wallet_tests.cpp | 2 +- 16 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index 1a65a8534c..2ef62b32be 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -76,6 +76,14 @@ bool BCLog::Logger::StartLogging() return true; } +void BCLog::Logger::DisconnectTestLogger() +{ + StdLockGuard scoped_lock(m_cs); + m_buffering = true; + if (m_fileout != nullptr) fclose(m_fileout); + m_fileout = nullptr; +} + void BCLog::Logger::EnableCategory(BCLog::LogFlags flag) { m_categories |= flag; diff --git a/src/logging.h b/src/logging.h index 14e49993e2..03fd968199 100644 --- a/src/logging.h +++ b/src/logging.h @@ -125,6 +125,8 @@ namespace BCLog { /** Start logging (and flush all buffered messages) */ bool StartLogging(); + /** Only for testing */ + void DisconnectTestLogger(); void ShrinkDebugFile(); diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index 64a965700a..3126c0958e 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -67,6 +68,9 @@ void AppTests::appTests() } #endif + ECC_Stop(); // Already started by the common test setup, so stop it to avoid interference + LogInstance().DisconnectTestLogger(); + m_app.parameterSetup(); GUIUtil::loadFonts(); m_app.createOptionsModel(true /* reset settings */); diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 1210a4379e..0b49755501 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -41,6 +41,7 @@ void RPCNestedTests::rpcNestedTests() tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]); //mempool.setSanityCheck(1.0); + LogInstance().DisconnectTestLogger(); // Already started by the common test setup, so stop it to avoid interference TestingSetup test; if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index e4dbc09e56..1d5fb6991e 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -12,10 +12,10 @@ #include #include #include -#include #include #include #include +#include #ifdef ENABLE_WALLET #include @@ -50,14 +50,8 @@ extern void noui_connect(); // This is all you need to run all the tests int main(int argc, char *argv[]) { - SetupEnvironment(); - SetupNetworking(); - SelectParams(CBaseChainParams::REGTEST); - noui_connect(); - ClearDatadirCache(); - fs::path pathTemp = fs::temp_directory_path() / strprintf("test_dash-qt_%lu_%i", (unsigned long)GetTime(), (int)GetRand(1 << 30)); - fs::create_directories(pathTemp); - gArgs.ForceSetArg("-datadir", pathTemp.string()); + BasicTestingSetup test{CBaseChainParams::REGTEST}; + auto node = interfaces::MakeNode(); bool fInvalid = false; diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index b41aed2d2a..2e413e1cf6 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -267,8 +267,6 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(blockfilter_index_init_destroy, BasicTestingSetup) { - SetDataDir("tempdir"); - BlockFilterIndex* filter_index; filter_index = GetBlockFilterIndex(BlockFilterType::BASIC_FILTER); diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 27ea7f91ca..ab86355324 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -27,7 +27,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper) { // Perform tests both obfuscated and non-obfuscated. for (const bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_obfuscate_true" : "dbwrapper_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); char key = 'k'; uint256 in = InsecureRand256(); @@ -46,7 +46,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_basic_data) { // Perform tests both obfuscated and non-obfuscated. for (bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper_1").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_1_obfuscate_true" : "dbwrapper_1_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), false, true, obfuscate); uint256 res; @@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch) { // Perform tests both obfuscated and non-obfuscated. for (const bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper_batch").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_batch_obfuscate_true" : "dbwrapper_batch_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); char key = 'i'; @@ -163,7 +163,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) { // Perform tests both obfuscated and non-obfuscated. for (const bool obfuscate : {false, true}) { - fs::path ph = SetDataDir(std::string("dbwrapper_iterator").append(obfuscate ? "_true" : "_false")); + fs::path ph = GetDataDir() / (obfuscate ? "dbwrapper_iterator_obfuscate_true" : "dbwrapper_iterator_obfuscate_false"); CDBWrapper dbw(ph, (1 << 20), true, false, obfuscate); // The two keys are intentionally chosen for ordering @@ -203,7 +203,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate) { // We're going to share this fs::path between two wrappers - fs::path ph = SetDataDir("existing_data_no_obfuscate"); + fs::path ph = GetDataDir() / "existing_data_no_obfuscate"; create_directories(ph); // Set up a non-obfuscated wrapper to write some initial data. @@ -244,7 +244,7 @@ BOOST_AUTO_TEST_CASE(existing_data_no_obfuscate) BOOST_AUTO_TEST_CASE(existing_data_reindex) { // We're going to share this fs::path between two wrappers - fs::path ph = SetDataDir("existing_data_reindex"); + fs::path ph = GetDataDir() / "existing_data_reindex"; create_directories(ph); // Set up a non-obfuscated wrapper to write some initial data. @@ -279,7 +279,7 @@ BOOST_AUTO_TEST_CASE(existing_data_reindex) BOOST_AUTO_TEST_CASE(iterator_ordering) { - fs::path ph = SetDataDir("iterator_ordering"); + fs::path ph = GetDataDir() / "iterator_ordering"; CDBWrapper dbw(ph, (1 << 20), true, false, false); for (int x=0x00; x<256; ++x) { uint8_t key = x; @@ -359,7 +359,7 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering) { char buf[10]; - fs::path ph = SetDataDir("iterator_string_ordering"); + fs::path ph = GetDataDir() / "iterator_string_ordering"; CDBWrapper dbw(ph, (1 << 20), true, false, false); for (int x=0x00; x<10; ++x) { for (int y = 0; y < 10; y++) { @@ -405,7 +405,7 @@ BOOST_AUTO_TEST_CASE(unicodepath) // On Windows this test will fail if the directory is created using // the ANSI CreateDirectoryA call and the code page isn't UTF8. // It will succeed if the created with CreateDirectoryW. - fs::path ph = SetDataDir("test_runner_₿_🏃_20191128_104644"); + fs::path ph = GetDataDir() / "test_runner_₿_🏃_20191128_104644"; CDBWrapper dbw(ph, (1 << 20)); fs::path lockPath = ph / "LOCK"; diff --git a/src/test/flatfile_tests.cpp b/src/test/flatfile_tests.cpp index 1db2f8054c..ef3946a115 100644 --- a/src/test/flatfile_tests.cpp +++ b/src/test/flatfile_tests.cpp @@ -11,7 +11,7 @@ BOOST_FIXTURE_TEST_SUITE(flatfile_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(flatfile_filename) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFilePos pos(456, 789); @@ -24,7 +24,7 @@ BOOST_AUTO_TEST_CASE(flatfile_filename) BOOST_AUTO_TEST_CASE(flatfile_open) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFileSeq seq(data_dir, "a", 16 * 1024); std::string line1("A purely peer-to-peer version of electronic cash would allow online " @@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE(flatfile_open) BOOST_AUTO_TEST_CASE(flatfile_allocate) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFileSeq seq(data_dir, "a", 100); bool out_of_space; @@ -105,7 +105,7 @@ BOOST_AUTO_TEST_CASE(flatfile_allocate) BOOST_AUTO_TEST_CASE(flatfile_flush) { - auto data_dir = SetDataDir("flatfile_test"); + const auto data_dir = GetDataDir(); FlatFileSeq seq(data_dir, "a", 100); bool out_of_space; diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index bdb6ef33ab..9261b14f22 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -12,7 +12,7 @@ BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(fsbridge_fstream) { - fs::path tmpfolder = SetDataDir("fsbridge_fstream"); + fs::path tmpfolder = GetDataDir(); // tmpfile1 should be the same as tmpfile2 fs::path tmpfile1 = tmpfolder / "fs_tests_₿_🏃"; fs::path tmpfile2 = tmpfolder / "fs_tests_₿_🏃"; diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 26ee0faf19..6dc14efea5 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -93,7 +93,6 @@ BOOST_AUTO_TEST_CASE(cnode_listen_port) BOOST_AUTO_TEST_CASE(caddrdb_read) { - SetDataDir("caddrdb_read"); CAddrManUncorrupted addrmanUncorrupted; addrmanUncorrupted.MakeDeterministic(); @@ -138,7 +137,6 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) { - SetDataDir("caddrdb_read_corrupted"); CAddrManCorrupted addrmanCorrupted; addrmanCorrupted.MakeDeterministic(); diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp index b02af8374c..2b36c55d43 100644 --- a/src/test/setup_common.cpp +++ b/src/test/setup_common.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -43,6 +44,13 @@ std::ostream& operator<<(std::ostream& os, const uint256& num) BasicTestingSetup::BasicTestingSetup(const std::string& chainName) : m_path_root(fs::temp_directory_path() / "test_common_" PACKAGE_NAME / strprintf("%lu_%i", (unsigned long)GetTime(), (int)(InsecureRandRange(1 << 30)))) { + fs::create_directories(m_path_root); + gArgs.ForceSetArg("-datadir", m_path_root.string()); + ClearDatadirCache(); + SelectParams(chainName); + gArgs.ForceSetArg("-printtoconsole", "0"); + InitLogging(); + LogInstance().StartLogging(); SHA256AutoDetect(); ECC_Start(); BLSInit(); @@ -52,7 +60,6 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName) InitScriptExecutionCache(); CCoinJoin::InitStandardDenominations(); fCheckBlockIndex = true; - SelectParams(chainName); evoDb.reset(new CEvoDB(1 << 20, true, true)); deterministicMNManager.reset(new CDeterministicMNManager(*evoDb)); static bool noui_connected = false; @@ -67,26 +74,17 @@ BasicTestingSetup::~BasicTestingSetup() deterministicMNManager.reset(); evoDb.reset(); + LogInstance().DisconnectTestLogger(); fs::remove_all(m_path_root); ECC_Stop(); } -fs::path BasicTestingSetup::SetDataDir(const std::string& name) -{ - fs::path ret = m_path_root / name; - fs::create_directories(ret); - gArgs.ForceSetArg("-datadir", ret.string()); - return ret; -} - TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) { - SetDataDir("tempdir"); const CChainParams& chainparams = Params(); // Ideally we'd move all the RPC tests to the functional testing framework // instead of unit tests, but for now we need these here. RegisterAllCoreRPCCommands(tableRPC); - ClearDatadirCache(); // We have to run a scheduler thread to prevent ActivateBestChain // from blocking due to queue overrun. diff --git a/src/test/setup_common.h b/src/test/setup_common.h index c3bbf35127..e3ea19e412 100644 --- a/src/test/setup_common.h +++ b/src/test/setup_common.h @@ -54,22 +54,19 @@ static inline bool InsecureRandBool() { return g_insecure_rand_ctx.randbool(); } static constexpr CAmount CENT{1000000}; /** Basic testing setup. - * This just configures logging and chain parameters. + * This just configures logging, data dir and chain parameters. */ struct BasicTestingSetup { ECCVerifyHandle globalVerifyHandle; explicit BasicTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); ~BasicTestingSetup(); - - fs::path SetDataDir(const std::string& name); - private: const fs::path m_path_root; }; /** Testing setup that configures a complete environment. - * Included are data directory, coins database, script check threads setup. + * Included are coins database, script check threads setup. */ struct TestingSetup : public BasicTestingSetup { boost::thread_group threadGroup; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 8a0263b4be..8b2828203f 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1136,7 +1136,7 @@ static constexpr char ExitCommand = 'X'; BOOST_AUTO_TEST_CASE(test_LockDirectory) { - fs::path dirname = SetDataDir("test_LockDirectory") / fs::unique_path(); + fs::path dirname = GetDataDir() / "lock_dir"; const std::string lockname = ".lock"; #ifndef WIN32 // Revert SIGCHLD to default, otherwise boost.test will catch and fail on @@ -1225,7 +1225,7 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory) BOOST_AUTO_TEST_CASE(test_DirIsWritable) { // Should be able to write to the data dir. - fs::path tmpdirname = SetDataDir("test_DirIsWritable"); + fs::path tmpdirname = GetDataDir(); BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), true); // Should not be able to write to a non-existent dir. diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index bed14ea7b1..fc27433dc1 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -16,7 +16,7 @@ BOOST_FIXTURE_TEST_SUITE(db_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(getwalletenv_file) { std::string test_name = "test_name.dat"; - fs::path datadir = SetDataDir("tempdir"); + const fs::path datadir = GetDataDir(); fs::path file_path = datadir / test_name; std::ofstream f(file_path.BOOST_FILESYSTEM_C_STR); f.close(); @@ -30,7 +30,7 @@ BOOST_AUTO_TEST_CASE(getwalletenv_file) BOOST_AUTO_TEST_CASE(getwalletenv_directory) { std::string expected_name = "wallet.dat"; - fs::path datadir = SetDataDir("tempdir"); + const fs::path datadir = GetDataDir(); std::string filename; std::shared_ptr env = GetWalletEnv(datadir, filename); @@ -40,8 +40,8 @@ BOOST_AUTO_TEST_CASE(getwalletenv_directory) BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_multiple) { - fs::path datadir = SetDataDir("tempdir"); - fs::path datadir_2 = SetDataDir("tempdir_2"); + fs::path datadir = GetDataDir() / "1"; + fs::path datadir_2 = GetDataDir() / "2"; std::string filename; std::shared_ptr env_1 = GetWalletEnv(datadir, filename); @@ -54,8 +54,8 @@ BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_multiple) BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_free_instance) { - fs::path datadir = SetDataDir("tempdir"); - fs::path datadir_2 = SetDataDir("tempdir_2"); + fs::path datadir = GetDataDir() / "1"; + fs::path datadir_2 = GetDataDir() / "2"; std::string filename; std::shared_ptr env_1_a = GetWalletEnv(datadir, filename); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index 3b828d57f9..fcd1f3fea8 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -13,7 +13,7 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam std::string sep; sep += fs::path::preferred_separator; - m_datadir = SetDataDir("tempdir"); + m_datadir = GetDataDir(); m_cwd = fs::current_path(); m_walletdir_path_cases["default"] = m_datadir / "wallets"; @@ -41,4 +41,4 @@ InitWalletDirTestingSetup::~InitWalletDirTestingSetup() void InitWalletDirTestingSetup::SetWalletDir(const fs::path& walletdir_path) { gArgs.ForceSetArg("-walletdir", walletdir_path.string()); -} \ No newline at end of file +} diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 4c39c5f84b..53b36aa9ff 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -195,7 +195,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) auto locked_chain = chain->lock(); LockAnnotation lock(::cs_main); - std::string backup_file = (SetDataDir("importwallet_rescan") / "wallet.backup").string(); + std::string backup_file = (GetDataDir() / "wallet.backup").string(); // Import key into wallet and call dumpwallet to create backup file. {