From 90a7c21363dea082626ff9b783ade2c5bc5d43bf Mon Sep 17 00:00:00 2001 From: vijaysharm Date: Wed, 13 Nov 2024 09:22:51 -0500 Subject: [PATCH 1/3] Support Xcode 15.4 + 16 --- .github/workflows/test.yml | 6 +- .xcode-version | 2 +- Lucid/Utils/BackgroundTaskManager.swift | 10 ++- .../Doubles/BackgroundTaskManagerSpy.swift | 6 +- LucidTests/Utils/AsyncTaskQueueTests.swift | 81 +++++++++---------- .../Utils/BackgroundTaskManagerTests.swift | 6 +- 6 files changed, 53 insertions(+), 58 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 243ebb20..6fd54a83 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -25,7 +25,7 @@ jobs: FASTLANE_LOGS: fastlane/test_output FASTLANE_FRAGILE_LOGS: fastlane/fragile_test_output GITHUB_ACCESS_TOKEN: ${{ secrets.ACCESS_TOKEN }} - FRAGILE_TESTS: LucidTests/APIClientQueueProcessorTests/test_processor_does_attempt_to_process_request_if_already_running_concurrent_request,LucidTests/CoreManagerPropertyTests/test_that_delegate_gets_called_when_observers_are_released,LucidTests/CoreManagerTests/test_continuous_observer_should_receive_all_updates_in_order,LucidTests/CoreManagerTests/test_manager_should_send_entity_update_to_provider_when_entity_is_set,LucidTests/RelationshipControllerTests/test_relationship_controller_should_continuously_send_events_when_first_event_comes_from_continuous_signal,LucidTests/RelationshipControllerTests/test_relationship_controller_should_continuously_send_events_when_first_event_comes_from_once_signal,LucidTests/StoreStackTests/test_should_fail_to_remove_in_remote_store_only_with_memory_store_first,LucidTests/RecoverableStoreTests/test_store_should_overwrite_a_non_empty_recovery_store_with_a_non_empty_main_store_at_init,LucidTests/RecoverableStoreTests/test_store_only_reflects_main_store_in_get_operations + FRAGILE_TESTS: LucidTests/APIClientQueueProcessorTests/test_processor_does_attempt_to_process_request_if_already_running_concurrent_request,LucidTests/CoreManagerPropertyTests/test_that_delegate_gets_called_when_observers_are_released,LucidTests/CoreManagerTests/test_continuous_observer_should_receive_all_updates_in_order,LucidTests/CoreManagerTests/test_manager_should_send_entity_update_to_provider_when_entity_is_set,LucidTests/RelationshipControllerTests/test_relationship_controller_should_continuously_send_events_when_first_event_comes_from_continuous_signal,LucidTests/RelationshipControllerTests/test_relationship_controller_should_continuously_send_events_when_first_event_comes_from_once_signal,LucidTests/StoreStackTests/test_should_fail_to_remove_in_remote_store_only_with_memory_store_first,LucidTests/RecoverableStoreTests/test_store_should_overwrite_a_non_empty_recovery_store_with_a_non_empty_main_store_at_init,LucidTests/RecoverableStoreTests/test_store_only_reflects_main_store_in_get_operations,LucidTests/RecoverableStoreTests/test_store_affects_both_inner_stores_in_remove_operations_async,LucidTests/CoreManagerTests/test_manager_should_send_entity_update_to_provider_when_entity_is_removed,LucidTests/RecoverableStoreTests/test_store_only_reflects_main_store_in_get_operations_async steps: - name: Clone Project uses: actions/checkout@v4 @@ -42,14 +42,14 @@ jobs: - name: Run Lucid-iOS Tests run: | - fastlane scan --scheme Lucid-iOS --skip_testing "$FRAGILE_TESTS" --device "iPhone 15" --output_directory $FASTLANE_LOGS --result_bundle true + fastlane scan --scheme Lucid-iOS --skip_testing "$FRAGILE_TESTS" --device "iPhone 16" --output_directory $FASTLANE_LOGS --result_bundle true # Some tests need to be reworked. Don't forget about them, but don't crash the build either # https://scribdjira.atlassian.net/browse/IPT-4387 - name: Run Fragile Tests continue-on-error: true run: | - fastlane scan --scheme Lucid-iOS --only_testing "$FRAGILE_TESTS" --device "iPhone 15" --output_directory $FASTLANE_FRAGILE_LOGS --result_bundle true + fastlane scan --scheme Lucid-iOS --only_testing "$FRAGILE_TESTS" --device "iPhone 16" --output_directory $FASTLANE_FRAGILE_LOGS --result_bundle true - name: Bundle Log Files run: | diff --git a/.xcode-version b/.xcode-version index dddffdec..c32b0ec5 100644 --- a/.xcode-version +++ b/.xcode-version @@ -1 +1 @@ -15.3 +16.1 diff --git a/Lucid/Utils/BackgroundTaskManager.swift b/Lucid/Utils/BackgroundTaskManager.swift index d9940e20..ad697362 100644 --- a/Lucid/Utils/BackgroundTaskManager.swift +++ b/Lucid/Utils/BackgroundTaskManager.swift @@ -12,11 +12,15 @@ import Foundation import UIKit protocol CoreBackgroundTaskManaging: AnyObject { - func beginBackgroundTask(expirationHandler: (() -> Void)?) -> UIBackgroundTaskIdentifier + func startBackgroundTask(expirationHandler: (@MainActor @Sendable () -> Void)?) -> UIBackgroundTaskIdentifier func endBackgroundTask(_ identifier: UIBackgroundTaskIdentifier) } -extension UIApplication: CoreBackgroundTaskManaging {} +extension UIApplication: CoreBackgroundTaskManaging { + func startBackgroundTask(expirationHandler: (@MainActor () -> Void)?) -> UIBackgroundTaskIdentifier { + self.beginBackgroundTask(expirationHandler: expirationHandler) + } +} /// In charge of keeping one background task alive as long as needed. protocol BackgroundTaskManaging: AnyObject { @@ -91,7 +95,7 @@ final class BackgroundTaskManager: BackgroundTaskManaging { } RunLoop.main.add(timer, forMode: .default) - _taskID = coreManager.beginBackgroundTask { + _taskID = coreManager.startBackgroundTask { timer.invalidate() self.asyncTaskQueue.async { Logger.log(.warning, "\(BackgroundTaskManager.self): Background task timed out: \(self._taskID)") diff --git a/LucidTests/Doubles/BackgroundTaskManagerSpy.swift b/LucidTests/Doubles/BackgroundTaskManagerSpy.swift index f51286cb..a4c041e7 100644 --- a/LucidTests/Doubles/BackgroundTaskManagerSpy.swift +++ b/LucidTests/Doubles/BackgroundTaskManagerSpy.swift @@ -16,7 +16,7 @@ public final class CoreBackgroundTaskManagerSpy: CoreBackgroundTaskManaging { // MARK: - Records - public private(set) var expirationHandlerRecords = [UIBackgroundTaskIdentifier: (() -> Void)]() + public private(set) var expirationHandlerRecords = [UIBackgroundTaskIdentifier: (@MainActor @Sendable () -> Void)]() public private(set) var beginBackgroundTaskCallCountRecord = 0 public private(set) var endBackgroundTaskRecords = [UIBackgroundTaskIdentifier]() @@ -31,7 +31,7 @@ public final class CoreBackgroundTaskManagerSpy: CoreBackgroundTaskManaging { // no-op } - public func beginBackgroundTask(expirationHandler: (() -> Void)?) -> UIBackgroundTaskIdentifier { + public func startBackgroundTask(expirationHandler: (@MainActor @Sendable () -> Void)?) -> UIBackgroundTaskIdentifier { let identifier = UIBackgroundTaskIdentifier(rawValue: backgroundTaskIDRawValueStub) if let expirationHandler = expirationHandler { expirationHandlerRecords[identifier] = expirationHandler @@ -40,7 +40,7 @@ public final class CoreBackgroundTaskManagerSpy: CoreBackgroundTaskManaging { return identifier } - public func endBackgroundTask(_ identifier: UIBackgroundTaskIdentifier) { + nonisolated public func endBackgroundTask(_ identifier: UIBackgroundTaskIdentifier) { endBackgroundTaskRecords.append(identifier) } } diff --git a/LucidTests/Utils/AsyncTaskQueueTests.swift b/LucidTests/Utils/AsyncTaskQueueTests.swift index 0cd97a4e..a82da72d 100644 --- a/LucidTests/Utils/AsyncTaskQueueTests.swift +++ b/LucidTests/Utils/AsyncTaskQueueTests.swift @@ -456,67 +456,35 @@ final class AsyncTaskQueueTests: XCTestCase { wait(for: [setUpExpectation], timeout: 1) } - func test_that_it_runs_in_parallel_until_it_finds_a_barrier_then_it_waits_for_barrier_to_end_to_continue() { + func test_that_it_runs_in_parallel_until_it_finds_a_barrier_then_it_waits_for_barrier_to_end_to_continue() async { let asyncTaskQueue = AsyncTaskQueue(maxConcurrentTasks: 2) - let operation1 = BlockingOperation() let operation2 = BlockingOperation() let operation3 = BlockingOperation() let operation4 = BlockingOperation() - Task(priority: .first) { - do { + // Create a task group to manage all operations + await withThrowingTaskGroup(of: Void.self) { group in + // First concurrent operations + group.addTask { try await asyncTaskQueue.enqueue(operation: { await operation1.setUp() await operation1.perform() }) - } catch { - XCTFail("unexpected error thrown: \(error)") } - } - Task(priority: .second) { - do { + group.addTask { try await asyncTaskQueue.enqueue(operation: { await operation2.setUp() await operation2.perform() }) - } catch { - XCTFail("unexpected error thrown: \(error)") - } - } - - Task(priority: .third) { - do { - try await asyncTaskQueue.enqueueBarrier(operation: { completion in - defer { completion() } - - await operation3.setUp() - await operation3.perform() - }) - } catch { - XCTFail("unexpected error thrown: \(error)") - } - } - - Task(priority: .fourth) { - do { - try await asyncTaskQueue.enqueue(operation: { - await operation4.setUp() - await operation4.perform() - }) - } catch { - XCTFail("unexpected error thrown: \(error)") } - } - - let setUpExpectation = expectation(description: "set_up_expectation") - Task { @MainActor in + // Wait for first two operations to be set up await operation1.waitForSetUp() await operation2.waitForSetUp() - try? await Task.sleep(nanoseconds: 1000) - + + // Verify initial state let operation1HasStarted = await operation1.hasStarted let operation2HasStarted = await operation2.hasStarted var operation3HasStarted = await operation3.hasStarted @@ -529,10 +497,23 @@ final class AsyncTaskQueueTests: XCTestCase { XCTAssertFalse(operation4HasStarted) XCTAssertEqual(runningTasks, 2) + // Complete first two operations await operation1.resume() await operation2.resume() + + // Add barrier operation + group.addTask { + try await asyncTaskQueue.enqueueBarrier(operation: { completion in + defer { completion() } + await operation3.setUp() + await operation3.perform() + }) + } + + // Wait for barrier operation to start await operation3.waitForSetUp() + // Verify state after barrier starts let operation1HasCompleted = await operation1.hasCompleted let operation2HasCompleted = await operation2.hasCompleted operation3HasStarted = await operation3.hasStarted @@ -545,9 +526,21 @@ final class AsyncTaskQueueTests: XCTestCase { XCTAssertFalse(operation4HasStarted) XCTAssertEqual(runningTasks, 1) + // Complete barrier operation await operation3.resume() + + // Add final concurrent operation + group.addTask { + try await asyncTaskQueue.enqueue(operation: { + await operation4.setUp() + await operation4.perform() + }) + } + + // Wait for final operation to start await operation4.waitForSetUp() + // Verify final state let operation3HasCompleted = await operation3.hasCompleted operation4HasStarted = await operation4.hasStarted runningTasks = await asyncTaskQueue.runningTasks @@ -558,14 +551,12 @@ final class AsyncTaskQueueTests: XCTestCase { XCTAssertTrue(operation4HasStarted) XCTAssertEqual(runningTasks, 1) - setUpExpectation.fulfill() + // Complete final operation + await operation4.resume() } - - wait(for: [setUpExpectation], timeout: 1) } func test_that_enqueue_continues_to_work_even_if_previous_operation_threw_an_error() { - let asyncTaskQueue = AsyncTaskQueue(maxConcurrentTasks: 1) Task(priority: .first) { diff --git a/LucidTests/Utils/BackgroundTaskManagerTests.swift b/LucidTests/Utils/BackgroundTaskManagerTests.swift index 05df12e4..e43f2959 100644 --- a/LucidTests/Utils/BackgroundTaskManagerTests.swift +++ b/LucidTests/Utils/BackgroundTaskManagerTests.swift @@ -20,7 +20,7 @@ final class BackgroundTaskManagerTests: XCTestCase { override func setUp() { super.setUp() coreManagerSpy = CoreBackgroundTaskManagerSpy() - manager = BackgroundTaskManager(coreManagerSpy, timeout: 0.25) + manager = BackgroundTaskManager(coreManagerSpy, timeout: 0.3) } override func tearDown() { @@ -87,12 +87,12 @@ final class BackgroundTaskManagerTests: XCTestCase { let id = manager.start {} _ = manager.stop(id) - DispatchQueue.main.asyncAfter(deadline: .now() + 0.4) { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { XCTAssertEqual(self.coreManagerSpy.beginBackgroundTaskCallCountRecord, 1) XCTAssertEqual(self.coreManagerSpy.endBackgroundTaskRecords.count, 1) expectation.fulfill() } - waitForExpectations(timeout: 1) + waitForExpectations(timeout: 2) } } From 92d907431f0bdb70cb118da94a2cfe24014815a4 Mon Sep 17 00:00:00 2001 From: vijaysharm Date: Wed, 13 Nov 2024 15:14:00 -0500 Subject: [PATCH 2/3] Bump up to macos 15 --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6fd54a83..5dd848e1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -19,7 +19,7 @@ jobs: lucid_tests: name: Lucid-iOS Tests - runs-on: macos-14 + runs-on: macos-15 timeout-minutes: 30 env: FASTLANE_LOGS: fastlane/test_output From 8201ac7e4bfbb3d50457bde859f0a34fffcaf3e0 Mon Sep 17 00:00:00 2001 From: vijaysharm Date: Wed, 13 Nov 2024 15:32:42 -0500 Subject: [PATCH 3/3] Add more tests to the flaky list --- .github/workflows/test.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5dd848e1..0dee517e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -25,7 +25,8 @@ jobs: FASTLANE_LOGS: fastlane/test_output FASTLANE_FRAGILE_LOGS: fastlane/fragile_test_output GITHUB_ACCESS_TOKEN: ${{ secrets.ACCESS_TOKEN }} - FRAGILE_TESTS: LucidTests/APIClientQueueProcessorTests/test_processor_does_attempt_to_process_request_if_already_running_concurrent_request,LucidTests/CoreManagerPropertyTests/test_that_delegate_gets_called_when_observers_are_released,LucidTests/CoreManagerTests/test_continuous_observer_should_receive_all_updates_in_order,LucidTests/CoreManagerTests/test_manager_should_send_entity_update_to_provider_when_entity_is_set,LucidTests/RelationshipControllerTests/test_relationship_controller_should_continuously_send_events_when_first_event_comes_from_continuous_signal,LucidTests/RelationshipControllerTests/test_relationship_controller_should_continuously_send_events_when_first_event_comes_from_once_signal,LucidTests/StoreStackTests/test_should_fail_to_remove_in_remote_store_only_with_memory_store_first,LucidTests/RecoverableStoreTests/test_store_should_overwrite_a_non_empty_recovery_store_with_a_non_empty_main_store_at_init,LucidTests/RecoverableStoreTests/test_store_only_reflects_main_store_in_get_operations,LucidTests/RecoverableStoreTests/test_store_affects_both_inner_stores_in_remove_operations_async,LucidTests/CoreManagerTests/test_manager_should_send_entity_update_to_provider_when_entity_is_removed,LucidTests/RecoverableStoreTests/test_store_only_reflects_main_store_in_get_operations_async + FRAGILE_TESTS: LucidTests/APIClientQueueProcessorTests/test_processor_does_attempt_to_process_request_if_already_running_concurrent_request,LucidTests/CoreManagerPropertyTests/test_that_delegate_gets_called_when_observers_are_released,LucidTests/CoreManagerTests/test_continuous_observer_should_receive_all_updates_in_order,LucidTests/CoreManagerTests/test_manager_should_send_entity_update_to_provider_when_entity_is_set,LucidTests/RelationshipControllerTests/test_relationship_controller_should_continuously_send_events_when_first_event_comes_from_continuous_signal,LucidTests/RelationshipControllerTests/test_relationship_controller_should_continuously_send_events_when_first_event_comes_from_once_signal,LucidTests/StoreStackTests/test_should_fail_to_remove_in_remote_store_only_with_memory_store_first,LucidTests/RecoverableStoreTests/test_store_should_overwrite_a_non_empty_recovery_store_with_a_non_empty_main_store_at_init,LucidTests/RecoverableStoreTests/test_store_only_reflects_main_store_in_get_operations,LucidTests/RecoverableStoreTests/test_store_affects_both_inner_stores_in_remove_operations_async,LucidTests/CoreManagerTests/test_manager_should_send_entity_update_to_provider_when_entity_is_removed,LucidTests/RecoverableStoreTests/test_store_only_reflects_main_store_in_get_operations_asyncLucidTests/BaseStoreTests/test_store_should_set_1000_entities_in_under_1_second,LucidTests/CoreManagerContractTests/test_continuous_obvserver_should_get_filtered_results_matching_entities_that_meet_contract_requirements,LucidTests/CoreManagerContractTests/test_core_manager_get_should_filter_results_when_enity_does_not_meet_contract_requirements,LucidTests/CoreManagerContractTests/test_core_manager_get_should_return_complete_results_when_entity_meets_contract_requirements,LucidTests/CoreManagerContractTests/test_core_manager_get_should_return_empty_results_when_no_entities_meet_contract_requirements_in_remote_store_for_remote_data_source,LucidTests/CoreManagerContractTests/test_core_manager_get_should_return_local_result_when_no_entities_meet_contract_requirements_in_remote_store_for_remote_or_local_data_source,LucidTestsCoreManagerTests/test_manager_should_send_entity_update_to_provider_with_different_query_when_entity_is_not_found + steps: - name: Clone Project uses: actions/checkout@v4