dn724/dncurrency
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

Unit test fix setup_chains() (#3982)

Browse source 
* Minor compilation-time fix to EXPECT_TIMELY

EXPECT_TIMELY defined the variable "ec" without taking precautions
to restrict its scope. One symptom was that EXPECT_TIMELY could not be
used twice in a function.

* Fix race condition in setup_chains() in bootstrap_server.cpp

* Fix possible race conditions in use of nano::test::confirm()

Most of these calls are race conditions because they use the following
pattern without allowing for propagation to occur:
process()
confirm()
This commit is contained in:
Dimitrios Siganos 2022-11-09 15:39:02 +00:00 • committed by GitHub
commit 3f0f63c202
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 18 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

4
nano/core_test/active_transactions.cpp
View file

@ -1433,7 +1433,7 @@ nano::keypair setup_rep (nano::test::system & system, nano::node & node, nano::u
.build_shared (); .build_shared ();
EXPECT_TRUE (nano::test::process (node, { send, open })); EXPECT_TRUE (nano::test::process (node, { send, open }));
EXPECT_TRUE (nano::test::confirm (node, { send, open })); EXPECT_TIMELY (5s, nano::test::confirm (node, { send, open }));
EXPECT_TIMELY (5s, nano::test::confirmed (node, { send, open })); EXPECT_TIMELY (5s, nano::test::confirmed (node, { send, open }));
return key; return key;
@ -1482,7 +1482,7 @@ std::vector<std::shared_ptr<nano::block>> setup_independent_blocks (nano::test::
} }
// Confirm whole genesis chain at once // Confirm whole genesis chain at once
EXPECT_TRUE (nano::test::confirm (node, { latest })); EXPECT_TIMELY (5s, nano::test::confirm (node, { latest }));
EXPECT_TIMELY (5s, nano::test::confirmed (node, { latest })); EXPECT_TIMELY (5s, nano::test::confirmed (node, { latest }));
return blocks; return blocks;

4
nano/core_test/bootstrap_server.cpp
View file

@ -41,7 +41,7 @@ block_list_t setup_chain (nano::test::system & system, nano::node & node, nano::
EXPECT_TRUE (nano::test::process (node, blocks)); EXPECT_TRUE (nano::test::process (node, blocks));
// Confirm whole chain at once // Confirm whole chain at once
EXPECT_TRUE (nano::test::confirm (node, { blocks.back () })); EXPECT_TIMELY (5s, nano::test::confirm (node, { blocks.back () }));
EXPECT_TIMELY (5s, nano::test::confirmed (node, blocks)); EXPECT_TIMELY (5s, nano::test::confirmed (node, blocks));
return blocks; return blocks;
@ -84,7 +84,7 @@ std::vector<std::pair<nano::account, block_list_t>> setup_chains (nano::test::sy
// Ensure blocks are in the ledger and confirmed // Ensure blocks are in the ledger and confirmed
EXPECT_TRUE (nano::test::process (node, { send, open })); EXPECT_TRUE (nano::test::process (node, { send, open }));
EXPECT_TRUE (nano::test::confirm (node, { send, open })); EXPECT_TIMELY (5s, nano::test::confirm (node, { send, open }));
EXPECT_TIMELY (5s, nano::test::confirmed (node, { send, open })); EXPECT_TIMELY (5s, nano::test::confirmed (node, { send, open }));
auto added_blocks = setup_chain (system, node, key, block_count); auto added_blocks = setup_chain (system, node, key, block_count);

6
nano/core_test/node.cpp
View file

@ -2573,7 +2573,7 @@ TEST (node, vote_by_hash_bundle)
} }
// Confirming last block will confirm whole chain and allow us to generate votes for those blocks later // Confirming last block will confirm whole chain and allow us to generate votes for those blocks later
ASSERT_TRUE (nano::test::confirm (node, { blocks.back () })); ASSERT_TIMELY (5s, nano::test::confirm (node, { blocks.back () }));
ASSERT_TIMELY (5s, nano::test::confirmed (node, { blocks.back () })); ASSERT_TIMELY (5s, nano::test::confirmed (node, { blocks.back () }));
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv); system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
@ -2752,7 +2752,7 @@ TEST (node, epoch_conflict_confirm)
ASSERT_TRUE (nano::test::process (node1, { send, send2, open })); ASSERT_TRUE (nano::test::process (node1, { send, send2, open }));
// Confirm open block in node1 to allow generating votes // Confirm open block in node1 to allow generating votes
ASSERT_TRUE (nano::test::confirm (node1, { open })); ASSERT_TIMELY (5s, nano::test::confirm (node1, { open }));
ASSERT_TIMELY (5s, nano::test::confirmed (node1, { open })); ASSERT_TIMELY (5s, nano::test::confirmed (node1, { open }));
// Process initial blocks on node0 // Process initial blocks on node0
@ -2766,7 +2766,7 @@ TEST (node, epoch_conflict_confirm)
ASSERT_TIMELY (5s, nano::test::exists (node1, { change, epoch_open })); ASSERT_TIMELY (5s, nano::test::exists (node1, { change, epoch_open }));
// Confirm initial blocks in node1 to allow generating votes later // Confirm initial blocks in node1 to allow generating votes later
ASSERT_TRUE (nano::test::confirm (node1, { change, epoch_open, send2 })); ASSERT_TIMELY (5s, nano::test::confirm (node1, { change, epoch_open, send2 }));
ASSERT_TIMELY (5s, nano::test::confirmed (node1, { change, epoch_open, send2 })); ASSERT_TIMELY (5s, nano::test::confirmed (node1, { change, epoch_open, send2 }));
// Start elections for node0 for conflicting change and epoch_open blocks (those two blocks have the same root) // Start elections for node0 for conflicting change and epoch_open blocks (those two blocks have the same root)

