From d9601413b6c8da88f774de3f4a16da1f49ea4ca9 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 29 Feb 2024 14:18:29 -0500 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 unexpectedly. This will make those types of errors distinct and easier to test for when expect and/or diagnose when not. --- 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_test.cpp | 5 +++-- src/test/jtx/impl/Env.cpp | 4 +++- src/test/net/DatabaseDownloader_test.cpp | 7 +++++++ src/test/protocol/Memo_test.cpp | 10 +++++----- 9 files changed, 34 insertions(+), 13 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 8ec86feadce..056ab5d91ee 100644 --- a/src/test/app/ValidatorSite_test.cpp +++ b/src/test/app/ValidatorSite_test.cpp @@ -239,7 +239,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_test.cpp b/src/test/jtx/Env_test.cpp index 6f09f49ed5d..73a3c6351b6 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..8b9fc907f06 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)); } 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