Remove boost as a unit test runner #35713

rustaceanrob commented at 2:23 PM on July 13, 2026: member

Continued from #35587. tl;dr: Make an equivalent test runner to boost with simpler macros and tighter coupling to bitcoin-specific code.

Motivation

There are a number of problems with using dependencies in general. Bugs must either be promptly upstreamed or patched, projects may be abandoned, and they are not tailored to a particular use case. This PR removes boost for writing and running unit tests. In the case of Boost.Test, there are a number of issues which are listed below.

My problems with Boost.Test

Macros

When using boost, there are 3 ways of writing the same thing, each with different outcomes

BOOST_CHECK(expr1 == expr2): failures show the expression, but no values for expr1 and expr2
BOOST_CHECK_EQUAL(expr1, expr2): failures show the expression and values, but requires developers remember CHECK_EQUAL and does not use usual operands
BOOST_TEST(expr1 == expr2): failures show the expression and values, and developers may use familiar operands (==, !=)

All three are preserved to maintain backwards compatibility - presumably, but this results in all 3 being used throughout the repository. As a bonus there is a 4th way, with added context

BOOST_CHECK_MESSAGE(expr1 == expr2, "the context"): failures show the expression and message, but no values for expr1 and expr2

To make debugging and review easier, all of these should be unified to a single macro. Test sites use operands, print helpful messages - values are always included, and do not vary in approach (deciding between BOOST_TEST, CHECK, and BOOST_CHECK_EQUAL is unintuitive)

<details> <summary>Example boost outputs</summary>

BOOST_AUTO_TEST_CASE(mustfail)
{
    auto a{1};
    auto b{2};
    BOOST_CHECK(a == b);
    BOOST_CHECK_EQUAL(a, b);
    BOOST_TEST(a == b);
    COutPoint outpoint_1{Txid{"0000000000000000000000000000000000000000000000000000000000000100"}, 0};
    COutPoint outpoint_2{Txid{"0000000000000000000000000000000000000000000000000000000000000100"}, 1};
    BOOST_CHECK(outpoint_1 == outpoint_2);
    // Doesn't even compile
    // BOOST_TEST(outpoint_1 == outpoint_2);
    // BOOST_CHECK_EQUAL(outpoint_1, outpoint_2);
}

Running 1 test case...
test/argsman_tests.cpp(31): error: in "argsman_tests/mustfail": check a == b has failed
test/argsman_tests.cpp(32): error: in "argsman_tests/mustfail": check a == b has failed [1 != 2]
test/argsman_tests.cpp(33): error: in "argsman_tests/mustfail": check a == b has failed [1 != 2]
test/argsman_tests.cpp(36): error: in "argsman_tests/mustfail": check outpoint_1 == outpoint_2 has failed

</details>

<details> <summary>New outputs</summary>

TEST_CASE(mustfail)
{
    auto a{1};
    auto b{2};
    CHECK(a == b);
    COutPoint outpoint_1{Txid{"0000000000000000000000000000000000000000000000000000000000000100"}, 0};
    COutPoint outpoint_2{Txid{"0000000000000000000000000000000000000000000000000000000000000100"}, 1};
    CHECK(outpoint_1 == outpoint_2);
}

Running 1 test cases...
[FAIL]: test/argsman_tests.cpp:30: CHECK(a == b)
1 == 2

[FAIL]: test/argsman_tests.cpp:33: CHECK(outpoint_1 == outpoint_2)
COutPoint(0000000000, 0) == COutPoint(0000000000, 1)

[FAIL] mustfail (2/2 checks failed)

</details>

In the depends system

We can simplify depends and a workaround in the guix build

No repository context

A custom framework allows for complete control of debug output, console output, CLI arguments, etc.

Other issues

Issues are mentioned in #34666, #8670, #36045. Using a test runner within the source also makes IWYU easier to work with.

High level changes

For those running tests, not a tremendous amount has changed in this PR. For instance, the doc page remains valid. Changes to the runner options include anything that is not help, run_test, log_level, list_content. This includes catch_system_errors which is always no. The log_levels have been reduced to 5 levels.

For an explanation of all the internal details, I have written a detailed personal blog

Changes in writing tests

Note that no changes BOOST_* macros are changed in this PR. Macro shims are added so developers may continue using the usual BOOST_* macros. Future changes to writing tests would include:

CHECK to compare two values with any ==, !=, >, etc
CHECK(a == b, "my message") to append a message
REQUIRE, same as CHECK, but will fail the test immediately
TEST_CASE(name) to add a test
FIXTURE_TEST_CASE(name, Fixture) to add a test with a fixture
TEST_SUITE_BEGIN/END to declare a suite, optionally with fixture for each test in the suite

Migration

Boost.Test is removed in this PR, but the macros are aliased by BOOST_* counterparts so there are no merge conflicts. The idea would be to migrate test files to the new macros when there are no/low number of conflicts on that file.

