From 11eb243adf1a75fa7010a675368fcae036b23811 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 11 Apr 2025 05:07:42 -0400 Subject: [PATCH] refactor: Clean up test logging to make it easier to search (#5396) This PR replaces the word `failed` with `failure` in any test names and renames some test files to fix MSVC warnings, so that it is easier to search through the test output to find tests that failed. --- src/test/app/AMMClawback_test.cpp | 57 +++---------------- src/test/app/AMM_test.cpp | 29 +++------- src/test/app/DepositAuth_test.cpp | 8 +-- src/test/jtx/Env.h | 6 +- src/test/jtx/impl/AMMTest.cpp | 11 +--- .../jtx/impl/{credentials.cpp => creds.cpp} | 0 src/test/jtx/impl/{did.cpp => dids.cpp} | 0 ...edgerStateFix.cpp => ledgerStateFixes.cpp} | 0 src/test/rpc/DepositAuthorized_test.cpp | 12 ++-- src/test/unit_test/multi_runner.cpp | 10 ++++ .../handlers/{Manifest.cpp => DoManifest.cpp} | 0 11 files changed, 45 insertions(+), 88 deletions(-) rename src/test/jtx/impl/{credentials.cpp => creds.cpp} (100%) rename src/test/jtx/impl/{did.cpp => dids.cpp} (100%) rename src/test/jtx/impl/{ledgerStateFix.cpp => ledgerStateFixes.cpp} (100%) rename src/xrpld/rpc/handlers/{Manifest.cpp => DoManifest.cpp} (100%) diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp index df2a6baf5..a452186f2 100644 --- a/src/test/app/AMMClawback_test.cpp +++ b/src/test/app/AMMClawback_test.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -1761,13 +1762,7 @@ class AMMClawback_test : public beast::unit_test::suite // Test AMMClawback for USD/XRP pool. Claw back USD, and XRP goes back // to the holder. - // Env env(*this, features, std::make_unique(&logs)); - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); Account gw{"gateway"}; Account alice{"alice"}; env.fund(XRP(1000000000), gw, alice); @@ -1853,13 +1848,7 @@ class AMMClawback_test : public beast::unit_test::suite // IOU/XRP pool. AMMClawback almost last holder's USD balance { - // Env env(*this, features, std::make_unique(&logs)); - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); Account gw{"gateway"}, alice{"alice"}, bob{"bob"}; auto const USD = setupAccounts(env, gw, alice, bob); @@ -1890,13 +1879,7 @@ class AMMClawback_test : public beast::unit_test::suite // IOU/XRP pool. AMMClawback part of last holder's USD balance { - // Env env(*this, features, std::make_unique(&logs)); - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); Account gw{"gateway"}, alice{"alice"}, bob{"bob"}; auto const USD = setupAccounts(env, gw, alice, bob); @@ -1927,13 +1910,7 @@ class AMMClawback_test : public beast::unit_test::suite // IOU/XRP pool. AMMClawback all of last holder's USD balance { - // Env env(*this, features, std::make_unique(&logs)); - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); Account gw{"gateway"}, alice{"alice"}, bob{"bob"}; auto const USD = setupAccounts(env, gw, alice, bob); @@ -1959,13 +1936,7 @@ class AMMClawback_test : public beast::unit_test::suite // IOU/IOU pool, different issuers { - // Env env(*this, features, std::make_unique(&logs)); - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); Account gw{"gateway"}, alice{"alice"}, bob{"bob"}; auto const USD = setupAccounts(env, gw, alice, bob); @@ -2001,13 +1972,7 @@ class AMMClawback_test : public beast::unit_test::suite // IOU/IOU pool, same issuer { - // Env env(*this, features, std::make_unique(&logs)); - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); Account gw{"gateway"}, alice{"alice"}, bob{"bob"}; auto const USD = setupAccounts(env, gw, alice, bob); @@ -2041,13 +2006,7 @@ class AMMClawback_test : public beast::unit_test::suite // IOU/IOU pool, larger asset ratio { - // Env env(*this, features, std::make_unique(&logs)); - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); Account gw{"gateway"}, alice{"alice"}, bob{"bob"}; auto const USD = setupAccounts(env, gw, alice, bob); diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 76e030fca..6268e33b4 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -5703,6 +5704,8 @@ private: testcase("Fix changeSpotPriceQuality"); using namespace jtx; + std::string logs; + enum class Status { SucceedShouldSucceedResize, // Succeed in pre-fix because // error allowance, succeed post-fix @@ -5785,12 +5788,7 @@ private: boost::smatch match; // tests that succeed should have the same amounts pre-fix and post-fix std::vector> successAmounts; - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); auto rules = env.current()->rules(); CurrentTransactionRulesGuard rg(rules); for (auto const& t : tests) @@ -5968,6 +5966,8 @@ private: using namespace std::chrono; FeatureBitset const all{featuresInitial}; + std::string logs; + Account const gatehub{"gatehub"}; Account const bitstamp{"bitstamp"}; Account const trader{"trader"}; @@ -6197,14 +6197,7 @@ private: testcase(input.testCase); for (auto const& features : {all}) { - // Env env(*this, features, - // std::make_unique(&logs)); - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); env.fund(XRP(5'000), gatehub, bitstamp, trader); env.close(); @@ -6430,13 +6423,7 @@ private: // Last Liquidity Provider is the issuer of one token { std::string logs; - // Env env(*this, features, std::make_unique(&logs)); - Env env( - *this, - envconfig(), - features, - nullptr, - beast::severities::kDisabled); + Env env(*this, features, std::make_unique(&logs)); fund( env, gw, diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index dc610c0a4..30440051c 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -829,7 +829,7 @@ struct DepositPreauth_test : public beast::unit_test::suite Account const john{"john"}; { - testcase("Payment failed with disabled credentials rule."); + testcase("Payment failure with disabled credentials rule."); Env env(*this, features - featureCredentials); @@ -931,7 +931,7 @@ struct DepositPreauth_test : public beast::unit_test::suite } { - testcase("Payment failed with invalid credentials."); + testcase("Payment failure with invalid credentials."); Env env(*this, features); @@ -1207,7 +1207,7 @@ struct DepositPreauth_test : public beast::unit_test::suite Account const zelda{"zelda"}; { - testcase("Payment failed with expired credentials."); + testcase("Payment failure with expired credentials."); Env env(*this, features); @@ -1354,7 +1354,7 @@ struct DepositPreauth_test : public beast::unit_test::suite { using namespace std::chrono; - testcase("Escrow failed with expired credentials."); + testcase("Escrow failure with expired credentials."); Env env(*this, features); diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index 92f6f4922..07df7c473 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -210,8 +210,10 @@ public: * @param args collection of features * */ - Env(beast::unit_test::suite& suite_, FeatureBitset features) - : Env(suite_, envconfig(), features) + Env(beast::unit_test::suite& suite_, + FeatureBitset features, + std::unique_ptr logs = nullptr) + : Env(suite_, envconfig(), features, std::move(logs)) { } diff --git a/src/test/jtx/impl/AMMTest.cpp b/src/test/jtx/impl/AMMTest.cpp index 73e56ebc2..c5211d352 100644 --- a/src/test/jtx/impl/AMMTest.cpp +++ b/src/test/jtx/impl/AMMTest.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -123,16 +124,10 @@ AMMTestBase::testAMM( for (auto const& features : arg.features) { - // Env env{ - // *this, - // features, - // arg.noLog ? std::make_unique(&logs) : nullptr}; - Env env( + Env env{ *this, - envconfig(), features, - nullptr, - beast::severities::kDisabled); + arg.noLog ? std::make_unique(&logs) : nullptr}; auto const [asset1, asset2] = arg.pool ? *arg.pool : std::make_pair(XRP(10000), USD(10000)); diff --git a/src/test/jtx/impl/credentials.cpp b/src/test/jtx/impl/creds.cpp similarity index 100% rename from src/test/jtx/impl/credentials.cpp rename to src/test/jtx/impl/creds.cpp diff --git a/src/test/jtx/impl/did.cpp b/src/test/jtx/impl/dids.cpp similarity index 100% rename from src/test/jtx/impl/did.cpp rename to src/test/jtx/impl/dids.cpp diff --git a/src/test/jtx/impl/ledgerStateFix.cpp b/src/test/jtx/impl/ledgerStateFixes.cpp similarity index 100% rename from src/test/jtx/impl/ledgerStateFix.cpp rename to src/test/jtx/impl/ledgerStateFixes.cpp diff --git a/src/test/rpc/DepositAuthorized_test.cpp b/src/test/rpc/DepositAuthorized_test.cpp index 64c3218a5..f75583152 100644 --- a/src/test/rpc/DepositAuthorized_test.cpp +++ b/src/test/rpc/DepositAuthorized_test.cpp @@ -65,6 +65,7 @@ public: void testValid() { + testcase("Valid"); using namespace jtx; Account const alice{"alice"}; Account const becky{"becky"}; @@ -162,6 +163,7 @@ public: void testErrors() { + testcase("Errors"); using namespace jtx; Account const alice{"alice"}; Account const becky{"becky"}; @@ -333,6 +335,8 @@ public: void testCredentials() { + testcase("Credentials"); + using namespace jtx; const char credType[] = "abcde"; @@ -363,7 +367,7 @@ public: { testcase( - "deposit_authorized with credentials failed: empty array."); + "deposit_authorized with credentials failure: empty array."); auto args = depositAuthArgs(alice, becky, "validated"); args[jss::credentials] = Json::arrayValue; @@ -376,7 +380,7 @@ public: { testcase( - "deposit_authorized with credentials failed: not a string " + "deposit_authorized with credentials failure: not a string " "credentials"); auto args = depositAuthArgs(alice, becky, "validated"); @@ -392,7 +396,7 @@ public: { testcase( - "deposit_authorized with credentials failed: not a hex string " + "deposit_authorized with credentials failure: not a hex string " "credentials"); auto args = depositAuthArgs(alice, becky, "validated"); @@ -412,7 +416,7 @@ public: { testcase( - "deposit_authorized with credentials failed: not a credential " + "deposit_authorized with credentials failure: not a credential " "index"); auto args = depositAuthArgs( diff --git a/src/test/unit_test/multi_runner.cpp b/src/test/unit_test/multi_runner.cpp index 0b7d08c5a..087e37dac 100644 --- a/src/test/unit_test/multi_runner.cpp +++ b/src/test/unit_test/multi_runner.cpp @@ -577,6 +577,16 @@ multi_runner_child::on_suite_begin(beast::unit_test::suite_info const& info) void multi_runner_child::on_suite_end() { + if (print_log_ || suite_results_.failed > 0) + { + std::stringstream s; + if (num_jobs_ > 1) + s << job_index_ << "> "; + s << (suite_results_.failed > 0 ? "failed: " : "") + << suite_results_.name << " had " << suite_results_.failed + << " failures." << std::endl; + message_queue_send(MessageType::log, s.str()); + } results_.add(suite_results_); message_queue_send(MessageType::test_end, suite_results_.name); } diff --git a/src/xrpld/rpc/handlers/Manifest.cpp b/src/xrpld/rpc/handlers/DoManifest.cpp similarity index 100% rename from src/xrpld/rpc/handlers/Manifest.cpp rename to src/xrpld/rpc/handlers/DoManifest.cpp