From 0839a202c91af50a444c1ef348adc90f8afa1dd2 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Fri, 13 May 2022 17:06:14 -0700 Subject: [PATCH] Reduce console noise coming from unit tests: A few unit tests have historically generated a lot of noise to the console from log writes. This noise was not useful and made it harder to locate actual test failures. By changing the log level of these tests from - severities::kError to - severities::kDisabled it was possible to remove that noise coming from the logs. --- src/test/app/LedgerLoad_test.cpp | 39 +++++++++++++------ src/test/app/LedgerReplay_test.cpp | 7 +++- src/test/app/NFTokenDir_test.cpp | 7 +++- src/test/app/ValidatorKeys_test.cpp | 12 +++++- src/test/app/ValidatorList_test.cpp | 6 ++- src/test/app/ValidatorSite_test.cpp | 8 +--- src/test/basics/PerfLog_test.cpp | 9 ++++- .../beast/beast_io_latency_probe_test.cpp | 3 -- src/test/core/ClosureCounter_test.cpp | 14 +++++-- src/test/core/Config_test.cpp | 5 +-- src/test/jtx/Env_test.cpp | 12 ++++-- src/test/ledger/View_test.cpp | 32 +++++++++++++-- src/test/overlay/short_read_test.cpp | 2 - src/test/rpc/LedgerRPC_test.cpp | 3 -- src/test/rpc/NodeToShardRPC_test.cpp | 3 +- src/test/rpc/ShardArchiveHandler_test.cpp | 9 +++-- src/test/server/Server_test.cpp | 1 - src/test/shamap/SHAMapSync_test.cpp | 1 - src/test/unit_test/FileDirGuard.h | 3 -- 19 files changed, 116 insertions(+), 60 deletions(-) diff --git a/src/test/app/LedgerLoad_test.cpp b/src/test/app/LedgerLoad_test.cpp index 7175df345..d78d25ea0 100644 --- a/src/test/app/LedgerLoad_test.cpp +++ b/src/test/app/LedgerLoad_test.cpp @@ -21,11 +21,12 @@ #include #include #include +#include +#include + #include #include #include -#include -#include namespace ripple { @@ -111,7 +112,9 @@ class LedgerLoad_test : public beast::unit_test::suite Env env( *this, envconfig( - ledgerConfig, sd.dbPath, sd.ledgerFile, Config::LOAD_FILE)); + ledgerConfig, sd.dbPath, sd.ledgerFile, Config::LOAD_FILE), + nullptr, + beast::severities::kDisabled); auto jrb = env.rpc("ledger", "current", "full")[jss::result]; BEAST_EXPECT( sd.ledger[jss::ledger][jss::accountState].size() == @@ -129,7 +132,9 @@ class LedgerLoad_test : public beast::unit_test::suite except([&] { Env env( *this, - envconfig(ledgerConfig, sd.dbPath, "", Config::LOAD_FILE)); + envconfig(ledgerConfig, sd.dbPath, "", Config::LOAD_FILE), + nullptr, + beast::severities::kDisabled); }); // file does not exist @@ -137,10 +142,9 @@ class LedgerLoad_test : public beast::unit_test::suite Env env( *this, envconfig( - ledgerConfig, - sd.dbPath, - "badfile.json", - Config::LOAD_FILE)); + ledgerConfig, sd.dbPath, "badfile.json", Config::LOAD_FILE), + nullptr, + beast::severities::kDisabled); }); // make a corrupted version of the ledger file (last 10 bytes removed). @@ -168,7 +172,9 @@ class LedgerLoad_test : public beast::unit_test::suite ledgerConfig, sd.dbPath, ledgerFileCorrupt.string(), - Config::LOAD_FILE)); + Config::LOAD_FILE), + nullptr, + beast::severities::kDisabled); }); } @@ -183,7 +189,9 @@ class LedgerLoad_test : public beast::unit_test::suite boost::erase_all(ledgerHash, "\""); Env env( *this, - envconfig(ledgerConfig, sd.dbPath, ledgerHash, Config::LOAD)); + envconfig(ledgerConfig, sd.dbPath, ledgerHash, Config::LOAD), + nullptr, + beast::severities::kDisabled); auto jrb = env.rpc("ledger", "current", "full")[jss::result]; BEAST_EXPECT(jrb[jss::ledger][jss::accountState].size() == 97); BEAST_EXPECT( @@ -199,7 +207,10 @@ class LedgerLoad_test : public beast::unit_test::suite // create a new env with the ledger "latest" specified for startup Env env( - *this, envconfig(ledgerConfig, sd.dbPath, "latest", Config::LOAD)); + *this, + envconfig(ledgerConfig, sd.dbPath, "latest", Config::LOAD), + nullptr, + beast::severities::kDisabled); auto jrb = env.rpc("ledger", "current", "full")[jss::result]; BEAST_EXPECT( sd.ledger[jss::ledger][jss::accountState].size() == @@ -213,7 +224,11 @@ class LedgerLoad_test : public beast::unit_test::suite using namespace test::jtx; // create a new env with specific ledger index at startup - Env env(*this, envconfig(ledgerConfig, sd.dbPath, "43", Config::LOAD)); + Env env( + *this, + envconfig(ledgerConfig, sd.dbPath, "43", Config::LOAD), + nullptr, + beast::severities::kDisabled); auto jrb = env.rpc("ledger", "current", "full")[jss::result]; BEAST_EXPECT( sd.ledger[jss::ledger][jss::accountState].size() == diff --git a/src/test/app/LedgerReplay_test.cpp b/src/test/app/LedgerReplay_test.cpp index 549495d40..cff94ee04 100644 --- a/src/test/app/LedgerReplay_test.cpp +++ b/src/test/app/LedgerReplay_test.cpp @@ -465,7 +465,7 @@ struct LedgerServer assert(param.initLedgers > 0); createAccounts(param.initAccounts); createLedgerHistory(); - app.logs().threshold(beast::severities::Severity::kWarning); + app.logs().threshold(beast::severities::kWarning); } /** @@ -567,7 +567,10 @@ public: PeerSetBehavior behavior = PeerSetBehavior::Good, InboundLedgersBehavior inboundBhvr = InboundLedgersBehavior::Good, PeerFeature peerFeature = PeerFeature::LedgerReplayEnabled) - : env(suite, jtx::envconfig(jtx::port_increment, 3)) + : env(suite, + jtx::envconfig(jtx::port_increment, 3), + nullptr, + beast::severities::kDisabled) , app(env.app()) , ledgerMaster(env.app().getLedgerMaster()) , inboundLedgers( diff --git a/src/test/app/NFTokenDir_test.cpp b/src/test/app/NFTokenDir_test.cpp index ae7eeeaf6..8f8d0f581 100644 --- a/src/test/app/NFTokenDir_test.cpp +++ b/src/test/app/NFTokenDir_test.cpp @@ -388,7 +388,12 @@ class NFTokenDir_test : public beast::unit_test::suite auto exerciseFixNFTokenDirV1 = [this, &features](std::initializer_list seeds) { - Env env{*this, features}; + Env env{ + *this, + envconfig(), + features, + nullptr, + beast::severities::kDisabled}; // Eventually all of the NFTokens will be owned by buyer. Account const buyer{"buyer"}; diff --git a/src/test/app/ValidatorKeys_test.cpp b/src/test/app/ValidatorKeys_test.cpp index 18061c7e4..3943fd858 100644 --- a/src/test/app/ValidatorKeys_test.cpp +++ b/src/test/app/ValidatorKeys_test.cpp @@ -23,8 +23,9 @@ #include #include #include +#include + #include -#include namespace ripple { namespace test { @@ -75,7 +76,14 @@ public: void run() override { - SuiteJournal journal("ValidatorKeys_test", *this); + // We're only using Env for its Journal. That Journal gives better + // coverage in unit tests. + test::jtx::Env env{ + *this, + test::jtx::envconfig(), + nullptr, + beast::severities::kDisabled}; + beast::Journal journal{env.app().journal("ValidatorKeys_test")}; // Keys/ID when using [validation_seed] SecretKey const seedSecretKey = diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index 860b8fc17..fead5563f 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -29,9 +29,10 @@ #include #include #include -#include #include +#include + namespace ripple { namespace test { @@ -217,7 +218,8 @@ private: { testcase("Config Load"); - jtx::Env env(*this); + jtx::Env env( + *this, jtx::envconfig(), nullptr, beast::severities::kDisabled); auto& app = env.app(); PublicKey emptyLocalKey; std::vector const emptyCfgKeys; diff --git a/src/test/app/ValidatorSite_test.cpp b/src/test/app/ValidatorSite_test.cpp index db9f5c977..8ec86fead 100644 --- a/src/test/app/ValidatorSite_test.cpp +++ b/src/test/app/ValidatorSite_test.cpp @@ -69,7 +69,7 @@ private: using namespace jtx; - Env env(*this); + Env env(*this, envconfig(), nullptr, beast::severities::kDisabled); auto trustedSites = std::make_unique(env.app(), env.journal); @@ -282,9 +282,6 @@ private: if (u.cfg.failFetch) { using namespace std::chrono; - log << " -- Msg: " - << myStatus[jss::last_refresh_message].asString() - << std::endl; std::stringstream nextRefreshStr{ myStatus[jss::next_refresh_time].asString()}; system_clock::time_point nextRefresh; @@ -357,9 +354,6 @@ private: sink.messages().str().find(u.expectMsg) != std::string::npos, sink.messages().str()); - log << " -- Msg: " - << myStatus[jss::last_refresh_message].asString() - << std::endl; } } } diff --git a/src/test/basics/PerfLog_test.cpp b/src/test/basics/PerfLog_test.cpp index a79dded90..79944e0ed 100644 --- a/src/test/basics/PerfLog_test.cpp +++ b/src/test/basics/PerfLog_test.cpp @@ -24,12 +24,13 @@ #include #include #include +#include + #include #include #include #include #include -#include #include //------------------------------------------------------------------------------ @@ -44,7 +45,11 @@ class PerfLog_test : public beast::unit_test::suite // We're only using Env for its Journal. That Journal gives better // coverage in unit tests. - test::jtx::Env env_{*this}; + test::jtx::Env env_{ + *this, + test::jtx::envconfig(), + nullptr, + beast::severities::kDisabled}; beast::Journal j_{env_.app().journal("PerfLog_test")}; struct Fixture diff --git a/src/test/beast/beast_io_latency_probe_test.cpp b/src/test/beast/beast_io_latency_probe_test.cpp index 9ae6a1341..b2bf67b10 100644 --- a/src/test/beast/beast_io_latency_probe_test.cpp +++ b/src/test/beast/beast_io_latency_probe_test.cpp @@ -200,9 +200,6 @@ class io_latency_probe_test : public beast::unit_test::suite, duration_cast(probe_duration).count()) / static_cast(tt.getMean()); #endif - log << "expected_probe_count_min: " << expected_probe_count_min << "\n"; - log << "expected_probe_count_max: " << expected_probe_count_max << "\n"; - test_sampler io_probe{interval, get_io_service()}; io_probe.start(); MyTimer timer{get_io_service(), probe_duration}; diff --git a/src/test/core/ClosureCounter_test.cpp b/src/test/core/ClosureCounter_test.cpp index 478816a89..c4199a0b0 100644 --- a/src/test/core/ClosureCounter_test.cpp +++ b/src/test/core/ClosureCounter_test.cpp @@ -19,9 +19,10 @@ #include #include +#include + #include #include -#include #include namespace ripple { @@ -31,9 +32,14 @@ namespace test { class ClosureCounter_test : public beast::unit_test::suite { - // We're only using Env for its Journal. - jtx::Env env{*this}; - beast::Journal j{env.app().journal("ClosureCounter_test")}; + // We're only using Env for its Journal. That Journal gives better + // coverage in unit tests. + test::jtx::Env env_{ + *this, + jtx::envconfig(), + nullptr, + beast::severities::kDisabled}; + beast::Journal j{env_.app().journal("ClosureCounter_test")}; void testConstruction() diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index 45afaf6cb..da29fafac 100644 --- a/src/test/core/Config_test.cpp +++ b/src/test/core/Config_test.cpp @@ -154,7 +154,7 @@ public: rmDataDir_ = !exists(dataDir_); config_.setup( file_.string(), - /*bQuiet*/ true, + /* bQuiet */ true, /* bSilent */ false, /* bStandalone */ false); } @@ -190,9 +190,6 @@ public: using namespace boost::filesystem; if (rmDataDir_) rmDir(dataDir_); - else - test_.log << "Skipping rm dir: " << dataDir_.string() - << std::endl; } catch (std::exception& e) { diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index b1a1a8125..6f09f49ed 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -888,10 +888,14 @@ public: testExceptionalShutdown() { except([this] { - jtx::Env env{*this, jtx::envconfig([](std::unique_ptr cfg) { - (*cfg).deprecatedClearSection("port_rpc"); - return cfg; - })}; + jtx::Env env{ + *this, + jtx::envconfig([](std::unique_ptr cfg) { + (*cfg).deprecatedClearSection("port_rpc"); + return cfg; + }), + nullptr, + beast::severities::kDisabled}; }); pass(); } diff --git a/src/test/ledger/View_test.cpp b/src/test/ledger/View_test.cpp index 45c8f007e..bbb1eec8f 100644 --- a/src/test/ledger/View_test.cpp +++ b/src/test/ledger/View_test.cpp @@ -130,6 +130,8 @@ class View_test : public beast::unit_test::suite void testLedger() { + testcase("Ledger"); + using namespace jtx; Env env(*this); Config config; @@ -165,6 +167,8 @@ class View_test : public beast::unit_test::suite void testMeta() { + testcase("Meta"); + using namespace jtx; Env env(*this); wipe(env.app().openLedger()); @@ -196,6 +200,8 @@ class View_test : public beast::unit_test::suite void testMetaSucc() { + testcase("Meta succ"); + using namespace jtx; Env env(*this); wipe(env.app().openLedger()); @@ -260,6 +266,8 @@ class View_test : public beast::unit_test::suite void testStacked() { + testcase("Stacked"); + using namespace jtx; Env env(*this); wipe(env.app().openLedger()); @@ -325,6 +333,8 @@ class View_test : public beast::unit_test::suite void testContext() { + testcase("Context"); + using namespace jtx; using namespace std::chrono; { @@ -387,6 +397,8 @@ class View_test : public beast::unit_test::suite void testUpperAndLowerBound() { + testcase("Upper and lower bound"); + using namespace jtx; Env env(*this); Config config; @@ -654,6 +666,8 @@ class View_test : public beast::unit_test::suite void testSles() { + testcase("Sles"); + using namespace jtx; Env env(*this); Config config; @@ -786,6 +800,8 @@ class View_test : public beast::unit_test::suite void testFlags() { + testcase("Flags"); + using namespace jtx; Env env(*this); @@ -949,6 +965,8 @@ class View_test : public beast::unit_test::suite void testTransferRate() { + testcase("Transfer rate"); + using namespace jtx; Env env(*this); @@ -975,12 +993,14 @@ class View_test : public beast::unit_test::suite // construct and manage two different Env instances at the same // time. So we can use two Env instances to produce mutually // incompatible ledgers. + testcase("Are compatible"); + using namespace jtx; auto const alice = Account("alice"); auto const bob = Account("bob"); // The first Env. - Env eA(*this); + Env eA(*this, envconfig(), nullptr, beast::severities::kDisabled); eA.fund(XRP(10000), alice); eA.close(); @@ -990,9 +1010,13 @@ class View_test : public beast::unit_test::suite eA.close(); auto const rdViewA4 = eA.closed(); - // The two Env's can't share the same ports, so modifiy the config + // The two Env's can't share the same ports, so modify the config // of the second Env to use higher port numbers - Env eB{*this, envconfig(port_increment, 3)}; + Env eB{ + *this, + envconfig(port_increment, 3), + nullptr, + beast::severities::kDisabled}; // Make ledgers that are incompatible with the first ledgers. Note // that bob is funded before alice. @@ -1029,6 +1053,8 @@ class View_test : public beast::unit_test::suite void testRegressions() { + testcase("Regressions"); + using namespace jtx; // Create a ledger with 1 item, put a diff --git a/src/test/overlay/short_read_test.cpp b/src/test/overlay/short_read_test.cpp index dd649bfd1..434b41008 100644 --- a/src/test/overlay/short_read_test.cpp +++ b/src/test/overlay/short_read_test.cpp @@ -195,8 +195,6 @@ private: { acceptor_.listen(); server_.endpoint_ = acceptor_.local_endpoint(); - test_.log << "[server] up on port: " << server_.endpoint_.port() - << std::endl; } void diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 1692b9806..2580c4bfe 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -1250,13 +1250,10 @@ class LedgerRPC_test : public beast::unit_test::suite // no amendments env.fund(XRP(10000), "alice"); env.close(); - log << env.closed()->info().hash; env.fund(XRP(10000), "bob"); env.close(); - log << env.closed()->info().hash; env.fund(XRP(10000), "jim"); env.close(); - log << env.closed()->info().hash; env.fund(XRP(10000), "jill"); { diff --git a/src/test/rpc/NodeToShardRPC_test.cpp b/src/test/rpc/NodeToShardRPC_test.cpp index edfaf6c20..07d8d8953 100644 --- a/src/test/rpc/NodeToShardRPC_test.cpp +++ b/src/test/rpc/NodeToShardRPC_test.cpp @@ -276,7 +276,8 @@ public: sectionNode.set("ledgers_per_shard", "256"); c->setupControl(true, true, true); - return jtx::Env(*this, std::move(c)); + return jtx::Env( + *this, std::move(c), nullptr, beast::severities::kDisabled); }(); std::uint8_t const numberOfShards = 10; diff --git a/src/test/rpc/ShardArchiveHandler_test.cpp b/src/test/rpc/ShardArchiveHandler_test.cpp index 37c9d0168..ee0bec1ea 100644 --- a/src/test/rpc/ShardArchiveHandler_test.cpp +++ b/src/test/rpc/ShardArchiveHandler_test.cpp @@ -173,7 +173,8 @@ public: } c->setupControl(true, true, true); - jtx::Env env(*this, std::move(c)); + jtx::Env env( + *this, std::move(c), nullptr, beast::severities::kDisabled); std::uint8_t const numberOfDownloads = 10; @@ -276,7 +277,8 @@ public: } c->setupControl(true, true, true); - jtx::Env env(*this, std::move(c)); + jtx::Env env( + *this, std::move(c), nullptr, beast::severities::kDisabled); std::uint8_t const numberOfDownloads = 10; @@ -380,7 +382,8 @@ public: } c->setupControl(true, true, true); - jtx::Env env(*this, std::move(c)); + jtx::Env env( + *this, std::move(c), nullptr, beast::severities::kDisabled); std::uint8_t const numberOfDownloads = 10; // Create some ledgers so that the ShardArchiveHandler diff --git a/src/test/server/Server_test.cpp b/src/test/server/Server_test.cpp index 35f9149ca..b5eb71f36 100644 --- a/src/test/server/Server_test.cpp +++ b/src/test/server/Server_test.cpp @@ -299,7 +299,6 @@ public: serverPort.back().port = 0; serverPort.back().protocol.insert("http"); auto eps = s->ports(serverPort); - log << "server listening on port " << eps[0].port() << std::endl; test_request(eps[0]); test_keepalive(eps[0]); // s->close(); diff --git a/src/test/shamap/SHAMapSync_test.cpp b/src/test/shamap/SHAMapSync_test.cpp index ba32f6e80..6b2648a96 100644 --- a/src/test/shamap/SHAMapSync_test.cpp +++ b/src/test/shamap/SHAMapSync_test.cpp @@ -184,7 +184,6 @@ public: BEAST_EXPECT(source.deepCompare(destination)); - log << "Checking destination invariants..." << std::endl; destination.invariants(); } }; diff --git a/src/test/unit_test/FileDirGuard.h b/src/test/unit_test/FileDirGuard.h index 6337365f0..3c79fb11b 100644 --- a/src/test/unit_test/FileDirGuard.h +++ b/src/test/unit_test/FileDirGuard.h @@ -86,9 +86,6 @@ public: if (rmSubDir_) rmDir(subDir_); - else - test_.log << "Skipping rm dir: " << subDir_.string() - << std::endl; } catch (std::exception& e) {