#35139 test: Add thread-safe fast-failing test macros
https://github.com/bitcoin/bitcoin/pull/35139 · · +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
- Concept ACK: sedited
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.