From d12ae81070398c22df9743aaaa67b17d9a68d8bb Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 19 Feb 2026 18:49:44 +0900 Subject: [PATCH] test: Env unit test RPC errors return a unique result: (#4877) * telENV_RPC_FAILED is a new code, reserved exclusively for unit tests when RPC fails. This will make those types of errors distinct and easier to test for when expected and/or diagnose when not. * Output RPC command result when result is not expected. --- src/ripple/protocol/TER.h | 1 + src/ripple/protocol/impl/TER.cpp | 2 + src/test/app/MultiSign_test.cpp | 12 ++++-- src/test/app/Regression_test.cpp | 2 +- src/test/app/ValidatorSite_test.cpp | 3 ++ src/test/jtx/Env.h | 6 ++- src/test/jtx/Env_test.cpp | 5 ++- src/test/jtx/impl/Env.cpp | 52 +++++++++++++++--------- src/test/net/DatabaseDownloader_test.cpp | 7 ++++ src/test/protocol/Memo_test.cpp | 10 ++--- 10 files changed, 69 insertions(+), 31 deletions(-) diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 3dc652d29..021be47ac 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -68,6 +68,7 @@ enum TELcodes : TERUnderlyingType { telNON_LOCAL_EMITTED_TXN, telIMPORT_VL_KEY_NOT_RECOGNISED, telCAN_NOT_QUEUE_IMPORT, + telENV_RPC_FAILED, }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index 283f9d621..6b6015bda 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -168,6 +168,8 @@ transResults() MAKE_ERROR(telNON_LOCAL_EMITTED_TXN, "Emitted transaction cannot be applied because it was not generated locally."), MAKE_ERROR(telIMPORT_VL_KEY_NOT_RECOGNISED, "Import vl key was not recognized."), MAKE_ERROR(telCAN_NOT_QUEUE_IMPORT, "Import transaction was not able to be directly applied and cannot be queued."), + MAKE_ERROR(telENV_RPC_FAILED, "Unit test RPC failure."), + MAKE_ERROR(temMALFORMED, "Malformed transaction."), MAKE_ERROR(temBAD_AMM_TOKENS, "Malformed: Invalid LPTokens."), MAKE_ERROR(temBAD_AMOUNT, "Can only send positive amounts."), diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 7dfbfdd78..f32a90788 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -245,7 +245,10 @@ public: // Duplicate signers should fail. aliceSeq = env.seq(alice); - env(noop(alice), msig(demon, demon), fee(3 * baseFee), ter(temINVALID)); + env(noop(alice), + msig(demon, demon), + fee(3 * baseFee), + ter(telENV_RPC_FAILED)); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq); @@ -356,7 +359,7 @@ public: msig phantoms{bogie, demon}; std::reverse(phantoms.signers.begin(), phantoms.signers.end()); std::uint32_t const aliceSeq = env.seq(alice); - env(noop(alice), phantoms, ter(temINVALID)); + env(noop(alice), phantoms, ter(telENV_RPC_FAILED)); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq); } @@ -1632,7 +1635,10 @@ public: // Duplicate signers should fail. aliceSeq = env.seq(alice); - env(noop(alice), msig(demon, demon), fee(3 * baseFee), ter(temINVALID)); + env(noop(alice), + msig(demon, demon), + fee(3 * baseFee), + ter(telENV_RPC_FAILED)); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq); diff --git a/src/test/app/Regression_test.cpp b/src/test/app/Regression_test.cpp index a2b639b0f..d9e7bb3f4 100644 --- a/src/test/app/Regression_test.cpp +++ b/src/test/app/Regression_test.cpp @@ -149,7 +149,7 @@ struct Regression_test : public beast::unit_test::suite secp256r1Sig->setFieldVL(sfSigningPubKey, *pubKeyBlob); jt.stx.reset(secp256r1Sig.release()); - env(jt, ter(temINVALID)); + env(jt, ter(telENV_RPC_FAILED)); }; Account const alice{"alice", KeyType::secp256k1}; diff --git a/src/test/app/ValidatorSite_test.cpp b/src/test/app/ValidatorSite_test.cpp index 79a80f5e5..405d55724 100644 --- a/src/test/app/ValidatorSite_test.cpp +++ b/src/test/app/ValidatorSite_test.cpp @@ -237,7 +237,10 @@ private: std::vector uris; for (auto const& u : servers) + { + log << "Testing " << u.uri << std::endl; uris.push_back(u.uri); + } sites->load(uris); sites->start(); sites->join(); diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index 96cd045e8..1fefabd0a 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -535,7 +535,11 @@ public: of JTx submission. */ void - postconditions(JTx const& jt, TER ter, bool didApply); + postconditions( + JTx const& jt, + TER ter, + bool didApply, + Json::Value const& jr = Json::Value()); /** Apply funclets and submit. */ /** @{ */ diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 85b6d5dc2..b0e7f6b81 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -749,8 +749,9 @@ public: // Force the factor low enough to fail params[jss::fee_mult_max] = 1; params[jss::fee_div_max] = 2; - // RPC errors result in temINVALID - envs(noop(alice), fee(none), seq(none), ter(temINVALID))(params); + // RPC errors result in telENV_RPC_FAILED + envs(noop(alice), fee(none), seq(none), ter(telENV_RPC_FAILED))( + params); auto tx = env.tx(); BEAST_EXPECT(!tx); diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 3af53877b..0b7d1e5aa 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -289,7 +289,9 @@ Env::parseResult(Json::Value const& jr) jr[jss::result].isMember(jss::engine_result_code)) ter = TER::fromInt(jr[jss::result][jss::engine_result_code].asInt()); else - ter = temINVALID; + // Use an error code that is not used anywhere in the transaction engine + // to distinguish this case. + ter = telENV_RPC_FAILED; return std::make_pair(ter, isTesSuccess(ter) || isTecClaim(ter)); } @@ -297,23 +299,29 @@ void Env::submit(JTx const& jt) { bool didApply; - if (jt.stx) - { - txid_ = jt.stx->getTransactionID(); - Serializer s; - jt.stx->add(s); - auto const jr = rpc("submit", strHex(s.slice())); + auto const jr = [&]() { + if (jt.stx) + { + txid_ = jt.stx->getTransactionID(); + Serializer s; + jt.stx->add(s); + auto const jr = rpc("submit", strHex(s.slice())); - std::tie(ter_, didApply) = parseResult(jr); - } - else - { - // Parsing failed or the JTx is - // otherwise missing the stx field. - ter_ = temMALFORMED; - didApply = false; - } - return postconditions(jt, ter_, didApply); + std::tie(ter_, didApply) = parseResult(jr); + + return jr; + } + else + { + // Parsing failed or the JTx is + // otherwise missing the stx field. + ter_ = temMALFORMED; + didApply = false; + + return Json::Value(); + } + }(); + return postconditions(jt, ter_, didApply, jr); } void @@ -351,11 +359,15 @@ Env::sign_and_submit(JTx const& jt, Json::Value params) std::tie(ter_, didApply) = parseResult(jr); - return postconditions(jt, ter_, didApply); + return postconditions(jt, ter_, didApply, jr); } void -Env::postconditions(JTx const& jt, TER ter, bool didApply) +Env::postconditions( + JTx const& jt, + TER ter, + bool didApply, + Json::Value const& jr) { if (jt.ter && !test.expect( @@ -365,6 +377,8 @@ Env::postconditions(JTx const& jt, TER ter, bool didApply) transToken(*jt.ter) + " (" + transHuman(*jt.ter) + ")")) { test.log << pretty(jt.jv) << std::endl; + if (jr) + test.log << pretty(jr) << std::endl; auto const& m = meta(); if (m) test.log << *m << std::endl; diff --git a/src/test/net/DatabaseDownloader_test.cpp b/src/test/net/DatabaseDownloader_test.cpp index d4ed2ebce..31c8abfd1 100644 --- a/src/test/net/DatabaseDownloader_test.cpp +++ b/src/test/net/DatabaseDownloader_test.cpp @@ -147,6 +147,7 @@ class DatabaseDownloader_test : public beast::unit_test::suite // server to request from. Use the /textfile endpoint // to get a simple text file sent as response. auto server = createServer(env); + log << "Downloading DB from " << server->local_endpoint() << std::endl; ripple::test::detail::FileDirGuard const data{ *this, "downloads", "data", "", false, false}; @@ -225,6 +226,8 @@ class DatabaseDownloader_test : public beast::unit_test::suite auto server = createServer(env); auto host = server->local_endpoint().address().to_string(); auto port = std::to_string(server->local_endpoint().port()); + log << "Downloading DB from " << server->local_endpoint() + << std::endl; server->stop(); BEAST_EXPECT(dl->download( host, @@ -249,6 +252,8 @@ class DatabaseDownloader_test : public beast::unit_test::suite ripple::test::detail::FileDirGuard const datafile{ *this, "downloads", "data", "", false, false}; auto server = createServer(env, false); + log << "Downloading DB from " << server->local_endpoint() + << std::endl; BEAST_EXPECT(dl->download( server->local_endpoint().address().to_string(), std::to_string(server->local_endpoint().port()), @@ -272,6 +277,8 @@ class DatabaseDownloader_test : public beast::unit_test::suite ripple::test::detail::FileDirGuard const datafile{ *this, "downloads", "data", "", false, false}; auto server = createServer(env); + log << "Downloading DB from " << server->local_endpoint() + << std::endl; BEAST_EXPECT(dl->download( server->local_endpoint().address().to_string(), std::to_string(server->local_endpoint().port()), diff --git a/src/test/protocol/Memo_test.cpp b/src/test/protocol/Memo_test.cpp index 695f35d43..a406d74dc 100644 --- a/src/test/protocol/Memo_test.cpp +++ b/src/test/protocol/Memo_test.cpp @@ -56,7 +56,7 @@ public: JTx memoSize = makeJtxWithMemo(); memoSize.jv[sfMemos.jsonName][0u][sfMemo.jsonName] [sfMemoData.jsonName] = std::string(2020, '0'); - env(memoSize, MEMOFEE, ter(temINVALID)); + env(memoSize, MEMOFEE, ter(telENV_RPC_FAILED)); // This memo is just barely small enough. memoSize.jv[sfMemos.jsonName][0u][sfMemo.jsonName] @@ -72,7 +72,7 @@ public: auto& m = mi[sfCreatedNode.jsonName]; // CreatedNode in Memos m[sfMemoData.jsonName] = "3030303030"; - env(memoNonMemo, MEMOFEE, ter(temINVALID)); + env(memoNonMemo, MEMOFEE, ter(telENV_RPC_FAILED)); } { // Put an invalid field in a Memo object. @@ -80,7 +80,7 @@ public: memoExtra .jv[sfMemos.jsonName][0u][sfMemo.jsonName][sfFlags.jsonName] = 13; - env(memoExtra, MEMOFEE, ter(temINVALID)); + env(memoExtra, MEMOFEE, ter(telENV_RPC_FAILED)); } { // Put a character that is not allowed in a URL in a MemoType field. @@ -88,7 +88,7 @@ public: memoBadChar.jv[sfMemos.jsonName][0u][sfMemo.jsonName] [sfMemoType.jsonName] = strHex(std::string_view("ONE