Merge pull request #2458 from onevcat/fix/actorbox-task-races

Harden tests against Task scheduling races

Tests/KingfisherTests/ImageCacheTests.swift

@@ -582,97 +582,80 @@ class ImageCacheTests: XCTestCase {
         waitForExpectations(timeout: 3, handler: nil)
     }
 
-#if os(iOS) || os(tvOS) || os(watchOS) || os(visionOS)
-    func testModifierShouldOnlyApplyForFinalResultWhenMemoryLoad() {
-        let exp = expectation(description: #function)
-        let key = testKeys[0]
-        
-        let modifierCalled = ActorBox(false)
-        let modifier = AnyImageModifier { image in
-            Task {
-                await modifierCalled.setValue(true)
-            }
-            return image.withRenderingMode(.alwaysTemplate)
-        }
-        
-        cache.store(testImage, original: testImageData, forKey: key) { _ in
-            self.cache.retrieveImage(forKey: key, options: [.imageModifier(modifier)]) { result in
-                XCTAssertEqual(result.value?.image?.renderingMode, .automatic)
-                Task {
-                    let called = await modifierCalled.value
-                    XCTAssertFalse(called)
-                    exp.fulfill()
-                    
-                }
-            }
-        }
-        waitForExpectations(timeout: 3, handler: nil)
-    }
-    
-    func testModifierShouldOnlyApplyForFinalResultWhenMemoryLoadAsync() async throws {
-        let key = testKeys[0]
+	#if os(iOS) || os(tvOS) || os(watchOS) || os(visionOS)
+	    func testModifierShouldOnlyApplyForFinalResultWhenMemoryLoad() {
+	        let exp = expectation(description: #function)
+	        let key = testKeys[0]
+	        
+	        let modifierCalled = LockIsolated(false)
+	        let modifier = AnyImageModifier { image in
+	            modifierCalled.setValue(true)
+	            return image.withRenderingMode(.alwaysTemplate)
+	        }
+	        
+	        cache.store(testImage, original: testImageData, forKey: key) { _ in
+	            self.cache.retrieveImage(forKey: key, options: [.imageModifier(modifier)]) { result in
+	                XCTAssertEqual(result.value?.image?.renderingMode, .automatic)
+	                XCTAssertFalse(modifierCalled.value)
+	                exp.fulfill()
+	            }
+	        }
+	        waitForExpectations(timeout: 3, handler: nil)
+	    }
+	    
+	    func testModifierShouldOnlyApplyForFinalResultWhenMemoryLoadAsync() async throws {
+	        let key = testKeys[0]
 
-        let modifierCalled = ActorBox(false)
-        let modifier = AnyImageModifier { image in
-            Task {
-                await modifierCalled.setValue(true)
-            }
-            return image.withRenderingMode(.alwaysTemplate)
-        }
+	        let modifierCalled = LockIsolated(false)
+	        let modifier = AnyImageModifier { image in
+	            modifierCalled.setValue(true)
+	            return image.withRenderingMode(.alwaysTemplate)
+	        }
 
-        try await cache.store(testImage, original: testImageData, forKey: key)
-        let result = try await cache.retrieveImage(forKey: key, options: [.imageModifier(modifier)])
-        let called = await modifierCalled.value
-        XCTAssertFalse(called)
-        XCTAssertEqual(result.image?.renderingMode, .automatic)
-    }
+	        try await cache.store(testImage, original: testImageData, forKey: key)
+	        let result = try await cache.retrieveImage(forKey: key, options: [.imageModifier(modifier)])
+	        XCTAssertFalse(modifierCalled.value)
+	        XCTAssertEqual(result.image?.renderingMode, .automatic)
+	    }
 
-    func testModifierShouldOnlyApplyForFinalResultWhenDiskLoad() {
-        let exp = expectation(description: #function)
-        let key = testKeys[0]
+	    func testModifierShouldOnlyApplyForFinalResultWhenDiskLoad() {
+	        let exp = expectation(description: #function)
+	        let key = testKeys[0]
 
-        let modifierCalled = ActorBox(false)
-        let modifier = AnyImageModifier { image in
-            Task {
-                await modifierCalled.setValue(true)
-            }
-            return image.withRenderingMode(.alwaysTemplate)
-        }
+	        let modifierCalled = LockIsolated(false)
+	        let modifier = AnyImageModifier { image in
+	            modifierCalled.setValue(true)
+	            return image.withRenderingMode(.alwaysTemplate)
+	        }
 
-        cache.store(testImage, original: testImageData, forKey: key) { _ in
-            self.cache.clearMemoryCache()
-            self.cache.retrieveImage(forKey: key, options: [.imageModifier(modifier)]) { result in
-                XCTAssertEqual(result.value?.image?.renderingMode, .automatic)
-                Task {
-                    let called = await modifierCalled.value
-                    XCTAssertFalse(called)
-                    exp.fulfill()
-                }
-            }
-        }
-        waitForExpectations(timeout: 3, handler: nil)
-    }
-    
-    func testModifierShouldOnlyApplyForFinalResultWhenDiskLoadAsync() async throws {
-        let key = testKeys[0]
-        let modifierCalled = ActorBox(false)
-        let modifier = AnyImageModifier { image in
-            Task {
-                await modifierCalled.setValue(true)
-            }
-            return image.withRenderingMode(.alwaysTemplate)
-        }
-        
-        try await cache.store(testImage, original: testImageData, forKey: key)
-        cache.clearMemoryCache()
-        let result = try await cache.retrieveImage(forKey: key, options: [.imageModifier(modifier)])
-        let called = await modifierCalled.value
-        XCTAssertFalse(called)
-        // The renderingMode is expected to be the default value `.automatic`. The image modifier should only apply to
-        // the image manager result.
-        XCTAssertEqual(result.image?.renderingMode, .automatic)
-    }
-#endif
+	        cache.store(testImage, original: testImageData, forKey: key) { _ in
+	            self.cache.clearMemoryCache()
+	            self.cache.retrieveImage(forKey: key, options: [.imageModifier(modifier)]) { result in
+	                XCTAssertEqual(result.value?.image?.renderingMode, .automatic)
+	                XCTAssertFalse(modifierCalled.value)
+	                exp.fulfill()
+	            }
+	        }
+	        waitForExpectations(timeout: 3, handler: nil)
+	    }
+	    
+	    func testModifierShouldOnlyApplyForFinalResultWhenDiskLoadAsync() async throws {
+	        let key = testKeys[0]
+	        let modifierCalled = LockIsolated(false)
+	        let modifier = AnyImageModifier { image in
+	            modifierCalled.setValue(true)
+	            return image.withRenderingMode(.alwaysTemplate)
+	        }
+	        
+	        try await cache.store(testImage, original: testImageData, forKey: key)
+	        cache.clearMemoryCache()
+	        let result = try await cache.retrieveImage(forKey: key, options: [.imageModifier(modifier)])
+	        XCTAssertFalse(modifierCalled.value)
+	        // The renderingMode is expected to be the default value `.automatic`. The image modifier should only apply to
+	        // the image manager result.
+	        XCTAssertEqual(result.image?.renderingMode, .automatic)
+	    }
+	#endif
     
     func testStoreToMemoryWithExpiration() {
         let exp = expectation(description: #function)

Tests/KingfisherTests/ImageDownloaderTests.swift

@@ -539,33 +539,28 @@ class ImageDownloaderTests: XCTestCase {
         waitForExpectations(timeout: 3, handler: nil)
     }
     
-#if os(iOS) || os(tvOS) || os(watchOS) || os(visionOS)
-    func testModifierShouldOnlyApplyForFinalResultWhenDownload() {
-        let exp = expectation(description: #function)
-
-        let url = testURLs[0]
-        stub(url, data: testImageData)
-
-        let modifierCalled = ActorBox(false)
-        let modifier = AnyImageModifier { image in
-            Task {
-                await modifierCalled.setValue(true)
-            }
-            return image.withRenderingMode(.alwaysTemplate)
-        }
-
-        downloader.downloadImage(with: url, options: [.imageModifier(modifier)]) { result in
-            XCTAssertEqual(result.value?.image.renderingMode, .automatic)
-            Task {
-                let called = await modifierCalled.value
-                XCTAssertFalse(called)
-                exp.fulfill()
-            }
-        }
-
-        waitForExpectations(timeout: 3, handler: nil)
-    }
-#endif
+	#if os(iOS) || os(tvOS) || os(watchOS) || os(visionOS)
+	    func testModifierShouldOnlyApplyForFinalResultWhenDownload() {
+	        let exp = expectation(description: #function)
+
+	        let url = testURLs[0]
+	        stub(url, data: testImageData)
+
+	        let modifierCalled = LockIsolated(false)
+	        let modifier = AnyImageModifier { image in
+	            modifierCalled.setValue(true)
+	            return image.withRenderingMode(.alwaysTemplate)
+	        }
+
+	        downloader.downloadImage(with: url, options: [.imageModifier(modifier)]) { result in
+	            XCTAssertEqual(result.value?.image.renderingMode, .automatic)
+	            XCTAssertFalse(modifierCalled.value)
+	            exp.fulfill()
+	        }
+
+	        waitForExpectations(timeout: 3, handler: nil)
+	    }
+	#endif
     
     func testDownloadTaskTakePriorityOption() {
         let exp = expectation(description: #function)

Tests/KingfisherTests/KingfisherManagerTests.swift

@@ -555,25 +555,20 @@ class KingfisherManagerTests: XCTestCase {
     }
     
     func testFailingProcessOnDataProviderImage() {
+        let exp = expectation(description: #function)
         let provider = SimpleImageDataProvider(cacheKey: "key") { .success(testImageData) }
-        let called = ActorBox(false)
         let p = FailingProcessor()
         let options = [KingfisherOptionsInfoItem.processor(p), .processingQueue(.mainCurrentOrAsync)]
         _ = manager.retrieveImage(with: .provider(provider), options: options) { result in
-            Task {
-                await called.setValue(true)
-            }
             XCTAssertNotNil(result.error)
             if case .processorError(reason: .processingFailed(let processor, _)) = result.error! {
                 XCTAssertEqual(processor.identifier, p.identifier)
             } else {
                 XCTFail()
             }
+            exp.fulfill()
         }
-        Task {
-            let result = await called.value
-            XCTAssertTrue(result)
-        }
+        waitForExpectations(timeout: 3, handler: nil)
     }
     
     func testCacheOriginalImageWithOriginalCache() {
@@ -841,20 +836,15 @@ class KingfisherManagerTests: XCTestCase {
         let url = testURLs[0]
         stub(url, data: testImageData)
         
-        let modifierCalled = ActorBox(false)
+        let modifierCalled = LockIsolated(false)
         let modifier = AnyImageModifier { image in
-            Task {
-                await modifierCalled.setValue(true)
-            }
+            modifierCalled.setValue(true)
             return image.withRenderingMode(.alwaysTemplate)
         }
         manager.retrieveImage(with: url, options: [.imageModifier(modifier)]) { result in
             XCTAssertEqual(result.value?.image.renderingMode, .alwaysTemplate)
-            Task {
-                let called = await modifierCalled.value
-                XCTAssertTrue(called)
-                exp.fulfill()
-            }
+            XCTAssertTrue(modifierCalled.value)
+            exp.fulfill()
         }
         waitForExpectations(timeout: 3, handler: nil)
     }
@@ -864,11 +854,9 @@ class KingfisherManagerTests: XCTestCase {
         let url = testURLs[0]
         stub(url, data: testImageData)
         
-        let modifierCalled = ActorBox(false)
+        let modifierCalled = LockIsolated(false)
         let modifier = AnyImageModifier { image in
-            Task {
-                await modifierCalled.setValue(true)
-            }
+            modifierCalled.setValue(true)
             return image.withRenderingMode(.alwaysTemplate)
         }
 
@@ -876,11 +864,8 @@ class KingfisherManagerTests: XCTestCase {
         manager.retrieveImage(with: url, options: [.imageModifier(modifier)]) { result in
             XCTAssertEqual(result.value?.cacheType, .memory)
             XCTAssertEqual(result.value?.image.renderingMode, .alwaysTemplate)
-            Task {
-                let called = await modifierCalled.value
-                XCTAssertTrue(called)
-                exp.fulfill()
-            }
+            XCTAssertTrue(modifierCalled.value)
+            exp.fulfill()
         }
         waitForExpectations(timeout: 3, handler: nil)
     }
@@ -890,11 +875,9 @@ class KingfisherManagerTests: XCTestCase {
         let url = testURLs[0]
         stub(url, data: testImageData)
 
-        let modifierCalled = ActorBox(false)
+        let modifierCalled = LockIsolated(false)
         let modifier = AnyImageModifier { image in
-            Task {
-                await modifierCalled.setValue(true)
-            }
+            modifierCalled.setValue(true)
             return image.withRenderingMode(.alwaysTemplate)
         }
 
@@ -903,10 +886,7 @@ class KingfisherManagerTests: XCTestCase {
             self.manager.retrieveImage(with: url, options: [.imageModifier(modifier)]) { result in
                 XCTAssertEqual(result.value!.cacheType, .disk)
                 XCTAssertEqual(result.value!.image.renderingMode, .alwaysTemplate)
-                Task {
-                    let result = await modifierCalled.value
-                    XCTAssertTrue(result)
-                }
+                XCTAssertTrue(modifierCalled.value)
                 exp.fulfill()
             }
         }
@@ -918,11 +898,9 @@ class KingfisherManagerTests: XCTestCase {
         let url = testURLs[0]
         stub(url, data: testImageData)
 
-        let modifierCalled = ActorBox(false)
+        let modifierCalled = LockIsolated(false)
         let modifier = AnyImageModifier { image in
-            Task {
-                await modifierCalled.setValue(true)
-            }
+            modifierCalled.setValue(true)
             return image.withRenderingMode(.alwaysTemplate)
         }
         manager.retrieveImage(with: url, options: [.imageModifier(modifier)]) { result in
@@ -935,12 +913,8 @@ class KingfisherManagerTests: XCTestCase {
             self.manager.cache.retrieveImageInDiskCache(forKey: url.absoluteString) { result in
                 XCTAssertNotNil(result.value!)
                 XCTAssertEqual(result.value??.renderingMode, .automatic)
-
-                Task {
-                    let result = await modifierCalled.value
-                    XCTAssertTrue(result)
-                    exp.fulfill()
-                }
+                XCTAssertTrue(modifierCalled.value)
+                exp.fulfill()
             }
         }
         
@@ -1078,24 +1052,18 @@ class KingfisherManagerTests: XCTestCase {
         let brokenURL = URL(string: "brokenurl")!
         stub(brokenURL, data: Data())
 
-        let downloadTaskUpdatedCount = ActorBox(0)
+        let downloadTaskUpdatedCount = LockIsolated(0)
         let task = manager.retrieveImage(
           with: .network(brokenURL),
           options: [.alternativeSources([.network(url)])],
           downloadTaskUpdated: { newTask in
-              Task {
-                  let value = await downloadTaskUpdatedCount.value + 1
-                  await downloadTaskUpdatedCount.setValue(value)
-              }
+              downloadTaskUpdatedCount.withValue { $0 += 1 }
               XCTAssertEqual(newTask?.sessionTask?.task.currentRequest?.url, url)
           })
         {
             result in
-            Task {
-                let result = await downloadTaskUpdatedCount.value
-                XCTAssertEqual(result, 1)
-                exp.fulfill()
-            }
+            XCTAssertEqual(downloadTaskUpdatedCount.value, 1)
+            exp.fulfill()
         }
 
         XCTAssertEqual(task?.sessionTask?.task.currentRequest?.url, brokenURL)
@@ -1131,7 +1099,7 @@ class KingfisherManagerTests: XCTestCase {
         let url = testURLs[0]
         let dataStub = delayedStub(url, data: testImageData)
         
-        let called = ActorBox(false)
+        let called = LockIsolated(false)
 
         let brokenURL = URL(string: "brokenurl")!
         stub(brokenURL, data: Data())
@@ -1142,9 +1110,7 @@ class KingfisherManagerTests: XCTestCase {
             downloadTaskUpdated: { newTask in
                 XCTAssertNotNil(newTask)
                 newTask?.cancel()
-                Task {
-                    await called.setValue(true)
-                }
+                called.setValue(true)
             }
         )
         {
@@ -1154,11 +1120,8 @@ class KingfisherManagerTests: XCTestCase {
 
             delay(0.3) {
                 _ = dataStub.go()
-                Task {
-                    let result = await called.value
-                    XCTAssertTrue(result)
-                    exp.fulfill()
-                }
+                XCTAssertTrue(called.value)
+                exp.fulfill()
             }
         }