4
nano/slow_test/vote_cache.cpp
View file

@ -38,7 +38,7 @@ nano::keypair setup_rep (nano::test::system & system, nano::node & node, nano::u
.build_shared (); .build_shared ();
EXPECT_TRUE (nano::test::process (node, { send, open })); EXPECT_TRUE (nano::test::process (node, { send, open }));
EXPECT_TRUE (nano::test::confirm (node, { send, open })); EXPECT_TIMELY (5s, nano::test::confirm (node, { send, open }));
EXPECT_TIMELY (5s, nano::test::confirmed (node, { send, open })); EXPECT_TIMELY (5s, nano::test::confirmed (node, { send, open }));
return key; return key;
@ -104,7 +104,7 @@ std::vector<std::shared_ptr<nano::block>> setup_blocks (nano::test::system & sys
EXPECT_TRUE (nano::test::process (node, receives)); EXPECT_TRUE (nano::test::process (node, receives));
// Confirm whole genesis chain at once // Confirm whole genesis chain at once
EXPECT_TRUE (nano::test::confirm (node, { sends.back () })); EXPECT_TIMELY (5s, nano::test::confirm (node, { sends.back () }));
EXPECT_TIMELY (5s, nano::test::confirmed (node, { sends })); EXPECT_TIMELY (5s, nano::test::confirmed (node, { sends }));
std::cout << "setup_blocks done" << std::endl; std::cout << "setup_blocks done" << std::endl;

16
nano/test_common/testutil.hpp
View file

@ -56,13 +56,15 @@
} }
/** Expects that the condition becomes true within the deadline */ /** Expects that the condition becomes true within the deadline */
#define EXPECT_TIMELY(time, condition) \ #define EXPECT_TIMELY(time, condition) \
system.deadline_set (time); \ system.deadline_set (time); \
std::error_code ec; \ { \
while (!(condition) && !(ec = system.poll ())) \ std::error_code _ec; \
{ \ while (!(condition) && !(_ec = system.poll ())) \
} \ { \
EXPECT_NO_ERROR (ec); } \
EXPECT_NO_ERROR (_ec); \
}
/* /*
* Asserts that the `val1 == val2` condition becomes true within the deadline * Asserts that the `val1 == val2` condition becomes true within the deadline