Skip to content

Commit

Permalink
Cleanup fixes for unit tests (#4184)
Browse files Browse the repository at this point in the history 
* Fixing incorrect bounds check in check_block_response_count and removing some code duplication.

* Removing usages of ledger cache block and confirmed count which are not fully synced with observing blocks in the ledger.

* Moving ASSERT_EQ out of function as this messes up googletest line reporting.

* Moving ASSERT_EQ out of pending_exists lambda since it interferes with googletest test failure reporting.

* Increasing timeouts to standard 5s.

* Removing duplicate/unneeded checks unrelated to correctness.
  • Loading branch information
@clemahieu
clemahieu committed Mar 16, 2023
1 parent 022a10f commit 6e3ef43
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 78 deletions.
4 changes: 2 additions & 2 deletions nano/core_test/active_transactions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -733,9 +733,9 @@ TEST (active_transactions, republish_winner)
node1.vote_processor.vote (vote, std::make_shared<nano::transport::inproc::channel> (node1, node1));
node1.vote_processor.flush ();
node1.block_processor.flush ();
ASSERT_TIMELY (3s, election->confirmed ());
ASSERT_TIMELY (5s, election->confirmed ());
ASSERT_EQ (fork->hash (), election->status.winner->hash ());
ASSERT_TIMELY (3s, node2.block_confirmed (fork->hash ()));
ASSERT_TIMELY (5s, node2.block_confirmed (fork->hash ()));
}

TEST (active_transactions, fork_filter_cleanup)
Expand Down
2 changes: 1 addition & 1 deletion nano/core_test/bootstrap.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,7 @@ TEST (bootstrap_processor, lazy_pruning_missing_block)
ASSERT_TIMELY (5s, node1->block (state_open->hash ()) != nullptr);
// Confirm last block to prune previous
nano::test::start_elections (system, *node1, { send1, send2, open, state_open }, true);
ASSERT_TIMELY (5s, node1->block_confirmed (send1->hash ()) && node1->block_confirmed (send2->hash ()) && node1->block_confirmed (open->hash ()) && node1->block_confirmed (state_open->hash ()) && node1->active.empty ());
ASSERT_TIMELY (5s, node1->block_confirmed (send2->hash ()) && node1->block_confirmed (open->hash ()) && node1->block_confirmed (state_open->hash ()));
ASSERT_EQ (5, node1->ledger.cache.block_count);
ASSERT_EQ (5, node1->ledger.cache.cemented_count);
// Pruning action
Expand Down
116 changes: 41 additions & 75 deletions nano/rpc_test/rpc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,18 +184,11 @@ boost::property_tree::ptree wait_response (nano::test::system & system, rpc_cont
return response_json;
}

void check_block_response_count (nano::test::system & system, rpc_context const & rpc_ctx, boost::property_tree::ptree & request, uint64_t size_count)
bool check_block_response_count (nano::test::system & system, rpc_context const & rpc_ctx, boost::property_tree::ptree & request, uint64_t size_count)
{
auto response (wait_response (system, rpc_ctx, request));
auto & blocks = response.get_child ("blocks");
if (size_count > 0)
{
ASSERT_EQ (size_count, blocks.front ().second.size ());
}
else
{
ASSERT_TRUE (blocks.empty ());
}
return size_count == blocks.size ();
}

rpc_context add_rpc (nano::test::system & system, std::shared_ptr<nano::node> const & node_a)
Expand Down Expand Up @@ -1932,9 +1925,7 @@ TEST (rpc, pending)
nano::keypair key1;
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
auto block1 (system.wallet (0)->send_action (nano::dev::genesis_key.pub, key1.pub, 100));
node->scheduler.flush ();
ASSERT_TIMELY (5s, !node->active.active (*block1));
ASSERT_TIMELY (5s, node->ledger.cache.cemented_count == 2 && node->confirmation_height_processor.current ().is_zero () && node->confirmation_height_processor.awaiting_processing_size () == 0);
ASSERT_TIMELY (5s, node->block_confirmed (block1->hash ()));
auto const rpc_ctx = add_rpc (system, node);
boost::property_tree::ptree request;
request.put ("action", "pending");
Expand Down Expand Up @@ -2008,19 +1999,14 @@ TEST (rpc, pending)
request.put ("source", "false");
request.put ("min_version", "false");

auto check_block_response_count_l = [&system, &request, &rpc_ctx] (size_t size) {
auto response (wait_response (system, rpc_ctx, request));
ASSERT_EQ (size, response.get_child ("blocks").size ());
};

