test: refine per-test framework context

Keep per-test mutable runner state in TestCaseContext while preserving the sequential runner model.

Use atomic check counters so joined child-thread CHECK calls are recorded, restrict aborting REQUIRE-style assertions to the owning test thread, and preserve check accounting when a test exits through an unexpected exception.

Update stale runner documentation for suite/case filters and --list_content.

Author: josibake

src/test/README.md

@@ -23,7 +23,7 @@ and tests weren't explicitly disabled.
 The unit tests can be run with `ctest --test-dir build`, which includes unit
 tests from subtrees.
 
-Run `build/bin/test_bitcoin --list` for the full list of tests.
+Run `build/bin/test_bitcoin --list_content` for the full list of tests.
 
 To run the unit tests manually, launch `build/bin/test_bitcoin`. To recompile
 after a test file was modified, run `cmake --build build` and then run the test again. If you
@@ -57,7 +57,7 @@ build/bin/test_bitcoin -l all -t getarg_tests
 or to run only the `doubledash` test in `getarg_tests`
 
 ```bash
-build/bin/test_bitcoin --run_test=getarg_tests::doubledash
+build/bin/test_bitcoin --run_test=getarg_tests/doubledash
 ```
 
 The `--log_level=` (or `-l`) argument controls the verbosity of the test output.

src/test/main.cpp

@@ -14,7 +14,7 @@
 /**
  * Retrieve the user-supplied command line arguments — everything after the
  * `--` separator on the command line. Allows usage like:
- * `test_bitcoin --run_test="net_tests::cnode_listen_port" -- -checkaddrman=1 -printtoconsole=1`
+ * `test_bitcoin --run_test="net_tests/cnode_listen_port" -- -checkaddrman=1 -printtoconsole=1`
  * which would return `["-checkaddrman=1", "-printtoconsole=1"]`.
  */
 const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS = []() {
@@ -23,7 +23,7 @@ const std::function<std::vector<const char*>()> G_TEST_COMMAND_LINE_ARGUMENTS =
 };
 
 /**
- * Retrieve the full name (`suite::case`) of the currently-running test.
+ * Retrieve the full name (`suite/case`) of the currently-running test.
  */
 const std::function<std::string()> G_TEST_GET_FULL_NAME = []() {
     return framework::current_test_full_name();

src/test/util/framework.hpp

@@ -19,6 +19,7 @@
 #include <stdexcept>
 #include <string>
 #include <string_view>
+#include <thread>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -53,41 +54,35 @@ struct Registrar {
     }
 };
 
-/** assertion counts for one test case. */
+/** Assertion counts for one test case. */
 struct TestStats {
     int checks = 0;
     int failed_checks = 0;
 };
 
-/** state owned by one running test case.
- * joined worker threads may use CHECK while the parent test is active. */
+/** State owned by one running test case.
+ *
+ * The runner is sequential: only one test case is active in a process.
+ * Non-aborting checks may be used from child threads that are joined before the
+ * test returns.
+ */
 struct TestCaseContext {
     explicit TestCaseContext(std::string name) : full_name{std::move(name)} {}
 
     const std::string full_name;
-    /** keep the test-thread path plain, worker threads pay the atomic cost. */
-    TestStats stats;
-    std::atomic<int> worker_checks{0};
-    std::atomic<int> worker_failed_checks{0};
+    const std::thread::id owner_thread{std::this_thread::get_id()};
+    std::atomic<int> checks{0};
+    std::atomic<int> failed_checks{0};
 };
 
-/** active context for the sequential runner.
- * parallel in-process tests need explicit context propagation. until then, fail closed. */
 inline std::atomic<TestCaseContext*>& active_test_context_storage()
 {
     static std::atomic<TestCaseContext*> context{nullptr};
     return context;
 }
 
-inline TestCaseContext*& local_test_context()
-{
-    thread_local TestCaseContext* context{nullptr};
-    return context;
-}
-
 inline TestCaseContext& active_test_context()
 {
-    if (auto* context{local_test_context()}) return *context;
     if (auto* context{active_test_context_storage().load(std::memory_order_acquire)}) return *context;
     throw std::logic_error{"test framework assertion used outside an active test case"};
 }
@@ -100,12 +95,10 @@ class ScopedTestCaseContext {
         if (!active_test_context_storage().compare_exchange_strong(expected, &context, std::memory_order_release, std::memory_order_relaxed)) {
             throw std::logic_error{"test framework supports only one active test case per process"};
         }
-        local_test_context() = &context;
     }
 
-    ~ScopedTestCaseContext()
+    ~ScopedTestCaseContext() noexcept
     {
-        local_test_context() = nullptr;
         active_test_context_storage().store(nullptr, std::memory_order_release);
     }
 
@@ -122,8 +115,8 @@ inline std::string current_test_full_name()
 inline TestStats snapshot_stats(const TestCaseContext& context)
 {
     return TestStats{
-        context.stats.checks + context.worker_checks.load(std::memory_order_relaxed),
-        context.stats.failed_checks + context.worker_failed_checks.load(std::memory_order_relaxed),
+        context.checks.load(std::memory_order_relaxed),
+        context.failed_checks.load(std::memory_order_relaxed),
     };
 }
 
