From 3819b7cae82b4a9ffe6f3272a89de515066a4499 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Sat, 26 Sep 2026 10:15:00 +0200 Subject: [PATCH] offline: wait for NetworkMonitor state instead of sleeping a fixed margin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tests/test_offline_integration.cpp decided two NetworkMonitor outcomes by sleeping a fixed wall-clock margin (80ms, then 150ms) before asserting on state that a probe thread sets asynchronously. Under load, the probe thread can be scheduled later than that margin, so the assertion races OS scheduling latency instead of the behaviour under test — reproduced on CI (PR #806) and, deterministically here, by widening probeInterval past the fixed margins on unmodified code. Replace both sites with morph::testing::waitUntil, the polling idiom already used for the same NetworkMonitor operations in tests/test_network_monitor.cpp. The test now fails only when the monitor genuinely does not reach the expected state, not when its thread is late. Removing the fixed 150ms sleep exposed a second, previously-masked race: the test called handler.execute() as soon as replayed.size()==3 became true, but bridge.switchBackend() runs immediately *after* the replay in the same onOnline callback, on the probe thread. The old fixed sleep almost always gave switchBackend() enough incidental slack to finish too; waitUntil can return the instant the replay condition is met, without that slack, letting the main thread call execute() while the switch is still in flight (observed as CI#830's Windows failure). Fixed by waiting on an explicit backendSwitched flag set after switchBackend() returns, instead of relying on replay completion as a stand-in for it. Also: two scoped_lock declarations flagged by clang-tidy-diff (misc-const-correctness) marked const. Fixes #821 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018fEUahMFF32wQLiWjbsfkc Signed-off-by: Yaraslau Tamashevich --- tests/test_offline_integration.cpp | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/tests/test_offline_integration.cpp b/tests/test_offline_integration.cpp index dcc0b9b83..3e0015a1a 100644 --- a/tests/test_offline_integration.cpp +++ b/tests/test_offline_integration.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include "test_support.hpp" @@ -91,32 +90,44 @@ TEST_CASE("Integration: offline queue replayed and backend switched on network r // Monitor: failureThreshold=1, onlineThreshold=1, probeInterval=30ms. // With networkOnline=false the monitor goes offline after ~30ms. // When networkOnline becomes true the monitor recovers after a further ~30ms. + std::atomic backendSwitched{false}; morph::offline::NetworkMonitor monitor{ [&] { return networkOnline.load(); }, [] {}, // onOffline — not exercised here [&] { // onOnline fires on the probe thread — replay then switch backend. + // `backendSwitched` is set only *after* switchBackend returns, so a + // waiter never observes "queue replayed" as a stand-in for "backend + // switched": the two used to be conflated by relying on the fixed + // 150ms sleep that preceded this test's waitUntil migration to have + // given switchBackend() enough slack to finish too, which held in + // practice but was never actually waited for. syncWorker.run(); bridge.switchBackend(std::make_unique(remotePool)); + backendSwitched.store(true); }, morph::offline::NetworkMonitor::Config{.probeInterval = 30ms, .failureThreshold = 1, .onlineThreshold = 1}}; - // Wait for monitor to detect offline (one probe interval + margin). - std::this_thread::sleep_for(80ms); - REQUIRE_FALSE(monitor.isOnline()); + // Wait for the monitor to detect offline. + REQUIRE(morph::testing::waitUntil([&] { return !monitor.isOnline(); })); // Bring network back online. networkOnline.store(true); - // Wait for monitor to detect recovery and fire onOnline (one probe interval + margin). - std::this_thread::sleep_for(150ms); - - // All queued items must have been replayed. + // Wait for the monitor to detect recovery, fire onOnline, and replay the queue. + REQUIRE(morph::testing::waitUntil([&] { + const std::scoped_lock lock{replayMtx}; + return replayed.size() == 3; + })); { - std::scoped_lock lock{replayMtx}; + const std::scoped_lock lock{replayMtx}; REQUIRE(replayed.size() == 3); } REQUIRE(queue.drain().empty()); + // Wait for the backend switch itself, not just the replay that precedes it + // in the same callback -- see the comment on `backendSwitched` above. + REQUIRE(morph::testing::waitUntil([&] { return backendSwitched.load(); })); + // morph::bridge::Bridge now routes to remotePool — execute still works. std::atomic result{-1}; handler.execute(OffAction{5}).then([&](int val) { result.store(val); }).onError([](const std::exception_ptr&) {});