<details> <summary>Commits</summary>

The addition of the framework:

test: Add header-only framework to util

Migration script (majority of the file changes):

scripted-diff: Migrate tests to header-only framework

Low usage count macro removals:

test: Remove low usage BOOST macros

Required for expression decomposition:

test: Wrap || expressions in macros
test: Wrap bitwise & expressions in macros

To pass CI

test: Inline CRecipient to avoid gcc12 uninitialized warning

Build and config

depends: drop test from Boost libraries
cmake: drop vcpkg Boost Test check
vcpkg: drop boost-test dependency
guix: Drop Boost.Test unused workaround

</details>

DrahtBot commented at 2:23 PM on July 13, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35713.

Reviews

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

Type	Reviewers
Concept ACK	josibake, jeanpablojp

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:

#36191 (kernel: Replace log callbacks with buffered reads by w0xlt)
#36159 (http: Improve HTTPRemoteClient::MaybeDisconnect() by hodlinator)
#36122 (BIP460: CISA for Taproot key path spends by fjahr)
#36091 (test: Add debug output to common tested types by rustaceanrob)
#36013 (test: Descriptor roundtrip and raw()/ addr() coverage by pablomartin4btc)
#35744 (coins: prevent DB resize from invalidating cursors by l0rinc)
#35662 (script: prevent stale sighash caches across transactions by l0rinc)
#35569 (Encapsulation for CTransaction by purpleKarrot)
#35511 (RFC: consensus: Make CAmount a class by hodlinator)
#35139 (test: Add thread-safe fast-failing test macros by maflcko)
#35003 (validation: improve block data I/O error handling in P2P paths by furszy)
#29278 (Wallet: Add maxfeerate wallet startup option by ismaelsadeeq)
#25573 (guix: produce a -static-pie bitcoind by fanquake)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

This lint is to avoid ODR violations, but this function is behind an \ifdef`, so it will only be defined in a TU that invoke `define`.->This lint is to avoid ODR violations, but this function is behind an `ifdef`, so it will only be defined in a TU that defines `BITCOIN_TEST_MAIN`.[“invokedefine`” is ungrammatical and obscures the intent]
unless the type does not having a meaningful output -> unless the type does not have a meaningful output [grammar error]

2026-09-03 14:11:30

josibake commented at 2:28 PM on July 13, 2026: member

Moving over my original ACK from the previous PR: #35587 (comment)

Strong Concept ACK!

Love to see it. While I broadly agree with the author's reasons stated in the PR, the ones I'll highlight specifically are:

No repository context
Not extensible

Having our own simple, in house test framework gives us the ability to customise it for exactly our needs. I believe we should do everything in our power to make testing an easy, intuitive, and feature rich experience as this helps to remove barriers to people writing and interpreting tests.

In particular, as a longer term project I am looking at ways to improve concurrency in our testing to help speed up CI. As @purpleKarrot points out:

The Boost.Test framework is notoriously bad at this. When selecting a single test, it spams out information about all tests that are not selected. While it can provide information about what tests are available, that output is optimized for human readability and very hard to parse by a test driver. We can definitely do better with a custom test framework!