check_block_response_count_l (1);
ASSERT_TRUE (check_block_response_count (system, rpc_ctx, request, 1));
rpc_ctx.io_scope->reset ();
reset_confirmation_height (system.nodes.front ()->store, block1->account ());
rpc_ctx.io_scope->renew ();
check_block_response_count_l (0);
ASSERT_TRUE (check_block_response_count (system, rpc_ctx, request, 0));
request.put ("include_only_confirmed", "false");
rpc_ctx.io_scope->renew ();
check_block_response_count_l (1);
ASSERT_TRUE (check_block_response_count (system, rpc_ctx, request, 1));
request.put ("include_only_confirmed", "true");

// Sorting with a smaller count than total should give absolute sorted amounts
Expand Down Expand Up @@ -2211,9 +2197,7 @@ TEST (rpc, pending_burn)
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
auto block1 (system.wallet (0)->send_action (nano::dev::genesis_key.pub, nano::dev::constants.burn_account, 100));
auto const rpc_ctx = add_rpc (system, node);
node->scheduler.flush ();
ASSERT_TIMELY (5s, !node->active.active (*block1));
ASSERT_TIMELY (5s, node->ledger.cache.cemented_count == 2 && node->confirmation_height_processor.current ().is_zero () && node->confirmation_height_processor.awaiting_processing_size () == 0);
ASSERT_TIMELY (5s, node->block_confirmed (block1->hash ()));
boost::property_tree::ptree request;
request.put ("action", "pending");
request.put ("account", nano::dev::constants.burn_account.to_account ());
Expand Down Expand Up @@ -2760,9 +2744,8 @@ TEST (rpc, block_count_pruning)
.work (*node1->work_generate_blocking (send1->hash ()))
.build_shared ();
node1->process_active (receive1);
node1->block_processor.flush ();
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
ASSERT_TIMELY (5s, node1->ledger.cache.cemented_count == 3 && node1->confirmation_height_processor.current ().is_zero () && node1->confirmation_height_processor.awaiting_processing_size () == 0);
ASSERT_TIMELY (5s, node1->block_confirmed (receive1->hash ()));
// Pruning action
{
auto transaction (node1->store.tx_begin_write ());
Expand Down Expand Up @@ -3681,9 +3664,7 @@ TEST (rpc, accounts_receivable)
nano::keypair key1;
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
auto block1 (system.wallet (0)->send_action (nano::dev::genesis_key.pub, key1.pub, 100));
node->scheduler.flush ();
ASSERT_TIMELY (5s, !node->active.active (*block1));
ASSERT_TIMELY (5s, node->ledger.cache.cemented_count == 2 && node->confirmation_height_processor.current ().is_zero () && node->confirmation_height_processor.awaiting_processing_size () == 0);
ASSERT_TIMELY (5s, node->block_confirmed (block1->hash ()));

auto const rpc_ctx = add_rpc (system, node);
boost::property_tree::ptree request;
Expand Down Expand Up @@ -3759,14 +3740,14 @@ TEST (rpc, accounts_receivable)
ASSERT_EQ (sources[block1->hash ()], nano::dev::genesis_key.pub);
}

check_block_response_count (system, rpc_ctx, request, 1);
ASSERT_TRUE (check_block_response_count (system, rpc_ctx, request, 1));
rpc_ctx.io_scope->reset ();
reset_confirmation_height (system.nodes.front ()->store, block1->account ());
rpc_ctx.io_scope->renew ();
check_block_response_count (system, rpc_ctx, request, 0);
ASSERT_TRUE (check_block_response_count (system, rpc_ctx, request, 0));
request.put ("include_only_confirmed", "false");
rpc_ctx.io_scope->renew ();
check_block_response_count (system, rpc_ctx, request, 1);
ASSERT_TRUE (check_block_response_count (system, rpc_ctx, request, 1));
}

TEST (rpc, blocks)
Expand Down Expand Up @@ -3801,20 +3782,16 @@ TEST (rpc, wallet_info)
nano::keypair key;
system.wallet (0)->insert_adhoc (key.prv);

// at first, 1 block and 1 confirmed -- the genesis
ASSERT_EQ (1, node->ledger.cache.block_count);
ASSERT_EQ (1, node->ledger.cache.cemented_count);

