From cba512068bfc113291f6bf33434a77902cd1bd41 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/AMM_test.cpp | 9 +++++++-- src/test/app/DepositAuth_test.cpp | 8 ++++---- src/test/jtx/Env.h | 6 ++++-- src/test/jtx/impl/{credentials.cpp => creds.cpp} | 0 src/test/jtx/impl/{did.cpp => dids.cpp} | 0 .../{ledgerStateFix.cpp => ledgerStateFixes.cpp} | 0 src/test/rpc/DepositAuthorized_test.cpp | 12 ++++++++---- src/test/unit_test/multi_runner.cpp | 10 ++++++++++ .../rpc/handlers/{Manifest.cpp => DoManifest.cpp} | 0 9 files changed, 33 insertions(+), 12 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/AMM_test.cpp b/src/test/app/AMM_test.cpp index e20102f224..a0be79913b 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 @@ -6079,6 +6080,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 @@ -6161,7 +6164,7 @@ private: boost::smatch match; // tests that succeed should have the same amounts pre-fix and post-fix std::vector> successAmounts; - Env env(*this, features); + Env env(*this, features, std::make_unique(&logs)); auto rules = env.current()->rules(); CurrentTransactionRulesGuard rg(rules); for (auto const& t : tests) @@ -6355,6 +6358,8 @@ private: using namespace std::chrono; FeatureBitset const all{features}; + std::string logs; + Account const gatehub{"gatehub"}; Account const bitstamp{"bitstamp"}; Account const trader{"trader"}; @@ -6583,7 +6588,7 @@ private: for (auto const& features : {all - fixAMMOverflowOffer, all | fixAMMOverflowOffer}) { - Env env(*this, features); + Env env(*this, features, std::make_unique(&logs)); env.fund(XRP(5'000), gatehub, bitstamp, trader); env.close(); diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index 6d9a3ac914..18f7b410b7 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -828,7 +828,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, supported_amendments() - featureCredentials); @@ -930,7 +930,7 @@ struct DepositPreauth_test : public beast::unit_test::suite } { - testcase("Payment failed with invalid credentials."); + testcase("Payment failure with invalid credentials."); Env env(*this); @@ -1206,7 +1206,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); @@ -1353,7 +1353,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); diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index 4e8bb64f59..399b176677 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/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 2ccad9d779..8162528ec2 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 0b7d08c5ae..087e37dac2 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