Skip to content

Commit e72103a

Browse files
committed
test: fix qml peer test reliability
The two-peer disconnect test was mixing up GUI peer ids and local peer processes, which could cause it to terminate the wrong bitcoind instance after clicking Disconnect. It also allowed peer2 processes to leak across phases and across runs, which made later iterations inherit extra inbound peers and fail nondeterministically. Fix this by: - mapping GUI peers back to local processes via session_id - cleaning up any existing peer2 process before starting a new one - asserting the exact remaining peer after the targeted disconnect - leaving the harness in a clean state after the two-peer disconnect test
1 parent a4afbe0 commit e72103a

1 file changed

Lines changed: 65 additions & 5 deletions

File tree

test/functional/qml_test_peers.py

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -228,14 +228,18 @@ def stop(self):
228228

229229
def rpc_call(self, method, params=None):
230230
"""Make a JSON-RPC call to the GUI node and return the result."""
231+
return self._rpc_call_to_port(_rpc_port(0), method, params)
232+
233+
def _rpc_call_to_port(self, port, method, params=None):
234+
"""Make a JSON-RPC call to a specific node RPC port."""
231235
payload = json.dumps({
232236
"jsonrpc": "1.0",
233237
"id": "qml_test",
234238
"method": method,
235239
"params": params or [],
236240
}).encode("utf-8")
237241

238-
conn = http.client.HTTPConnection("127.0.0.1", _rpc_port(0), timeout=10)
242+
conn = http.client.HTTPConnection("127.0.0.1", port, timeout=10)
239243
credentials = base64.b64encode(
240244
f"{GUI_RPC_USER}:{GUI_RPC_PASS}".encode("utf-8")
241245
).decode("ascii")
@@ -254,6 +258,15 @@ def rpc_call(self, method, params=None):
254258
raise RuntimeError(f"RPC error: {body['error']}")
255259
return body["result"]
256260

261+
def peer_rpc_call(self, peer_idx, method, params=None):
262+
"""Make a JSON-RPC call to a peer node.
263+
264+
peer_idx:
265+
1 -> harness.peer_process
266+
2 -> harness.peer2_process
267+
"""
268+
return self._rpc_call_to_port(_rpc_port(peer_idx), method, params)
269+
257270
def wait_for_peer(self, timeout=30):
258271
"""Poll getpeerinfo until at least one peer is connected."""
259272
deadline = time.time() + timeout
@@ -363,6 +376,8 @@ def restart_gui(self):
363376

364377
def start_additional_peer(self):
365378
"""Start a second peer bitcoind that connects to the GUI node."""
379+
_terminate_process(self.peer2_process)
380+
366381
datadir = os.path.join(self.tmpdir, "peer2_node")
367382
os.makedirs(datadir, exist_ok=True)
368383
conf_path = os.path.join(datadir, "bitcoin.conf")
@@ -395,14 +410,48 @@ def wait_for_n_peers(self, n, timeout=30):
395410
try:
396411
peers = self.rpc_call("getpeerinfo")
397412
if len(peers) >= n:
398-
ids = [p["id"] for p in peers]
413+
ids = sorted(p["id"] for p in peers)
399414
print(f" {n} peer(s) connected: ids={ids}")
400415
return ids
401416
except Exception:
402417
pass
403418
time.sleep(0.5)
404419
raise RuntimeError(f"Expected {n} peers after {timeout}s")
405420

421+
def wait_for_exact_peer_ids(self, expected_ids, timeout=PEER_ACTION_TIMEOUT_SECS):
422+
"""Poll getpeerinfo until the peer set matches expected_ids."""
423+
expected = sorted(expected_ids)
424+
deadline = time.time() + timeout
425+
while time.time() < deadline:
426+
try:
427+
actual = sorted(p["id"] for p in self.rpc_call("getpeerinfo"))
428+
if actual == expected:
429+
return
430+
except Exception:
431+
pass
432+
time.sleep(0.3)
433+
raise RuntimeError(
434+
f"Expected peer ids {expected} after {timeout}s, "
435+
f"got {sorted(p['id'] for p in self.rpc_call('getpeerinfo'))}"
436+
)
437+
438+
def gui_peer_for_process(self, peer_idx):
439+
"""Return the GUI-side getpeerinfo entry for a peer process via session_id."""
440+
peer_connections = self.peer_rpc_call(peer_idx, "getpeerinfo")
441+
if len(peer_connections) != 1:
442+
raise RuntimeError(
443+
f"Expected peer {peer_idx} to have exactly 1 connection, got: {peer_connections}"
444+
)
445+
446+
session_id = peer_connections[0]["session_id"]
447+
for peer in self.rpc_call("getpeerinfo"):
448+
if peer.get("session_id") == session_id:
449+
return peer
450+
451+
raise RuntimeError(
452+
f"Could not match peer {peer_idx} session {session_id} to GUI peer list"
453+
)
454+
406455

407456
# ── Navigation helpers ────────────────────────────────────────────────────────
408457

@@ -543,9 +592,15 @@ def test_disconnect_specific_peer(gui, harness):
543592
print("\n── test_disconnect_specific_peer ────────────────────────────")
544593

545594
harness.start_additional_peer()
546-
peer_ids = harness.wait_for_n_peers(2)
547-
target_id = peer_ids[0]
548-
other_id = peer_ids[1]
595+
harness.wait_for_n_peers(2)
596+
597+
# Use session_id to map the GUI-side node id back to a specific peer
598+
# process. Relying on getpeerinfo ordering is brittle and can disconnect
599+
# the correct peer while terminating the wrong process.
600+
target_peer = harness.gui_peer_for_process(1)
601+
other_peer = harness.gui_peer_for_process(2)
602+
target_id = target_peer["id"]
603+
other_id = other_peer["id"]
549604

550605
gui.wait_for_property(f"peerListItem_{target_id}", "visible", True, timeout_ms=PEER_LIST_ITEM_VISIBLE_TIMEOUT_MS)
551606

@@ -558,6 +613,7 @@ def test_disconnect_specific_peer(gui, harness):
558613
_terminate_process(harness.peer_process)
559614

560615
_wait_for_peer_gone(harness, target_id)
616+
harness.wait_for_exact_peer_ids([other_id])
561617

562618
peers = harness.rpc_call("getpeerinfo")
563619
remaining_ids = [p["id"] for p in peers]
@@ -569,6 +625,10 @@ def test_disconnect_specific_peer(gui, harness):
569625
f"Peer {other_id} should still be connected"
570626
print(f" PASSED: peer {target_id} gone; peer {other_id} still present")
571627

628+
# Leave the harness in a clean state for subsequent tests and future runs.
629+
_terminate_process(harness.peer2_process)
630+
harness.wait_for_no_peers()
631+
572632

573633
def test_ban_one_of_two_peers(gui, harness):
574634
"""Ban one peer and verify the other is also disconnected due to subnet ban.

0 commit comments

Comments
 (0)