Merge #11067: [qa] TestNode: Add wait_until_stopped helper method

faa8d9581 [qa] TestNode: Add wait_until_stopped helper method (MarcoFalke)

Pull request description:

  This adds a helper method `wait_until_stopped` to the `TestNode` class. This should prevent numerous `time.sleep()` over all places.

  Additionally, the timeout behavior is restored. (Was removed by the introduction of `TestNode`.)
  This should prevent tests from running indefinitely by accident.

Tree-SHA512: 7133fc64d55711869c4e372e9d30625c98f1237fb3578c24a26900d9319831f10eb95592d7b08e536fa706158dffb0abf9197f11c5d9ef605c880628e1a6533f

5 files changed, 24 insertions, 22 deletions

diff --git a/test/functional/blockchain.py b/test/functional/blockchain.py
index 5d04de9940..50be9262e4 100755
--- a/test/functional/blockchain.py
+++ b/test/functional/blockchain.py
@@ -21,7 +21,7 @@ from decimal import Decimal
 import http.client
 import subprocess
 
-from test_framework.test_framework import (BitcoinTestFramework, BITCOIND_PROC_WAIT_TIMEOUT)
+from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises,
@@ -141,7 +141,7 @@ class BlockchainTest(BitcoinTestFramework):
         except (ConnectionError, http.client.BadStatusLine):
             pass  # The node already shut down before response
         self.log.debug('Node should stop at this height...')
-        self.nodes[0].process.wait(timeout=BITCOIND_PROC_WAIT_TIMEOUT)
+        self.nodes[0].wait_until_stopped()
         self.start_node(0)
         assert_equal(self.nodes[0].getblockcount(), 207)
 
diff --git a/test/functional/fundrawtransaction.py b/test/functional/fundrawtransaction.py
index 75a0dc5f9d..71e88009b6 100755
--- a/test/functional/fundrawtransaction.py
+++ b/test/functional/fundrawtransaction.py
@@ -4,7 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the fundrawtransaction RPC."""
 
-from test_framework.test_framework import BitcoinTestFramework, BITCOIND_PROC_WAIT_TIMEOUT
+from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 
 
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 103651f175..a53eb51799 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -43,8 +43,6 @@ TEST_EXIT_PASSED = 0
 TEST_EXIT_FAILED = 1
 TEST_EXIT_SKIPPED = 77
 
-BITCOIND_PROC_WAIT_TIMEOUT = 60
-
 class BitcoinTestFramework(object):
     """Base class for a bitcoin test script.
 
@@ -263,8 +261,7 @@ class BitcoinTestFramework(object):
     def stop_node(self, i):
         """Stop a bitcoind test node"""
         self.nodes[i].stop_node()
-        while not self.nodes[i].is_node_stopped():
-            time.sleep(0.1)
+        self.nodes[i].wait_until_stopped()
 
     def stop_nodes(self):
         """Stop multiple bitcoind test nodes"""
@@ -274,8 +271,7 @@ class BitcoinTestFramework(object):
 
         for node in self.nodes:
             # Wait for nodes to stop
-            while not node.is_node_stopped():
-                time.sleep(0.1)
+            node.wait_until_stopped()
 
     def assert_start_raises_init_error(self, i, extra_args=None, expected_msg=None):
         with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index efb3ac9d16..f58a372a14 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -17,9 +17,12 @@ from .util import (
     assert_equal,
     get_rpc_proxy,
     rpc_url,
+    wait_until,
 )
 from .authproxy import JSONRPCException
 
+BITCOIND_PROC_WAIT_TIMEOUT = 60
+
 class TestNode():
     """A class for representing a bitcoind node under test.
 
@@ -125,14 +128,20 @@ class TestNode():
         if not self.running:
             return True
         return_code = self.process.poll()
-        if return_code is not None:
-            # process has stopped. Assert that it didn't return an error code.
-            assert_equal(return_code, 0)
-            self.running = False
-            self.process = None
-            self.log.debug("Node stopped")
-            return True
-        return False
+        if return_code is None:
+            return False
+
+        # process has stopped. Assert that it didn't return an error code.
+        assert_equal(return_code, 0)
+        self.running = False
+        self.process = None
+        self.rpc_connected = False
+        self.rpc = None
+        self.log.debug("Node stopped")
+        return True
+
+    def wait_until_stopped(self, timeout=BITCOIND_PROC_WAIT_TIMEOUT):
+        wait_until(self.is_node_stopped, timeout=timeout)
 
     def node_encrypt_wallet(self, passphrase):
         """"Encrypts the wallet.
@@ -140,10 +149,7 @@ class TestNode():
         This causes bitcoind to shutdown, so this method takes
         care of cleaning up resources."""
         self.encryptwallet(passphrase)
-        while not self.is_node_stopped():
-            time.sleep(0.1)
-        self.rpc = None
-        self.rpc_connected = False
+        self.wait_until_stopped()
 
 class TestNodeCLI():
     """Interface to bitcoin-cli for an individual node"""
diff --git a/test/functional/wallet-encryption.py b/test/functional/wallet-encryption.py
index f63bb2ea5e..ce1e7744e9 100755
--- a/test/functional/wallet-encryption.py
+++ b/test/functional/wallet-encryption.py
@@ -6,7 +6,7 @@
 
 import time
 
-from test_framework.test_framework import BitcoinTestFramework, BITCOIND_PROC_WAIT_TIMEOUT
+from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_raises_jsonrpc,