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

Code before:
        else:
            assert self.rpc_connected and self.rpc, self._node_msg("RPC not connected")
            wallet_path = "wallet/{}".format(urllib.parse.quote(wallet_name))
            return RPCOverloadWrapper(self.rpc / wallet_path)

    def version_is_at_least(self, ver):
        return self.version is None or self.version >= ver

    def stop_node(self, expected_stderr='', *, wait=0, wait_until_stopped=True):
        """Stop the node."""
        if not self.running:
            return
        self.log.debug("Stopping node")
        try:
            # Do not use wait argument when testing older nodes, e.g. in wallet_backwards_compatibility.py
            if self.version_is_at_least(180000):
                self.stop(wait=wait)
            else:
                self.stop()
        except http.client.CannotSendRequest:
            self.log.exception("Unable to stop node.")

        # If there are any running perf processes, stop them.
        for profile_name in tuple(self.perf_subprocesses.keys()):
            self._stop_perf(profile_name)

        del self.p2ps[:]

        assert (not expected_stderr) or wait_until_stopped  # Must wait to check stderr
        if wait_until_stopped:
            self.wait_until_stopped(expected_stderr=expected_stderr)

    def is_node_stopped(self, *, expected_stderr="", expected_ret_code=0):
        """Checks whether the node has stopped.

        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))

Review comment (refactor):
In commit "qa: Avoid calling stop-RPC if not connected" (8102f3d8e0b6715433eeda78b9b57a83109fc45e)

This change seems good, but unclear what motivation is. Is it supposed to be a refactoring that makes code more robust, or were there actually any cases where CannotSendRequest exceptions were caught and suppressed? If so what were those cases? Commit description should say what goals of this change are and if CannotSendRequest is an actual or theoretical change in behavior.

Code after:
        assert called_by_framework, "Direct call of this mining RPC is discouraged. Please use one of the self.generate* methods on the test framework, which sync the nodes to avoid intermittent test issues. You may use sync_fun=self.no_op to disable the sync explicitly."
        return self.__getattr__('generatetodescriptor')(*args, **kwargs)

    def setmocktime(self, timestamp):
        """Wrapper for setmocktime RPC, sets self.mocktime"""
        if timestamp == 0:
            # setmocktime(0) resets to system time.
            self.mocktime = None
        else:
            self.mocktime = timestamp
        return self.__getattr__('setmocktime')(timestamp)

    def get_wallet_rpc(self, wallet_name):
        if self.use_cli:
            return RPCOverloadWrapper(self.cli("-rpcwallet={}".format(wallet_name)), True, self.descriptors)
        else:
            assert self.rpc_connected and self.rpc, self._node_msg("RPC not connected")
            wallet_path = "wallet/{}".format(urllib.parse.quote(wallet_name))
            return RPCOverloadWrapper(self.rpc / wallet_path, descriptors=self.descriptors)

    def version_is_at_least(self, ver):
        return self.version is None or self.version >= ver

    def stop_node(self, expected_stderr='', *, wait=0, wait_until_stopped=True):
        """Stop the node."""
        if not self.running:
            return
        assert self.rpc_connected, self._node_msg(
            "Should only call stop_node() on a running node after wait_for_rpc_connection() succeeded. "
            f"Did you forget to call the latter after start()? Not connected to process: {self.process.pid}")
        self.log.debug("Stopping node")
        # Do not use wait argument when testing older nodes, e.g. in wallet_backwards_compatibility.py
        if self.version_is_at_least(180000):
            self.stop(wait=wait)
        else:
            self.stop()

        # If there are any running perf processes, stop them.
        for profile_name in tuple(self.perf_subprocesses.keys()):
            self._stop_perf(profile_name)

        del self.p2ps[:]

        assert (not expected_stderr) or wait_until_stopped  # Must wait to check stderr
        if wait_until_stopped:
            self.wait_until_stopped(expected_stderr=expected_stderr)

    def is_node_stopped(self, *, expected_stderr="", expected_ret_code=0):
        """Checks whether the node has stopped.

        Returns True if the node has stopped. False otherwise.