AB#292773 test: make the SessionHelper tests deterministic

[eligutovsky]

AB#292773

Description of Changes

SessionHelperTests asserted the wrong thing. sessionDidEnd() must return without waiting for
the background flush it kicks off — that is the whole point of the change in 6.2.6, "remove
semaphore wait in session end to avoid QoS inversion during background flush" — but the test only
checked that the tracking completion eventually ran. It would have passed just as happily if
sessionDidEnd() had gone back to blocking.

The test now asserts the ordering directly. Tracking blocks on a semaphore the test releases only
after sessionDidEnd() has returned, so a version that waited for tracking could not get past it:

let releaseTracking = DispatchSemaphore(value: 0)
let trackingInvoked = expectation(description: "trackBackground was invoked")
let sessionDidEndReturned = expectation(description: "sessionDidEnd returned")

let blockingMock: (Date, @escaping SyncCompletedBlock) -> Void = { _, done in
    trackingInvoked.fulfill()
    DispatchQueue.global().async { releaseTracking.wait(); done(nil) }
}
...
DispatchQueue.global().async { helper.sessionDidEnd(); sessionDidEndReturned.fulfill() }
wait(for: [trackingInvoked, sessionDidEndReturned], timeout: timeoutInSeconds)
releaseTracking.signal()

The old test is kept as test_sessionDidEnd_invokesTrackingCompletion — renamed to say what it
actually checks, since "does not wait" was never what it measured.

Scope was narrowed — the AnalyticsHelper half is gone

This PR originally also reworked AnalyticsHelperTests. #134 fixes that flake better and this
branch now stays out of its way.

Both of us independently found the same root cause: the simulator does not enforce entitlements, so
containerURL(forSecurityApplicationGroupIdentifier:) resolves for any group identifier and
AnalyticsHelper silently opens the shared-container database — while the test helper cleared the
Documents one, which was never the database in use. So the tests never had isolation and events
accumulated across the class.

Where the fixes differ:

	this PR (dropped)	#134 (kept)
Approach	clear the shared database in both locations	storeUrlOverride: a fresh store per test
Isolation	only as good as the deletion is complete	real, by construction
Production code	untouched	a test-only hook on the initialiser

Per-test stores are the right answer — the weakness I flagged in my own version was exactly that it
still rested on deleting a shared file. Keeping both would also have conflicted: they touch the same
setUp, tearDown and count assertions. AnalyticsHelperTests is therefore back to master's
version here, and the remaining timing work on it lives in #150, on top of #134.

Verification

Full suite: 75 tests, 0 failures.

Mutation-checked — restoring the semaphore wait in SessionHelper.sessionDidEnd() fails
test_sessionDidEnd_doesNotWaitForTrackingToComplete and leaves
test_sessionDidEnd_invokesTrackingCompletion passing, which is the split the rename describes.

Both tests run in 0.003s.

One thing this does not claim

I could not reproduce the CI flake locally, so I am not claiming a measured reduction anywhere.
Running the suite 8 times under full 11-core CPU load on unfixed master passed 8/8 — the run
slowed from 2.8s to 31s, so the load was real, but the flake needs something about the
GitHub-hosted runner. The argument here is about what the tests assert, not about flake rates.

Breaking Changes

• None

Test-only. No version bump.

Release Checklist

• Detail any breaking changes — none, tests only
• pod lib lint — n/a, no shipped code changed
• Wiki — n/a

Integration tests

n/a — no shipped code changed.

[Copilot]

Pull request overview

This PR aims to eliminate intermittent CI failures by making AnalyticsHelperTests and SessionHelperTests deterministic, removing reliance on wall-clock sleeps, and preventing late async completions from accessing torn-down test state.

Changes:

• Reworked SessionHelperTests to assert ordering (non-blocking behavior) via semaphores/expectations instead of elapsed-time assertions and sleeps.
• Updated AnalyticsHelperTests to (a) capture strong references used in async completions to avoid post-tearDown crashes, and (b) replace asyncAfter sequencing with completion-based sequencing.
• Added basic synchronization (NSLock) around shared mutable state in the HTTP client mocks used by the tests.

Reviewed changes

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

File	Description
OptimoveSDK/Tests/Sources/Optimobile/SessionHelperTests.swift	Replaces timing-based “does not block” tests with ordering-based assertions using expectations/semaphores.
OptimoveSDK/Tests/Sources/Optimobile/AnalyticsHelperTests.swift	Makes analytics tests deterministic by avoiding sleeps, capturing needed references in async completions, and synchronizing mock state with locks.

---

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

OptimoveSDK/Tests/Sources/Optimobile/SessionHelperTests.swift:20

• In blockingMock, the background block waits on releaseTracking with no timeout. If the test fails before reaching releaseTracking.signal() (e.g., an early assertion failure or the wait times out), that thread will remain blocked for the rest of the test run. Add a teardown cleanup that signals the semaphore to avoid leaking blocked threads across subsequent tests.

        let releaseTracking = DispatchSemaphore(value: 0)
        let trackingInvoked = expectation(description: "trackBackground was invoked")
        let sessionDidEndReturned = expectation(description: "sessionDidEnd returned")

        let blockingMock: (Date, @escaping SyncCompletedBlock) -> Void = { _, done in
            trackingInvoked.fulfill()
            DispatchQueue.global().async {
                releaseTracking.wait()
                done(nil)
            }

OptimoveSDK/Tests/Sources/Optimobile/AnalyticsHelperTests.swift:176

• Typo in the test event type string: "immeditate_event" is likely meant to be "immediate_event". While it doesn’t affect the assertion, fixing it avoids confusion when debugging logs or failures.

        analyticsHelper.trackEvent(eventType: "immeditate_event", properties: nil, immediateFlush: true)

[Copilot]

Pull request overview

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

Suppressed comments (1)

OptimoveSDK/Tests/Sources/Optimobile/SessionHelperTests.swift:13

• PR description/title indicate changes to both AnalyticsHelperTests and SessionHelperTests to make tests deterministic, but in the current branch snapshot only SessionHelperTests reflects those determinism changes. For example, OptimoveSDK/Tests/Sources/Optimobile/AnalyticsHelperTests.swift still contains wall-clock sleeps (asyncAfter(... + 2)) and its clearAnalyticsStore() only deletes the Documents DB path, which contradicts the described fix for the app-group shared store. Please either include the missing AnalyticsHelperTests updates in this PR or adjust the PR title/description to match the actual changes.

    // Both tests previously encoded "does not block" as wall-clock measurements:
    // an elapsed-time assertion of under 100ms, and a 2 second sleep inside a 3
    // second timeout. Neither survives a loaded CI runner, where scheduling delay
    // alone can exceed the margin even though the behaviour is correct. They now
    // assert ordering instead, so the timeouts are only a backstop against a real
    // hang rather than the thing being measured.