From 39bd882a2709b5bbdedc70f6b8993debb688fb65 Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Tue, 14 Jul 2026 18:01:36 +0200 Subject: [PATCH] Fix for flaky tests (#5869) * possible logging tests fix * fix for flaky tests * use timelimit for max test time * pr suggestions * remove comment --- ElementX.xcodeproj/project.pbxproj | 12 +++ .../ChatsTabFlowCoordinatorTests.swift | 22 ------ .../ComposerToolbarViewModelTests.swift | 16 ++-- .../Sources/ElementCallServiceTests.swift | 79 +++++++++++-------- UnitTests/Sources/LoggingTests.swift | 5 ++ ...diaUploadPreviewScreenViewModelTests.swift | 14 ++-- .../MediaUploadingPreprocessorTests.swift | 16 +++- .../RoomDetailsScreenViewModelTests.swift | 12 ++- .../Sources/RoomFlowCoordinatorTests.swift | 17 ---- ...ficationSettingsScreenViewModelTests.swift | 3 +- .../Sources/SpaceScreenViewModelTests.swift | 47 ++++++----- .../Extensions/NavigationCoordinators.swift | 53 +++++++++++++ .../TimelineMediaPreviewViewModelTests.swift | 8 +- .../UserSessionFlowCoordinatorTests.swift | 59 +++++++++++--- 14 files changed, 231 insertions(+), 132 deletions(-) create mode 100644 UnitTests/Sources/TestUtilities/Extensions/NavigationCoordinators.swift diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj index 1b5216bb5..85e67ea2b 100644 --- a/ElementX.xcodeproj/project.pbxproj +++ b/ElementX.xcodeproj/project.pbxproj @@ -487,6 +487,7 @@ 4EAC427267424192964B16B3 /* AppSettingsHook.swift in Sources */ = {isa = PBXBuildFile; fileRef = 13BE9781699FB510E9263192 /* AppSettingsHook.swift */; }; 4ED764A24F2A715C25CF07F1 /* TimelineMediaPreviewFileExportPicker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 30856520F3263D0E195710D7 /* TimelineMediaPreviewFileExportPicker.swift */; }; 4F2DF6138E87A4B8C2488CA3 /* VoiceMessageCacheProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 43A84EE187D0C772E18A4E39 /* VoiceMessageCacheProtocol.swift */; }; + 4F51C7C8131D060108B80575 /* NavigationCoordinators.swift in Sources */ = {isa = PBXBuildFile; fileRef = 79351405CD99312A182B2488 /* NavigationCoordinators.swift */; }; 4FDC8A9764CFDA90CE035725 /* Duration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7FB2253D36E81E045E1CB432 /* Duration.swift */; }; 4FE688FE9375B2FBF424146A /* TextBasedRoomTimelineViewProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = A6EA0D8B0BBD8805F7D5A133 /* TextBasedRoomTimelineViewProtocol.swift */; }; 4FF90E2242DBD596E1ED2E27 /* AppCoordinatorStateMachine.swift in Sources */ = {isa = PBXBuildFile; fileRef = 077D7C3BE199B6E5DDEC07EC /* AppCoordinatorStateMachine.swift */; }; @@ -2338,6 +2339,7 @@ 78910787F967CBC6042A101E /* StartChatScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StartChatScreenViewModelProtocol.swift; sourceTree = ""; }; 78913D6E120D46138E97C107 /* NavigationSplitCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NavigationSplitCoordinatorTests.swift; sourceTree = ""; }; 78BBDF7A05CF53B5CDC13682 /* landscape_test_video.mov */ = {isa = PBXFileReference; lastKnownFileType = video.quicktime; path = landscape_test_video.mov; sourceTree = ""; }; + 79351405CD99312A182B2488 /* NavigationCoordinators.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NavigationCoordinators.swift; sourceTree = ""; }; 796CBD0C56FA0D3AEDAB255B /* SessionVerificationScreenCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionVerificationScreenCoordinator.swift; sourceTree = ""; }; 79A106D76741914F82664959 /* StateStoreViewModelV2.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StateStoreViewModelV2.swift; sourceTree = ""; }; 7A03E073077D92AA19C43DCF /* IdentityConfirmationScreenCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IdentityConfirmationScreenCoordinator.swift; sourceTree = ""; }; @@ -3284,6 +3286,14 @@ path = RoomNotificationSettingsScreen; sourceTree = ""; }; + 0345B849A6A802EB2F72C44A /* Extensions */ = { + isa = PBXGroup; + children = ( + 79351405CD99312A182B2488 /* NavigationCoordinators.swift */, + ); + path = Extensions; + sourceTree = ""; + }; 0371482D36C95ABAF9D4C651 /* Recorder */ = { isa = PBXGroup; children = ( @@ -4793,6 +4803,7 @@ children = ( B91AD590B0B40718A0AA0C61 /* DeferredFulfillment.swift */, 9E21DC54FCC2A7F78532923F /* WaitingConfirmation.swift */, + 0345B849A6A802EB2F72C44A /* Extensions */, ); path = TestUtilities; sourceTree = ""; @@ -8043,6 +8054,7 @@ 1146E9EDCF8344F7D6E0D553 /* MockCoder.swift in Sources */, DC68E866D6E664B0D2B06E74 /* MockImageCache.swift in Sources */, 3F791C4E0F7B373DB217EC90 /* NSItemProviderTests.swift in Sources */, + 4F51C7C8131D060108B80575 /* NavigationCoordinators.swift in Sources */, 981853650217B6C8ECDD998C /* NavigationRootCoordinatorTests.swift in Sources */, 69C7B956B74BEC3DB88224EA /* NavigationSplitCoordinatorTests.swift in Sources */, 4BB282209EA82015D0DF8F89 /* NavigationStackCoordinatorTests.swift in Sources */, diff --git a/UnitTests/Sources/ChatsTabFlowCoordinatorTests.swift b/UnitTests/Sources/ChatsTabFlowCoordinatorTests.swift index 2310d1892..867ccf39a 100644 --- a/UnitTests/Sources/ChatsTabFlowCoordinatorTests.swift +++ b/UnitTests/Sources/ChatsTabFlowCoordinatorTests.swift @@ -329,25 +329,3 @@ struct ChatsTabFlowCoordinatorTests { withExtendedLifetime(previousDetailRootCoordinator) { } } } - -// Sendable projections of the split coordinator's non-Sendable coordinator references, -// allowing the tests to `observe` them and wait for navigation with `deferFulfillment`. -private extension NavigationSplitCoordinator { - var detailCoordinatorID: ObjectIdentifier? { - detailCoordinator.map { ObjectIdentifier($0) } - } - - /// The root coordinator inside the detail navigation stack. - var detailRootCoordinator: (any CoordinatorProtocol)? { - (detailCoordinator as? NavigationStackCoordinator)?.rootCoordinator - } - - /// The identity of the root coordinator inside the detail navigation stack. - var detailRootCoordinatorID: ObjectIdentifier? { - detailRootCoordinator.map { ObjectIdentifier($0) } - } - - var sheetCoordinatorID: ObjectIdentifier? { - sheetCoordinator.map { ObjectIdentifier($0) } - } -} diff --git a/UnitTests/Sources/ComposerToolbarViewModelTests.swift b/UnitTests/Sources/ComposerToolbarViewModelTests.swift index 056d1ae72..57c0d3a9c 100644 --- a/UnitTests/Sources/ComposerToolbarViewModelTests.swift +++ b/UnitTests/Sources/ComposerToolbarViewModelTests.swift @@ -741,11 +741,7 @@ final class ComposerToolbarViewModelTests { } } - // There are 2 violations, ensure that resolving the first one is not enough - let mockSubject = CurrentValueSubject<[IdentityStatusChange], Never>([ - IdentityStatusChange(userId: "@alice:localhost", changedTo: .verificationViolation), - IdentityStatusChange(userId: "@bob:localhost", changedTo: .verificationViolation) - ]) + let mockSubject = CurrentValueSubject<[IdentityStatusChange], Never>([]) roomProxyMock.identityStatusChangesPublisher = mockSubject.asCurrentValuePublisher() @@ -761,12 +757,16 @@ final class ComposerToolbarViewModelTests { composerDraftService: draftServiceMock) var fulfillment = deferFulfillment(viewModel.context.$viewState, message: "Composer is disabled") { $0.canSend == false } - mockSubject.send([IdentityStatusChange(userId: "@alice:localhost", changedTo: .verificationViolation)]) + mockSubject.send([ + IdentityStatusChange(userId: "@alice:localhost", changedTo: .verificationViolation), + IdentityStatusChange(userId: "@bob:localhost", changedTo: .verificationViolation) + ]) try await fulfillment.fulfill() - fulfillment = deferFulfillment(viewModel.context.$viewState, message: "Composer is still disabled") { $0.canSend == false } + // There are 2 violations, ensure that resolving the first one is not enough. + let failure = deferFailure(viewModel.context.$viewState, timeout: .seconds(1), message: "Composer should still be disabled") { $0.canSend == true } mockSubject.send([IdentityStatusChange(userId: "@alice:localhost", changedTo: .pinned)]) - try await fulfillment.fulfill() + try await failure.fulfill() fulfillment = deferFulfillment(viewModel.context.$viewState, message: "Composer is now enabled") { $0.canSend == true } mockSubject.send([IdentityStatusChange(userId: "@bob:localhost", changedTo: .pinned)]) diff --git a/UnitTests/Sources/ElementCallServiceTests.swift b/UnitTests/Sources/ElementCallServiceTests.swift index 4de0ef197..3c92ca1a0 100644 --- a/UnitTests/Sources/ElementCallServiceTests.swift +++ b/UnitTests/Sources/ElementCallServiceTests.swift @@ -11,6 +11,8 @@ import Clocks import PushKit import Testing +// The TestClock advances by yielding to background tasks which can be starved on a busy CI runner. +@Suite(.timeLimit(.minutes(2))) @MainActor final class ElementCallServiceTests { private var callProvider: CXProviderMock! @@ -111,27 +113,28 @@ final class ElementCallServiceTests { } @Test - func lifetimeIsCapped() async { - await confirmation { confirmation in - callProvider.reportCallWithEndedAtReasonClosure = { _, _, reason in - if reason == .unanswered { - confirmation() - } else { - Issue.record("Call should have ended as unanswered") - } - } - - #expect(!callProvider.reportNewIncomingCallWithUpdateCompletionCalled) - - let pushPayload = PKPushPayloadMock().updatingExpiration(currentDate, lifetime: 300) - - service.pushRegistry(pushRegistry, - didReceiveIncomingPushWith: pushPayload, - for: .voIP) { } - - // Advance past the max timeout but below the 300 - await testClock.advance(by: .seconds(100)) + func lifetimeIsCapped() async throws { + #expect(!callProvider.reportNewIncomingCallWithUpdateCompletionCalled) + + let (endedCalls, endedCallsContinuation) = AsyncStream.makeStream() + callProvider.reportCallWithEndedAtReasonClosure = { _, _, reason in + endedCallsContinuation.yield(reason) } + let deferredEndedCall = deferFulfillment(endedCalls, timeout: .seconds(30)) { _ in true } + + let pushPayload = PKPushPayloadMock().updatingExpiration(currentDate, lifetime: 300) + + service.pushRegistry(pushRegistry, + didReceiveIncomingPushWith: pushPayload, + for: .voIP) { } + + await waitForScheduledSleep() + + // Advance past the max timeout but below the 300 + await testClock.advance(by: .seconds(100)) + + let reason = try await deferredEndedCall.fulfill() + #expect(reason == .unanswered, "Call should have ended as unanswered") } @Test @@ -142,20 +145,20 @@ final class ElementCallServiceTests { } @Test - func timeoutClearsIncomingCallStateBeforeNextPush() async { + func timeoutClearsIncomingCallStateBeforeNextPush() async throws { // Drive push #1 to its 60s unanswered timeout - await confirmation { confirmation in - callProvider.reportCallWithEndedAtReasonClosure = { _, _, reason in - if reason == .unanswered { - confirmation() - } - } - - let firstPayload = PKPushPayloadMock().updatingExpiration(currentDate, lifetime: 60) - service.pushRegistry(pushRegistry, didReceiveIncomingPushWith: firstPayload, for: .voIP) { } - - await testClock.advance(by: .seconds(70)) + let (endedCalls, endedCallsContinuation) = AsyncStream.makeStream() + callProvider.reportCallWithEndedAtReasonClosure = { _, _, reason in + endedCallsContinuation.yield(reason) } + let deferredEndedCall = deferFulfillment(endedCalls, timeout: .seconds(30)) { $0 == .unanswered } + + let firstPayload = PKPushPayloadMock().updatingExpiration(currentDate, lifetime: 60) + service.pushRegistry(pushRegistry, didReceiveIncomingPushWith: firstPayload, for: .voIP) { } + + await waitForScheduledSleep() + await testClock.advance(by: .seconds(70)) + try await deferredEndedCall.fulfill() callProvider.reportCallWithEndedAtReasonClosure = nil let firstCallUUID = callProvider.reportNewIncomingCallWithUpdateCompletionReceivedArguments?.uuid @@ -189,6 +192,9 @@ final class ElementCallServiceTests { } } + // Make sure the cancellation below exercises a scheduled timer rather than one that never started. + await waitForScheduledSleep() + // Simulate the answer flow handing off to setupCallSession, which must cancel // the pending endUnansweredCallTask as part of clearing the incoming state. await service.setupCallSession(roomID: "!room:example.com", roomDisplayName: "welcome") @@ -235,6 +241,15 @@ final class ElementCallServiceTests { #expect(service.ongoingCallRoomIDPublisher.value == "!room:example.com") } + /// Waits until the unanswered-call timer is sleeping on the test clock — it runs in an + /// unstructured task, so advancing the clock before that would never wake it. + /// Note: `checkSuspension()` throws when a sleep is scheduled, which is the state we want. + private func waitForScheduledSleep() async { + while await (try? testClock.checkSuspension()) != nil { + await Task.yield() + } + } + private func expectImmediatelyEndedCallReported(forPayload payload: PKPushPayloadMock, expectedReason: CXCallEndedReason) async { let baselineNewIncomingCount = callProvider.reportNewIncomingCallWithUpdateCompletionCallsCount diff --git a/UnitTests/Sources/LoggingTests.swift b/UnitTests/Sources/LoggingTests.swift index 17bcf24b9..629824c07 100644 --- a/UnitTests/Sources/LoggingTests.swift +++ b/UnitTests/Sources/LoggingTests.swift @@ -331,6 +331,11 @@ final class LoggingTests { /// to start with a fresh state (as calling ``Tracing.deleteLogFiles`` would trigger the bug). private func setupTest(name: String = #function, redirectTracingFileWriter: Bool = true) throws { let testDirectory = URL.appGroupLogsDirectory.appending(component: name, directoryHint: .isDirectory) + + // CI retries re-run the suite on the same simulator, so clear out any log files left + // behind by a previous attempt. No writer points at the directory yet, so this is safe. + try? FileManager.default.removeItem(at: testDirectory) + Tracing.logsDirectoryOverride = testDirectory try? FileManager.default.createDirectory(at: testDirectory, withIntermediateDirectories: true) diff --git a/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift b/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift index 80ce152c5..efc49841b 100644 --- a/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift +++ b/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift @@ -56,19 +56,21 @@ final class MediaUploadPreviewScreenViewModelTests { try await send() } - @Test + @Test(.timeLimit(.minutes(2))) // Transcoding videos can be slow on a busy CI runner. func videoUploadWithoutCaption() async throws { setUpViewModel(urls: [videoURL], expectedCaption: nil) context.caption = .init("") - try await send() + // The preprocessor transcodes the video for real which can take a while on a busy CI runner. + try await send(timeout: .seconds(60)) } - @Test + @Test(.timeLimit(.minutes(2))) // Transcoding videos can be slow on a busy CI runner. func videoUploadWithCaption() async throws { let caption = "Check out this video!" setUpViewModel(urls: [videoURL], expectedCaption: caption) context.caption = .init(string: caption) - try await send() + // The preprocessor transcodes the video for real which can take a while on a busy CI runner. + try await send(timeout: .seconds(60)) } @Test @@ -306,10 +308,10 @@ final class MediaUploadPreviewScreenViewModelTests { return .success(()) } - private func send() async throws { + private func send(timeout: Duration = .seconds(10)) async throws { #expect(!context.viewState.shouldDisableInteraction, "Attempting to send when interaction is disabled.") - let deferred = deferFulfillment(viewModel.actions) { $0 == .dismiss } + let deferred = deferFulfillment(viewModel.actions, timeout: timeout) { $0 == .dismiss } context.send(viewAction: .send) #expect(context.viewState.shouldDisableInteraction, "The interaction should be disabled while sending.") diff --git a/UnitTests/Sources/MediaUploadingPreprocessorTests.swift b/UnitTests/Sources/MediaUploadingPreprocessorTests.swift index 139994ab7..51f7b6560 100644 --- a/UnitTests/Sources/MediaUploadingPreprocessorTests.swift +++ b/UnitTests/Sources/MediaUploadingPreprocessorTests.swift @@ -40,7 +40,7 @@ final class MediaUploadingPreprocessorTests { #expect(isEqual(audioInfo.size ?? 0, 194_811, within: 100)) } - @Test + @Test(.timeLimit(.minutes(2))) // Transcoding videos can be slow on a busy CI runner. func landscapeMovVideoProcessing() async throws { let url = try #require(Bundle(for: Self.self).url(forResource: "landscape_test_video.mov", withExtension: nil), "Failed retrieving test asset") @@ -74,8 +74,12 @@ final class MediaUploadingPreprocessorTests { #expect(isEqual(videoInfo.thumbnailInfo?.size ?? 0, 183_093, within: 100)) #expect(videoInfo.thumbnailInfo?.width == 800) #expect(videoInfo.thumbnailInfo?.height == 450) + } + + @Test(.timeLimit(.minutes(2))) // Transcoding videos can be slow on a busy CI runner. + func landscapeMovVideoOptimizedProcessing() async throws { + let url = try #require(Bundle(for: Self.self).url(forResource: "landscape_test_video.mov", withExtension: nil), "Failed retrieving test asset") - // Repeat with optimised media setting appSettings.optimizeMediaUploads = true guard case let .success(optimizedResult) = await mediaUploadingPreprocessor.processMedia(at: url, maxUploadSize: maxUploadSize), @@ -95,7 +99,7 @@ final class MediaUploadingPreprocessorTests { #expect(isEqual(optimizedVideoInfo.duration ?? 0, 30, within: 100)) } - @Test + @Test(.timeLimit(.minutes(2))) // Transcoding videos can be slow on a busy CI runner. func portraitMp4VideoProcessing() async throws { let url = try #require(Bundle(for: Self.self).url(forResource: "portrait_test_video.mp4", withExtension: nil), "Failed retrieving test asset") @@ -129,8 +133,12 @@ final class MediaUploadingPreprocessorTests { #expect(isEqual(videoInfo.thumbnailInfo?.size ?? 0, 40976, within: 100)) #expect(videoInfo.thumbnailInfo?.width == 337) #expect(videoInfo.thumbnailInfo?.height == 600) + } + + @Test(.timeLimit(.minutes(2))) // Transcoding videos can be slow on a busy CI runner. + func portraitMp4VideoOptimizedProcessing() async throws { + let url = try #require(Bundle(for: Self.self).url(forResource: "portrait_test_video.mp4", withExtension: nil), "Failed retrieving test asset") - // Repeat with optimised media setting appSettings.optimizeMediaUploads = true guard case let .success(optimizedResult) = await mediaUploadingPreprocessor.processMedia(at: url, maxUploadSize: maxUploadSize), diff --git a/UnitTests/Sources/RoomDetailsScreenViewModelTests.swift b/UnitTests/Sources/RoomDetailsScreenViewModelTests.swift index 1dfd4fb58..82cc6d46c 100644 --- a/UnitTests/Sources/RoomDetailsScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomDetailsScreenViewModelTests.swift @@ -175,8 +175,9 @@ struct RoomDetailsScreenViewModelTests { #expect(context.viewState.dmRecipientInfo?.member == RoomMemberDetails(withProxy: recipient)) + #expect(!context.viewState.isProcessingIgnoreRequest) let deferredProcessing = deferFulfillment(viewModel.context.observe(\.viewState.isProcessingIgnoreRequest), - transitionValues: [false, true, false]) + transitionValues: [true, false]) context.send(viewAction: .ignoreConfirmed) @@ -206,8 +207,9 @@ struct RoomDetailsScreenViewModelTests { #expect(context.viewState.dmRecipientInfo?.member == RoomMemberDetails(withProxy: recipient)) + #expect(!context.viewState.isProcessingIgnoreRequest) let deferredProcessing = deferFulfillment(viewModel.context.observe(\.viewState.isProcessingIgnoreRequest), - transitionValues: [false, true, false]) + transitionValues: [true, false]) context.send(viewAction: .ignoreConfirmed) @@ -236,8 +238,9 @@ struct RoomDetailsScreenViewModelTests { #expect(context.viewState.dmRecipientInfo?.member == RoomMemberDetails(withProxy: recipient)) + #expect(!context.viewState.isProcessingIgnoreRequest) let deferredProcessing = deferFulfillment(viewModel.context.observe(\.viewState.isProcessingIgnoreRequest), - transitionValues: [false, true, false]) + transitionValues: [true, false]) context.send(viewAction: .unignoreConfirmed) @@ -267,8 +270,9 @@ struct RoomDetailsScreenViewModelTests { #expect(context.viewState.dmRecipientInfo?.member == RoomMemberDetails(withProxy: recipient)) + #expect(!context.viewState.isProcessingIgnoreRequest) let deferredProcessing = deferFulfillment(viewModel.context.observe(\.viewState.isProcessingIgnoreRequest), - transitionValues: [false, true, false]) + transitionValues: [true, false]) context.send(viewAction: .unignoreConfirmed) diff --git a/UnitTests/Sources/RoomFlowCoordinatorTests.swift b/UnitTests/Sources/RoomFlowCoordinatorTests.swift index efd642b22..fb0d4e0a5 100644 --- a/UnitTests/Sources/RoomFlowCoordinatorTests.swift +++ b/UnitTests/Sources/RoomFlowCoordinatorTests.swift @@ -550,20 +550,3 @@ final class RoomFlowCoordinatorTests { private enum RoomType { case invited(roomID: String) } - -// Sendable projections of the stack coordinator's non-Sendable coordinator references, -// allowing the tests to `observe` them and wait for navigation with `deferFulfillment`. -private extension NavigationStackCoordinator { - var rootCoordinatorID: ObjectIdentifier? { - rootCoordinator.map { ObjectIdentifier($0) } - } - - /// The identity of the topmost coordinator on the stack. - var topCoordinatorID: ObjectIdentifier? { - stackCoordinators.last.map { ObjectIdentifier($0) } - } - - var sheetCoordinatorID: ObjectIdentifier? { - sheetCoordinator.map { ObjectIdentifier($0) } - } -} diff --git a/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift b/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift index c054e0dd0..ee1eefceb 100644 --- a/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift @@ -298,8 +298,9 @@ struct RoomNotificationSettingsScreenViewModelTests { } .store(in: &cancellables) + #expect(!viewModel.context.viewState.deletingCustomSetting) let deferredViewState = deferFulfillment(viewModel.context.observe(\.viewState.deletingCustomSetting), - transitionValues: [false, true, false]) + transitionValues: [true, false]) viewModel.context.send(viewAction: .deleteCustomSettingTapped) diff --git a/UnitTests/Sources/SpaceScreenViewModelTests.swift b/UnitTests/Sources/SpaceScreenViewModelTests.swift index c7f6d10ce..76873909a 100644 --- a/UnitTests/Sources/SpaceScreenViewModelTests.swift +++ b/UnitTests/Sources/SpaceScreenViewModelTests.swift @@ -48,22 +48,15 @@ struct SpaceScreenViewModelTests { #expect(!response.isEmpty, "There should be some test rooms.") // When the pagination is triggered. - var deferred = deferFulfillment(spaceRoomListProxy.paginationStatePublisher) { $0 == .loading } + let deferredState = deferFulfillment(context.observe(\.viewState.paginationState), transitionValues: [.paginating, .endReached]) paginationStateSubject.send(.idle(endReached: false)) // Invert the default to allow paginate to be called. - try await deferred.fulfill() + try await deferredState.fulfill() - // Then the screen should show a paginating indicator. - #expect(context.viewState.paginationState == .paginating) + // Then a single pagination should be made and the space rooms should be populated. + // The rooms arrive on a separate publisher to the pagination state, so wait for them too. + let deferredRooms = deferFulfillment(context.observe(\.viewState.rooms)) { [response] in $0.map(\.id) == response.map(\.id) } + try await deferredRooms.fulfill() #expect(spaceRoomListProxy.paginateCallsCount == 1) - - // When waiting for the pagination to finish. - deferred = deferFulfillment(spaceRoomListProxy.paginationStatePublisher) { $0 == .idle(endReached: true) } - try await deferred.fulfill() - - // Then no more pagination requests should be made the the space rooms should be populated. - #expect(context.viewState.paginationState == .endReached) - #expect(spaceRoomListProxy.paginateCallsCount == 1) - #expect(context.viewState.rooms.map(\.id) == response.map(\.id)) } @Test @@ -153,16 +146,18 @@ struct SpaceScreenViewModelTests { let selectedSpace = try #require(mockSpaceRooms.first { $0.isSpace && $0.state != .joined }, "There should be a space to select.") - let deferredState = deferFulfillment(viewModel.context.observe(\.viewState.joiningRoomIDs), transitionValues: [[selectedSpace.id], []]) - - try await confirmation("Join room") { confirm in - clientProxy.joinRoomViaClosure = { _, _ in - confirm() + // Assert the transient joining state inside the closure: it can be cleared again before an observer is serviced. + await waitForConfirmation("Join room") { confirmation in + clientProxy.joinRoomViaClosure = { [context] _, _ in + await MainActor.run { #expect(context.viewState.joiningRoomIDs == [selectedSpace.id]) } + confirmation() return .success(()) } viewModel.context.send(viewAction: .spaceAction(.join(selectedSpace))) - try await deferredState.fulfill() } + + let deferredJoined = deferFulfillment(viewModel.context.observe(\.viewState.joiningRoomIDs)) { $0.isEmpty } + try await deferredJoined.fulfill() } @Test @@ -171,16 +166,18 @@ struct SpaceScreenViewModelTests { let selectedRoom = try #require(mockSpaceRooms.first { !$0.isSpace }, "There should be a room to select.") - let deferredState = deferFulfillment(viewModel.context.observe(\.viewState.joiningRoomIDs), transitionValues: [[selectedRoom.id], []]) - - try await confirmation("Join room") { confirm in - clientProxy.joinRoomViaClosure = { _, _ in - confirm() + // Assert the transient joining state inside the closure: it can be cleared again before an observer is serviced. + await waitForConfirmation("Join room") { confirmation in + clientProxy.joinRoomViaClosure = { [context] _, _ in + await MainActor.run { #expect(context.viewState.joiningRoomIDs == [selectedRoom.id]) } + confirmation() return .success(()) } viewModel.context.send(viewAction: .spaceAction(.join(selectedRoom))) - try await deferredState.fulfill() } + + let deferredJoined = deferFulfillment(viewModel.context.observe(\.viewState.joiningRoomIDs)) { $0.isEmpty } + try await deferredJoined.fulfill() } @Test diff --git a/UnitTests/Sources/TestUtilities/Extensions/NavigationCoordinators.swift b/UnitTests/Sources/TestUtilities/Extensions/NavigationCoordinators.swift new file mode 100644 index 000000000..f7c83a0e3 --- /dev/null +++ b/UnitTests/Sources/TestUtilities/Extensions/NavigationCoordinators.swift @@ -0,0 +1,53 @@ +// +// Copyright 2026 Element Creations Ltd. +// +// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. +// Please see LICENSE files in the repository root for full details. +// + +@testable import ElementX +import Foundation + +// Sendable projections of the navigation coordinators' non-Sendable coordinator references, +// allowing the tests to `observe` them and wait for navigation with `deferFulfillment`. + +extension NavigationSplitCoordinator { + var detailCoordinatorID: ObjectIdentifier? { + detailCoordinator.map { ObjectIdentifier($0) } + } + + /// The root coordinator inside the detail navigation stack. + var detailRootCoordinator: (any CoordinatorProtocol)? { + (detailCoordinator as? NavigationStackCoordinator)?.rootCoordinator + } + + /// The identity of the root coordinator inside the detail navigation stack. + var detailRootCoordinatorID: ObjectIdentifier? { + detailRootCoordinator.map { ObjectIdentifier($0) } + } + + var sheetCoordinatorID: ObjectIdentifier? { + sheetCoordinator.map { ObjectIdentifier($0) } + } +} + +extension NavigationStackCoordinator { + var rootCoordinatorID: ObjectIdentifier? { + rootCoordinator.map { ObjectIdentifier($0) } + } + + /// The identity of the topmost coordinator on the stack. + var topCoordinatorID: ObjectIdentifier? { + stackCoordinators.last.map { ObjectIdentifier($0) } + } + + var sheetCoordinatorID: ObjectIdentifier? { + sheetCoordinator.map { ObjectIdentifier($0) } + } +} + +extension NavigationTabCoordinator { + var sheetCoordinatorID: ObjectIdentifier? { + sheetCoordinator.map { ObjectIdentifier($0) } + } +} diff --git a/UnitTests/Sources/TimelineMediaPreviewViewModelTests.swift b/UnitTests/Sources/TimelineMediaPreviewViewModelTests.swift index 710e4eb93..5cf8f9469 100644 --- a/UnitTests/Sources/TimelineMediaPreviewViewModelTests.swift +++ b/UnitTests/Sources/TimelineMediaPreviewViewModelTests.swift @@ -181,11 +181,15 @@ struct TimelineMediaPreviewViewModelTests { // When confirming the redaction. let deferred = deferFulfillment(viewModel.actions) { $0 == .dismiss } - context.send(viewAction: .redactConfirmation(item: mediaItem)) + + // The redaction runs in an unstructured task, so wait for the call rather than asserting after the dismiss. + await waitForConfirmation { confirmation in + timelineController.redactClosure = { _ in confirmation() } + context.send(viewAction: .redactConfirmation(item: mediaItem)) + } // Then the item should be redacted and the view should be dismissed. try await deferred.fulfill() - #expect(timelineController.redactCalled) } @Test diff --git a/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift b/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift index 6a00fba63..168f8272d 100644 --- a/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift +++ b/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift @@ -101,7 +101,9 @@ struct UserSessionFlowCoordinatorTests { #expect((tabCoordinator?.sheetCoordinator as? NavigationStackCoordinator)?.rootCoordinator is SettingsScreenCoordinator) #expect(detailCoordinator == nil) - try await process(route: .room(roomID: "1", via: []), expectedChatsState: .roomList(detailState: .room(roomID: "1"))) + try await process(route: .room(roomID: "1", via: []), + expectedUserSessionState: .tabBar, + expectedChatsState: .roomList(detailState: .room(roomID: "1"))) #expect(tabCoordinator?.sheetCoordinator == nil) #expect(detailNavigationStack?.rootCoordinator is RoomScreenCoordinator) #expect(detailCoordinator != nil) @@ -148,9 +150,13 @@ struct UserSessionFlowCoordinatorTests { try await process(route: .share(sharePayload), expectedChatsState: .roomList(detailState: .room(roomID: "2"))) + let splitCoordinator = try #require(chatsSplitCoordinator) + let deferredSheet = deferFulfillment(splitCoordinator.observe(\.sheetCoordinatorID)) { $0 != nil } + try await deferredSheet.fulfill() + #expect(detailNavigationStack?.rootCoordinator is RoomScreenCoordinator) #expect(tabCoordinator?.sheetCoordinator == nil) - #expect((chatsSplitCoordinator?.sheetCoordinator as? NavigationStackCoordinator)?.rootCoordinator is MediaUploadPreviewScreenCoordinator) + #expect((splitCoordinator.sheetCoordinator as? NavigationStackCoordinator)?.rootCoordinator is MediaUploadPreviewScreenCoordinator) } @Test @@ -240,21 +246,22 @@ struct UserSessionFlowCoordinatorTests { // MARK: - Helpers - private mutating func process(route: AppRoute, - expectedUserSessionState: UserSessionFlowCoordinator.State? = nil, - expectedChatsState: ChatsTabFlowCoordinatorStateMachine.State? = nil) async throws { + private func process(route: AppRoute, + expectedUserSessionState: UserSessionFlowCoordinator.State? = nil, + expectedChatsState: ChatsTabFlowCoordinatorStateMachine.State? = nil) async throws { + // Keep the previous root coordinator alive while waiting, otherwise a newly presented + // coordinator could be allocated at the same address and be mistaken for it below. + let previousDetailRootCoordinator = chatsSplitCoordinator?.detailRootCoordinator + let previousDetailRootCoordinatorID = previousDetailRootCoordinator.map { ObjectIdentifier($0) } + let deferredUserSession: DeferredFulfillment? = if let expectedUserSessionState { - deferFulfillment(stateMachineFactory.userSessionFlowStatePublisher.delay(for: .milliseconds(100), scheduler: DispatchQueue.main)) { - $0 == expectedUserSessionState - } + deferFulfillment(stateMachineFactory.userSessionFlowStatePublisher) { $0 == expectedUserSessionState } } else { nil } let deferredChatsState: DeferredFulfillment? = if let expectedChatsState { - deferFulfillment(stateMachineFactory.chatsTabFlowStatePublisher.delay(for: .milliseconds(100), scheduler: DispatchQueue.main)) { - $0 == expectedChatsState - } + deferFulfillment(stateMachineFactory.chatsTabFlowStatePublisher) { $0 == expectedChatsState } } else { nil } @@ -262,6 +269,36 @@ struct UserSessionFlowCoordinatorTests { userSessionFlowCoordinator.handleAppRoute(route, animated: true) try await deferredUserSession?.fulfill() try await deferredChatsState?.fulfill() + + // The state machines' states change before the coordinators have updated their stacks, + // so also wait for the navigation side effects implied by the expected states. + switch expectedUserSessionState { + case .settingsScreen: + let tabCoordinator = try #require(tabCoordinator) + let deferredSheet = deferFulfillment(tabCoordinator.observe(\.sheetCoordinatorID)) { $0 != nil } + try await deferredSheet.fulfill() + case .tabBar: + let tabCoordinator = try #require(tabCoordinator) + let deferredSheet = deferFulfillment(tabCoordinator.observe(\.sheetCoordinatorID)) { $0 == nil } + try await deferredSheet.fulfill() + default: + break + } + + switch expectedChatsState { + case .roomList(detailState: .some(.room)): + let splitCoordinator = try #require(chatsSplitCoordinator) + let deferredDetail = deferFulfillment(splitCoordinator.observe(\.detailRootCoordinatorID)) { $0 != nil && $0 != previousDetailRootCoordinatorID } + try await deferredDetail.fulfill() + case .shareExtensionRoomList: + let splitCoordinator = try #require(chatsSplitCoordinator) + let deferredSheet = deferFulfillment(splitCoordinator.observe(\.sheetCoordinatorID)) { $0 != nil } + try await deferredSheet.fulfill() + default: + break + } + + withExtendedLifetime(previousDetailRootCoordinator) { } } /// Other services retract indicators, so this filters based on the reachability ID.