Refactor unit tests

element-hq · Feb 2, 2023 · a2b1177 · a2b1177
1 parent e5e4035
commit a2b1177
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 48 deletions.
diff --git a/Riot/Managers/PushRulesUpdater/PushRulesUpdater.swift b/Riot/Managers/PushRulesUpdater/PushRulesUpdater.swift
@@ -20,6 +20,11 @@ final class PushRulesUpdater {
     private var cancellables: Set<AnyCancellable> = .init()
     private var rules: [NotificationPushRuleType] = []
     private let notificationSettingsService: NotificationSettingsServiceType
+    private let didCompleteUpdateSubject: PassthroughSubject<Void, Never> = .init()
+
+    var didCompleteUpdate: AnyPublisher<Void, Never> {
+        didCompleteUpdateSubject.eraseToAnyPublisher()
+    }
 
     init(notificationSettingsService: NotificationSettingsServiceType, needsCheck: AnyPublisher<Void, Never>) {
         self.notificationSettingsService = notificationSettingsService
@@ -39,31 +44,35 @@ final class PushRulesUpdater {
 
 private extension PushRulesUpdater {
     func syncRulesIfNeeded() {
-        for rule in rules {
-            syncRelatedRulesIfNeeded(for: rule)
-        }
-    }
-
-    func syncRelatedRulesIfNeeded(for rule: NotificationPushRuleType) {
-        guard let ruleId = rule.pushRuleId else {
-            return
-        }
+        let dispatchGroup: DispatchGroup = .init()
 
-        let relatedRules = ruleId.syncedRules(in: rules)
-
-        for relatedRule in relatedRules {
-            guard rule.hasSameContentOf(relatedRule) == false else {
+        for rule in rules {
+            guard let ruleId = rule.pushRuleId else {
                 continue
             }
 
-            sync(relatedRuleId: relatedRule.ruleId, with: rule)
+            let relatedRules = ruleId.syncedRules(in: rules)
+
+            for relatedRule in relatedRules {
+                guard rule.hasSameContentOf(relatedRule) == false else {
+                    continue
+                }
+
+                dispatchGroup.enter()
+                Task {
+                    try? await sync(relatedRuleId: relatedRule.ruleId, with: rule)
+                    dispatchGroup.leave()
+                }
+            }
+        }
+
+        dispatchGroup.notify(queue: .main) { [weak self] in
+            self?.didCompleteUpdateSubject.send(())
         }
     }
 
-    func sync(relatedRuleId: String, with rule: NotificationPushRuleType) {
-        Task {
-            try? await notificationSettingsService.updatePushRuleActions(for: relatedRuleId, enabled: rule.enabled, actions: rule.ruleActions)
-        }
+    func sync(relatedRuleId: String, with rule: NotificationPushRuleType) async throws {
+        try await notificationSettingsService.updatePushRuleActions(for: relatedRuleId, enabled: rule.enabled, actions: rule.ruleActions)
     }
 }
 

diff --git a/RiotTests/PushRulesUpdaterTests.swift b/RiotTests/PushRulesUpdaterTests.swift
@@ -22,12 +22,17 @@ final class PushRulesUpdaterTests: XCTestCase {
     private var notificationService: MockNotificationSettingsService!
     private var pushRulesUpdater: PushRulesUpdater!
     private var needsCheckPublisher: PassthroughSubject<Void, Never> = .init()
+    private var subscriptions: Set<AnyCancellable> = .init()
 
     override func setUpWithError() throws {
         notificationService = .init()
         notificationService.rules = [MockNotificationPushRule].default
         pushRulesUpdater = .init(notificationSettingsService: notificationService, needsCheck: needsCheckPublisher.eraseOutput())
     }
+
+    override func tearDownWithError() throws {
+        subscriptions.removeAll()
+    }
 
     func testNoRuleIsUpdated() throws {
         needsCheckPublisher.send()
@@ -42,11 +47,14 @@ final class PushRulesUpdaterTests: XCTestCase {
 
         needsCheckPublisher.send(())
 
-        DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
-            XCTAssertEqual(self.notificationService.rules[targetRuleIndex].ruleActions, NotificationStandardActions.notifyDefaultSound.actions)
-            XCTAssertTrue(self.notificationService.rules[targetRuleIndex].enabled)
-            expectation.fulfill()
-        }
+        pushRulesUpdater
+            .didCompleteUpdate
+            .sink { _ in
+                XCTAssertEqual(self.notificationService.rules[targetRuleIndex].ruleActions, NotificationStandardActions.notifyDefaultSound.actions)
+                XCTAssertTrue(self.notificationService.rules[targetRuleIndex].enabled)
+                expectation.fulfill()
+            }
+            .store(in: &subscriptions)
 
         waitForExpectations(timeout: 2.0)
     }
@@ -60,21 +68,24 @@ final class PushRulesUpdaterTests: XCTestCase {
 
         needsCheckPublisher.send(())
 
-        DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
-            for rule in self.notificationService.rules {
-                guard let id = rule.pushRuleId else {
-                    continue
-                }
-
-                if affectedRules.contains(id) {
-                    XCTAssertEqual(rule.ruleActions, targetActions)
-                } else {
-                    XCTAssertEqual(rule.ruleActions, NotificationStandardActions.notifyDefaultSound.actions)
+        pushRulesUpdater
+            .didCompleteUpdate
+            .sink { _ in
+                for rule in self.notificationService.rules {
+                    guard let id = rule.pushRuleId else {
+                        continue
+                    }
+
+                    if affectedRules.contains(id) {
+                        XCTAssertEqual(rule.ruleActions, targetActions)
+                    } else {
+                        XCTAssertEqual(rule.ruleActions, NotificationStandardActions.notifyDefaultSound.actions)
+                    }
                 }
+                expectation.fulfill()
             }
-            expectation.fulfill()
-        }
-
+            .store(in: &subscriptions)
+
         waitForExpectations(timeout: 2.0)
     }
 
@@ -87,20 +98,23 @@ final class PushRulesUpdaterTests: XCTestCase {
 
         needsCheckPublisher.send(())
 
-        DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
-            for rule in self.notificationService.rules {
-                guard let id = rule.pushRuleId else {
-                    continue
-                }
-
-                if affectedRules.contains(id) {
-                    XCTAssertEqual(rule.ruleActions, targetActions)
-                } else {
-                    XCTAssertEqual(rule.ruleActions, NotificationStandardActions.notifyDefaultSound.actions)
+        pushRulesUpdater
+            .didCompleteUpdate
+            .sink { _ in
+                for rule in self.notificationService.rules {
+                    guard let id = rule.pushRuleId else {
+                        continue
+                    }
+
+                    if affectedRules.contains(id) {
+                        XCTAssertEqual(rule.ruleActions, targetActions)
+                    } else {
+                        XCTAssertEqual(rule.ruleActions, NotificationStandardActions.notifyDefaultSound.actions)
+                    }
                 }
+                expectation.fulfill()
             }
-            expectation.fulfill()
-        }
+            .store(in: &subscriptions)
 
         waitForExpectations(timeout: 2.0)
     }