test: remove some avoidable test waits #36446

pull willcl-ark wants to merge 4 commits into bitcoin:master from willcl-ark:speedup-tests changing 4 files +43 −10

willcl-ark commented at 8:27 AM on October 6, 2026: member
A few of our longer-running tests have some unnecessary waits and timeouts in them. Reduce or replace them to shorten test times:
- in p2p_node_network_limited replace the expected sync timeout with header/ping barriers and a bounded negative check. At CI timeout factor 40, saves roughly 195 seconds per transport.
- in interface_ipc_mining cap three expected no-event waits at five seconds, saving roughly 105 seconds at factor 40 in CI.
- in WalletLoadingDescriptors honor sanity mode’s single epoch, avoiding four unnecessary wallet unload/load cycles.
- two wallet unit tests switch to BasicTestingSetup avoiding construction of two unused 100-block chains.
The timeout changes alone save approximately eight minutes of accumulated test-worker time on CI jobs running both transports and IPC mining. Wall-clock savings depend on parallel scheduling, CPU etc.

test: shorten network limited negative wait

CI scaled a negative sync timeout to 200 seconds per transport. The test
now waits for header sync and a ping roundtrip, then checks that the IBD
node sends no block requests for up to five seconds.

06cb869112

test: cap expected IPC timeouts

Expected no-event checks can otherwise each wait for the full timeout
factor, which reaches 40 seconds on CI. Cap those checks at five
seconds while keeping readiness waits on the normal scaled timeout.

79bb94f092

bench: honor configured wallet epochs

The benchmark sanity runner selects one epoch, but the benchmark
unconditionally replaced that setting with five. Cap the configured
epoch count at five so sanity runs remain short.

40619d0687

test: avoid unused wallet test chains

These tests do not use chain state, but their fixtures mine 100 blocks
before each case. BasicTestingSetup provides the node context without
the unnecessary chain construction.

ce1d88b263

DrahtBot added the label Tests on Oct 6, 2026
DrahtBot commented at 8:27 AM on October 6, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36446
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36446.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36446#author-ralph


Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK Ayoazeez26, maflcko, ViniciusCestarii

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #35598 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35598.svg"></sub> (test: cover feeThreshold = MAX_MONEY in interface_ipc_mining.py by Sjors)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
willcl-ark commented at 8:27 AM on October 6, 2026: member

I run these on free GHA runners here: https://github.com/willcl-ark/bitcoin/actions/runs/37357329900
Ayoazeez26 commented at 10:26 PM on October 7, 2026: none

tACK ce1d88b263

Built on macOS (arm64) and compared against master:

p2p_node_network_limited.py: 15 s → 8–9 s (v1 and v2 transport) interface_ipc_mining.py --timeout-factor=10: 50 s → 34 s wallet_disableprivkeys + dummy_input_size_test: pass, small speedup (user CPU 0.15 s → 0.11 s) WalletLoadingDescriptors: normal results unchanged (~335 ms/op), as expected; -sanity-check runs fine All wallet_tests pass
fanquake commented at 6:19 AM on October 14, 2026: member

cc @ViniciusCestarii
in test/functional/p2p_node_network_limited.py:170 in 06cb869112
```
 171 | +                and peer["bytessent_per_msg"].get("getdata", 0) == 0
 172 | +            )
 173 | +
 174 | +        ensure_for(
 175 | +            duration=max(1.0, min(5.0, self.options.timeout_factor)),
 176 | +            f=no_block_request_from_limited_peer,
```
maflcko commented at 6:45 AM on October 14, 2026:

nit in 06cb869112da6bb1c43f938b2e5a9794f825ff92: Is the ensure_for needed? There is no "trickle delay" for blocks, so just a plain assert_equal like before should be enough?

maflcko commented at 10:16 AM on October 15, 2026:

There are also other issues and typos in the test, so I fixed it in https://github.com/bitcoin/bitcoin/pull/36541
maflcko approved

Type	Reviewers
ACK	Ayoazeez26, maflcko, ViniciusCestarii

maflcko commented at 6:49 AM on October 14, 2026: member

review ACK ce1d88b2632cc3950f47d8b191bdd74b806eb3ae 🐠

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK ce1d88b2632cc3950f47d8b191bdd74b806eb3ae 🐠
ljalevF1PRZiYJ9dTCCTkGiL/FY1wHlrxkYPn5ihrbJlBfMDv3OfjWF5tzjiTTnqm8hbnpM+rtLr/WAnh3a7BQ==

</details>

ViniciusCestarii commented at 11:47 PM on October 14, 2026: contributor

ACK ce1d88b2632cc3950f47d8b191bdd74b806eb3ae

Checked the speed ups on CI and it looks pretty good! "test: avoid unused wallet test chains" ce1d88b2632cc3950f47d8b191bdd74b806eb3ae doens't seem to speed up much, but it is a fine change.
fanquake merged this on Oct 15, 2026
fanquake closed this on Oct 15, 2026

Contributors

Labels

Linked (view graph)

#36541 test: Small p2p_node_network_limited.py cleanups #36582 test: reorder functional tests by current runtimes

test: remove some avoidable test waits #36446

External sites

Reviews

Conflicts