Skip to content

Commit 3cc9a06

Browse files
ryanofskysipa
authored andcommitted
test: Add TestNode ipcbind option
With this change, tests can specify `self.extra_init = [{ipcbind: True}]` to start a node listening on an IPC socket, instead of needing to choose which node binary to invoke and what `self.extra_args=[["-ipcbind=..."]]` value to pass to it. The eliminates boilerplate code #30437 (interface_ipc_mining.py), #32297 (interface_ipc_cli.py), and #33201 (interface_ipc.py) previously needed in their test setup.
1 parent 3cceb60 commit 3cc9a06

File tree

2 files changed

+46
-14
lines changed

2 files changed

+46
-14
lines changed

test/functional/test_framework/test_framework.py

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ def __init__(self, paths, bin_dir):
7676
self.paths = paths
7777
self.bin_dir = bin_dir
7878

79-
def node_argv(self):
79+
def node_argv(self, **kwargs):
8080
"Return argv array that should be used to invoke bitcoind"
81-
return self._argv("node", self.paths.bitcoind)
81+
return self._argv("node", self.paths.bitcoind, **kwargs)
8282

8383
def rpc_argv(self):
8484
"Return argv array that should be used to invoke bitcoin-cli"
@@ -101,16 +101,19 @@ def chainstate_argv(self):
101101
"Return argv array that should be used to invoke bitcoin-chainstate"
102102
return self._argv("chainstate", self.paths.bitcoinchainstate)
103103

104-
def _argv(self, command, bin_path):
104+
def _argv(self, command, bin_path, need_ipc=False):
105105
"""Return argv array that should be used to invoke the command. It
106-
either uses the bitcoin wrapper executable (if BITCOIN_CMD is set), or
107-
the direct binary path (bitcoind, etc). When bin_dir is set (by tests
108-
calling binaries from previous releases) it always uses the direct
109-
path."""
106+
either uses the bitcoin wrapper executable (if BITCOIN_CMD is set or
107+
need_ipc is True), or the direct binary path (bitcoind, etc). When
108+
bin_dir is set (by tests calling binaries from previous releases) it
109+
always uses the direct path."""
110110
if self.bin_dir is not None:
111111
return [os.path.join(self.bin_dir, os.path.basename(bin_path))]
112-
elif self.paths.bitcoin_cmd is not None:
113-
return self.paths.bitcoin_cmd + [command]
112+
elif self.paths.bitcoin_cmd is not None or need_ipc:
113+
# If the current test needs IPC functionality, use the bitcoin
114+
# wrapper binary and append -m so it calls multiprocess binaries.
115+
bitcoin_cmd = self.paths.bitcoin_cmd or [self.paths.bitcoin_bin]
116+
return bitcoin_cmd + (["-m"] if need_ipc else []) + [command]
114117
else:
115118
return [bin_path]
116119

@@ -158,6 +161,7 @@ def __init__(self, test_file) -> None:
158161
self.noban_tx_relay: bool = False
159162
self.nodes: list[TestNode] = []
160163
self.extra_args = None
164+
self.extra_init = None
161165
self.network_thread = None
162166
self.rpc_timeout = 60 # Wait for up to 60 seconds for the RPC server to respond
163167
self.supports_cli = True
@@ -553,15 +557,15 @@ def bin_dir_from_version(version):
553557

554558
bin_dirs.append(bin_dir)
555559

560+
extra_init = [{}] * num_nodes if self.extra_init is None else self.extra_init # type: ignore[var-annotated]
561+
assert_equal(len(extra_init), num_nodes)
556562
assert_equal(len(extra_confs), num_nodes)
557563
assert_equal(len(extra_args), num_nodes)
558564
assert_equal(len(versions), num_nodes)
559565
assert_equal(len(bin_dirs), num_nodes)
560566
for i in range(num_nodes):
561567
args = list(extra_args[i])
562-
test_node_i = TestNode(
563-
i,
564-
get_datadir_path(self.options.tmpdir, i),
568+
init = dict(
565569
chain=self.chain,
566570
rpchost=rpchost,
567571
timewait=self.rpc_timeout,
@@ -578,6 +582,11 @@ def bin_dir_from_version(version):
578582
v2transport=self.options.v2transport,
579583
uses_wallet=self.uses_wallet,
580584
)
585+
init.update(extra_init[i])
586+
test_node_i = TestNode(
587+
i,
588+
get_datadir_path(self.options.tmpdir, i),
589+
**init)
581590
self.nodes.append(test_node_i)
582591
if not test_node_i.version_is_at_least(170000):
583592
# adjust conf for pre 17

test/functional/test_framework/test_node.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import json
1212
import logging
1313
import os
14+
import pathlib
1415
import platform
1516
import re
1617
import subprocess
@@ -19,6 +20,7 @@
1920
import urllib.parse
2021
import collections
2122
import shlex
23+
import shutil
2224
import sys
2325
from pathlib import Path
2426

@@ -52,6 +54,13 @@
5254
NULL_BLK_XOR_KEY = bytes([0] * NUM_XOR_BYTES)
5355
BITCOIN_PID_FILENAME_DEFAULT = "bitcoind.pid"
5456

57+
if sys.platform.startswith("linux"):
58+
UNIX_PATH_MAX = 108 # includes the trailing NUL
59+
elif sys.platform.startswith(("darwin", "freebsd", "netbsd", "openbsd")):
60+
UNIX_PATH_MAX = 104
61+
else: # safest portable value
62+
UNIX_PATH_MAX = 92
63+
5564

5665
class FailedToStartError(Exception):
5766
"""Raised when a node fails to start correctly."""
@@ -77,7 +86,7 @@ class TestNode():
7786
To make things easier for the test writer, any unrecognised messages will
7887
be dispatched to the RPC connection."""
7988

