#35139 test: Add thread-safe fast-failing test macros

full analysis

https://github.com/bitcoin/bitcoin/pull/35139 · maflcko · +337/-162 in 4 files, 4 commits · labels: Tests · draft

Goal

  • Provide thread-safe test assertion macros that stop on first failure and can be shared with fuzz tests
  • Prevent silent signedness comparison bugs and reduce log noise that hides root causes in CI

Adds lightweight, thread-safe, and fail-fast test assertion macros (`ASSERT`, `ASSERT_EQ`, `ASSERT_EQ_COLLECTIONS`, `ASSERT_EXCEPTION`) in `src/test/util/common.h` as drop-in replacements for Boost test macros. It enforces signedness matching at compile time and demonstrates the new macros by converting `src/test/streams_tests.cpp`.

Problem: Boost check macros are tied to the Boost test framework and cannot be reused in fuzz tests. They also lack thread safety under TSAN, silently permit invalid mixed-signedness comparisons, and continue running after failures, generating excessive log noise that obscures the original error in CI.

Category: Test infrastructure (#13 of 45)

P3 · cleanup

  • P3 because it fixes test edge cases like TSAN false positives and silent signedness comparison bugs
  • Enables sharing assertion macros with fuzz tests while existing Boost checks remain functional

P3 because replacing Boost check macros fixes real edge cases in testing (TSAN false positives, silent signed/unsigned comparison bugs) and enables assertion sharing with fuzz tests, but existing Boost macros function adequately and migration is reasonably deferrable.

Membership: Touches test assertion macros in `src/test/util/common.h` and updates unit tests in `src/test/streams_tests.cpp`.

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

Reviewability: Ready

  • Ready to review now

The macro implementation and test conversions are complete, CI is passing, and no known blocking changes are pending.

Author status: active; addressed review feedback and rebased

Resolved concerns:

  • mercie-ux noted an unused `<typeindex>` header in `common.h`, which the author removed.
  • sedited questioned whether fast-failing was desirable over seeing all failures at once, but gave a Concept ACK after the author demonstrated how downstream log bloat truncates CI error output.

Agreement: Positive

  • Concept approval after author showed how post-failure log spam truncates CI output (sedited)
  • Minor cleanups resolved (mercie-ux)

Positive; sedited gave Concept ACK after author addressed questions regarding fast-fail behavior

Initial skepticism regarding fast-failing versus collecting all test failures was addressed by showing real-world CI log truncation examples, leading to a Concept ACK.

  • sedited questioned fast-failing behavior on 2026-05-26
  • maflcko provided CI log examples demonstrating why post-failure output obscures root causes
  • sedited gave Concept ACK on 2026-07-07

Review verdicts (DrahtBot): 0

Dependencies

Enables:

  • #34666 tracking issue for test framework improvements

Files

499 lines under test/bench/ci.

  • src/test/streams_tests.cpp +155/-161
  • src/test/util/common.h +176/-1
  • src/test/util/common.cpp +5/-0
  • src/test/util/CMakeLists.txt +1/-0

Card

This PR introduces custom assertion macros (ASSERT, ASSERT_EQ, and related helpers) in test utilities to replace Boost test macros. The new macros provide thread safety under TSAN, strict signedness comparison checks, and fail-fast behavior on failure to prevent CI log bloat, while enabling test assertions to be shared with fuzz tests. It converts streams_tests.cpp to demonstrate the replacements. Review is ready and sedited gave Concept ACK after initial questions about fail-fast behavior were resolved.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T15:52, confidence high, input hash e5725cf0a191d40b