Skip to content

Commit

Permalink
Fix flaky tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@kean
kean committed May 4, 2024
1 parent 09f3620 commit 42671e5
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 170 deletions.
4 changes: 0 additions & 4 deletions Nuke.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
0C1C201E29ABBF19004B38FD /* Nuke.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 0C9174901BAE99EE004A7905 /* Nuke.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
0C1E620B1D6F817700AD5CF5 /* ImageRequestTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C1E620A1D6F817700AD5CF5 /* ImageRequestTests.swift */; };
0C1ECA421D526461009063A9 /* ImageCacheTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C7C06871BCA888800089D7F /* ImageCacheTests.swift */; };
0C211E502856328500F48AA6 /* DataLoaderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C211E4F2856328500F48AA6 /* DataLoaderTests.swift */; };
0C222DE3294E2DEA00012288 /* NukeUI.docc in Sources */ = {isa = PBXBuildFile; fileRef = 0C222DE2294E2DEA00012288 /* NukeUI.docc */; };
0C222DE5294E2E0300012288 /* NukeExtensions.docc in Sources */ = {isa = PBXBuildFile; fileRef = 0C222DE4294E2E0200012288 /* NukeExtensions.docc */; };
0C2A368B26437BF100F1D000 /* TaskLoadData.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0C2A368A26437BF100F1D000 /* TaskLoadData.swift */; };
Expand Down Expand Up @@ -362,7 +361,6 @@
0C1E59682372EBBC00674B63 /* validate.sh */ = {isa = PBXFileReference; lastKnownFileType = text.script.sh; name = validate.sh; path = Scripts/validate.sh; sourceTree = "<group>"; };
0C1E596A2372EBBC00674B63 /* test.sh */ = {isa = PBXFileReference; lastKnownFileType = text.script.sh; name = test.sh; path = Scripts/test.sh; sourceTree = "<group>"; };
0C1E620A1D6F817700AD5CF5 /* ImageRequestTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImageRequestTests.swift; sourceTree = "<group>"; };
0C211E4F2856328500F48AA6 /* DataLoaderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataLoaderTests.swift; sourceTree = "<group>"; };
0C222DE2294E2DEA00012288 /* NukeUI.docc */ = {isa = PBXFileReference; lastKnownFileType = folder.documentationcatalog; path = NukeUI.docc; sourceTree = "<group>"; };
0C222DE4294E2E0200012288 /* NukeExtensions.docc */ = {isa = PBXFileReference; lastKnownFileType = folder.documentationcatalog; path = NukeExtensions.docc; sourceTree = "<group>"; };
0C2A368A26437BF100F1D000 /* TaskLoadData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TaskLoadData.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -714,7 +712,6 @@
0C64F73A24383043001983C6 /* ImageEncoderTests.swift */,
0CD195291D4348AC00E011BB /* ImagePrefetcherTests.swift */,
0CB26806208F25C2004C83F4 /* DataCacheTests.swift */,
0C211E4F2856328500F48AA6 /* DataLoaderTests.swift */,
0CE5F681215638300046609F /* ResumableDataTests.swift */,
0C4AF1E91FE8551D002F86CB /* LinkedListTest.swift */,
0CB2EFD52110F52C00F7C63F /* RateLimiterTests.swift */,
Expand Down Expand Up @@ -1659,7 +1656,6 @@
0CE3992D1D4697CE00A87D47 /* ImagePipelineTests.swift in Sources */,
0C64F73B24383043001983C6 /* ImageEncoderTests.swift in Sources */,
0CF58FF726DAAC3800D2650D /* ImageDownsampleTests.swift in Sources */,
0C211E502856328500F48AA6 /* DataLoaderTests.swift in Sources */,
0C967EB328688B3F0050E083 /* DocumentationTests.swift in Sources */,
0C91B0EC2438E287007F9100 /* ResizeTests.swift in Sources */,
0CA3BA63285C11EA0079A444 /* ImagePipelineTaskDelegateTests.swift in Sources */,
Expand Down
2 changes: 2 additions & 0 deletions Sources/Nuke/Pipeline/ImagePipeline.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public final class ImagePipeline: @unchecked Sendable {

let rateLimiter: RateLimiter?
let id = UUID()
var onTaskStarted: ((ImageTask) -> Void)? // Debug purposes

deinit {
lock.deinitialize(count: 1)
Expand Down Expand Up @@ -309,6 +310,7 @@ public final class ImagePipeline: @unchecked Sendable {
task?.process(.init($0))
}
delegate.imageTaskDidStart(task, pipeline: self)
onTaskStarted?(task)
}

// MARK: - Image Task Events
Expand Down
77 changes: 0 additions & 77 deletions Tests/NukeTests/DataLoaderTests.swift
View file

This file was deleted.

144 changes: 66 additions & 78 deletions Tests/NukeTests/ImagePipelineTests/ImagePipelineCoalescingTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,25 +160,25 @@ class ImagePipelineCoalescingTests: XCTestCase {
// MARK: - Thumbnail

func testDeduplicationGivenSameURLButDifferentThumbnailOptions() {
dataLoader.queue.isSuspended = true

// GIVEN requests with the same URLs but one accesses thumbnail
let request1 = ImageRequest(url: Test.url, userInfo: [.thumbnailKey: ImageRequest.ThumbnailOptions(maxPixelSize: 400)])
let request2 = ImageRequest(url: Test.url)

// WHEN loading images for those requests
expect(pipeline).toLoadImage(with: request1) { result in
// THEN
guard let image = result.value?.image else { return XCTFail() }
XCTAssertEqual(image.sizeInPixels, CGSize(width: 400, height: 300))
}
expect(pipeline).toLoadImage(with: request2) { result in
// THEN
guard let image = result.value?.image else { return XCTFail() }
XCTAssertEqual(image.sizeInPixels, CGSize(width: 640.0, height: 480.0))
}
suspendDataLoading(for: pipeline, expectedRequestCount: 2) {

dataLoader.queue.isSuspended = false
// WHEN loading images for those requests
expect(pipeline).toLoadImage(with: request1) { result in
// THEN
guard let image = result.value?.image else { return XCTFail() }
XCTAssertEqual(image.sizeInPixels, CGSize(width: 400, height: 300))
}
expect(pipeline).toLoadImage(with: request2) { result in
// THEN
guard let image = result.value?.image else { return XCTFail() }
XCTAssertEqual(image.sizeInPixels, CGSize(width: 640.0, height: 480.0))
}

}

wait { _ in
// THEN the image data is fetched once
Expand All @@ -187,27 +187,25 @@ class ImagePipelineCoalescingTests: XCTestCase {
}

func testDeduplicationGivenSameURLButDifferentThumbnailOptionsReversed() {
dataLoader.queue.isSuspended = true

// GIVEN requests with the same URLs but one accesses thumbnail
// (in this test, order is reversed)
let request1 = ImageRequest(url: Test.url)
let request2 = ImageRequest(url: Test.url, userInfo: [.thumbnailKey: ImageRequest.ThumbnailOptions(maxPixelSize: 400)])

// WHEN loading images for those requests
expect(pipeline).toLoadImage(with: request1) { result in
// THEN
guard let image = result.value?.image else { return XCTFail() }
XCTAssertEqual(image.sizeInPixels, CGSize(width: 640.0, height: 480.0))
}
expect(pipeline).toLoadImage(with: request2) { result in
// THEN
guard let image = result.value?.image else { return XCTFail() }
XCTAssertEqual(image.sizeInPixels, CGSize(width: 400, height: 300))
suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
// WHEN loading images for those requests
expect(pipeline).toLoadImage(with: request1) { result in
// THEN
guard let image = result.value?.image else { return XCTFail() }
XCTAssertEqual(image.sizeInPixels, CGSize(width: 640.0, height: 480.0))
}
expect(pipeline).toLoadImage(with: request2) { result in
// THEN
guard let image = result.value?.image else { return XCTFail() }
XCTAssertEqual(image.sizeInPixels, CGSize(width: 400, height: 300))
}
}

dataLoader.queue.isSuspended = false

wait { _ in
// THEN the image data is fetched once
XCTAssertEqual(self.dataLoader.createdTaskCount, 1)
Expand All @@ -217,20 +215,18 @@ class ImagePipelineCoalescingTests: XCTestCase {
// MARK: - Processing

func testProcessorsAreDeduplicated() {
dataLoader.queue.isSuspended = true

// Given
// Make sure we don't start processing when some requests haven't
// started yet.
let processors = MockProcessorFactory()
let queueObserver = OperationQueueObserver(queue: pipeline.configuration.imageProcessingQueue)

// When
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url, processors: [processors.make(id: "1")]))
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url, processors: [processors.make(id: "2")]))
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url, processors: [processors.make(id: "1")]))

dataLoader.queue.isSuspended = false
suspendDataLoading(for: pipeline, expectedRequestCount: 3) {
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url, processors: [processors.make(id: "1")]))
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url, processors: [processors.make(id: "2")]))
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url, processors: [processors.make(id: "1")]))
}

