← Watch feed
test: functional: drop unused --keepcache argument
Public commit record
100/100 · Strong
What the developer wrote
Authored by David Gumberg
test: functional: drop unused --keepcache argument
At the time this was added in #10197, building the test cache took 21
seconds, as described in that PR, but this is no longer true, as
demonstrated by running the functional test framework with and without
the --keepcache arguments on master prior to this commit:
```
hyperfine --warmup 1 --export-markdown results.md --runs 3 \
-n 'without --keepcache' './build/test/functional/test_runner.py -j $(nproc)' \
-n 'with --keepcache' './build/test/functional/test_runner.py -j $(nproc) --keepcache'
```
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|:----------------------|---------------:|--------:|---:|---:|
| `without --keepcache` | 76.373 ± 3.058 | 74.083 | 79.846 | 1.00 |
| `with --keepcache` | 77.384 ± 1.836 | 75.952 | 79.454 | 1.01 ± 0.05 |
As a consequence, this argument can be removed from the test runner and
this also has the benefit of being able to use an RAII-like
`tempfile.TemporaryDirectory` instead of having to clean up the cache
manually at the end of test runs.
bitcoin/bitcoin#10197: https://github.com/bitcoin/bitcoin/pull/10197
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This commit removes an unused command-line option called --keepcache from Bitcoin Core's functional test runner. It is a cleanup change that switches test cache handling to an automatic temporary directory. There is no security relevance.
Recommended action
No security action needed. Treat as ordinary maintenance/test cleanup.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch deletes the –keepcache/-k argument and all related logic from test/functional/test_runner.py. Previously the runner could optionally preserve a persistent build/test/cache directory between runs; now it always uses a fresh tempfile.TemporaryDirectory for the cache. This is purely a test-infrastructure simplification with no production code changes.
Changed components
test/functional/test_runner.pyInspect captured patch +3 / −11
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index cef49a38..6cf4aa92 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -415,7 +415,6 @@ def main():
parser.add_argument('--extended', action='store_true', help='run the extended test suite in addition to the basic tests')
parser.add_argument('--help', '-h', '-?', action='store_true', help='print help text and exit')
parser.add_argument('--jobs', '-j', type=int, default=4, help='how many test scripts to run in parallel. Default=4.')
- parser.add_argument('--keepcache', '-k', action='store_true', help='the default behavior is to flush the cache directory on startup. --keepcache retains the cache from the previous testrun.')
parser.add_argument('--quiet', '-q', action='store_true', help='only print dots, results summary and failure logs')
parser.add_argument('--tmpdirprefix', '-t', default=tempfile.gettempdir(), help="Root directory for datadirs")
parser.add_argument('--failfast', '-F', action='store_true', help='stop execution after the first test failure')
@@ -565,9 +564,6 @@ def main():
check_script_list(src_dir=config["environment"]["SRCDIR"], fail_on_warn=fail_on_warn)
check_script_prefixes()
- if not args.keepcache:
- shutil.rmtree("%s/test/cache" % config["environment"]["BUILDDIR"], ignore_errors=True)
-
run_tests(
test_list=test_list,
build_dir=config["environment"]["BUILDDIR"],
@@ -598,11 +594,6 @@ def run_tests(*, test_list, build_dir, tmpdir, jobs=1, enable_coverage=False, ar
# pgrep not supported
pass
- # Warn if there is a cache directory
- cache_dir = "%s/test/cache" % build_dir
- if os.path.isdir(cache_dir):
- print("%sWARNING!%s There is a cache directory here: %s. If tests fail unexpectedly, try deleting the cache directory." % (BOLD[1], BOLD[0], cache_dir))
-
# Warn if there is not enough space on the testing dir
min_space = MIN_FREE_SPACE + (jobs - 1) * ADDITIONAL_SPACE_PER_JOB
if shutil.disk_usage(tmpdir).free < min_space:
@@ -614,7 +605,8 @@ def run_tests(*, test_list, build_dir, tmpdir, jobs=1, enable_coverage=False, ar
# a hard link or a copy on any platform. See https://github.com/bitcoin/bitcoin/pull/27561.
sys.path.append(tests_dir)
- flags = ['--cachedir={}'.format(cache_dir)] + args
+ cache_tmp_dir = tempfile.TemporaryDirectory(prefix="functional_test_cache")
+ flags = [f"--cachedir={cache_tmp_dir.name}"] + args
if enable_coverage:
coverage = RPCCoverage()
@@ -631,7 +623,7 @@ def run_tests(*, test_list, build_dir, tmpdir, jobs=1, enable_coverage=False, ar
sys.stdout.buffer.write(e.output)
raise
- #Run Tests
+ # Run Tests
job_queue = TestHandler(
num_tests_parallel=jobs,
tests_dir=tests_dir,
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.