test: Remove polling loop from test_runner
What changed, and why it matters
This commit is a cleanup of Bitcoin Core's internal functional test runner. It replaces a manual polling loop that repeatedly checked whether test subprocesses had finished with a standard Python thread-pool mechanism. There is no change to the Bitcoin network protocol, wallet handling, consensus rules, or any code that runs in production. It only affects how developers run tests locally and in CI.
No security action required. Treat as a normal test-infrastructure refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors test/functional/test_runner.py’s TestHandler class. Previously it spawned subprocess.Popen instances and polled each proc.poll() in a loop with time.sleep(.5). The change uses concurrent.futures.ThreadPoolExecutor to submit blocking proc.wait() calls and then uses futures.wait(…, return_when=FIRST_COMPLETED) to return finished tests. The visible behavior (parallelism, timeouts, result handling, logging) is intended to remain equivalent. No production code, RPC interface, P2P logic, or cryptographic operation is modified.
Changed components
test/functional/test_runner.pyInspect captured patch +46 / −33
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 7c8c15f3..ea8b2bc1 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -14,6 +14,7 @@ For a description of arguments recognized by test scripts, see
import argparse
from collections import deque
+from concurrent import futures
import configparser
import csv
import datetime
@@ -703,15 +704,15 @@ class TestHandler:
"""
Trigger the test scripts passed in via the list.
"""
-
def __init__(self, *, num_tests_parallel, tests_dir, tmpdir, test_list, flags, use_term_control):
assert num_tests_parallel >= 1
+ self.executor = futures.ThreadPoolExecutor(max_workers=num_tests_parallel)
self.num_jobs = num_tests_parallel
self.tests_dir = tests_dir
self.tmpdir = tmpdir
self.test_list = test_list
self.flags = flags
- self.jobs = []
+ self.jobs = {}
self.use_term_control = use_term_control
def done(self):
@@ -728,47 +729,59 @@ class TestHandler:
test_argv = test.split()
testdir = "{}/{}_{}".format(self.tmpdir, re.sub(".py$", "", test_argv[0]), portseed)
tmpdir_arg = ["--tmpdir={}".format(testdir)]
- self.jobs.append((test,
- time.time(),
- subprocess.Popen([sys.executable, self.tests_dir + test_argv[0]] + test_argv[1:] + self.flags + portseed_arg + tmpdir_arg,
- text=True,
- stdout=log_stdout,
- stderr=log_stderr),
- testdir,
- log_stdout,
- log_stderr))
+
+ def proc_wait(task):
+ task[2].wait()
+ return task
+
+ task = [
+ test,
+ time.time(),
+ subprocess.Popen(
+ [sys.executable, self.tests_dir + test_argv[0]] + test_argv[1:] + self.flags + portseed_arg + tmpdir_arg,
+ text=True,
+ stdout=log_stdout,
+ stderr=log_stderr,
+ ),
+ testdir,
+ log_stdout,
+ log_stderr,
+ ]
+ fut = self.executor.submit(proc_wait, task)
+ self.jobs[fut] = test
if not self.jobs:
raise IndexError('pop from empty list')
# Print remaining running jobs when all jobs have been started.
if not self.test_list:
- print("Remaining jobs: [{}]".format(", ".join(j[0] for j in self.jobs)))
+ print("Remaining jobs: [{}]".format(", ".join(sorted(self.jobs.values()))))
dot_count = 0
while True:
# Return all procs that have finished, if any. Otherwise sleep until there is one.
- time.sleep(.5)
+ procs = futures.wait(self.jobs.keys(), timeout=.5, return_when=futures.FIRST_COMPLETED)
+ self.jobs = {fut: self.jobs[fut] for fut in procs.not_done}
ret = []
- for job in self.jobs:
- (name, start_time, proc, testdir, log_out, log_err) = job
- if proc.poll() is not None:
- log_out.seek(0), log_err.seek(0)
- [stdout, stderr] = [log_file.read().decode('utf-8') for log_file in (log_out, log_err)]
- log_out.close(), log_err.close()
- skip_reason = None
- if proc.returncode == TEST_EXIT_PASSED and stderr == "":
- status = "Passed"
- elif proc.returncode == TEST_EXIT_SKIPPED:
- status = "Skipped"
- skip_reason = re.search(r"Test Skipped: (.*)", stdout).group(1)
- else:
- status = "Failed"
- self.jobs.remove(job)
- if self.use_term_control:
- clearline = '\r' + (' ' * dot_count) + '\r'
- print(clearline, end='', flush=True)
- dot_count = 0
- ret.append((TestResult(name, status, int(time.time() - start_time)), testdir, stdout, stderr, skip_reason))
+ for job in procs.done:
+ (name, start_time, proc, testdir, log_out, log_err) = job.result()
+
+ log_out.seek(0), log_err.seek(0)
+ [stdout, stderr] = [log_file.read().decode('utf-8') for log_file in (log_out, log_err)]
+ log_out.close(), log_err.close()
+ skip_reason = None
+ if proc.returncode == TEST_EXIT_PASSED and stderr == "":
+ status = "Passed"
+ elif proc.returncode == TEST_EXIT_SKIPPED:
+ status = "Skipped"
+ skip_reason = re.search(r"Test Skipped: (.*)", stdout).group(1)
+ else:
+ status = "Failed"
+
+ if self.use_term_control:
+ clearline = '\r' + (' ' * dot_count) + '\r'
+ print(clearline, end='', flush=True)
+ dot_count = 0
+ ret.append((TestResult(name, status, int(time.time() - start_time)), testdir, stdout, stderr, skip_reason))
if ret:
return ret
if self.use_term_control:
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.