From d902128b12df51dbdcb604d0d88059a4860926d3 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Fri, 19 Mar 2021 23:34:20 +0100 Subject: [PATCH] [Test] Fix intermittent sync_blocks failures >>> backports bitcoin/bitcoin@fa3f9a05660687bf4146e089050e944a1d6cbe3c Github-Pull: #2254 Rebased-From: d076c814bd9b8bdbcf98c7df87297cc8cd2a533c --- test/functional/feature_reindex.py | 6 ++---- test/functional/mempool_persist.py | 11 +++++------ test/functional/test_framework/test_node.py | 21 +++++++++++++++++++-- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py index 234d1ad899f30..5d2333a481c8a 100755 --- a/test/functional/feature_reindex.py +++ b/test/functional/feature_reindex.py @@ -10,7 +10,7 @@ """ from test_framework.test_framework import PivxTestFramework -from test_framework.util import wait_until +from test_framework.util import assert_equal import time class ReindexTest(PivxTestFramework): @@ -23,11 +23,9 @@ def reindex(self): self.nodes[0].generate(3) blockcount = self.nodes[0].getblockcount() self.stop_nodes() - time.sleep(5) extra_args = [["-reindex", "-checkblockindex=1"]] self.start_nodes(extra_args) - time.sleep(15) - wait_until(lambda: self.nodes[0].getblockcount() == blockcount) + assert_equal(self.nodes[0].getblockcount(), blockcount) # start_node is blocking on reindex self.log.info("Success") def run_test(self): diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index d6d2e2105b2f4..e81a9244d938f 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -70,9 +70,8 @@ def run_test(self): self.start_node(1, extra_args=["-persistmempool=0"]) self.start_node(0) self.start_node(2) - - wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"], timeout=1) - wait_until(lambda: self.nodes[2].getmempoolinfo()["loaded"], timeout=1) + assert self.nodes[0].getmempoolinfo()["loaded"] # start_node is blocking on the mempool being loaded + assert self.nodes[2].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[0].getrawmempool()), 5) assert_equal(len(self.nodes[2].getrawmempool()), 5) # The others have loaded their mempool. If node_1 loaded anything, we'd probably notice by now: @@ -85,13 +84,13 @@ def run_test(self): self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.") self.stop_nodes() self.start_node(0, extra_args=["-persistmempool=0"]) - wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"]) + assert self.nodes[0].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[0].getrawmempool()), 0) self.log.debug("Stop-start node0. Verify that it has the transactions in its mempool.") self.stop_nodes() self.start_node(0) - wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"]) + assert self.nodes[0].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[0].getrawmempool()), 5) # Following code is ahead of our current repository state. Future back port. @@ -107,7 +106,7 @@ def run_test(self): os.rename(mempooldat0, mempooldat1) self.stop_nodes() self.start_node(1, extra_args=[]) - wait_until(lambda: self.nodes[1].getmempoolinfo()["loaded"]) + assert self.nodes[0].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[1].getrawmempool()), 5) self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails") diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 5436a3b40c6b8..e9664b7200235 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -121,8 +121,25 @@ def wait_for_rpc_connection(self): assert self.process.poll() is None, "pivxd exited with status %i during initialization" % self.process.returncode try: self.rpc = get_rpc_proxy(rpc_url(self.datadir, self.index, self.rpchost), self.index, timeout=self.rpc_timeout, coveragedir=self.coverage_dir) - while self.rpc.getblockcount() < 0: - time.sleep(1) + self.rpc.getblockcount() + wait_until(lambda: self.rpc.getmempoolinfo()['loaded']) + # Wait for the node to finish reindex, block import, and + # loading the mempool. Usually importing happens fast or + # even "immediate" when the node is started. However, there + # is no guarantee and sometimes ThreadImport might finish + # later. This is going to cause intermittent test failures, + # because generally the tests assume the node is fully + # ready after being started. + # + # For example, the node will reject block messages from p2p + # when it is still importing with the error "Unexpected + # block message received" + # + # The wait is done here to make tests as robust as possible + # and prevent racy tests and intermittent failures as much + # as possible. Some tests might not need this, but the + # overhead is trivial, and the added gurantees are worth + # the minimal performance cost. # If the call to getblockcount() succeeds then the RPC connection is up self.rpc_connected = True self.url = self.rpc.url