Repository: bitcoin/bitcoin
File: test/functional/test_framework/test_node.py (Python)

Code before:
            raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))

        self.stdout.close()
        self.stderr.close()

        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, expect_error=False, **kwargs):
        if "expected_ret_code" not in kwargs:
            kwargs["expected_ret_code"] = 1 if expect_error else 0  # Whether node shutdown return EXIT_FAILURE or EXIT_SUCCESS
        self.wait_until(lambda: self.is_node_stopped(**kwargs), timeout=timeout)

    def kill_process(self):
        self.process.kill()
        self.wait_until_stopped(expected_ret_code=1 if platform.system() == "Windows" else -9)
        assert self.is_node_stopped()

    def replace_in_config(self, replacements):
        """
        Perform replacements in the configuration file.
        The substitutions are passed as a list of search-replace-tuples, e.g.
            [("old", "new"), ("foo", "bar"), ...]
        """
        with open(self.bitcoinconf, 'r', encoding='utf8') as conf:
            conf_data = conf.read()
        for replacement in replacements:
            assert_equal(len(replacement), 2)
            old, new = replacement[0], replacement[1]
            conf_data = conf_data.replace(old, new)
        with open(self.bitcoinconf, 'w', encoding='utf8') as conf:
            conf.write(conf_data)

    @property
    def chain_path(self) -> Path:
        return self.datadir_path / self.chain

    @property
    def debug_log_path(self) -> Path:
        return self.chain_path / 'debug.log'

    @property
    def blocks_path(self) -> Path:
        return self.chain_path / "blocks"

    @property
    def blocks_key_path(self) -> Path:

Review comment (bug):
In commit "test: Allow changing expected internal return code" (db0e1580a5212a162908ac4f23e935e1f76e06dc)

Now that this logic is getting more complicated with `self.expected_ret_code` I think the `expect_error` argument should be dropped so the code can be more understandable. It looks like there is only a single `expect_error` usage in `feature_abortnode.py` and it can be easily replaced by using `expected_ret_code`. Would suggest:

```diff
--- a/test/functional/feature_abortnode.py
+++ b/test/functional/feature_abortnode.py
@@ -36,7 +36,7 @@ class AbortNodeTest(BitcoinTestFramework):
 
             # Check that node0 aborted
             self.log.info("Waiting for crash")
-            self.nodes[0].wait_until_stopped(timeout=5, expect_error=True, expected_stderr="Error: A fatal internal error occurred, see debug.log for details: Failed to disconnect block.")
+            self.nodes[0].wait_until_stopped(timeout=5, expected_ret_code=1, expected_stderr="Error: A fatal internal error occurred, see debug.log for details: Failed to disconnect block.")
         self.log.info("Node crashed - now verifying restart fails")
         self.nodes[0].assert_start_raises_init_error()
 
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -460,11 +460,11 @@ class TestNode():
         self.log.debug("Node stopped")
         return True
 
-    def wait_until_stopped(self, *, timeout=BITCOIND_PROC_WAIT_TIMEOUT, expect_error=False, **kwargs):
+    def wait_until_stopped(self, *, timeout=BITCOIND_PROC_WAIT_TIMEOUT, **kwargs):
         if "expected_ret_code" not in kwargs:
             # Whether node shutdown is expected to return EXIT_FAILURE (1) or
             # EXIT_SUCCESS (self.expected_ret_code).
-            kwargs["expected_ret_code"] = 1 if expect_error else self.expected_ret_code
+            kwargs["expected_ret_code"] = self.expected_ret_code
         self.wait_until(lambda: self.is_node_stopped(**kwargs), timeout=timeout)
 
     def replace_in_config(self, replacements):
```


Code after:
        Returns True if the node has stopped. False otherwise.
        This method is responsible for freeing resources (self.process)."""
        if not self.running:
            return True
        return_code = self.process.poll()
        if return_code is None:
            return False

        # process has stopped. Assert that it didn't return an error code.
        assert return_code == expected_ret_code, self._node_msg(
            f"Node returned unexpected exit code ({return_code}) vs ({expected_ret_code}) when stopping")
        # Check that stderr is as expected
        self.stderr.seek(0)
        stderr = self.stderr.read().decode('utf-8').strip()
        if stderr != expected_stderr:
            raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))

        self.stdout.close()
        self.stderr.close()

        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, expect_error=False, **kwargs):
        if "expected_ret_code" not in kwargs:
            kwargs["expected_ret_code"] = 1 if expect_error else 0  # Whether node shutdown return EXIT_FAILURE or EXIT_SUCCESS
        self.wait_until(lambda: self.is_node_stopped(**kwargs), timeout=timeout)

    def kill_process(self):
        self.process.kill()
        self.wait_until_stopped(expected_ret_code=1 if platform.system() == "Windows" else -9)
        assert self.is_node_stopped()

    def replace_in_config(self, replacements):
        """
        Perform replacements in the configuration file.
        The substitutions are passed as a list of search-replace-tuples, e.g.
            [("old", "new"), ("foo", "bar"), ...]
        """
        with open(self.bitcoinconf, 'r', encoding='utf8') as conf:
            conf_data = conf.read()
        for replacement in replacements:
            assert_equal(len(replacement), 2)
            old, new = replacement[0], replacement[1]
            conf_data = conf_data.replace(old, new)
        with open(self.bitcoinconf, 'w', encoding='utf8') as conf:
            conf.write(conf_data)