fix: reduce test flakiness from timing races and resource leaks

[jonathannorris]

Summary

• testSseReopenDoesntGetCalledWhenForegroundedBeforeInactivityDelay: race between build()'s pending getConfig callback and the test's mock SSE connection — the callback fired during wait(), called setupSSEConnection(), which closed the mock (setting closeCalled=true) and replaced it with a real SSE connection; also the asyncAfter(0.5) + 2s timeout was too tight on slow CI runners. Fix: use disableRealtimeUpdates(true) to prevent SSE setup during init, wait for init to complete before injecting the mock, and replace the asyncAfter+wait pattern with RunLoop.current.run(until:) + direct assertions.
• testSseReopenGetsCalledWhenForegrounded: asyncAfter(1) + 2s timeout left only 1s margin. Reduced delay to 0.1s and increased timeout to 5s.
• Variable and flush-event tests that called build() / initialize() but never called close(): the pending getConfig callbacks eventually fired and created real SSEConnection instances (via setupSSEConnection()) that were never cleaned up. Accumulated open connections and timers degraded main queue throughput late in the suite, explaining the tvOS 15-minute CI timeout. Added client.close(callback: nil) to: testVariableMethodReturnsDefaultedVariableWhenKeyIsNotInConfig, testVariableStringDefaultValue, testVariableBooleanDefaultValue, testVariableNumberDefaultValue, testVariableJSONDefaultValue, testFlushEventsWithOneEventInQueueAndCallback.

Related

Stacked on #266

[Copilot]

Pull request overview

This PR aims to reduce flakiness and suite slowdowns in DevCycleClientTests by removing timing-sensitive waits, preventing unintended real SSE connections from being created during tests, and explicitly closing clients to clean up timers/resources.

Changes:

• Added client.close(callback: nil) to several tests to avoid leaking timers/SSE connections across the suite.
• Adjusted SSE foreground/background tests to use wider timeouts and less timing-sensitive patterns.
• Updated testSseReopenDoesntGetCalledWhenForegroundedBeforeInactivityDelay to wait for init completion and to prevent SSE setup during initialization via disableRealtimeUpdates(true).

Comments suppressed due to low confidence (4)

DevCycleTests/Models/DevCycleClientTests.swift:380

• client.close(callback: nil) will flush variable events; because this test calls client.initialize(callback: nil) (which creates a real DevCycleService), close() may publish events over the network. To keep the test hermetic and avoid flakiness, avoid re-initializing with the real service and/or explicitly initialize/setup using the existing MockService (or disable automatic event logging).

        let nsString: NSString = "nsString"
        let varNSString = client.variable(key: "some_non_existent_variable", defaultValue: nsString)
        XCTAssertEqual(varNSString.defaultValue, nsString)
        XCTAssertEqual(varNSString.type, DVCVariableTypes.String)
        client.close(callback: nil)

DevCycleTests/Models/DevCycleClientTests.swift:394

• With client.initialize(callback: nil) in this test, the client ends up using a real DevCycleService; the newly added client.close(callback: nil) can therefore flush variable events via live network requests. Consider keeping the mock service for initialization (or disabling automatic event logging) so closing the client can’t perform external I/O.

        let variable = client.variable(key: "some_non_existent_variable", defaultValue: true)
        XCTAssertEqual(variable.value, true)
        XCTAssert(variable.isDefaulted)
        XCTAssertEqual(variable.defaultValue, true)
        XCTAssertEqual(variable.type, DVCVariableTypes.Boolean)
        client.close(callback: nil)

DevCycleTests/Models/DevCycleClientTests.swift:414

• This test re-calls client.initialize(callback: nil), which creates a real DevCycleService. Since client.close(callback: nil) now flushes queued variable events, it may issue real network requests from the test runner. Prefer initializing/setting up with the MockService (or disabling automatic event logging) to keep the test deterministic.

        let nsNum: NSNumber = 10.1
        let variableNum = client.variable(key: "some_non_existent_variable", defaultValue: nsNum)
        XCTAssertEqual(variableNum.defaultValue, nsNum)
        XCTAssertEqual(variableNum.type, DVCVariableTypes.Number)
        client.close(callback: nil)

