From 354f79c839dac7e29f6910ae7afd475fd0454ba8 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Thu, 13 Aug 2026 10:35:42 +0300 Subject: [PATCH] tests: replace test_conflict_resolution.cpp's fixed 80ms sleeps with a poll on notifyCount Pre-existing, unrelated to any open PR (present since #17): each test called bridge.switchBackend() then slept a fixed 80ms hoping onBackendChanged() (which drains the offline queue on the new backend's thread pool) had finished by then, before asserting on the queue/notifyCount. Under contention -- a coverage-instrumented build, a busy CI runner, Valgrind's own overhead -- 80ms isn't always enough, so REQUIRE(queue.drain().empty()) intermittently sees a queue that hasn't drained yet. Confirmed reproducing on 3 independent CI jobs (Linux clang-debug, Linux Qt6-WebSockets, Valgrind memcheck) and, separately, in a local WSL coverage build. Adds waitForBackendChanged(), which polls the model's own notifyCount (via the existing read-only OrderQueryAction, already used by these tests) until it reaches 1 -- the real signal that onBackendChanged has run, and therefore that its queue drain has too -- instead of guessing a fixed delay. Bounded at 2 seconds, matching waitInt's own budget. Co-Authored-By: Claude Sonnet 5 --- tests/test_conflict_resolution.cpp | 33 +++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/tests/test_conflict_resolution.cpp b/tests/test_conflict_resolution.cpp index c8ef7de7..bfd3027e 100644 --- a/tests/test_conflict_resolution.cpp +++ b/tests/test_conflict_resolution.cpp @@ -131,6 +131,27 @@ static int waitInt(auto completion) { return result.load(); } +// ── Helper: poll until onBackendChanged has actually run ───────────────────── +// +// switchBackend() dispatches the new model's construction and its +// onBackendChanged() call (which drains the offline queue) onto the new +// backend's own thread pool, asynchronously -- there is no signal the caller +// can block on directly. A fixed sleep_for() guessing "surely long enough" +// is exactly the failure mode this helper replaces: it polls the model's own +// notifyCount via OrderQueryAction (read-only, safe to call repeatedly) +// until it reaches 1, the real signal that onBackendChanged has completed +// -- and therefore that the queue drain it performs has too. Bounded, not +// unbounded: returns false (rather than hanging) if the count never reaches 1. +static bool waitForBackendChanged(morph::bridge::BridgeHandler& handler) { + for (int i = 0; i < 200; ++i) { + if (waitInt(handler.execute(OrderQueryAction{})) >= 1) { + return true; + } + std::this_thread::sleep_for(10ms); + } + return false; +} + // ── Tests ───────────────────────────────────────────────────────────────────── TEST_CASE("ConflictResolution: no conflicts - all items markDone on switchBackend", "[conflict]") { @@ -151,7 +172,7 @@ TEST_CASE("ConflictResolution: no conflicts - all items markDone on switchBack morph::bridge::BridgeHandler handler{bridge, &cbExec, binding}; bridge.switchBackend(std::make_unique(pool2)); - std::this_thread::sleep_for(80ms); // let notifyBackendChanged complete + REQUIRE(waitForBackendChanged(handler)); // All items removed from queue after clean replay. REQUIRE(queue.drain().empty()); @@ -178,7 +199,7 @@ TEST_CASE("ConflictResolution: conflicting items discarded - resolver returns morph::bridge::BridgeHandler handler{bridge, &cbExec, binding}; bridge.switchBackend(std::make_unique(pool2)); - std::this_thread::sleep_for(80ms); + REQUIRE(waitForBackendChanged(handler)); // All three items removed regardless of outcome (discard also calls markDone). REQUIRE(queue.drain().empty()); @@ -202,7 +223,7 @@ TEST_CASE("ConflictResolution: conflicting items merged - resolver returns non morph::bridge::BridgeHandler handler{bridge, &cbExec, binding}; bridge.switchBackend(std::make_unique(pool2)); - std::this_thread::sleep_for(80ms); + REQUIRE(waitForBackendChanged(handler)); // All three items processed and removed. REQUIRE(queue.drain().empty()); @@ -228,18 +249,16 @@ TEST_CASE("ConflictResolution: framework fires onBackendChanged exactly once per morph::bridge::BridgeHandler handler{bridge, &cbExec, binding}; // Switch once - new model instance created, notifyCount becomes 1. + // waitInt's own poll loop is the wait here: no separate fixed sleep needed. bridge.switchBackend(std::make_unique(pool2)); - std::this_thread::sleep_for(80ms); REQUIRE(waitInt(handler.execute(OrderQueryAction{})) == 1); // Switch again - another fresh instance, again notifyCount == 1. bridge.switchBackend(std::make_unique(pool3)); - std::this_thread::sleep_for(80ms); REQUIRE(waitInt(handler.execute(OrderQueryAction{})) == 1); // Third switch. bridge.switchBackend(std::make_unique(pool4)); - std::this_thread::sleep_for(80ms); REQUIRE(waitInt(handler.execute(OrderQueryAction{})) == 1); } @@ -277,7 +296,7 @@ TEST_CASE("ConflictResolution: full offline scenario - accumulate offline, syn // Simulate reconnection - switch to remote backend. bridge.switchBackend(std::make_unique(remotePool)); - std::this_thread::sleep_for(80ms); + REQUIRE(waitForBackendChanged(handler)); // Queue fully drained: 2 clean replays + 1 merge = 3 markDone calls. REQUIRE(queue.drain().empty());