// When/Then
wait { _ in
Expand Down Expand Up @@ -488,18 +484,11 @@ class ImagePipelineCoalescingTests: XCTestCase {
$0.isTaskCoalescingEnabled = false
}

dataLoader.queue.isSuspended = true

// When/Then
let request1 = Test.request
let request2 = Test.request

expect(pipeline).toLoadImage(with: request1)
expect(pipeline).toLoadImage(with: request2)

pipeline.queue.sync {}
dataLoader.queue.isSuspended = false

suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
expect(pipeline).toLoadImage(with: Test.request)
expect(pipeline).toLoadImage(with: Test.request)
}
wait { _ in
XCTAssertEqual(self.dataLoader.createdTaskCount, 2)
}
Expand Down Expand Up @@ -528,18 +517,18 @@ class ImagePipelineProcessingDeduplicationTests: XCTestCase {
let request2 = ImageRequest(url: Test.url, processors: [processors.make(id: "1"), processors.make(id: "2")])

// When
dataLoader.queue.isSuspended = true
expect(pipeline).toLoadImage(with: request1) { result in
let image = result.value?.image
XCTAssertEqual(image?.nk_test_processorIDs ?? [], ["1"])
}
expect(pipeline).toLoadImage(with: request2) { result in
let image = result.value?.image
XCTAssertEqual(image?.nk_test_processorIDs ?? [], ["1", "2"])
suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
expect(pipeline).toLoadImage(with: request1) { result in
let image = result.value?.image
XCTAssertEqual(image?.nk_test_processorIDs ?? [], ["1"])
}
expect(pipeline).toLoadImage(with: request2) { result in
let image = result.value?.image
XCTAssertEqual(image?.nk_test_processorIDs ?? [], ["1", "2"])
}
}

// Then the processor "1" is only applied once
dataLoader.queue.isSuspended = false
wait { _ in
XCTAssertEqual(processors.numberOfProcessorsApplied, 2)
}
Expand All @@ -557,12 +546,12 @@ class ImagePipelineProcessingDeduplicationTests: XCTestCase {
let request2 = ImageRequest(url: Test.url, processors: [processors.make(id: "1"), processors.make(id: "2"), processors.make(id: "3")])

// When
dataLoader.queue.isSuspended = true
expect(pipeline).toLoadImage(with: request1)
expect(pipeline).toLoadImage(with: request2)
suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
expect(pipeline).toLoadImage(with: request1)
expect(pipeline).toLoadImage(with: request2)
}

// Then
dataLoader.queue.isSuspended = false
wait { _ in
XCTAssertNotNil(cache[request1])
XCTAssertNotNil(cache[request2])
Expand Down Expand Up @@ -634,18 +623,18 @@ class ImagePipelineProcessingDeduplicationTests: XCTestCase {
let request2 = ImageRequest(url: Test.url, processors: [processors.make(id: "1"), processors.make(id: "2")])

// When
dataLoader.queue.isSuspended = true
expect(pipeline).toLoadImage(with: request1) { result in
let image = result.value?.image
XCTAssertEqual(image?.nk_test_processorIDs ?? [], ["1"])
}
expect(pipeline).toLoadImage(with: request2) { result in
let image = result.value?.image
XCTAssertEqual(image?.nk_test_processorIDs ?? [], ["1", "2"])
suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
expect(pipeline).toLoadImage(with: request1) { result in
let image = result.value?.image
XCTAssertEqual(image?.nk_test_processorIDs ?? [], ["1"])
}
expect(pipeline).toLoadImage(with: request2) { result in
let image = result.value?.image
XCTAssertEqual(image?.nk_test_processorIDs ?? [], ["1", "2"])
}
}

// Then the processor "1" is applied twice
dataLoader.queue.isSuspended = false
wait { _ in
XCTAssertEqual(processors.numberOfProcessorsApplied, 3)
}
Expand All @@ -658,16 +647,15 @@ class ImagePipelineProcessingDeduplicationTests: XCTestCase {
pipeline = pipeline.reconfigured {
$0.dataCache = dataCache
}
dataLoader.queue.isSuspended = true

// When
func makeRequest(options: ImageRequest.Options) -> ImageRequest {
ImageRequest(urlRequest: URLRequest(url: Test.url), options: options)
}
expect(pipeline).toLoadImage(with: makeRequest(options: []))
expect(pipeline).toLoadImage(with: makeRequest(options: [.reloadIgnoringCachedData]))
pipeline.queue.sync {}
dataLoader.queue.isSuspended = false
suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
expect(pipeline).toLoadImage(with: makeRequest(options: []))
expect(pipeline).toLoadImage(with: makeRequest(options: [.reloadIgnoringCachedData]))
}

// Then
wait { _ in
Expand All @@ -681,17 +669,17 @@ class ImagePipelineProcessingDeduplicationTests: XCTestCase {
pipeline = pipeline.reconfigured {
$0.dataCache = dataCache
}
dataLoader.queue.isSuspended = true

// When
// - One request reloading cache data, another one not
func makeRequest(options: ImageRequest.Options) -> ImageRequest {
ImageRequest(urlRequest: URLRequest(url: Test.url), options: options)
}
expect(pipeline).toLoadImage(with: makeRequest(options: []))
expect(pipeline).toLoadImage(with: makeRequest(options: [.reloadIgnoringCachedData]))
pipeline.queue.sync {}
dataLoader.queue.isSuspended = false

suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
expect(pipeline).toLoadImage(with: makeRequest(options: []))
expect(pipeline).toLoadImage(with: makeRequest(options: [.reloadIgnoringCachedData]))
}

// Then
wait { _ in
Expand Down
6 changes: 3 additions & 3 deletions Tests/NukeTests/ImagePipelineTests/ImagePipelineDataCacheTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ class ImagePipelineDataCachePolicyTests: XCTestCase {
}

// WHEN
pipeline.registerMultipleRequests {
suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url, processors: [MockImageProcessor(id: "p1")]))
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url))
}
Expand Down Expand Up @@ -479,7 +479,7 @@ class ImagePipelineDataCachePolicyTests: XCTestCase {
}

// WHEN
pipeline.registerMultipleRequests {
suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url, processors: [MockImageProcessor(id: "p1")]))
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url))
}
Expand Down Expand Up @@ -544,7 +544,7 @@ class ImagePipelineDataCachePolicyTests: XCTestCase {
}

// WHEN
pipeline.registerMultipleRequests {
suspendDataLoading(for: pipeline, expectedRequestCount: 2) {
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url, processors: [MockImageProcessor(id: "p1")]))
expect(pipeline).toLoadImage(with: ImageRequest(url: Test.url))
}
Expand Down
Loading

0 comments on commit 42671e5

Please sign in to comment.