auto send (system.wallet (0)->send_action (nano::dev::genesis_key.pub, key.pub, nano::Gxrb_ratio));
// after the send, expect 2 blocks immediately, then 2 confirmed in a timely manner,
// and finally 3 blocks and 3 confirmed after the wallet generates the receive block for this send
ASSERT_EQ (2, node->ledger.cache.block_count);
ASSERT_TIMELY (5s, 2 == node->ledger.cache.cemented_count);
ASSERT_TIMELY (5s, 3 == node->ledger.cache.block_count && 3 == node->ledger.cache.cemented_count);
ASSERT_TIMELY (5s, node->block_confirmed (send->hash ())); // Send gets confirmed
ASSERT_TIMELY (5s, !node->latest (key.pub).is_zero ()); // Receive gets generated
ASSERT_TIMELY (5s, node->block_confirmed (node->latest (key.pub))); // Receive gets confirmed

// do another send to be able to expect some "pending" down below
auto send2 (system.wallet (0)->send_action (nano::dev::genesis_key.pub, key.pub, 1));
ASSERT_TIMELY (5s, 4 == node->ledger.cache.block_count && 4 == node->ledger.cache.cemented_count);
ASSERT_TIMELY (5s, node->block_confirmed (send2->hash ()));

nano::account account (system.wallet (0)->deterministic_insert ());
{
Expand Down Expand Up @@ -3890,35 +3867,33 @@ TEST (rpc, pending_exists)
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
auto hash0 (node->latest (nano::dev::genesis->account ()));
auto block1 (system.wallet (0)->send_action (nano::dev::genesis_key.pub, key1.pub, 100));
node->scheduler.flush ();
ASSERT_TIMELY (5s, !node->active.active (*block1));
ASSERT_TIMELY (5s, node->ledger.cache.cemented_count == 2 && node->confirmation_height_processor.current ().is_zero () && node->confirmation_height_processor.awaiting_processing_size () == 0);
ASSERT_TIMELY (5s, node->block_confirmed (block1->hash ()));

auto const rpc_ctx = add_rpc (system, node);
boost::property_tree::ptree request;

auto pending_exists = [&system, &rpc_ctx, &request] (char const * exists_a) {
auto response0 (wait_response (system, rpc_ctx, request));
std::string exists_text (response0.get<std::string> ("exists"));
ASSERT_EQ (exists_a, exists_text);
return exists_a == exists_text;
};

request.put ("action", "pending_exists");
request.put ("hash", hash0.to_string ());
pending_exists ("0");
ASSERT_TRUE (pending_exists ("0"));

node->store.pending.exists (node->store.tx_begin_read (), nano::pending_key (nano::dev::genesis_key.pub, block1->hash ()));
request.put ("hash", block1->hash ().to_string ());
pending_exists ("1");
ASSERT_TRUE (pending_exists ("1"));

pending_exists ("1");
ASSERT_TRUE (pending_exists ("1"));
rpc_ctx.io_scope->reset ();
reset_confirmation_height (node->store, block1->account ());
rpc_ctx.io_scope->renew ();
pending_exists ("0");
ASSERT_TRUE (pending_exists ("0"));
request.put ("include_only_confirmed", "false");
rpc_ctx.io_scope->renew ();
pending_exists ("1");
ASSERT_TRUE (pending_exists ("1"));
}

TEST (rpc, wallet_pending)
Expand All @@ -3945,27 +3920,20 @@ TEST (rpc, wallet_pending)

