test: Remove redundant warning about missing binaries
What the developer wrote
Authored by MarcoFalke
test: Remove redundant warning about missing binaries
The error was added in commit 1ea7e45a1f445d32a2b690d52befb2e63418653b,
because there was an additional confusing `AssertionError: [node 0]
Error: no RPC connection` instead of just a single `FileNotFoundError:
[Errno 2] No such file or directory`.
This is no longer needed on current master.
Also, the test is incomplete, because it was just checking bitcoind and
bitcoin-cli, not any other missing binaries.
Also, after the previous commit, it would not work in combination with
--valgrind.
Instead of trying to make it complete, and work in all combinations,
just remove it, because the already existing error will be clear in any
case.
This can be tested via:
```sh
./test/get_previous_releases.py
mv releases releases_backup
# Confirm the test is skipped due to missing releases
./bld-cmake/test/functional/wallet_migration.py
# Confirm the test fails due to missing releases
./bld-cmake/test/functional/wallet_migration.py --previous-releases
mv releases_backup releases
mv ./releases/v28.2 ./releases/v28.2_backup
# Confirm the test fails with a single FileNotFoundError
./bld-cmake/test/functional/wallet_migration.py
mv ./releases/v28.2_backup ./releases/v28.2
# Confirm the test runs and passes
./bld-cmake/test/functional/wallet_migration.py
rm ./bld-cmake/bin/bitcoind
# Confirm the test fails with a single "No such file or directory",
# testing with and without --valgrind
./bld-cmake/test/functional/wallet_migration.py
./bld-cmake/test/functional/wallet_migration.py --valgrind
```
What changed, and why it matters
This commit removes a redundant warning in Bitcoin Core's test framework. It does not change production code, network behavior, or wallet security. The change only affects how functional tests report missing test binaries, and the commit message explicitly says the removed check is no longer needed because the underlying error message is now clear enough on its own.
No security action required. Reviewers may optionally verify that missing binaries now produce a single, comprehensible FileNotFoundError as described in the commit message.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch deletes an explicit pre-flight check in test/functional/test_framework/test_framework.py that verified bitcoind/bitcoin-cli (and previous release binaries) existed before starting a test. The removed code raised AssertionError with a friendly message when binaries were missing. The commit argues this is redundant because the subprocess launch already produces a clear FileNotFoundError, the check was incomplete (missed other binaries), and it broke –valgrind. A help string and one error message are updated to point users to test/get_previous_releases.py.
Changed components
test/functional/test_framework/test_framework.pyInspect captured patch +3 / −15
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 79fcfafd..e3893eb4 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -179,7 +179,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
help="The seed to use for assigning port numbers (default: current process id)")
parser.add_argument("--previous-releases", dest="prev_releases", action="store_true",
default=os.path.isdir(previous_releases_path) and bool(os.listdir(previous_releases_path)),
- help="Force test of previous releases (default: %(default)s)")
+ help="Force test of previous releases (default: %(default)s). Previous releases binaries can be downloaded via `test/get_previous_releases.py`.")
parser.add_argument("--coveragedir", dest="coveragedir",
help="Write tested RPC commands into this directory")
parser.add_argument("--configfile", dest="configfile",
@@ -458,18 +458,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
bin_dirs = []
for v in versions:
bin_dir = bin_dir_from_version(v)
-
- # Fail test if any of the needed release binaries is missing
- for bin_path in (argv[0] for binaries in (self.get_binaries(bin_dir),)
- for argv in (binaries.node_argv(), binaries.rpc_argv())):
-
- if shutil.which(bin_path) is None:
- self.log.error(f"Binary not found: {bin_path}")
- if v is None:
- raise AssertionError("At least one binary is missing, did you compile?")
- raise AssertionError("At least one release binary is missing. "
- "Previous releases binaries can be downloaded via `test/get_previous_releases.py`.")
-
bin_dirs.append(bin_dir)
extra_init = [{}] * num_nodes if self.extra_init is None else self.extra_init # type: ignore[var-annotated]
@@ -968,8 +956,8 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
"""Checks whether previous releases are present and enabled."""
if not os.path.isdir(self.options.previous_releases_path):
if self.options.prev_releases:
- raise AssertionError("Force test of previous releases but releases missing: {}".format(
- self.options.previous_releases_path))
+ raise AssertionError(f"Force test of previous releases but releases missing: {self.options.previous_releases_path}\n"
+ "Previous releases binaries can be downloaded via `test/get_previous_releases.py`.")
return self.options.prev_releases
def skip_if_no_external_signer(self):
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.