#36074 scripted-diff: [test] Add util/check.h includes for assertions

ReviewabilityReviewsAgreementSize
#36074 maflcko scripted-diff: [test] Add util/check.h includes for assertionsReady0Strong14 + 319 tests
  • Ensure test assertions consistently use check utilities and avoid compilation failures or NDEBUG ambiguity
  • Enable automated include analysis across tests by standardizing assertion headers
  • Ready to review
  • Support for systematically cleaning up assertion includes in tests (l0rinc)
  • Concept approval without detailed feedback (l0rinc)
  • Script regex gaps and path syntax errors were raised and resolved (jeanpablojp)
  • 14 lines added or modified outside tests
  • 319 lines added or modified in tests
  • 220 lines removed in total
  • 154 files, 3 commits
Build and CI#52 of 56
P4 · cleanup
  • P4 because it only adds minor tooling support to install clang-format for scripted-diff CI linting
  • Negligible impact on overall build reliability or platform support
Ranking notes
Test infrastructure#42 of 54
P4 · cleanup
  • P4 because standardizing assertion includes across tests is an internal test cleanup
  • Prevents rare compile issues without assertions in check headers but offers limited developer leverage
  • Ranking pass: Grouped with the other assertion-hygiene scripted diffs.
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Mechanically adds `#include <util/check.h>` and removes `<cassert>` includes across unit test, fuzz, and benchmark files invoking `assert` or `Assert`. Updates the scripted-diff lint environment to allow running `clang-format` and sorts the affected include blocks.

Problem: Test code relying directly on `<cassert>` or transitive headers creates ambiguity around whether assertions can be stripped by `NDEBUG`. It also causes compilation failures when compiling without assertions in `util/check.h` and complicates automated Include-What-You-Use (IWYU) analysis.

Evidence of importance:

  • Commit 9f18b0f183: "The project has a compile error when compiled without assertions in util/check.h. Thus, util/check.h should be included for all assertions."
  • PR description: "In test code, `assert` is used. This is perfectly fine, but sometimes confusion arises, when it is unclear whether `NDEBUG` can disable the assertions, or whether to use `assert` or `Assert`."

Scope: A scripted-diff across 154 test and benchmark files, accompanied by a CI lint setup change to install clang-format for scripted-diff checks.

Risk of a mistake: low (tests, tooling). Only touches test include lists and a CI lint script; an error would only manifest as a test compilation or CI lint failure.

Changed since the previous assessment: Rebased with no changes to the approach or commit structure.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • author: resolving conflicts while PR is in draft 2026-10-29
  • author: PR converted back to draft due to conflicts 2026-10-29

Agreement

Strong

substantive support, no open objection (l0rinc)

Thread notes: Added missing suggestion from jeanpablojp regarding src/ipc/test/ exclusion in lint_cpp.rs. Updated clears_with to change for concrete defects in scripts. Author converted PR back to draft due to merge conflicts. / Added jeanpablojp's missing scope question regarding omitting src/ipc/test/ (rc:3866101304), which was fixed by adding the path. Updated the PR status to waiting on the author as it was converted back to draft due to merge conflicts.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
jeanpablojp (contributor)objection, correctnessassert calls with spaces are missed by both scripted-diff replacement and the lint scriptfixed · fix pushed · author dropped the commits containing the replacement and lint scriptchangedyes2026-08-26: 'This one doesn't match `<assert(` because of the space, and it's the only one left in the swept paths. `lint_c_assert` uses the same regex, so it doesn't catch it either.'
Status from: 2026-08-29: maflcko: 'Thx, but I've dropped the third and forth commit for now'
jeanpablojp (contributor)suggestion, scopeinclude src/ipc/test/ in the lint checkfixed · fix pushed · author added src/ipc/test/ to the lint path list and fixed the syntax erroransweredno2026-08-26: '`src/ipc/test/` is left out here, and `src/ipc/test/fuzz/ipc.cpp` has six `assert(`. `lint-tests.py` already counts that directory as part of the test suite. Is this intentional?'
Status from: 2026-09-01: maflcko: 'Whoops, fixed. Thx'
jeanpablojp (contributor)objection, correctnessmisplaced comma in string literal causes the ipc test directory filter to match nothingfixed · fix pushed · author fixed the comma placementchangedyes2026-08-29: 'The comma in `'src/ipc/test/',` ends up inside the element, so it still matches nothing.'
Status from: 2026-09-01: maflcko: 'Whoops, fixed. Thx'
l0rinc (member)suggestion, scopeextract the lint commit into a separate PR to merge quickly and combine the scripted diffs into a single commitanswered · author prefers keeping bundled and noted conflicts are trivialansweredyes2026-09-25: 'maybe we could extract the lint commit into a separate PR - that one is conflict-free and should pass quickly.'
Status from: 2026-09-28: maflcko: 'The changes and scripts should be easy enough to keep bundled here.'
l0rinc (member)question, approachcould the include insertion skip the first-#include line-number lookup and just insert at the top of the file?answered · author explained clang-format limitationansweredyes2026-09-25: 'could the include insertion skip the first-`#include` line-number lookup and just insert at the top of the file?'
Status from: 2026-09-28: maflcko: 'clang-format can't move includes over `//` comment lines. So this won't work.'

