From 0c32fc5f2adb5a1598dd7e97fc83ba0f64fdee20 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 19 Mar 2024 12:13:52 -0400 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 | 3 +- src/ripple/protocol/impl/TER.cpp | 1 + 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(+), 32 deletions(-) diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h index 8cd5e824608..41c23a2d6a8 100644 --- a/src/ripple/protocol/TER.h +++ b/src/ripple/protocol/TER.h @@ -64,7 +64,8 @@ enum TELcodes : TERUnderlyingType { telCAN_NOT_QUEUE_FULL, telWRONG_NETWORK, telREQUIRES_NETWORK_ID, - telNETWORK_ID_MAKES_TX_NON_CANONICAL + telNETWORK_ID_MAKES_TX_NON_CANONICAL, + telENV_RPC_FAILED }; //------------------------------------------------------------------------------ diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index caba034a1c4..93bc60a98ba 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -154,6 +154,7 @@ transResults() MAKE_ERROR(telWRONG_NETWORK, "Transaction specifies a Network ID that differs from that of the local node."), MAKE_ERROR(telREQUIRES_NETWORK_ID, "Transactions submitted to this node/network must include a correct NetworkID field."), MAKE_ERROR(telNETWORK_ID_MAKES_TX_NON_CANONICAL, "Transactions submitted to this node/network must NOT include a NetworkID field."), + MAKE_ERROR(telENV_RPC_FAILED, "Unit test RPC failure."), MAKE_ERROR(temMALFORMED, "Malformed transaction."), MAKE_ERROR(temBAD_AMM_TOKENS, "Malformed: Invalid LPTokens."), diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 433cf2f7cd8..6e918e36c79 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -247,7 +247,10 @@ class MultiSign_test : public beast::unit_test::suite // 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); @@ -358,7 +361,7 @@ class MultiSign_test : public beast::unit_test::suite 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); } @@ -1634,7 +1637,10 @@ class MultiSign_test : public beast::unit_test::suite // 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 6431f81dbd6..e2c4b355a9d 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 8acd2063358..00512d157ec 100644 --- a/src/test/app/ValidatorSite_test.cpp +++ b/src/test/app/ValidatorSite_test.cpp @@ -237,7 +237,10 @@ class ValidatorSite_test : public beast::unit_test::suite 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 74e9e057de8..fd05d1c6c5a 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -512,7 +512,11 @@ class Env 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 154abe44ce5..8a42b554b8e 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -747,8 +747,9 @@ class Env_test : public beast::unit_test::suite // 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 e82183c0001..6496f7df1d2 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -280,7 +280,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)); } @@ -288,23 +290,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 @@ -342,11 +350,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( @@ -356,6 +368,8 @@ Env::postconditions(JTx const& jt, TER ter, bool didApply) transHuman(*jt.ter) + ")")) { test.log << pretty(jt.jv) << std::endl; + if (jr) + test.log << pretty(jr) << std::endl; // Don't check postconditions if // we didn't get the expected result. return; diff --git a/src/test/net/DatabaseDownloader_test.cpp b/src/test/net/DatabaseDownloader_test.cpp index d4ed2ebcedf..31c8abfd12c 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 b39482e42d0..89ae6dfe18a 100644 --- a/src/test/protocol/Memo_test.cpp +++ b/src/test/protocol/Memo_test.cpp @@ -56,7 +56,7 @@ class Memo_test : public beast::unit_test::suite JTx memoSize = makeJtxWithMemo(); memoSize.jv[sfMemos.jsonName][0u][sfMemo.jsonName] [sfMemoData.jsonName] = std::string(2020, '0'); - env(memoSize, ter(temINVALID)); + env(memoSize, ter(telENV_RPC_FAILED)); // This memo is just barely small enough. memoSize.jv[sfMemos.jsonName][0u][sfMemo.jsonName] @@ -72,7 +72,7 @@ class Memo_test : public beast::unit_test::suite auto& m = mi[sfCreatedNode.jsonName]; // CreatedNode in Memos m[sfMemoData.jsonName] = "3030303030"; - env(memoNonMemo, ter(temINVALID)); + env(memoNonMemo, ter(telENV_RPC_FAILED)); } { // Put an invalid field in a Memo object. @@ -80,7 +80,7 @@ class Memo_test : public beast::unit_test::suite memoExtra .jv[sfMemos.jsonName][0u][sfMemo.jsonName][sfFlags.jsonName] = 13; - env(memoExtra, ter(temINVALID)); + env(memoExtra, ter(telENV_RPC_FAILED)); } { // Put a character that is not allowed in a URL in a MemoType field. @@ -88,7 +88,7 @@ class Memo_test : public beast::unit_test::suite memoBadChar.jv[sfMemos.jsonName][0u][sfMemo.jsonName] [sfMemoType.jsonName] = strHex(std::string_view("ONE