80-
def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor, binaries, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, v2transport=False, uses_wallet=False):
89+
def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor, binaries, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, v2transport=False, uses_wallet=False, ipcbind=False):
8190
"""
8291
Kwargs:
8392
start_perf (bool): If True, begin profiling the node with `perf` as soon as
@@ -109,7 +118,7 @@ def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor,
109118
# Configuration for logging is set as command-line args rather than in the bitcoin.conf file.
110119
# This means that starting a bitcoind using the temp dir to debug a failed test won't
111120
# spam debug.log.
112-
self.args = self.binaries.node_argv() + [
121+
self.args = self.binaries.node_argv(need_ipc=ipcbind) + [
113122
f"-datadir={self.datadir_path}",
114123
"-logtimemicros",
115124
"-debug",
@@ -121,6 +130,17 @@ def __init__(self, i, datadir_path, *, chain, rpchost, timewait, timeout_factor,
121130
if uses_wallet is not None and not uses_wallet:
122131
self.args.append("-disablewallet")
123132

133+
self.ipc_tmp_dir = None
134+
if ipcbind:
135+
self.ipc_socket_path = self.chain_path / "node.sock"
136+
if len(os.fsencode(self.ipc_socket_path)) < UNIX_PATH_MAX:
137+
self.args.append("-ipcbind=unix")
138+
else:
139+
# Work around default CI path exceeding maximum socket path length.
140+
self.ipc_tmp_dir = pathlib.Path(tempfile.mkdtemp(prefix="test-ipc-"))
141+
self.ipc_socket_path = self.ipc_tmp_dir / "node.sock"
142+
self.args.append(f"-ipcbind=unix:{self.ipc_socket_path}")
143+
124144
# Use valgrind, expect for previous release binaries
125145
if use_valgrind and version is None:
126146
default_suppressions_file = Path(__file__).parents[3] / "contrib" / "valgrind.supp"
@@ -208,6 +228,9 @@ def __del__(self):
208228
# this destructor is called.
209229
print(self._node_msg("Cleaning up leftover process"), file=sys.stderr)
210230
self.process.kill()
231+
if self.ipc_tmp_dir:
232+
print(self._node_msg(f"Cleaning up ipc directory {str(self.ipc_tmp_dir)!r}"))
233+
shutil.rmtree(self.ipc_tmp_dir)
211234

212235
def __getattr__(self, name):
213236
"""Dispatches any unrecognised messages to the RPC connection or a CLI instance."""

0 commit comments

Comments
 (0)