From b9bbbd7583fd34f649e328ad3f5b10887e31f148 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 24 Aug 2026 00:12:07 +0300 Subject: [PATCH] testkit: a worker-side executor double, and ledger's report tests driven by it Closes #161. examples/common/testkit/step_executor.hpp adds StepExecutor: an IExecutor that queues posted tasks and runs them only on runOne()/runAll(). Substituted for the ThreadPoolExecutor a model or App would otherwise own, it turns a submit-then-poll job into an exact sequence -- submit, assert still pending, run the worker, assert done -- with no sleep, no retry cap, and no flakiness budget. It is a deliberate mirror of morph::testing::StepExecutor (tests/test_support.hpp), same name and same API, duplicated for the reason DeterministicExecutor is already duplicated from that header: it is private to morph_tests' own translation units and has no reachable include path from examples/. That reachability gap, not the absence of the semantics, is what left every ladder async-job test spinning a real pool -- worth stating plainly because #161 reads the other way, listing StepExecutor among the doubles that "sit on the callback side". An IExecutor has no callback side; what it drives is decided by what it is plugged into. LedgerModel grows the constructor its own _reportExecutor comment has always claimed a caller could use ("so a caller can substitute a different executor ... without this class changing shape"). Until now there was no way to. test_ledger_reports.cpp converts three of its four report jobs and keeps ONE on a real ThreadPoolExecutor, deliberately: that case is the only coverage of the default-constructed model -- the constructor the bridge registry actually uses -- and the only one where the worker runs on a genuinely different thread, with the caller's thread-local session context absent. A worker that wrongly reached for morph::session::current() would pass every converted case and fail only there. Two cases are new, and neither was expressible before: - "A submitted report stays Pending until its worker actually runs" -- the negative assertion a real pool cannot support. - "Running one report job settles that job and no other" -- two jobs outstanding at once, settled one at a time. Mutation-verified rather than asserted. Replacing _reportExecutor->post(task) with an inline call -- SubmitReport computing the whole report synchronously on the caller's thread -- leaves the pre-change suite entirely green (44 assertions, 5 cases, all passed) and fails 20 assertions across 4 cases of the converted one. Three further mutations (finishReportJob made a no-op; finishReportJob settling the oldest job row instead of its own; the monthly period filter disabled) each fail the converted suite, the last of them only in the timezone case, which confirms the conversion did not hollow that case out. Measured, and smaller than #161 implies: the sleeps were costing about 26 ms, not the seconds the issue's framing suggests. [reports] runs 1.28-1.29 s before and 1.54-1.57 s after -- but the after has two more cases; on the five cases common to both it is 1.223 s -> 1.197 s. An instrumented run shows pollUntilSettled slept 7-15 times per call site, so ~2 s of sleep() calls did disappear; they had simply been overlapping the worker's real work, and only the last partial 10 ms slice was ever wall-clock. What the conversion actually buys is the determinism and the two assertions above, not speed. Local verification: Debug/Ninja, /opt/homebrew/opt/llvm/bin/clang++, ledger rung only. ladder_common_tests and ladder_ledger_tests built and run (ctest -L ladder -LE stress, 177/177), plus scripts/check_spec_citations.sh. Both changed translation units compile under -Weverything -Werror (verified from the ninja command line). Every other rung, the sanitizer legs and the WASM leg are left to CI. Doxygen's input is include/morph only, which this change does not touch, so the Docs job is unaffected; the new symbols carry full @param/@return/@throws docs regardless. Co-Authored-By: Claude Opus 5 (1M context) --- examples/TESTING.md | 23 +++ examples/common/CMakeLists.txt | 4 +- examples/common/testkit/step_executor.hpp | 129 +++++++++++++++ .../common/testkit/test_step_executor.cpp | 114 +++++++++++++ .../include/ledger/models/ledger_model.hpp | 33 +++- examples/ledger/src/models/ledger_model.cpp | 12 ++ examples/ledger/tests/test_ledger_reports.cpp | 150 ++++++++++++++++-- 7 files changed, 450 insertions(+), 15 deletions(-) create mode 100644 examples/common/testkit/step_executor.hpp create mode 100644 examples/common/testkit/test_step_executor.cpp diff --git a/examples/TESTING.md b/examples/TESTING.md index 5e06b38f..82f39bb5 100644 --- a/examples/TESTING.md +++ b/examples/TESTING.md @@ -156,6 +156,28 @@ A `sleep_for` outside `pump.hpp` is a review-rejectable defect. The test binary uses the Qt-owning `main()` (QCoreApplication + `Catch::Session` + DeferredDelete drain) copied from `tests/qt/test_qt_websocket.cpp`. +`pump.hpp` covers waiting on the *Qt loop*. Waiting on a **background job** +has its own answer, and it is not a wait at all: + +- `step_executor.hpp` — `StepExecutor`, an `IExecutor` that queues posted + tasks and runs them only on `runOne()`/`runAll()`. Substituted for the + `ThreadPoolExecutor` a model or App would otherwise own, it turns + submit-then-poll into an exact sequence: submit, `CHECK(pending() == 1)`, + `runOne()`, assert done. The negative half — "the worker has **not** run + yet" — is assertable only this way; against a real pool it can only be + sampled. `runAll()` picks up tasks a running task posts, so a chained job + runs to completion instead of stranding its own continuation, and is + bounded so a self-reposting task fails loudly rather than hanging. + It mirrors `morph::testing::StepExecutor` (`tests/test_support.hpp`), which + the framework's own suite has always had; the ladder copy exists because + that header has no reachable include path from `examples/`. + +A test that keeps a real `ThreadPoolExecutor` under an async job — because it +is covering the production wiring, or the fact that the worker runs on a +genuinely different thread with no session context — says so at the test case +and pays the retry loop knowingly. `examples/ledger/tests/test_ledger_reports.cpp` +keeps exactly one such case and converts the rest. + ## Multi-client stress harness Testkit components, with the rung that **first needs** each (this ordering @@ -167,6 +189,7 @@ DoD): | `testkit_main.cpp`, `pump.hpp`, `backend_rig.hpp`, `db_fixture.hpp`, `db_fault_fixture.hpp`, **fault proxy + strand interleaver** (pulled forward, round-7) | rung 0/1 | | `client_pool.hpp`, `convergence.hpp` | rung 3 | | `action_driver.hpp`, `process_pool.hpp`, `offline_rig.hpp` | rung 4 | +| `step_executor.hpp` | rung 5 | - `db_fault_fixture.hpp` — holds a real `Lightweight::SqlScopedLock` on a second, independent `SqlConnection` to the shared test database, producing diff --git a/examples/common/CMakeLists.txt b/examples/common/CMakeLists.txt index 560a2b78..d7966b71 100644 --- a/examples/common/CMakeLists.txt +++ b/examples/common/CMakeLists.txt @@ -156,7 +156,8 @@ if(NOT Catch2_FOUND) endif() # ── morph_ladder_testkit: pump/fixtures/rig/fault-proxy/interleaver ───────── -# strand_interleaver.hpp (DeterministicExecutor), db_fixture.hpp and +# strand_interleaver.hpp (DeterministicExecutor), step_executor.hpp +# (StepExecutor), db_fixture.hpp and # db_fault_fixture.hpp are fully header-defined and have no .cpp: none is a # QObject, none needs MOC, and the library already links a non-empty TU # (fault_proxy.cpp), so content-free placeholder TUs would be dead weight. @@ -200,6 +201,7 @@ add_executable(ladder_common_tests testkit/test_event_poller.cpp testkit/test_fault_proxy.cpp testkit/test_strand_interleaver.cpp + testkit/test_step_executor.cpp testkit/test_wasm_registration_path_native.cpp testkit/test_action_driver.cpp testkit/test_offline_rig.cpp diff --git a/examples/common/testkit/step_executor.hpp b/examples/common/testkit/step_executor.hpp new file mode 100644 index 00000000..4d893e14 --- /dev/null +++ b/examples/common/testkit/step_executor.hpp @@ -0,0 +1,129 @@ +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include +#include +#include +#include +#include +#include + +/// @file +/// The ladder testkit's worker-side executor double (examples/TESTING.md, +/// "Pumping discipline"): the substitute for a real `ThreadPoolExecutor` +/// underneath a submit-now/compute-later job, so a test asserts the exact +/// sequence submit -> still pending -> `runOne()` -> done, instead of polling +/// with a sleep and a retry cap. +/// +/// This is a deliberate mirror of `morph::testing::StepExecutor` +/// (`tests/test_support.hpp`) under the ladder's own namespace, not a new +/// design: same name, same API, same rationale. It is duplicated rather than +/// shared for exactly the reason `strand_interleaver.hpp`'s +/// `DeterministicExecutor` is duplicated from that same header -- +/// `tests/test_support.hpp` is a private header for `morph_tests`' own +/// translation units and has no reachable include path from `examples/`. +/// That reachability gap, not the absence of the semantics, is what left +/// every ladder async-job test spinning a real pool (morph#161). + +namespace morph::ladder::testkit { + +/// @brief An `IExecutor` that queues every posted task and runs them only when +/// the test explicitly asks, one at a time -- never on its own thread. +/// +/// Where `DeterministicExecutor` (strand_interleaver.hpp) sits *underneath* a +/// `StrandExecutor` to control the delivery order of continuations, this sits +/// where a production `ThreadPoolExecutor` would: it is the worker. Injected +/// as the executor a model or App posts background work to, it turns +/// "eventually the job finishes" into a sequence of exact, assertable states: +/// +/// ```cpp +/// StepExecutor worker; +/// const auto jobId = model.execute(SubmitReport{...}); +/// CHECK(worker.pending() == 1); // queued, and not run +/// CHECK(status(jobId) == ReportStatus::Pending); // asserted, not sampled +/// REQUIRE(worker.runOne()); +/// CHECK(status(jobId) == ReportStatus::Done); // no sleep, no cap +/// ``` +/// +/// The `pending()`/`runOne()` pair is what makes the *negative* half of that +/// assertable at all: "the worker has not run yet" is an ordinary `CHECK` +/// here, where against a real pool it can only be sampled and hoped for. +/// +/// Single-threaded by construction: `post()` appends to a deque under a mutex +/// (posts can legitimately arrive from other threads -- code under test +/// posting a continuation from inside a running task -- so `post()` still +/// honours `IExecutor`'s thread-safe contract, `docs/spec/core/executor.md`), +/// but every task itself runs synchronously on whichever thread calls +/// `runOne()`/`runAll()`. Concurrent `runOne()`/`runAll()` calls are not +/// supported: a test reasoning about exact task ordering drives it from one +/// thread. +/// +/// Unlike `ThreadPoolExecutor`, a task's exception is not caught and logged +/// here: it propagates straight out of `runOne()`/`runAll()` to the caller. +/// That is deliberate -- the caller is a test, and the exception is often a +/// `REQUIRE` failure the test needs to see rather than have swallowed. +class StepExecutor : public ::morph::exec::IExecutor { +public: + /// @brief Enqueues @p task; does not run it. + /// @param task Callable to run on a later `runOne()`/`runAll()` call. + void post(std::function task) override { + std::scoped_lock const lock{_mtx}; + _queue.push_back(std::move(task)); + } + + /// @brief Runs exactly one queued task, oldest first (FIFO). + /// @return `true` if a task was run, `false` if the queue was empty -- + /// returning rather than throwing so that "nothing more was + /// queued" is an ordinary `CHECK_FALSE`, which is half of what a + /// worker-side double is for. + bool runOne() { + std::function task; + { + std::scoped_lock const lock{_mtx}; + if (_queue.empty()) { + return false; + } + task = std::move(_queue.front()); + _queue.pop_front(); + } + task(); + return true; + } + + /// @brief Runs every task currently queued, including ones a running task + /// itself posts -- so a chained job runs to completion rather than + /// stranding its own continuation. + /// + /// Bounded at @p maxSteps rather than looping until the queue is empty: a + /// task that keeps re-posting work to this executor (a bug in the code + /// under test, or a harness misuse) would otherwise be an undetectable + /// infinite loop, hanging the test process with no assertion failure. + /// @param maxSteps Upper bound on tasks run before giving up. + /// @return Number of tasks run. + /// @throws std::runtime_error if @p maxSteps is reached. + std::size_t runAll(std::size_t maxSteps = 10'000) { + std::size_t ran = 0; + while (ran < maxSteps && runOne()) { + ++ran; + } + if (ran == maxSteps) { + throw std::runtime_error( + "StepExecutor::runAll: exceeded maxSteps -- a task is likely re-posting " + "indefinitely; use runOne() to step through and find it"); + } + return ran; + } + + /// @brief Number of tasks currently queued, awaiting a `runOne()`/`runAll()`. + /// @return Queue depth. + [[nodiscard]] std::size_t pending() const { + std::scoped_lock const lock{_mtx}; + return _queue.size(); + } + +private: + mutable std::mutex _mtx; + std::deque> _queue; +}; + +} // namespace morph::ladder::testkit diff --git a/examples/common/testkit/test_step_executor.cpp b/examples/common/testkit/test_step_executor.cpp new file mode 100644 index 00000000..a6faacf9 --- /dev/null +++ b/examples/common/testkit/test_step_executor.cpp @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: Apache-2.0 +#include +#include +#include +#include +#include +#include + +#include "step_executor.hpp" + +namespace { + +using morph::ladder::testkit::StepExecutor; + +} // namespace + +TEST_CASE("StepExecutor queues a posted task instead of running it", "[ladder][testkit][executor]") { + StepExecutor exec; + bool ran = false; + exec.post([&] { ran = true; }); + + // The whole point: after post() returns, the worker has provably not run. + // Against a real pool this is exactly the state that can only be sampled. + CHECK(exec.pending() == 1); + CHECK_FALSE(ran); + + REQUIRE(exec.runOne()); + CHECK(ran); + CHECK(exec.pending() == 0); +} + +TEST_CASE("StepExecutor::runOne reports an empty queue rather than throwing", "[ladder][testkit][executor]") { + StepExecutor exec; + CHECK_FALSE(exec.runOne()); + + exec.post([] {}); + REQUIRE(exec.runOne()); + // "Nothing more was queued" is an ordinary assertion, not a CHECK_THROWS -- + // which is what distinguishes this from DeterministicExecutor::step(). + CHECK_FALSE(exec.runOne()); +} + +TEST_CASE("StepExecutor runs tasks oldest-first", "[ladder][testkit][executor]") { + StepExecutor exec; + std::vector order; + exec.post([&] { order.push_back(1); }); + exec.post([&] { order.push_back(2); }); + exec.post([&] { order.push_back(3); }); + + REQUIRE(exec.pending() == 3); + REQUIRE(exec.runOne()); + CHECK(order == std::vector{1}); + CHECK(exec.pending() == 2); + + CHECK(exec.runAll() == 2); + CHECK(order == std::vector{1, 2, 3}); +} + +TEST_CASE("StepExecutor::runAll picks up tasks posted by a running task", "[ladder][testkit][executor]") { + // A chained job must run to completion rather than stranding its own + // continuation -- the property that separates runAll() from "run the + // N tasks that happened to be queued when I was called". + StepExecutor exec; + std::vector steps; + exec.post([&] { + steps.emplace_back("first"); + exec.post([&] { + steps.emplace_back("second"); + exec.post([&] { steps.emplace_back("third"); }); + }); + }); + + CHECK(exec.runAll() == 3); + CHECK(steps == std::vector{"first", "second", "third"}); + CHECK(exec.pending() == 0); +} + +TEST_CASE("StepExecutor::runAll is bounded against a self-reposting task", "[ladder][testkit][executor]") { + StepExecutor exec; + int runs = 0; + // Deliberate harness misuse: without the bound this hangs the process with + // no assertion failure and no diagnostic. + std::function repost; + repost = [&] { + ++runs; + exec.post(repost); + }; + exec.post(repost); + + CHECK_THROWS_AS(exec.runAll(/*maxSteps=*/16), std::runtime_error); + CHECK(runs == 16); +} + +TEST_CASE("StepExecutor lets a task's exception reach the test", "[ladder][testkit][executor]") { + // Unlike ThreadPoolExecutor, which catches and logs: a swallowed exception + // here would be a swallowed REQUIRE failure. + StepExecutor exec; + exec.post([] { throw std::runtime_error{"boom"}; }); + CHECK_THROWS_AS(exec.runOne(), std::runtime_error); + CHECK(exec.pending() == 0); +} + +TEST_CASE("StepExecutor is usable through the IExecutor interface", "[ladder][testkit][executor]") { + // How production code sees it: a model or App holding a + // shared_ptr substitutes this for its ThreadPoolExecutor. + StepExecutor exec; + ::morph::exec::IExecutor& iface = exec; + bool ran = false; + iface.post([&] { ran = true; }); + CHECK(exec.pending() == 1); + CHECK_FALSE(ran); + REQUIRE(exec.runOne()); + CHECK(ran); +} diff --git a/examples/ledger/include/ledger/models/ledger_model.hpp b/examples/ledger/include/ledger/models/ledger_model.hpp index 53679882..9bc7fa4c 100644 --- a/examples/ledger/include/ledger/models/ledger_model.hpp +++ b/examples/ledger/include/ledger/models/ledger_model.hpp @@ -65,6 +65,34 @@ namespace ledger { /// explicitly. class LedgerModel { public: + /// @brief The ordinary shape: owns a one-thread `ThreadPoolExecutor` for + /// `execute(SubmitReport)`'s worker, per `_reportExecutor`'s own + /// default. This is the constructor the bridge registry uses -- + /// every non-test caller reaches the model this way. + LedgerModel() = default; + + /// @brief Substitutes a caller-supplied executor for the default worker + /// pool. `_reportExecutor`'s comment has always claimed a caller + /// could do this "without this class changing shape"; until this + /// constructor existed there was in fact no way to, which is why + /// `test_ledger_reports.cpp` had to poll a real pool with sleeps + /// (morph#161). + /// + /// Nothing else changes: `execute(SubmitReport)` still posts the + /// same task, capturing the same plain values. Passing + /// `morph::ladder::testkit::StepExecutor` makes the worker run + /// exactly when the test says `runOne()`, so "submitted, still + /// Pending, the worker has not run yet" becomes an assertion + /// rather than a sample. + /// @param reportExecutor Where `execute(SubmitReport)` posts the report + /// aggregation. + /// @throws std::invalid_argument if @p reportExecutor is null -- checked + /// here rather than left to a null dereference inside + /// `execute(SubmitReport)`, which would fire on whichever call + /// first submits a report, arbitrarily far from the construction + /// that caused it. + explicit LedgerModel(std::shared_ptr<::morph::exec::IExecutor> reportExecutor); + /// @brief Creates an account in the ledger named by `action.ledgerId`. /// The model's first keyed action -- see the hand-written /// `ModelKeyTraits`/`ActionKeyTraits` specialisations below this @@ -309,7 +337,10 @@ class LedgerModel { /// A `shared_ptr` rather than a `ThreadPoolExecutor` /// by value so a caller can substitute a different executor /// (a `MainThreadExecutor`, a deterministic double) without this - /// class changing shape. + /// class changing shape -- reachable through the + /// `explicit LedgerModel(std::shared_ptr)` constructor + /// above, which is what `test_ledger_reports.cpp` uses to drive + /// the worker deterministically. std::shared_ptr<::morph::exec::IExecutor> _reportExecutor = std::make_shared<::morph::exec::ThreadPoolExecutor>(1); }; diff --git a/examples/ledger/src/models/ledger_model.cpp b/examples/ledger/src/models/ledger_model.cpp index 036869b8..03ac8b67 100644 --- a/examples/ledger/src/models/ledger_model.cpp +++ b/examples/ledger/src/models/ledger_model.cpp @@ -21,7 +21,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -459,6 +461,16 @@ void LedgerModel::logAction(const Action& action, const Result& result, std::str _log->flush(); } +LedgerModel::LedgerModel(std::shared_ptr<::morph::exec::IExecutor> reportExecutor) + : _reportExecutor{std::move(reportExecutor)} { + // Checked rather than left to a later null dereference inside + // execute(SubmitReport): the crash would happen on whichever call first + // submits a report, arbitrarily far from the construction that caused it. + if (_reportExecutor == nullptr) { + throw std::invalid_argument{"LedgerModel: reportExecutor must not be null"}; + } +} + AccountInfo LedgerModel::execute(const OpenAccount& action) { const auto* ctx = morph::session::current(); if (ctx == nullptr || ctx->principal.empty()) { diff --git a/examples/ledger/tests/test_ledger_reports.cpp b/examples/ledger/tests/test_ledger_reports.cpp index b1b51525..84914967 100644 --- a/examples/ledger/tests/test_ledger_reports.cpp +++ b/examples/ledger/tests/test_ledger_reports.cpp @@ -3,6 +3,7 @@ #include "ledger/db/ledger_entity.hpp" #include "ledger/models/ledger_model.hpp" #include "testkit/db_fixture.hpp" +#include "testkit/step_executor.hpp" #include #include @@ -11,6 +12,7 @@ #include #include +#include #include #include @@ -35,18 +37,17 @@ class ScopedPrincipal { morph::session::detail::ScopedContext _scope; }; +using morph::ladder::testkit::StepExecutor; + /// @brief Polls @p model's `GetReportStatus` for @p jobId until it leaves /// `Pending`, or until the hard iteration cap is reached. /// -/// A bounded retry loop with a hard cap (100 x 10ms = 1s), matching the only -/// precedent for testing an async job in this codebase -/// (`examples/bookmarks/tests/test_app.cpp`, -/// `examples/pastebin/tests/test_paste_model.cpp`): no deferred-executor test -/// double exists for the worker-pool side of a job, so this genuinely spins -/// the real `ThreadPoolExecutor` and sleeps between polls. A single report -/// job over a tiny test ledger completes far inside that budget; exhausting -/// the cap means a real stall (e.g. a SQLite lock held by another -/// connection), not a slow machine. +/// A bounded retry loop with a hard cap (100 x 10ms = 1s). Used by exactly +/// ONE test case below -- the one that deliberately keeps a real +/// `ThreadPoolExecutor` underneath the model (see its own comment). Every +/// other case in this file injects a `StepExecutor` and drives the worker by +/// hand, so it neither sleeps nor guesses at a budget. Do not reach for this +/// helper for a new case without the same explicit justification. /// @param model The model to poll. /// @param jobId The submitted job. /// @return The last status observed -- still `Pending` only if the cap was hit. @@ -63,8 +64,40 @@ class ScopedPrincipal { return status; } +/// @brief Runs the one report job @p worker is holding, asserting on the way +/// through that it really was holding exactly one and that the job +/// posted no follow-up work of its own. +/// +/// The `pending() == 1` on entry is the assertion a real pool cannot make: +/// against a `ThreadPoolExecutor` the worker may already have finished by the +/// time the test looks, so "submitted but not yet run" can only be sampled. +/// @param worker The executor injected into the model under test. +void runReportJob(StepExecutor& worker) { + REQUIRE(worker.pending() == 1); + REQUIRE(worker.runOne()); + CHECK_FALSE(worker.runOne()); +} + } // namespace +// The one case in this file that deliberately keeps a REAL ThreadPoolExecutor, +// and the reason not everything here was converted to `StepExecutor`. It is +// the only test that exercises the production shape end to end: +// +// * the default-constructed `LedgerModel` -- the constructor the bridge +// registry actually uses, and the one that owns the pool. Every converted +// case below goes through the injecting constructor instead, so without +// this case nothing covers the default wiring at all. +// * the worker running on a genuinely different thread. `execute(SubmitReport)` +// is written on the premise that nothing from the caller's stack frame +// survives into the task -- not its `DataMapper`, and in particular not +// `morph::session::current()`, a thread-local. Under `StepExecutor` the +// task runs inline on the test's thread, where the `ScopedPrincipal` above +// is still installed: a worker that wrongly reached for the caller's +// session would pass every converted case and fail only here. +// +// The cost is the retry loop `pollUntilSettled` still carries. Paying it once, +// for the properties only a real thread can show, beats paying it five times. TEST_CASE("SubmitReport returns immediately; GetReportStatus transitions Pending to Done", "[ledger][reports]") { morph::ladder::testkit::DbFixture fixture; Lightweight::DataMapper mapper; @@ -132,7 +165,8 @@ TEST_CASE("A 23:30-local transaction is reported in its local month, not its UTC mapper.Create(ledgerRow); const auto ledgerId = ledger::LedgerId{static_cast(ledgerRow.id.Value())}; - ledger::LedgerModel model; + auto worker = std::make_shared(); + ledger::LedgerModel model{worker}; const ScopedPrincipal principal{"alice"}; model.execute(ledger::OpenAccount{.ledgerId = ledgerId, .name = "Checking", @@ -174,7 +208,12 @@ TEST_CASE("A 23:30-local transaction is reported in its local month, not its UTC const auto jobId = model.execute(ledger::SubmitReport{ .ledgerId = ledgerId, .kind = ledger::ReportKind::MonthlyStatement, .params = paramsJson}); REQUIRE(jobId.hasValue()); - const auto status = pollUntilSettled(model, jobId); + // Three report jobs run in this test case; each is submitted, asserted + // still Pending, then run by hand. Previously this was three passes of + // a 10ms-granularity poll loop, and the bulk of this file's runtime. + CHECK(model.execute(ledger::GetReportStatus{.jobId = jobId}).status == ledger::ReportStatus::Pending); + runReportJob(*worker); + const auto status = model.execute(ledger::GetReportStatus{.jobId = jobId}); REQUIRE(status.status == ledger::ReportStatus::Done); REQUIRE(status.result.has_value()); std::vector lines; @@ -203,7 +242,8 @@ TEST_CASE("Re-polling the same completed job returns byte-identical results", "[ mapper.Create(ledgerRow); const auto ledgerId = ledger::LedgerId{static_cast(ledgerRow.id.Value())}; - ledger::LedgerModel model; + auto worker = std::make_shared(); + ledger::LedgerModel model{worker}; const ScopedPrincipal principal{"alice"}; model.execute(ledger::OpenAccount{ .ledgerId = ledgerId, .name = "Checking", .kind = ledger::AccountKind::Asset, .currency = ledger::Currency::USD}); @@ -211,7 +251,8 @@ TEST_CASE("Re-polling the same completed job returns byte-identical results", "[ auto jobId = model.execute( ledger::SubmitReport{.ledgerId = ledgerId, .kind = ledger::ReportKind::MonthlyStatement, .params = "{}"}); - const auto status = pollUntilSettled(model, jobId); + runReportJob(*worker); + const auto status = model.execute(ledger::GetReportStatus{.jobId = jobId}); REQUIRE(status.status == ledger::ReportStatus::Done); // Two more polls of the SAME completed job -- byte-identical results @@ -224,6 +265,89 @@ TEST_CASE("Re-polling the same completed job returns byte-identical results", "[ CHECK(secondPoll.status == thirdPoll.status); } +TEST_CASE("A submitted report stays Pending until its worker actually runs", "[ledger][reports]") { + // The assertion a real thread pool cannot support. Against a + // `ThreadPoolExecutor` the worker may already have finished by the time + // the test looks, so "submitted, and the aggregation has NOT happened yet" + // can only be sampled and hoped for -- which means a `SubmitReport` that + // quietly computed the report inline, on the caller's thread, would pass + // every previous test in this file. Here it is an ordinary CHECK. + morph::ladder::testkit::DbFixture fixture; + Lightweight::DataMapper mapper; + ledger::db::LedgerRecord ledgerRow; + ledgerRow.name = "Personal"; + mapper.Create(ledgerRow); + const auto ledgerId = ledger::LedgerId{static_cast(ledgerRow.id.Value())}; + + auto worker = std::make_shared(); + ledger::LedgerModel model{worker}; + const ScopedPrincipal principal{"alice"}; + model.execute(ledger::OpenAccount{ + .ledgerId = ledgerId, .name = "Checking", .kind = ledger::AccountKind::Asset, .currency = ledger::Currency::USD}); + + const auto jobId = model.execute( + ledger::SubmitReport{.ledgerId = ledgerId, .kind = ledger::ReportKind::MonthlyStatement, .params = "{}"}); + REQUIRE(jobId.hasValue()); + + // Submitted: the aggregation is queued and provably has not run. + CHECK(worker->pending() == 1); + // Stable, not merely "not yet": polled repeatedly, it stays Pending with + // no result body for as long as the worker is not run. On a real pool the + // same three polls race the job and prove nothing. + for (int poll = 0; poll < 3; ++poll) { + const auto pending = model.execute(ledger::GetReportStatus{.jobId = jobId}); + CHECK(pending.status == ledger::ReportStatus::Pending); + CHECK_FALSE(pending.result.has_value()); + } + CHECK(worker->pending() == 1); + + REQUIRE(worker->runOne()); + + const auto done = model.execute(ledger::GetReportStatus{.jobId = jobId}); + CHECK(done.status == ledger::ReportStatus::Done); + CHECK(done.result.has_value()); + // The job is one task, not a chain: it left nothing queued behind it. + CHECK_FALSE(worker->runOne()); +} + +TEST_CASE("Running one report job settles that job and no other", "[ledger][reports]") { + // Two jobs outstanding at once, settled one at a time. Only a hand-driven + // worker can hold a second job at Pending while the first completes, so a + // completion that wrote the wrong row -- or every row -- was previously + // untestable here: with a real pool both jobs finish before either can be + // observed, and "job B is Done" looks the same either way. + morph::ladder::testkit::DbFixture fixture; + Lightweight::DataMapper mapper; + ledger::db::LedgerRecord ledgerRow; + ledgerRow.name = "Personal"; + mapper.Create(ledgerRow); + const auto ledgerId = ledger::LedgerId{static_cast(ledgerRow.id.Value())}; + + auto worker = std::make_shared(); + ledger::LedgerModel model{worker}; + const ScopedPrincipal principal{"alice"}; + model.execute(ledger::OpenAccount{ + .ledgerId = ledgerId, .name = "Checking", .kind = ledger::AccountKind::Asset, .currency = ledger::Currency::USD}); + + const auto first = model.execute( + ledger::SubmitReport{.ledgerId = ledgerId, .kind = ledger::ReportKind::MonthlyStatement, .params = "{}"}); + const auto second = model.execute( + ledger::SubmitReport{.ledgerId = ledgerId, .kind = ledger::ReportKind::MonthlyStatement, .params = "{}"}); + REQUIRE(first.hasValue()); + REQUIRE(second.hasValue()); + REQUIRE(*first != *second); + REQUIRE(worker->pending() == 2); + + // FIFO: the first submitted is the first queued, so this settles `first`. + REQUIRE(worker->runOne()); + CHECK(model.execute(ledger::GetReportStatus{.jobId = first}).status == ledger::ReportStatus::Done); + CHECK(model.execute(ledger::GetReportStatus{.jobId = second}).status == ledger::ReportStatus::Pending); + + REQUIRE(worker->runOne()); + CHECK(model.execute(ledger::GetReportStatus{.jobId = second}).status == ledger::ReportStatus::Done); + CHECK_FALSE(worker->runOne()); +} + TEST_CASE("SubmitReport rejects a disengaged ledgerId and an unknown ledger", "[ledger][reports]") { morph::ladder::testkit::DbFixture fixture;