TEST (rpc, wallet_receivable)
{
nano::test::system system0;
auto node = add_ipc_enabled_node (system0);
nano::test::system system;
auto node = add_ipc_enabled_node (system);
nano::keypair key1;
system0.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
system0.wallet (0)->insert_adhoc (key1.prv);
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
system.wallet (0)->insert_adhoc (key1.prv);
auto iterations (0);
auto block1 (system0.wallet (0)->send_action (nano::dev::genesis_key.pub, key1.pub, 100));
node->scheduler.flush ();
while (node->active.active (*block1) || node->ledger.cache.cemented_count < 2 || !node->confirmation_height_processor.current ().is_zero () || node->confirmation_height_processor.awaiting_processing_size () != 0)
{
system0.poll ();
++iterations;
ASSERT_LT (iterations, 200);
}

auto const rpc_ctx = add_rpc (system0, node);
auto block1 (system.wallet (0)->send_action (nano::dev::genesis_key.pub, key1.pub, 100));
ASSERT_TIMELY (5s, node->block_confirmed (block1->hash ()));
auto const rpc_ctx = add_rpc (system, node);
boost::property_tree::ptree request;
request.put ("action", "wallet_receivable");
request.put ("wallet", node->wallets.items.begin ()->first.to_string ());
request.put ("count", "100");
auto response (wait_response (system0, rpc_ctx, request));
auto response (wait_response (system, rpc_ctx, request));
ASSERT_EQ (1, response.get_child ("blocks").size ());
for (auto & pending : response.get_child ("blocks"))
{
Expand All @@ -3975,7 +3943,7 @@ TEST (rpc, wallet_receivable)
ASSERT_EQ (block1->hash (), hash1);
}
request.put ("threshold", "100"); // Threshold test
auto response0 (wait_response (system0, rpc_ctx, request));
auto response0 (wait_response (system, rpc_ctx, request));
std::unordered_map<nano::block_hash, nano::uint128_union> blocks;
ASSERT_EQ (1, response0.get_child ("blocks").size ());
for (auto & pending : response0.get_child ("blocks"))
Expand All @@ -3997,13 +3965,13 @@ TEST (rpc, wallet_receivable)
}
ASSERT_EQ (blocks[block1->hash ()], 100);
request.put ("threshold", "101");
auto response1 (wait_response (system0, rpc_ctx, request));
auto response1 (wait_response (system, rpc_ctx, request));
auto & pending1 (response1.get_child ("blocks"));
ASSERT_EQ (0, pending1.size ());
request.put ("threshold", "0");
request.put ("source", "true");
request.put ("min_version", "true");
auto response2 (wait_response (system0, rpc_ctx, request));
auto response2 (wait_response (system, rpc_ctx, request));
std::unordered_map<nano::block_hash, nano::uint128_union> amounts;
std::unordered_map<nano::block_hash, nano::account> sources;
ASSERT_EQ (1, response2.get_child ("blocks").size ());
Expand All @@ -4023,14 +3991,14 @@ TEST (rpc, wallet_receivable)
ASSERT_EQ (amounts[block1->hash ()], 100);
ASSERT_EQ (sources[block1->hash ()], nano::dev::genesis_key.pub);

check_block_response_count (system0, rpc_ctx, request, 1);
ASSERT_TRUE (check_block_response_count (system, rpc_ctx, request, 1));
rpc_ctx.io_scope->reset ();
reset_confirmation_height (system0.nodes.front ()->store, block1->account ());
reset_confirmation_height (system.nodes.front ()->store, block1->account ());
rpc_ctx.io_scope->renew ();
check_block_response_count (system0, rpc_ctx, request, 0);
ASSERT_TRUE (check_block_response_count (system, rpc_ctx, request, 0));
request.put ("include_only_confirmed", "false");
rpc_ctx.io_scope->renew ();
check_block_response_count (system0, rpc_ctx, request, 1);
ASSERT_TRUE (check_block_response_count (system, rpc_ctx, request, 1));
}

TEST (rpc, receive_minimum)
Expand Down Expand Up @@ -4898,9 +4866,8 @@ TEST (rpc, block_info_pruning)
.work (*node1->work_generate_blocking (send1->hash ()))
.build_shared ();
node1->process_active (receive1);
node1->block_processor.flush ();
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
ASSERT_TIMELY (5s, node1->ledger.cache.cemented_count == 3 && node1->confirmation_height_processor.current ().is_zero () && node1->confirmation_height_processor.awaiting_processing_size () == 0);
ASSERT_TIMELY (5s, node1->block_confirmed (receive1->hash ()));
// Pruning action
{
auto transaction (node1->store.tx_begin_write ());
Expand Down Expand Up @@ -4965,9 +4932,8 @@ TEST (rpc, pruned_exists)
.work (*node1->work_generate_blocking (send1->hash ()))
.build_shared ();
node1->process_active (receive1);
node1->block_processor.flush ();
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
ASSERT_TIMELY (5s, node1->ledger.cache.cemented_count == 3 && node1->confirmation_height_processor.current ().is_zero () && node1->confirmation_height_processor.awaiting_processing_size () == 0);
ASSERT_TIMELY (5s, node1->block_confirmed (receive1->hash ()));
// Pruning action
{
auto transaction (node1->store.tx_begin_write ());
Expand Down Expand Up @@ -7300,7 +7266,7 @@ TEST (rpc, receive_pruned)
// Extra send frontier
auto send3 (wallet1->send_action (nano::dev::genesis_key.pub, key1.pub, node2->config.receive_minimum.number (), *node2->work_generate_blocking (send1->hash ())));
// Pruning
ASSERT_TIMELY (5s, node2->ledger.cache.cemented_count == 6 && node2->confirmation_height_processor.current ().is_zero () && node2->confirmation_height_processor.awaiting_processing_size () == 0);
ASSERT_TIMELY (5s, node2->block_confirmed (send3->hash ()));
{
auto transaction (node2->store.tx_begin_write ());
ASSERT_EQ (2, node2->ledger.pruning_action (transaction, send2->hash (), 1));
Expand Down

0 comments on commit 6e3ef43

Please sign in to comment.