From 58fb7c4fcb7ba9a8f54a595c8eb421126b93c9c6 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 28 Jul 2026 18:52:10 +0200 Subject: [PATCH] Wait on durable state in three flaky test suites These three suites assert against state that is still being loaded or has already moved on, so they fail whenever a busy runner stretches the window. All three passed on CI only because `-retry-tests-on-failure` re-ran them. - MediaUploadPreviewScreenViewModelTests: the processing failure tests treated a one second `deferFailure` window as if it also meant the send had finished. Preprocessing the valid files outlasts it, so wait for the interaction to be re-enabled instead. - SecurityAndPrivacyScreenViewModelTests: the screen loads its parent spaces and its room directory visibility in two independent tasks, and `isSaveDisabled` needs both. Every wait now gates on the visibility having landed too. - RoomNotificationSettingsScreenViewModelTests: the delete tests waited on a transient `deletingCustomSetting` flag that the observation coalesces away under load. They now wait on the dismiss action and the alert. --- ...diaUploadPreviewScreenViewModelTests.swift | 10 +++++-- ...ficationSettingsScreenViewModelTests.swift | 30 ++++++++----------- ...curityAndPrivacyScreenViewModelTests.swift | 27 +++++++++++------ 3 files changed, 39 insertions(+), 28 deletions(-) diff --git a/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift b/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift index a087583dc..f5020f734 100644 --- a/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift +++ b/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift @@ -117,8 +117,11 @@ final class MediaUploadPreviewScreenViewModelTests { #expect(userIndicatorController.submitIndicatorDelayCallsCount == 1) // Loading indicator // Then the failure should occur preventing the screen from being dismissed. + // The interaction being re-enabled is what marks the end of the send, the timeout above only proves + // that no dismissal happened within it — processing may still be in flight when it elapses. + let deferredInteraction = deferFulfillment(context.observe(\.viewState.shouldDisableInteraction)) { !$0 } try await deferredFailure.fulfill() - #expect(!context.viewState.shouldDisableInteraction) + try await deferredInteraction.fulfill() #expect(userIndicatorController.submitIndicatorDelayCallsCount == 2, "An error indicator should be shown.") } @@ -213,8 +216,11 @@ final class MediaUploadPreviewScreenViewModelTests { #expect(userIndicatorController.submitIndicatorDelayCallsCount == 1) // Loading indicator // Then the failure should occur preventing the screen from being dismissed. + // Preprocessing the valid files can outlast the timeout above, so wait for the interaction to be + // re-enabled rather than assuming the send has finished by the time it elapses. + let deferredInteraction = deferFulfillment(context.observe(\.viewState.shouldDisableInteraction)) { !$0 } try await deferredFailure.fulfill() - #expect(!context.viewState.shouldDisableInteraction) + try await deferredInteraction.fulfill() #expect(userIndicatorController.submitIndicatorDelayCallsCount == 2, "An error indicator should be shown.") } diff --git a/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift b/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift index ee1eefceb..0612b79a9 100644 --- a/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift +++ b/UnitTests/Sources/RoomNotificationSettingsScreenViewModelTests.swift @@ -254,22 +254,16 @@ struct RoomNotificationSettingsScreenViewModelTests { notificationSettingsProxyMock.callbacks.send(.settingsDidChange) try await deferred.fulfill() - var actionSent: RoomNotificationSettingsScreenViewModelAction? - viewModel.actions - .sink { action in - actionSent = action - } - .store(in: &cancellables) - - let deferredViewState = deferFulfillment(viewModel.context.observe(\.viewState.deletingCustomSetting), - transitionValues: [false, true, false]) + // The `deletingCustomSetting` flag is only raised for the duration of the call, which is short enough + // that the observation can coalesce it away. Wait on the outcome instead. + let deferredDismiss = deferFulfillment(viewModel.actions) { $0 == .dismiss } viewModel.context.send(viewAction: .deleteCustomSettingTapped) - try await deferredViewState.fulfill() - // the `dismiss` action must have been sent - #expect(actionSent == .dismiss) + try await deferredDismiss.fulfill() + + #expect(!viewModel.context.viewState.deletingCustomSetting) // `restoreDefaultNotificationMode` should have been called #expect(notificationSettingsProxyMock.restoreDefaultNotificationModeRoomIdCalled) #expect(notificationSettingsProxyMock.restoreDefaultNotificationModeRoomIdReceivedInvocations == [roomProxyMock.id]) @@ -299,15 +293,17 @@ struct RoomNotificationSettingsScreenViewModelTests { .store(in: &cancellables) #expect(!viewModel.context.viewState.deletingCustomSetting) - let deferredViewState = deferFulfillment(viewModel.context.observe(\.viewState.deletingCustomSetting), - transitionValues: [true, false]) + // The `deletingCustomSetting` flag is only raised for the duration of the call, which is short enough + // that the observation can coalesce it away. Wait on the alert instead, it is raised before the flag + // is lowered again. + let deferredAlert = deferFulfillment(viewModel.context.observe(\.viewState.bindings.alertInfo)) { $0?.id == .restoreDefaultFailed } viewModel.context.send(viewAction: .deleteCustomSettingTapped) - try await deferredViewState.fulfill() - // an alert is expected - #expect(viewModel.context.alertInfo?.id == .restoreDefaultFailed) + try await deferredAlert.fulfill() + + #expect(!viewModel.context.viewState.deletingCustomSetting) // the `dismiss` action must not have been sent #expect(actionSent == nil) } diff --git a/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift b/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift index c4797a14c..8a0748185 100644 --- a/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift +++ b/UnitTests/Sources/SecurityAndPrivacyScreenViewModelTests.swift @@ -31,7 +31,7 @@ final class SecurityAndPrivacyScreenViewModelTests { let space = singleRoom[0] setupViewModel(joinedParentSpaces: singleRoom, joinRule: .public) - let deferred = deferFulfillment(context.$viewState) { $0.selectableJoinedSpaces.count == 1 } + let deferred = deferScreenLoaded { $0.selectableJoinedSpaces.count == 1 } try await deferred.fulfill() #expect(context.viewState.currentSettings.accessType == .anyone) @@ -63,7 +63,7 @@ final class SecurityAndPrivacyScreenViewModelTests { let space = singleRoom[0] setupViewModel(joinedParentSpaces: singleRoom, joinRule: .public) - let deferred = deferFulfillment(context.$viewState) { $0.selectableJoinedSpaces.count == 1 } + let deferred = deferScreenLoaded { $0.selectableJoinedSpaces.count == 1 } try await deferred.fulfill() #expect(context.viewState.currentSettings.accessType == .anyone) @@ -95,7 +95,7 @@ final class SecurityAndPrivacyScreenViewModelTests { let space = singleRoom[0] setupViewModel(joinedParentSpaces: [], joinRule: .restricted(rules: [.roomMembership(roomID: space.id)])) - let deferred = deferFulfillment(context.$viewState) { $0.selectableJoinedSpaces.isEmpty } + let deferred = deferScreenLoaded { $0.selectableJoinedSpaces.isEmpty } try await deferred.fulfill() #expect(context.viewState.currentSettings.accessType == .spaceMembers(spaceIDs: [space.id])) @@ -127,7 +127,7 @@ final class SecurityAndPrivacyScreenViewModelTests { let spaces = [SpaceServiceRoom].mockJoinedSpaces2 setupViewModel(joinedParentSpaces: spaces, joinRule: .public) - let deferred = deferFulfillment(context.$viewState) { $0.selectableJoinedSpaces.count == 3 } + let deferred = deferScreenLoaded { $0.selectableJoinedSpaces.count == 3 } try await deferred.fulfill() #expect(context.viewState.currentSettings.accessType == .anyone) @@ -172,7 +172,7 @@ final class SecurityAndPrivacyScreenViewModelTests { let spaces = [SpaceServiceRoom].mockJoinedSpaces2 setupViewModel(joinedParentSpaces: spaces, joinRule: .public) - let deferred = deferFulfillment(context.$viewState) { $0.selectableJoinedSpaces.count == 3 } + let deferred = deferScreenLoaded { $0.selectableJoinedSpaces.count == 3 } try await deferred.fulfill() #expect(context.viewState.currentSettings.accessType == .anyone) @@ -218,7 +218,7 @@ final class SecurityAndPrivacyScreenViewModelTests { setupViewModel(joinedParentSpaces: spaces, joinRule: .restricted(rules: [.roomMembership(roomID: "unknownSpaceID")])) - let deferred = deferFulfillment(context.$viewState) { $0.selectableSpacesCount == 4 } + let deferred = deferScreenLoaded { $0.selectableSpacesCount == 4 } try await deferred.fulfill() #expect(context.viewState.currentSettings.accessType.isSpaceMembers) @@ -270,7 +270,7 @@ final class SecurityAndPrivacyScreenViewModelTests { joinRule: .restricted(rules: [.roomMembership(roomID: space.id), .roomMembership(roomID: "unknownSpaceID")])) - let deferred = deferFulfillment(context.$viewState) { $0.selectableSpacesCount == 5 } + let deferred = deferScreenLoaded { $0.selectableSpacesCount == 5 } try await deferred.fulfill() #expect(context.viewState.currentSettings.accessType.isSpaceMembers) @@ -309,7 +309,7 @@ final class SecurityAndPrivacyScreenViewModelTests { setupViewModel(joinedParentSpaces: [], joinRule: .restricted(rules: [])) - let deferred = deferFulfillment(context.$viewState) { $0.selectableSpacesCount == 0 } + let deferred = deferScreenLoaded { $0.selectableSpacesCount == 0 } try await deferred.fulfill() #expect(context.viewState.currentSettings.accessType.isSpaceMembers) @@ -330,7 +330,7 @@ final class SecurityAndPrivacyScreenViewModelTests { setupViewModel(joinedParentSpaces: singleRoom, joinRule: .restricted(rules: [])) - let deferred = deferFulfillment(context.$viewState) { $0.selectableSpacesCount == 1 } + let deferred = deferScreenLoaded { $0.selectableSpacesCount == 1 } try await deferred.fulfill() #expect(context.viewState.currentSettings.accessType.isSpaceMembers) @@ -444,6 +444,15 @@ final class SecurityAndPrivacyScreenViewModelTests { // MARK: - Helpers + /// Waits for `condition` **and** for the screen to have finished loading. + /// + /// The view model fetches its parent spaces and its room directory visibility in two independent tasks. + /// Waiting on the spaces alone leaves `isSaveDisabled` reading `true` whilst the visibility is unknown, + /// so every wait needs to gate on both having landed. + private func deferScreenLoaded(until condition: @escaping (SecurityAndPrivacyScreenViewState) -> Bool) -> DeferredFulfillment { + deferFulfillment(context.$viewState) { condition($0) && $0.currentSettings.isVisibileInRoomDirectory != nil } + } + private func setupViewModel(joinedParentSpaces: [SpaceServiceRoom], topLevelSpaces: [SpaceServiceRoom] = [], joinRule: ElementX.JoinRule,