in src/test/util/framework.h:140 in ae1ee1b007 outdated

 135 | +        os << value;
 136 | +        return os.str();
 137 | +    } else if constexpr (has_to_string<T>) {
 138 | +        return value.ToString();
 139 | +    } else {
 140 | +        return typeid(T).name();

maflcko commented at 2:41 PM on July 13, 2026:

As explained in #35625 (comment), I still don't think this makes sense. The only change is that some compile-time type name may be printed for both sides.

This should remain a compile failure, like it is on current master.

rustaceanrob commented at 9:38 AM on July 14, 2026:

Agree this should be a static assert but I haven't gotten to this yet because BOOST_CHECK doesn't require a meaningful string representation. There are many types that would then require a ToString or similar for stringify to work properly. I have a couple ideas of how to do this.

maflcko commented at 9:54 AM on July 14, 2026:

Why not just keep the pre-existing approach of using the helpers from src/test/util/common.h (std::ostream& operator<<)?

rustaceanrob commented at 10:20 AM on July 14, 2026:

Those don't cover most of the types. For instance, a few random ones, ByRatio<FeeFrac> and Coin do not have a ToString or <<.

maflcko commented at 11:14 AM on July 14, 2026:

Ok, I see. It could make sense to split this fix up into its own pull request? Otherwise, it looks like it is hidden in a single line in a ~1500line pull request.

To explain the background:

No code today (in master) exists that compares Coin in the test framework with pretty debug output.
If someone were to trigger pretty output, e.g. by using BOOST_CHECK_EQUAL, then the compilation would fail:

diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index 6ae5f2b7cb..9ba2e67e30 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -168,3 +168,3 @@ void SimulationTest(CCoinsView* base, bool fake_best_block)
                 AccessByTxid(*stack.back(), txid) : stack.back()->AccessCoin(COutPoint(txid, 0));
-            BOOST_CHECK(coin == entry);
+            BOOST_CHECK_EQUAL(coin,entry);

To fix this, I think it would be fine to provide a ToString or operator<<, maybe in a separate pull request? However, I haven't checked how many classes are affected in total.

Maybe there is a better solution available (we don't have C++26, heh), but I think whatever the solution is, should (and can) be a separate pull request. Also, seems fine to keep it here, but I think this pull request should try to preserve the behavior, or at least list all behavior changes and why they are done.

rustaceanrob commented at 1:52 PM on July 14, 2026:

I think some have a natural fix, like if HexStr(v) works than it is probably a good representation. Also, if an type implements a v.Serialize then we can also pass that as a hex string, which should be suitable for debugging (maybe the typeid as a prefix to the hex string). Also std::vector<Foo> for Foo with a stringifyable can just print the list of Foo. Same goes for std::pair and some others.

For the rest of the cases, I was thinking it could be possible to add a macro BOOST_NO_DISPLAY_CHECK. All of these callsites can be fixed by adding a ToString or << as that file is migrated to new macros.

maflcko commented at 1:57 PM on July 14, 2026:

Ah nice. Hex could work here.

Though, for a vector I am not sure if it makes sense to dump the full list. See check_eq_collections in https://github.com/bitcoin/bitcoin/pull/35139/changes, which only prints the failing index. IIRC this is also what boost does.

maflcko commented at 3:39 PM on July 15, 2026:

I think the fallback return typeid(T).name(); still exists in the latest push and is still wrong.

The correct lowest fallback is still a compilation-failure, just like the prior behavior with boost.

rustaceanrob commented at 3:56 PM on July 15, 2026:

Yeah, agree, however there are around 80 callsites that still do not have a string representation. Some of which appear to be the wrong check, like using BOOST_CHECK on two ranges instead of BOOST_CHECK_EQUAL_RANGES. Others would need a custom ToString or <<.

For the time being, in a separate commit after the scripted diff, I was going to introduce the static assert and a macro CHECK_NO_DISPLAY that omits the stringify for those sites. I am also open to other approaches.

maflcko commented at 12:22 PM on July 16, 2026:

Yeah, seems fine to have way to skip having to provide a stringifier. One hacky way would be to just use double-braces: CHECK((a==b)), but that may be a bit ugly and unintuitive. CHECK_NO_DISPLAY sounds fine.

The benefit of the static assert is that it can print the problem and solution at compile time, instead of silently running into issues without any suggestion on how to fix them. E.g. something like:

        static_assert(requires(std::ostream& os) { os << value; }, "Please provide an operator<<(std::ostream&, const T&) for formatting. Otherwise, a test failure will not be able to display the differing objects. Alternatively, use CHECK_NO_DISPLAY to disable display of the objects completely.");

rustaceanrob commented at 12:32 PM on July 20, 2026:

01be0daef8f8e003d4cda75cf5b226fa0eff0696 and efa00a2cded95fb39ca820a2eae4da527da6e89a updates stringify overloads and 05397f7c0881265f1b108e8e2544c889d7c8b830 moves to a static_assert. In doing this exercise I saw a lot of BOOST_CHECK(r1 == r2) that compares two ranges, where almost certainly it would be better to use BOOST_CHECK_EQUAL_COLLECTIONS. I think it would be a nice property to allow devs to use CHECK(r1 == r2), so I added a concept that checks if a developer is trying to compare two ranges and that type does not already have a ToString or <<. This allowed me two remove the CHECK_EQUAL_COLLECTIONS macro entirely, which I prefer.

Unfortunately, two iterators are compared quite a few times in the tests, however it is difficult to make a meaningful stringify for these cases. I am hoping to leave those conversions to CHECK_NO_DISPLAY as followup, so as to not completely bloat this PR.

in src/test/dbwrapper_tests.cpp:360 in 421271dbcc

 355 | @@ -356,8 +356,8 @@ BOOST_AUTO_TEST_CASE(iterator_ordering)
 356 |      for (const int seek_start : {0x00, 0x80}) {
 357 |          it->Seek((uint8_t)seek_start);
 358 |          for (unsigned int x=seek_start; x<255; ++x) {
 359 | -            uint8_t key;
 360 | -            uint32_t value;

fanquake commented at 8:39 AM on July 14, 2026:

In 6eaefedb79598eedf00b24e4e480a7e005440e40:

libboost_unit_test_framework is prebuilt without MSan/UBSan,

We don't use any Boost libraries (header-only since #24301), so not sure how the library missing instrumentation could be the cause of issues here?

maflcko commented at 9:02 AM on July 14, 2026:

Maybe this is just one of the GCC false positive bugs about -w-uninit? Though, hard to tell without seeing the exact compiler/sanitizer output in the commit message.

Maybe the commit messages could be expanded with the exact output?

rustaceanrob commented at 9:06 AM on July 14, 2026:

I'm running the CI on a different branch to see if this still fails. If so, I'll paste some output.

rustaceanrob commented at 11:49 AM on July 14, 2026:

Looks like an older state of the framwork or commit history was causing this, dropped the commit.

maflcko commented at 12:08 PM on July 14, 2026:

Could still make sense to expand the other commit with the exact failure reason.

Stuff like 9e1247067dbf51fab4a532a8ffda707361886021 looks like bugs in the bitcoin core code itself. About 17b37d2fdded4cb02203a07e27408f4001abe894 I wonder what the error was.

rustaceanrob commented at 2:10 PM on July 14, 2026:

For the constexpr commits, it is a linker error, which I will add to the commit messages:

undefined reference to `LockedPool::ARENA_SIZE'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

AFICT there are no downsides to const -> constexpr, so it feels like these commits should belong in a broader sweep, but then again I don't see much of a motivation to do so outside of this PR. Would there be any benefit?

maflcko commented at 2:53 PM on July 14, 2026:

Ah, it is a linker error due to the missing inline via constexpr. Thanks for explaining.

For reference, the compile failure for the other commit would be something like:

error: invalid operands to binary expression ('const AddressPosition' and 'const AddressPosition')
  851 |                     (void)(ref==ref);
      |                            ~~~^ ~~~
src/addrman.h:76:10: note: candidate function not viable: 'this' argument has type 'const AddressPosition', but method is not marked const
   76 |     bool operator==(AddressPosition other) {
      |          ^

rustaceanrob commented at 12:47 PM on July 22, 2026:

683a06f0869e471239a75f672eb1f8841c424c26 circumvents the linker issue by just removing the use of const T& for the static const class member variables. I looked at the number of static const member variables in src and there were quite a few, so I would rather not convert all of these to constexpr at the moment.

maflcko commented at 1:27 PM on July 22, 2026:

I mean it looks like ODR violations that just happen to get optimized away by the compiler, but I guess there is no tool to find/fix all of them?

rustaceanrob commented at 3:47 PM on July 22, 2026:

I think https://github.com/llvm/llvm-project/pull/162741 would do it but looks a bit stalled

fanquake commented at 4:06 PM on July 22, 2026:

I think it'd be fine to open a PR that just changes all class usage of static const to static constexpr? That seems straightforward, and correct (happy to do). Somewhat related, I had been looking at debug symbols (from the 31.1 release), to check where symbols were being duplicated across TUs, rather than squashed to a single definition by the linker; MAX_SCRIPT_ELEMENT_SIZE is one of a few. Fixing these is basically just changing static const -> inline constexpr, which also seems worthwhile, and if anything, shrinks our release binary.

maflcko commented at 12:37 PM on July 30, 2026:

Everything in this thread is resolved and it can be closed?

rustaceanrob commented at 12:42 PM on July 30, 2026:

Somewhat related, I had been looking at debug symbols (from the 31.1 release), to check where symbols were being duplicated across TUs, rather than squashed to a single definition by the linker; MAX_SCRIPT_ELEMENT_SIZE is one of a few. Fixing these is basically just changing static const -> inline constexpr, which also seems worthwhile

Perhaps this can be made an issue if there are other associated changes or this impact on the binary size is noticeable.

maflcko commented at 6:14 PM on July 30, 2026:

Did some inline constexpr scripted diff in #35852. Let's see what reviewers think.

in src/test/util/framework.h:66 in 421271dbcc outdated

  61 | +    {
  62 | +        registry().emplace_back(TestCase{current_test_suite(), name, fn});
  63 | +    }
  64 | +};
  65 | +
  66 | +/** Path that the test binary */

maflcko commented at 9:16 AM on July 14, 2026:

llm-nit: (Also looks like the LLM broke down):

Possible typos and grammar issues:

/** Path that the test binary */ -> /** Path to the test binary */ [the original comment is grammatically incomplete and अस्पष्ट]

rustaceanrob commented at 9:38 AM on July 14, 2026: member

Draft while I work on #35713 (review)

rustaceanrob marked this as a draft on Jul 14, 2026

rustaceanrob force-pushed on Jul 14, 2026

DrahtBot added the label CI failed on Jul 14, 2026

DrahtBot removed the label CI failed on Jul 14, 2026

rustaceanrob force-pushed on Jul 15, 2026

DrahtBot added the label CI failed on Jul 15, 2026

DrahtBot commented at 12:03 PM on July 15, 2026: contributor

🚧 At least one of the CI tasks failed. Task OpenBSD Cross: https://github.com/bitcoin/bitcoin/actions/runs/29411082055/job/87338055592 LLM reason (✨ experimental): CI failed due to a linker error: undefined symbol HexStr(std::span<const unsigned char,...>) while building test_kernel (missing HexStr implementation).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

rustaceanrob marked this as ready for review on Jul 15, 2026

DrahtBot removed the label CI failed on Jul 15, 2026

rustaceanrob marked this as a draft on Jul 15, 2026

rustaceanrob force-pushed on Jul 20, 2026

DrahtBot added the label CI failed on Jul 20, 2026

DrahtBot commented at 10:54 AM on July 20, 2026: contributor

🚧 At least one of the CI tasks failed. Task NetBSD Cross: https://github.com/bitcoin/bitcoin/actions/runs/29734060337/job/88325092997 LLM reason (✨ experimental): CI failed during compilation of test_kernel.cpp due to Clang errors (notably ambiguous Txid and missing operator<< needed by the test framework for BOOST_CHECK output).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

DrahtBot removed the label CI failed on Jul 20, 2026

rustaceanrob force-pushed on Jul 20, 2026

rustaceanrob marked this as ready for review on Jul 20, 2026

rustaceanrob force-pushed on Jul 22, 2026

rustaceanrob referenced this in commit 075e7f4218 on Jul 22, 2026

DrahtBot added the label CI failed on Jul 22, 2026

fanquake referenced this in commit 6ced9ad782 on Jul 22, 2026

rustaceanrob force-pushed on Jul 22, 2026

DrahtBot removed the label CI failed on Jul 22, 2026

fanquake referenced this in commit fb22682c45 on Jul 24, 2026

fanquake referenced this in commit 3a2c52f9d7 on Jul 24, 2026

rustaceanrob force-pushed on Jul 24, 2026

rustaceanrob force-pushed on Jul 26, 2026

rustaceanrob force-pushed on Jul 28, 2026

DrahtBot added the label Needs rebase on Jul 29, 2026

rustaceanrob force-pushed on Jul 29, 2026

DrahtBot removed the label Needs rebase on Jul 29, 2026

maflcko commented at 8:09 AM on July 30, 2026: member

Could rebase and drop the unused boost workaround in guix?

+# Use LINK_WARNING_AS_ERROR when using CMake 4.x
+case "$HOST" in
+    riscv64-linux-gnu) ;; # https://github.com/boostorg/test/issues/345
+    *) HOST_LDFLAGS="${HOST_LDFLAGS} -Wl,--fatal-warnings" ;;
+esac
+

rustaceanrob force-pushed on Jul 30, 2026

rustaceanrob commented at 9:40 AM on July 30, 2026: member

Given the response to #35729, which I was surprised to see pushback on, I found a solution to preserve && and || at the cost of the debug output. By implementing operator bool we can chain operations within the macros to preserve the perceived expressiveness of writing two conditions in one check. In the current state, this makes the loss of debug output implicit, but the PR becomes a lot more concise. We can revisit if we should delete the && and || at a later point, or perhaps fix these as files are migrated.

rustaceanrob force-pushed on Aug 1, 2026

rustaceanrob commented at 10:20 AM on August 3, 2026: member

I walked back the comment above on the basis that the test framework should not hide debug behavior implicitly. && and || are delete in the latest push, which also includes the changes from #35729 once more.

rustaceanrob force-pushed on Aug 4, 2026

sedited referenced this in commit aa0e0f793f on Aug 11, 2026

DrahtBot added the label Needs rebase on Aug 11, 2026

rustaceanrob force-pushed on Aug 11, 2026

DrahtBot removed the label Needs rebase on Aug 11, 2026

DrahtBot added the label CI failed on Aug 11, 2026

DrahtBot commented at 7:03 PM on August 11, 2026: contributor

🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/31517920788/job/93867572289 LLM reason (✨ experimental): CI failed because clang-tidy reported an error: main is defined in a header file (test/util/framework.h), triggering misc-definitions-in-headers (warnings-as-errors).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

rustaceanrob force-pushed on Aug 12, 2026

DrahtBot removed the label CI failed on Aug 12, 2026

DrahtBot added the label Needs rebase on Aug 14, 2026

rustaceanrob referenced this in commit 4e1c45341e on Aug 17, 2026

rustaceanrob referenced this in commit 28289ac065 on Aug 18, 2026

in src/wallet/test/wallet_transaction_tests.cpp:52 in 312b1ec3b1

  46 | @@ -47,12 +47,8 @@ BOOST_AUTO_TEST_CASE(deserialize_rejects_mismatched_variant_txid)
  47 |  
  48 |      // A variant whose txid doesn't match the canonical txid must be rejected.
  49 |      std::map<Wtxid, CTransactionRef> bad_variants{{tx_b->GetWitnessHash(), tx_b}};
  50 | -    try {
  51 | -        CWalletTx(deserialize, ss, bad_variants);
  52 | -        BOOST_FAIL("expected std::runtime_error was not thrown");

maflcko commented at 8:44 AM on August 19, 2026:

This points at a larger problem of all the verbose, brittle and confusing manual exception checks. I'd say all of those should be removed. Done in https://github.com/bitcoin/bitcoin/pull/36018

rustaceanrob referenced this in commit 1156ce6754 on Aug 19, 2026

fanquake referenced this in commit 8c1d776bf2 on Aug 19, 2026

rustaceanrob force-pushed on Aug 20, 2026

in src/test/validation_chainstatemanager_tests.cpp:181 in 6a4ae21f18

 180 | -    BOOST_CHECK_CLOSE(double(c2.m_coinstip_cache_size_bytes), max_cache * 0.95, 1);
 181 | -    BOOST_CHECK_CLOSE(double(c2.m_coinsdb_cache_size_bytes), max_cache * 0.95, 1);
 182 | +    auto close_to = [](double a, double b, double tol_pct) {
 183 | +        return std::abs(a - b) <= (tol_pct / 100.0) * std::max(std::abs(a), std::abs(b));
 184 | +    };
 185 | +    BOOST_CHECK(close_to(double(c1.m_coinstip_cache_size_bytes), max_cache * 0.05, 1));

maflcko commented at 2:28 PM on August 20, 2026:

nit: Use clang-tidy named-args here for the third arg, to avoid confusion with fraction vs pct https://stackoverflow.com/questions/1093453/difference-between-boost-check-close-and-boost-check-close-fraction?

    BOOST_CHECK(close_to(double{c1.m_coinstip_cache_size_bytes}, max_cache * 0.05, /*tol_pct=*/1));

DrahtBot removed the label Needs rebase on Aug 20, 2026

DrahtBot added the label Needs rebase on Aug 21, 2026

rustaceanrob force-pushed on Aug 23, 2026

DrahtBot removed the label Needs rebase on Aug 23, 2026

Kino1994 referenced this in commit 46a55dfc92 on Aug 23, 2026

fanquake referenced this in commit aed80c7395 on Aug 24, 2026

rustaceanrob force-pushed on Aug 24, 2026

jeanpablojp commented at 8:02 PM on August 30, 2026: contributor

Concept ACK

The whole suite passes here and the scripted-diff reproduces.

BOOST_TEST_RUN_FILTERS is no longer read, so deterministic-unittest-coverage runs the whole binary instead of the suite it was given.

bitcoin_core_ci.sh in the libmultiprocess subtree still passes three Boost.Test flags, and any one of them now makes it exit with code 2 without running anything. It doesn't run in CI here, but it checks out Core master, so it breaks as soon as this lands.

One question on scope. That's 1096 new lines in framework.h and I couldn't find anything covering their failure paths. util_check_tests covers both paths for Assert. Would something similar work here, or is it worth a follow-up? Happy to help if useful.

test: Add display `operator<<` overloads to `common.{h,cpp}`

If the `<<` operator exists, boost can use this as a debug output when a
check fails (besides `BOOST_CHECK`). This adds `<<` for types commonly
used across the unit tests that do not yet have a display output.

40adeb6d30

test: Convert `BOOST_CHECK` macros with known debug string

Using the previous commit we can improve the error output of these
checks by updating the boost macros to a version that prints the values
on failure.

e23ce049ec

test: Remove low usage `BOOST` macros

`BOOST_FAIL`: This check may be simplified by using the timeout as the
condition for a `BOOST_REQUIRE`

`BOOST_TEST_INFO`: In the case of `descriptor_tests.cpp`, the info is only used to get the
diagonsics of `BOOST_CHECK_EQUAL` along with the human readable form of
the descriptor. This is irrelevant if the test framework implements
expression decomposition, which will print the string representation on
failure (e.g. `BOOST_TEST`).

b5de66e4cc

test: Inline `CRecipient` construction to avoid gcc12 uninitialized warning

gcc12 warns with uninitialized when boost is not used for this test.
Unfortunately, I think this is a false positive, as `CRecipient` seems
obivously initialized?

ref: https://github.com/boostorg/variant2/issues/55

error:
```
>::_Vector_impl::<anonymous>.std::_Vector_base<unsigned char, std::allocator<unsigned char> >::_Vector_impl_data::_M_end_of_storage’ may be used uninitialized [-Werror=maybe-uninitialized]

367 | _M_impl._M_end_of_storage - _M_impl._M_start);

| ~~~~~~~~^~~~~~~~~~~~~~~~~

/home/runner/work/_temp/src/wallet/test/spend_tests.cpp: In lambda function:

/home/runner/work/_temp/src/wallet/test/spend_tests.cpp:47:20: note: ‘recipient’ declared here

47 | CRecipient recipient{PubKeyDestination({}), 50 * COIN - leftover_input_amount, /*subtract_fee=*/true};

| ^~~~~~~~~

cc1plus: all warnings being treated as errors

gmake[2]: *** [src/test/CMakeFiles/test_bitcoin.dir/build.make:2142: src/test/CMakeFiles/test_bitcoin.dir/__/wallet/test/spend_tests.cpp.o] Error 1

gmake[1]: *** [CMakeFiles/Makefile2:2621: src/test/CMakeFiles/test_bitcoin.dir/all] Error 2

gmake: *** [Makefile:146: all] Error 2

Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', 'e5149787b8a1739029b127ba5d3b9cf034d955ca907ea03ab0cfe5de2af9e8d9', '/home/runner/work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 2.
```

839d87cb62

test: Wrap bitwise `&` expressions in macros

Logical operators are not usable with test frameworks that decompose
expressions, as introduced in a later commit.

ref: https://catch2-temp.readthedocs.io/en/latest/assertions.html#other-limitations
ref: https://fekir.info/post/decomposing-an-expression/

0583e4c39f

test: Wrap `||`, `&&` expressions in macros

The `||` and `&&` re not usable with `BOOST_TEST` and other modern test
frameworks like Catch2. This is due to operator precedence used in such
macros. Here is an additional opinion from Catch2:

> There is no simple rewrite rule for ||, but I generally believe
that if you have || in your test expression, you should rethink your tests.

ref: https://catch2-temp.readthedocs.io/en/latest/assertions.html#other-limitations
ref: https://fekir.info/post/decomposing-an-expression/

2879a4768f

test: Add header-only framework to util

ODR violation lint suppression: https://clang.llvm.org/extra/clang-tidy/checks/misc/definitions-in-headers.html

f8aba43fa6

scripted-diff: Migrate tests to header-only framework

This migrates the test runner, boost header imports, and linter.
`include <memory>` line is to fix an IWYU.

-BEGIN VERIFY SCRIPT-
mv src/test/new_main.cpp src/test/main.cpp
git grep -l '<boost/test/unit_test.hpp>' | xargs sed -i 's|<boost/test/unit_test.hpp>|<test/util/framework.h>|'
sed -i 's|BOOST_TEST_MODULE Bitcoin Kernel Test Suite|BITCOIN_TEST_MAIN|; s|<boost/test/included/unit_test.hpp>|<test/util/framework.h>|' src/test/kernel/test_kernel.cpp
sed -i 's|boost::unit_test::framework::master_test_suite().argv\[0\]|framework::executable_path()|' src/test/system_tests.cpp
sed -i '/boost\/test\/\(included\/\)\?unit_test.hpp/d' test/lint/lint-includes.py
sed -i 's| --catch_system_error=no||' src/test/CMakeLists.txt
sed -i 's|#include <test/util/framework.h>|&\n\n#include <memory>|' src/test/result_tests.cpp
-END VERIFY SCRIPT-

25169cb26a

test: Require `stringify` implementation or explicit omission

If there is no meaningful string representation for a type being
checked, this fails with a `static_assert`. If the test writer does not
want to add a string representation, there is a `CHECK_NO_DISPLAY` macro
for explicit opt-out.

6789b16fa3

depends: drop test from Boost libraries 14c813e58c

cmake: drop vcpkg Boost Test check 2ab4babeef

vcpkg: drop boost-test dependency 22f2652965

guix: Drop `Boost.Test` unused workaround c7677eae85

in src/test/util/framework.h:481 in 8be2320bfe outdated

 476 | +        if constexpr (std::string_view{#op} == "==" && is_range_check<T> && is_range_check<U>) {                   \
 477 | +            return check_equal_ranges(lhs, rhs);                                                                   \
 478 | +        } else {                                                                                                   \
 479 | +            bool btc_test_result;                                                                                  \
 480 | +            if constexpr (is_cstring<T> && is_cstring<U>) {                                                        \
 481 | +                btc_test_result = (std::strcmp(lhs, rhs) op 0);                                                    \

jeanpablojp commented at 8:02 PM on August 30, 2026:

Comparing contents here is right, BOOST_CHECK_EQUAL already did that. What comes with it is that there's no null check, and is_cstring matches after decay, so literals land here too. With either side null this segfaults instead of reporting the failure. On master two nulls pass and one null reports a failure.

Today the branch is only reached from NullOverride in settings_tests, where neither side can be null, so nothing breaks yet.

in src/test/util/framework.h:121 in 8be2320bfe outdated

 116 | +template <typename... Args>
 117 | +inline void log(LogLevel level, const char* fmt, Args&&... args)
 118 | +{
 119 | +    if (current_log_level() >= level) {
 120 | +        std::scoped_lock lock{log_mutex()};
 121 | +        tfm::vformat(std::cout, fmt, tfm::makeFormatList(std::forward<Args>(args)...));

jeanpablojp commented at 8:02 PM on August 30, 2026:

This never flushes. With a crash inside a test case and output redirected, master keeps the preceding lines and here the file comes back empty. Boost catches the signal and names the case that died, while the PR description explicitly calls out disabling catch_system_errors. I didn't see the flush mentioned, and it's what makes a crash leave no trace at all.

Under CTest the loss is partial, because the node log with -printtoconsole=1 flushes stdout per line and carries the framework's lines along with it.

The same buffer causes a second, smaller thing. test_LockDirectory forks and the child exits with exit(0), reprinting what it inherited, so the output comes out duplicated.

in contrib/guix/libexec/build_linux.sh:41 in 8be2320bfe outdated

  34 | @@ -35,12 +35,6 @@ esac
  35 |  # LDFLAGS
  36 |  HOST_LDFLAGS="-Wl,--as-needed -Wl,--dynamic-linker=$(glibc_dynamic_linker "$HOST") -Wl,-O2"
  37 |  
  38 | -# Use LINK_WARNING_AS_ERROR when using CMake 4.x
  39 | -case "$HOST" in
  40 | -    riscv64-linux-gnu) ;; # https://github.com/boostorg/test/issues/345
  41 | -    *) HOST_LDFLAGS="${HOST_LDFLAGS} -Wl,--fatal-warnings" ;;

jeanpablojp commented at 8:02 PM on August 30, 2026:

I saw the request to drop the workaround. Looking at b12a70f330, the workaround looks to me like the riscv64-linux-gnu arm of the case rather than the flag, since the exception was there because of the linker warnings from execution_monitor.ipp, which this PR removes. Dropping the whole block means no Linux host links with -Wl,--fatal-warnings any more, while build_win.sh and build_win_gui.sh keep it. CMake's working_linker_werror_flag doesn't cover this. Would it make sense to keep the flag for every host now that riscv64 is clean?

in src/test/util/framework.h:696 in 8be2320bfe outdated

 691 | +        return status;
 692 | +    }
 693 | +    current_log_level() = opts.log_level;
 694 | +    user_args() = std::move(opts.passthrough);
 695 | +    // To avoid breaking user scripts unexpectedly, reuse boost naming here.
 696 | +    if (std::getenv("BOOST_TEST_RANDOM")) {

jeanpablojp commented at 8:02 PM on August 30, 2026:

Here the value is only checked for presence, never read, so 0 and an empty string both turn shuffling on and a fixed seed no longer reproduces. With BOOST_TEST_RANDOM=5 master repeats the order across runs and this doesn't. Boost also prints the seed it used, which here doesn't come out at any level, so there's no way to reproduce an order-dependent failure. 00_setup_env.sh already uses ${BOOST_TEST_RANDOM:-1}.

in src/test/util/framework.h:216 in 8be2320bfe outdated

 211 | +{
 212 | +    return "\"" + std::string(v.begin(), v.end()) + "\"";
 213 | +}
 214 | +
 215 | +template <typename T>
 216 | +std::string stringify(const std::optional<T>& v)

jeanpablojp commented at 8:02 PM on August 30, 2026:

This overload wins over the primary by partial ordering, so the static_assert never reaches std::optional. With BOOST_CHECK_EQUAL and a T that has no operator<<, comparing two optional<T> compiles here and prints the same type-only text on both sides, whereas the merge base refuses to compile.

Making it recurse into stringify(*v) does reach the assert, and then three existing sites stop building: the optional<CCoinsStats> in coinstatsindex_tests, the optional<FeeRateEstimate> in mempool_fee_estimator_tests, and the optional<Node<CPubKey>> in miniscript_tests. This looks like the same case as the iterators, so is this one for the CHECK_NO_DISPLAY follow-up too?

in src/test/util/framework.h:784 in 8be2320bfe outdated

 779 | + * static void btc_test_fn34();
 780 | + * static Registrar btc_test_reg{my_test, &btc_test_fn34};
 781 | + * static void btc_test_fn34()
 782 | + *
 783 | + * ... user defines behavior */
 784 | +#define TEST_CASE(name)                                                                                                      \

jeanpablojp commented at 8:02 PM on August 30, 2026:

TEST_SUITE_BEGIN(suite, Fixture) sets btc_suite_fixture, but TEST_CASE registers a plain function and never uses it. With a fixture whose constructor bumps a counter, a TEST_CASE in that suite sees 0 while a BOOST_AUTO_TEST_CASE in an otherwise identical suite sees 1. The description says the suite fixture applies to each test, and the comment above TEST_CASE says it's the no-fixture case. Should TEST_CASE delegate to FIXTURE_TEST_CASE(name, btc_suite_fixture)?

rustaceanrob force-pushed on Sep 3, 2026

rustaceanrob commented at 2:12 PM on September 3, 2026: member

Rebased with changes from #36091. Thanks for the review @jeanpablojp will address it soon