Support:

  • l0rinc (member) (2026-08-29) [read: src/test/fuzz/mini_miner.cpp]: glad you're taking a stab at it systemically, even if just for tests for now
  • l0rinc (member) (Concept ACK, 2026-09-25) [read]: (no reason given) [not substantive]

Participants:

  • jeanpablojp (contributor, objection, 3 statements 2026-08-26 to 2026-08-29): pointed out assert regex missed whitespace, omitted ipc test directory, and a syntax typo in the ipc test path list
  • l0rinc (member, support, 2 statements 2026-08-29 to 2026-09-25): Concept ACK; suggested extracting the lint commit and asked about simplifying include insertion

Files

526 lines under test/bench/ci.

  • src/qt/test/wallettests.cpp +10/-9
  • src/ipc/test/ipc_tests.cpp +9/-9
  • ci/lint/01_install.sh +6/-11
  • src/qt/test/addressbooktests.cpp +8/-8
  • src/test/miniscript_tests.cpp +8/-7
  • src/test/transaction_tests.cpp +7/-8
  • src/test/fuzz/feeratediagram.cpp +5/-7
  • src/ipc/test/fuzz/ipc.cpp +5/-4
  • src/test/blockmanager_tests.cpp +5/-4
  • src/test/fuzz/versionbits.cpp +5/-4
  • src/test/txrequest_tests.cpp +4/-4
  • src/test/validation_tests.cpp +4/-4
  • src/test/crypto_tests.cpp +4/-3
  • src/test/fuzz/base_encode_decode.cpp +3/-4
  • src/test/fuzz/merkle.cpp +4/-3
  • src/test/miniminer_tests.cpp +4/-3
  • src/test/blockencodings_tests.cpp +3/-3
  • src/test/blockfilter_tests.cpp +3/-3
  • src/test/fuzz/feefrac.cpp +4/-2
  • src/test/fuzz/muhash.cpp +4/-2
  • src/test/key_io_tests.cpp +3/-3
  • src/test/validation_chainstatemanager_tests.cpp +3/-3
  • src/test/compress_tests.cpp +3/-2
  • src/test/fuzz/addrman.cpp +3/-2
  • src/test/fuzz/bech32.cpp +3/-2
  • src/test/fuzz/block.cpp +3/-2
  • src/test/fuzz/key.cpp +3/-2
  • src/test/fuzz/key_io.cpp +3/-2
  • src/test/fuzz/locale.cpp +3/-2
  • src/test/fuzz/script.cpp +3/-2
  • src/test/fuzz/string.cpp +3/-2
  • src/test/fuzz/transaction.cpp +3/-2
  • src/test/fuzz/util/mempool.cpp +3/-2
  • src/test/headers_sync_chainwork_tests.cpp +3/-2
  • src/test/mempool_tests.cpp +3/-2
  • src/test/node_init_tests.cpp +3/-2
  • src/test/orphanage_tests.cpp +3/-2
  • src/test/rbf_tests.cpp +3/-2
  • src/test/sigopcount_tests.cpp +3/-2
  • src/test/txdownload_tests.cpp +3/-2
  • src/wallet/test/ismine_tests.cpp +3/-2
  • src/test/fuzz/asmap.cpp +3/-1
  • src/test/fuzz/asmap_direct.cpp +2/-2
  • src/test/fuzz/bitdeque.cpp +3/-1
  • src/test/fuzz/bitset.cpp +3/-1
  • src/test/fuzz/fees.cpp +3/-1
  • src/test/fuzz/headerssync.cpp +3/-1
  • src/test/fuzz/miniscript.cpp +3/-1
  • src/test/fuzz/net.cpp +3/-1
  • src/test/fuzz/private_broadcast.cpp +3/-1
  • src/test/fuzz/txdownloadman.cpp +3/-1
  • src/test/fuzz/txgraph.cpp +3/-1
  • src/test/fuzz/txrequest.cpp +3/-1
  • src/test/fuzz/vecdeque.cpp +3/-1
  • src/test/private_broadcast_tests.cpp +3/-1
  • src/test/util/chainstate.h +2/-2
  • src/test/util/net.h +2/-2
  • src/test/util/transaction_utils.cpp +3/-1
  • src/test/util_tests.cpp +2/-2
  • src/wallet/test/coinselector_tests.cpp +3/-1
  • src/wallet/test/fuzz/coincontrol.cpp +3/-1
  • src/wallet/test/fuzz/coinselection.cpp +3/-1
  • src/wallet/test/fuzz/spend.cpp +3/-1
  • src/wallet/test/util.cpp +2/-2
  • src/qt/test/apptests.cpp +2/-1
  • src/test/fuzz/banman.cpp +2/-1
  • src/test/fuzz/difference_formatter.cpp +2/-1
  • src/test/fuzz/flatfile.cpp +2/-1
  • src/test/fuzz/net_permissions.cpp +2/-1
  • src/test/fuzz/netaddress.cpp +2/-1
  • src/test/fuzz/threadpool.cpp +2/-1
  • src/test/fuzz/util/wallet.h +2/-1
  • src/test/interfaces_tests.cpp +2/-1
  • src/test/pcp_tests.cpp +2/-1
  • src/test/scriptnum10.h +2/-1
  • src/test/txvalidation_tests.cpp +2/-1
  • src/wallet/test/wallet_test_fixture.cpp +2/-1
  • src/test/fuzz/bip324.cpp +2/-0
  • src/test/fuzz/block_header.cpp +1/-1
  • src/test/fuzz/bloom_filter.cpp +1/-1
  • src/test/fuzz/cluster_linearize.cpp +2/-0
  • src/test/fuzz/coins_view.cpp +1/-1
  • src/test/fuzz/coinscache_sim.cpp +1/-1
  • src/test/fuzz/crypto_aes256.cpp +1/-1
  • src/test/fuzz/crypto_aes256cbc.cpp +1/-1
  • src/test/fuzz/crypto_common.cpp +1/-1
  • src/test/fuzz/cuckoocache.cpp +2/-0
  • src/test/fuzz/decode_tx.cpp +1/-1
  • src/test/fuzz/float.cpp +1/-1
  • src/test/fuzz/golomb_rice.cpp +1/-1
  • src/test/fuzz/hex.cpp +1/-1
  • src/test/fuzz/message.cpp +1/-1
  • src/test/fuzz/node_eviction.cpp +1/-1
  • src/test/fuzz/p2p_transport_serialization.cpp +1/-1
  • src/test/fuzz/parse_hd_keypath.cpp +1/-1
  • src/test/fuzz/parse_iso8601.cpp +1/-1
  • src/test/fuzz/parse_numbers.cpp +1/-1
  • src/test/fuzz/prevector.cpp +2/-0
  • src/test/fuzz/rolling_bloom_filter.cpp +1/-1
  • src/test/fuzz/rpc.cpp +1/-1
  • src/test/fuzz/script_flags.cpp +1/-1
  • src/test/fuzz/script_sign.cpp +1/-1
  • src/test/fuzz/scriptnum_ops.cpp +1/-1
  • src/test/fuzz/util/net.cpp +1/-1
  • src/test/httpserver_tests.cpp +2/-0
  • src/test/random_tests.cpp +1/-1
  • src/test/sock_tests.cpp +1/-1
  • src/wallet/test/group_outputs_tests.cpp +1/-1
  • src/test/blockchain_tests.cpp +1/-0
  • src/test/btcsignals_tests.cpp +1/-0
  • src/test/chainstate_write_tests.cpp +1/-0
  • src/test/checkqueue_tests.cpp +1/-0
  • src/test/cuckoocache_tests.cpp +1/-0
  • src/test/flatfile_tests.cpp +1/-0
  • src/test/fuzz/addition_overflow.cpp +1/-0
  • src/test/fuzz/block_index.cpp +1/-0
  • src/test/fuzz/block_index_tree.cpp +1/-0
  • src/test/fuzz/connect_block.cpp +1/-0
  • src/test/fuzz/connman.cpp +1/-0
  • src/test/fuzz/crypto_chacha20.cpp +1/-0
  • src/test/fuzz/crypto_chacha20poly1305.cpp +1/-0
  • src/test/fuzz/crypto_diff_fuzz_chacha20.cpp +1/-0
  • src/test/fuzz/crypto_poly1305.cpp +1/-0
  • src/test/fuzz/dbwrapper.cpp +0/-1
  • src/test/fuzz/descriptor_parse.cpp +1/-0
  • src/test/fuzz/deserialize.cpp +1/-0
  • src/test/fuzz/fee_rate.cpp +1/-0
  • src/test/fuzz/http_request.cpp +0/-1
  • src/test/fuzz/integer.cpp +0/-1
  • src/test/fuzz/multiplication_overflow.cpp +1/-0
  • src/test/fuzz/netbase_dns_lookup.cpp +1/-0
  • src/test/fuzz/p2p_headers_presync.cpp +1/-0
  • src/test/fuzz/p2p_private_broadcast.cpp +1/-0
  • src/test/fuzz/poolresource.cpp +1/-0
  • src/test/fuzz/primitives_transaction.cpp +1/-0
  • src/test/fuzz/script_descriptor_cache.cpp +1/-0
  • src/test/fuzz/secp256k1_ec_seckey_import_export_der.cpp +1/-0
  • src/test/fuzz/span.cpp +0/-1
  • src/test/fuzz/tx_in.cpp +0/-1
  • src/test/fuzz/tx_out.cpp +1/-0
  • src/test/fuzz/util.h +1/-0
  • src/test/logging_tests.cpp +1/-0
  • src/test/mempool_fee_estimator_tests.cpp +1/-0
  • src/test/net_tests.cpp +1/-0
  • src/test/scheduler_tests.cpp +1/-0
  • src/test/streams_tests.cpp +1/-0
  • src/test/threadpool_tests.cpp +1/-0
  • src/test/util/cluster_linearize.h +1/-0
  • src/test/util/net.cpp +1/-0
  • src/test/util/poolresourcetester.h +0/-1
  • src/wallet/test/fuzz/wallet_bdb_parser.cpp +1/-0
  • src/wallet/test/psbt_wallet_tests.cpp +1/-0
  • src/wallet/test/rescan_tests.cpp +0/-1
  • src/wallet/test/spend_tests.cpp +1/-0

