mempool_persist.py was failing very intermittently because after starting the node, there was a race between calling getrawmempool and the mempool being reloaded by a background thread.
This PR removes that raciness in the test. It also removes the is_network_split parameter and tidies up the start of the test (this test was written between #10198 being opened and merged)
65 | @@ -72,20 +66,24 @@ def run_test(self): 66 | self.nodes = [] 67 | self.nodes.append(start_node(0, self.options.tmpdir)) 68 | self.nodes.append(start_node(1, self.options.tmpdir)) 69 | - assert_equal(len(self.nodes[0].getrawmempool()), 5) 70 | - assert_equal(len(self.nodes[1].getrawmempool()), 0) 71 | + # Give bitcoind a second to reload the mempool 72 | + time.sleep(1) 73 | + assert wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5) 74 | + assert wait_until(lambda: len(self.nodes[1].getrawmempool()) == 0)
Does the wait_until accomplish anything here, or at line 80 below? Either the mempool will be empty when this RPC is first invoked, in which case the wait_until will immediately return, or else it will have something -- in which case waiting and then failing later doesn't seem useful?
No - this could just as well be a straight assert that node1's mempool is empty. The time.sleep(1) above is what actually guarantees that we're not just hitting a window before node1's mempool fills up.
I changed this to a wait_until() call for consistency with the line above, but could quite easily change it back if you think that would be clearer for the reader.
Yeah imo it would be clearer without the wait_until.
51 | - self.nodes.append(start_node(1, self.options.tmpdir, ["-persistmempool=false"])) 52 | - self.nodes.append(start_node(2, self.options.tmpdir)) 53 | - connect_nodes_bi(self.nodes, 0, 2) 54 | - connect_nodes_bi(self.nodes, 1, 2) 55 | - self.is_network_split = False 56 | + self.extra_args = [[], ["-persistmempool=false"], []]
Our parser does not support string representations of booleans. You should only use integers here...
:%s/false/0/g
utACK after global substitution.
Added fixup commit:
persistmempool=false with -persistmempool=0assert wait_until(lambda: len(self.nodes[1].getrawmempool()) == 0) to assert_equal(len(self.nodes[1].getrawmempool()), 0)sync_all() call after self.nodes[0].generate(1) to ensure that we really are out of IBD.Will squash once this is ACKed and ready for merge.
ACK
utACK 6658e7fc4f3117beb82d02d71f2de2f0500d37c1, please squash.
squashed