Remove boost as a unit test runner #35713

pull rustaceanrob wants to merge 18 commits into bitcoin:master from rustaceanrob:remove-boost-test changing 165 files +1343 −364
  1. 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>

    No repository context

    Many types implement ToString, but boost has no way of meaningfully using these representations (will change w/ future std::format). With a repository-specific test runner we can use these today. See example above.

    Not extensible

    With the test runner in this repository we can add things such as #35139, implement string representations for more types, and implement ideas from #8670.

    Other issues

    More issues with boost are raised in #34666, #8670. This PR does not resolve all of them, but one it does address immediately is banning comparisons of integers with different signs, previously allowed by boost. 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.

    Changes in writing tests

    • 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
    • CHECK_EQUAL_RANGES(it1, it2) to compare two ranges
    • 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.

    Commits

    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 use of BOOST_FAIL
    • test: Remove use of BOOST_TEST_INFO_*
    • test: Remove use of BOOST_CHECK_CLOSE

    Required for expression decomposition:

    • crypto: Use constexpr to define output size
    • util: Use constexpr in LockedPool definitions
    • net: Make AddressPosition comparitor const
    • test: Unroll && conditions in macros
    • test: Wrap || expressions in macros
    • test: Wrap bitwise & expressions in macros

    To pass CI

    • test: Use lambda for CRecipient to avoid gcc12 uninitialized warning
    • test: Include memory in result_tests for IWYU

    Build and config

    • depends: drop test from Boost libraries
    • cmake: drop vcpkg Boost Test check
    • vcpkg: drop boost-test dependency
  2. DrahtBot commented at 2:23 PM on July 13, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

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

    Type Reviewers
    Concept ACK josibake

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35662 (script: make txdata non-default-constructible by l0rinc)
    • #35531 (txindex: hash keys and pack positions to reduce disk usage by andrewtoth)
    • #35511 (RFC: consensus: Make CAmount a class by hodlinator)
    • #35195 (coins: cache UTXO outpoint hash codes by l0rinc)
    • #35084 (ipc: Add nonunix platform support by ryanofsky)
    • #34864 (coins: tighten cache entry state invariants by l0rinc)
    • #34075 (fees: Introduce Mempool Based Fee Estimation to reduce overestimation by ismaelsadeeq)
    • #33540 (argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments) by pablomartin4btc)
    • #31682 ([IBD] specialize CheckBlock's input & coinbase checks by l0rinc)
    • #31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
    • #29278 (Wallet: Add maxfeerate wallet startup option by ismaelsadeeq)
    • #25665 (refactor: Add util::Result failure types and ability to merge result values by ryanofsky)
    • #17783 (common: Disallow calling IsArgSet() on ALLOW_LIST options by ryanofsky)
    • #17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
    • #17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
    • #17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)

    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.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

    LLM Linter (✨ experimental)

    Possible typos and grammar issues:

    • The usual forward declaration of the test function is implement with the runner() -> is implemented with the runner() [grammar is broken; “implement” should be “implemented”]

    Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

    • close_to(double, double, double) in src/test/validation_chainstatemanager_tests.cpp
    • Lookup(..., 1, false) in src/test/pcp_tests.cpp

    <sup>2026-07-14 11:54:10</sup>

  3. 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!

  4. 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.

  5. crypto: Use `constexpr` to define output size
    Expression decomposition in `BOOST_TEST` and the framework introduced
    later binds the operand to a `const T&`, odr-using these members and
    requiring a definition. Marking them `constexpr` implicitly inlines
    them in C++17, supplying the definition.
    17b37d2fdd
  6. util: Use `constexpr` in `LockedPool` definitions
    Expression decomposition in `BOOST_TEST` and the framework introduced
    later binds the operand to a `const T&`, odr-using these constants and
    requiring a definition. Marking them `constexpr` implicitly inlines
    them in C++17, supplying the definition.
    d01f1eb984
  7. net: Make `AddressPosition` comparitor `const`
    Needed to use `==` operator in tests.
    9e1247067d
  8. test: Include `memory` in `result_tests` for IWYU
    This is being brought in transitively by boost
    68cf66930c
  9. 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) {
          |          ^
    
  10. 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 अस्पष्ट]
  11. rustaceanrob commented at 9:38 AM on July 14, 2026: member

    Draft while I work on #35713 (review)

  12. rustaceanrob marked this as a draft on Jul 14, 2026
  13. test: Remove use of `BOOST_FAIL`
    There is only a single case of this macro and the if conditional that
    triggers it may simply be used in a `BOOST_REQUIRE`
    295f026539
  14. test: Remove use of `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`).
    
    Removing `BOOST_TEST_INFO_SCOPE` requires a few duplications, but these are
    very little cost compared to a port of this macro, which does not appear
    very useful.
    62236cdc76
  15. test: Remove use of `BOOST_CHECK_CLOSE`
    This is only used in a single test and would be a low-motivation add to
    porting the test framework. A lambda is sufficient to enforce the check.
    b00498afea
  16. test: Move `Coin` `==` operator out of anon namespace fec54bc8dc
  17. test: Use lambda for `CRecipient` to avoid gcc12 uninitialized warning
    gcc12 warns with uninitialized when boost is not used for this test.
    This side-steps the warning.
    711ee396d3
  18. test: Avoid unnecessary type conversions in check conditions
    `EraseTx` already returns a `bool`, so these implicit casts within the
    comparisons are not necessary.
    87f176edde
  19. test: Unroll `&&` conditions in macros
    Using `&&` in `BOOST_CHECK` is problematic as failures will not indicate
    which condition failed. By unrolling these checks, the user knows
    exactly which expression is the failing case.
    
    This is also required when using test macros that support value
    decomposition, which requires `&&` and `||` are `delete`. Examples
    include `BOOST_TEST`, doctest, Catch2, etc.
    
    ref: https://fekir.info/post/decomposing-an-expression/
    b8e8bd1d1c
  20. test: Wrap `||` expressions in macros
    The `||` is 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/
    8e6b37ba38
  21. test: Wrap bitwise `&` expressions in macros 593031f389
  22. rustaceanrob force-pushed on Jul 14, 2026
  23. test: Add header-only framework to util 71967f9acb
  24. scripted-diff: Migrate tests to header-only framework
    -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
    -END VERIFY SCRIPT-
    5110e41262
  25. depends: drop test from Boost libraries 359d51f313
  26. cmake: drop vcpkg Boost Test check f4a7b09fbf
  27. vcpkg: drop boost-test dependency 16926e7dae
  28. rustaceanrob force-pushed on Jul 14, 2026
  29. DrahtBot added the label CI failed on Jul 14, 2026
  30. DrahtBot removed the label CI failed on Jul 14, 2026

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-07-14 16:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me