Stabilize timing-sensitive tests

[bmehta001]

Summary

• use an injectable monotonic clock for kill-switch and Retry-After deadlines
• add deterministic unit coverage for exact expiry boundaries
• replace fixed sleeps in the temporary kill-switch functional test with bounded waits for observable drops and delivery
• simulate expired SQLite reservation leases directly instead of depending on scheduler timing
• rename the temporary kill-switch local logger to avoid MSVC /WX shadowing failures, and harden injected clocks by falling back from an empty Clock and invoking it outside the kill-switch mutex
• replace the bad-network teardown test's external-endpoint loop with an injected HTTP client that waits for teardown cancellation and delivers exactly-once NetworkFailure callbacks deterministically

Motivation

The affected tests depended on wall-clock time and fixed sleeps. Under slower or
uneven macOS CI scheduling, they could observe state too early or wait longer than
necessary. These changes preserve the production and integration coverage while
making expiry behavior deterministic and every asynchronous wait bounded.

They also remove external network timing from the bad-network teardown coverage and
harden the injected-clock path for empty-clock callers and Windows CI.

[Copilot]

Pull request overview

This PR reduces flakiness in timing-sensitive kill-switch and offline-storage tests by removing real sleeps and making expiry behavior deterministic via an injectable clock and direct lease manipulation.

Changes:

• Introduce an injectable monotonic clock in KillSwitchManager and compute expiry times in milliseconds with overflow protection.
• Replace the sleep-based SQLite reservation timeout test with a deterministic “force lease expiry” approach via SQL update.
• Remove the long, sleep-heavy functional test for temporary kill-switch behavior and replace it with deterministic unit tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tests/unittests/OfflineStorageTests_SQLite.cpp	Makes the reservation-timeout test deterministic by simulating expired leases directly in SQLite.
tests/unittests/KillSwitchManagerTests.cpp	Adds unit tests that verify kill-switch expiry exactly at the deadline using an injected clock.
tests/functests/BasicFuncTests.cpp	Removes a timing/sleep-heavy functional test that was sensitive to CI scheduling and wall-clock delays.
lib/offline/KillSwitchManager.hpp	Adds injectable monotonic clock support and overflow-safe expiry computation for kill-switch deadlines.

Comments suppressed due to low confidence (2)

lib/offline/KillSwitchManager.hpp:124

• KillSwitchManager now accepts an injectable Clock, but isTokenBlocked() calls m_clock() while holding m_lock. Since the injected clock can be arbitrary user code, this can deadlock (e.g., if the clock calls back into the manager) or unnecessarily extend the critical section. Evaluate the clock outside the mutex, then lock only for shared state access.

        bool isTokenBlocked(const std::string& tokenId)
        {
            std::lock_guard<std::mutex> guard(m_lock);
            const int64_t now = m_clock();

lib/offline/KillSwitchManager.hpp:115

• addToken() calls expiryFromNow() while holding m_lock, which in turn calls the injectable m_clock(). To avoid executing arbitrary code under the mutex (deadlock / contention risk), compute the expiry before taking the lock and only lock around the map mutation.

        void addToken(const std::string& tokenId, int64_t timeInSeconds)
        {
            std::lock_guard<std::mutex> guard(m_lock);
            if (timeInSeconds > 0)
            {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

tests/functests/BasicFuncTests.cpp:566

• waitForEvent holds mtx_requests while decoding/parsing every received request. That blocks the HTTP server callback from appending new requests, and it also re-decodes the same requests on every poll, which can make this bounded wait slower/flakier under load. Consider snapshotting only the new requests under the mutex, then decoding outside the lock.

    bool waitForEvent(const std::string& name, unsigned timeoutMs)
    {
        const auto deadline = PAL::getMonotonicTimeMs() + timeoutMs;
        while (PAL::getMonotonicTimeMs() < deadline)
        {

tests/functests/BasicFuncTests.cpp:1297

• listener.waitForAtLeast(numHttpOK, 1, ...) is a prerequisite for the rest of the test (it ensures the kill-switch response was actually observed). Using EXPECT_TRUE allows the test to continue and fail later with less actionable errors; this should be an ASSERT_TRUE precondition.

    EXPECT_TRUE(listener.waitForAtLeast(listener.numHttpOK, 1, 10000));

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.