DevCycleTests/Models/DevCycleClientTests.swift:436

• Because this test invokes client.initialize(callback: nil) (real DevCycleService) and variables enqueue aggregate events, the added client.close(callback: nil) can flush those events via real network calls. To avoid nondeterminism, keep the mock service for init/setup or disable automatic event logging for this test.

        let nsDicDefault: NSDictionary = ["key": "val"]
        let variable2 = client.variable(
            key: "some_non_existent_variable", defaultValue: nsDicDefault)
        XCTAssertEqual(variable2.defaultValue, nsDicDefault)
        XCTAssertEqual(variable2.type, DVCVariableTypes.JSON)
        client.close(callback: nil)

---

💡 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 2 comments.

Comments suppressed due to low confidence (5)

DevCycleTests/Models/EventQueueTests.swift:36

• This test still depends on an arbitrary asyncAfter delay to observe the results of EventQueue.queue() (which appends on its own serial queue). With the delay reduced to 0.1s, the test can intermittently fail under load; prefer waiting on eventDispatchQueue (or fulfilling an expectation from within it) rather than sleeping.

        let expectation = XCTestExpectation(description: "Events are serially queued")
        let event1 = try! DevCycleEvent.builder().type("event1").build()
        let event2 = try! DevCycleEvent.builder().type("event2").build()
        eventQueue.queue(event1)
        eventQueue.queue(event2)
        DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {
            XCTAssert(eventQueue.events.first?.type == "event1")
            XCTAssert(eventQueue.events.last?.type == "event2")
            expectation.fulfill()

DevCycleTests/Models/DevCycleClientTests.swift:380

• This test closes the client without waiting for the async initialization/config-fetch callbacks to finish. Since config delivery happens on DispatchQueue.main.async, a late callback can still run after close() and create a new SSEConnection (via updateUserConfig()), leaking resources. To make cleanup reliable, wait for onInitialized (or build with disableRealtimeUpdates(true)) before calling close().

        let nsString: NSString = "nsString"
        let varNSString = client.variable(key: "some_non_existent_variable", defaultValue: nsString)
        XCTAssertEqual(varNSString.defaultValue, nsString)
        XCTAssertEqual(varNSString.type, DVCVariableTypes.String)
        client.close(callback: nil)

DevCycleTests/Models/DevCycleClientTests.swift:394

• client.close(callback: nil) is called without ensuring the build/initialization async callbacks have drained. If getConfig completes after close, it can still trigger setupSSEConnection() and open a real SSE stream after the test finishes. Consider waiting for onInitialized (or disabling realtime updates) before closing to guarantee cleanup.

        XCTAssertEqual(variable.value, true)
        XCTAssert(variable.isDefaulted)
        XCTAssertEqual(variable.defaultValue, true)
        XCTAssertEqual(variable.type, DVCVariableTypes.Boolean)
        client.close(callback: nil)

DevCycleTests/Models/DevCycleClientTests.swift:414

• The client is closed without waiting for the async config fetch to complete. A delayed getConfig completion can still execute after close() and establish an SSEConnection, which won’t be shut down, undermining the leak fix. Prefer waiting for initialization completion (or disabling realtime updates) before closing.

        let nsNum: NSNumber = 10.1
        let variableNum = client.variable(key: "some_non_existent_variable", defaultValue: nsNum)
        XCTAssertEqual(variableNum.defaultValue, nsNum)
        XCTAssertEqual(variableNum.type, DVCVariableTypes.Number)
        client.close(callback: nil)

DevCycleTests/Models/DevCycleClientTests.swift:436

• Closing the client here may happen before the async initialization/config callback runs (it’s queued on the main queue). If the callback arrives after close(), it can still create an SSEConnection via updateUserConfig(), leaving resources running past the test. Consider waiting for onInitialized (or using disableRealtimeUpdates(true)) before close() for deterministic cleanup.

        let variable2 = client.variable(
            key: "some_non_existent_variable", defaultValue: nsDicDefault)
        XCTAssertEqual(variable2.defaultValue, nsDicDefault)
        XCTAssertEqual(variable2.type, DVCVariableTypes.JSON)
        client.close(callback: nil)