@@ -287,21 +280,10 @@ inline std::optional<LogLevel> to_log_level(std::string_view s)
 inline void record_check(const Result& result, const char* kind, const char* expr,
                          const char* file, int line, const std::string& message = {})
 {
-    TestCaseContext* context{local_test_context()};
-    const bool worker_thread{context == nullptr};
-    if (worker_thread) context = &active_test_context();
-
-    if (worker_thread) {
-        context->worker_checks.fetch_add(1, std::memory_order_relaxed);
-    } else {
-        ++context->stats.checks;
-    }
+    auto& context = active_test_context();
+    context.checks.fetch_add(1, std::memory_order_relaxed);
     if (!result.is_ok()) {
-        if (worker_thread) {
-            context->worker_failed_checks.fetch_add(1, std::memory_order_relaxed);
-        } else {
-            ++context->stats.failed_checks;
-        }
+        context.failed_checks.fetch_add(1, std::memory_order_relaxed);
         if (current_log_level() >= LogLevel::Error) {
             std::scoped_lock lock{log_mutex()};
             log_unlocked("[FAIL]: %s:%d: %s(%s)\n", file, line, kind, expr);
@@ -313,6 +295,15 @@ inline void record_check(const Result& result, const char* kind, const char* exp
     log(LogLevel::All, "[PASS]: %s:%d: %s(%s)\n", file, line, kind, expr);
 }
 
+inline bool aborting_check_allowed(const char* kind, const char* file, int line)
+{
+    const auto& context = active_test_context();
+    if (context.owner_thread == std::this_thread::get_id()) return true;
+    record_check(Result::failed(std::string{kind} + " used from a child thread; use non-aborting checks in child threads"),
+                 kind, "test-thread-only assertion", file, line);
+    return false;
+}
+
 template <std::ranges::range R1, std::ranges::range R2>
     requires std::same_as<std::ranges::range_value_t<R1>, std::ranges::range_value_t<R2>>
 Result check_equal_ranges(const R1& r1, const R2& r2)
@@ -506,22 +497,25 @@ inline int run(int argc, char** argv)
             continue;
         }
         TestCaseContext context{test_name(test_case)};
+        bool failed_by_exception{false};
         try {
             ScopedTestCaseContext active_context{context};
             test_case.fn();
         } catch (const RequireFailed&) {
         } catch (const std::exception& e) {
-            ++summary.failed;
+            failed_by_exception = true;
             log(LogLevel::Error, "EXCEPTION in %s: %s\n", test_case.name, e.what());
-            continue;
         } catch (...) {
-            ++summary.failed;
+            failed_by_exception = true;
             log(LogLevel::Error, "EXCEPTION in %s: %s\n", test_case.name, "unknown exception");
-            continue;
         }
         const auto stats = snapshot_stats(context);
         summary.total_checks += stats.checks;
-        if (stats.failed_checks == 0) {
+        if (failed_by_exception) {
+            ++summary.failed;
+            log(LogLevel::Error, "[FAIL] %s (exception after %d checks, %d checks failed)\n",
+                test_case.name, stats.checks, stats.failed_checks);
+        } else if (stats.failed_checks == 0) {
             ++summary.passed;
             log(LogLevel::Info, "[ OK ] %s (%d checks)\n", test_case.name, stats.checks);
         } else {
@@ -634,13 +628,16 @@ inline int run(int argc, char** argv)
                                   btc_test_res_.is_ok() ? std::string{} : btc_test_os_.str()); \
     } while (false)
 
-/** Like CHECK, but aborts the current test on failure by throwing.
- * Subsequent checks in the test are skipped.
- * keep this on the test thread, use CHECK from worker threads.
+/** Like CHECK, but aborts the current test's control flow on failure by throwing.
+ * Subsequent checks in that control flow are skipped. Use CHECK from child
+ * threads, because throwing in a child thread cannot abort the parent test.
  *
  * Accepts the same optional stream-style failure message as CHECK. */
 #define REQUIRE(expr, ...)                                                                     \
     do {                                                                                       \
+        if (!::framework::aborting_check_allowed("REQUIRE", __FILE__, __LINE__)) {             \
+            break;                                                                             \
+        }                                                                                      \
         BITCOIN_TEST_DIAG_PUSH                                                                 \
         ::framework::Result btc_test_res_ = ::framework::Decomposer{} <= expr;                 \
         BITCOIN_TEST_DIAG_POP                                                                  \
@@ -678,9 +675,13 @@ inline int run(int argc, char** argv)
                                   #expr, __FILE__, __LINE__);                  \
     } while (false)
 
-/** Like CHECK_NOTHROW, but aborts the current test on failure. */
+/** Like CHECK_NOTHROW, but aborts the current test's control flow on failure. */
 #define REQUIRE_NOTHROW(expr)                                                  \
     do {                                                                       \
+        if (!::framework::aborting_check_allowed("REQUIRE_NOTHROW", __FILE__,  \
+                                                 __LINE__)) {                  \
+            break;                                                             \
+        }                                                                      \
         ::framework::Result btc_test_res_ = ::framework::Result::ok();         \
         try {                                                                  \
             expr;                                                              \