Card

The short text the ranking pass compares across PRs.

Adds explicit `#include <util/check.h>` and drops `<cassert>` via scripted-diff across unit tests, benchmarks, and fuzz harnesses that invoke `assert` or `Assert`. This resolves ambiguity about whether assertions are compiled out by NDEBUG, avoids compile failures when assertions are disabled in `util/check.h`, and facilitates running IWYU tooling. It also enables `clang-format` in the scripted-diff CI verification environment to format and sort headers.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-31T01:14, confidence high, input hash aacf728b6d6ac16d
thread read through 2026-10-29 (10 statements), head 96f006f283
labels: Refactoring · draft

Categories

Build and CI

P4 · cleanup

Updates CI lint installation scripts to support clang-format in scripted diffs. This is minor tooling support with negligible impact on build reliability or platforms.

Membership: Modifies `ci/lint/01_install.sh` to install clang-format for scripted-diff CI linting.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0

Test infrastructure

P4 · cleanup (ranking pass: Grouped with the other assertion-hygiene scripted diffs.)

Standardizes assertion includes across test files to use `util/check.h`. It resolves potential compile errors when assertions are disabled in `util/check.h` and removes ambiguity between `assert` and `Assert`, but remains an internal test-code cleanup.

Membership: Updates assertion header includes across unit tests, fuzz harnesses, and benchmarks.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0