Add SwiftPM Testing; Make Tests More Reliable (#3102)

* Add support for testing through SPM.

* Test on GitHub using SPM.

* Make SPM step independent.

* Make completion more deterministic.

* Remove reference to completion once run.

.github/workflows/ci.yml

@@ -57,10 +57,11 @@ jobs:
       - name: watchOS - ${{ matrix.destination }}
         run: set -o pipefail && env NSUnbufferedIO=YES xcodebuild -project "Alamofire.xcodeproj" -scheme "Alamofire watchOS" -destination "${{ matrix.destination }}" clean build | xcpretty
   spm:
-    name: Test SPM Integration
+    name: Test with SPM
     runs-on: macOS-latest    
-    needs: [macOS]
+    env: 
+      DEVELOPER_DIR: /Applications/Xcode_11.3.1.app/Contents/Developer
     steps:
       - uses: actions/checkout@v1
-      - name: SPM Build
-        run: swift build
+      - name: SPM Test
+        run: swift test -c debug

Package.swift

@@ -33,5 +33,8 @@ let package = Package(name: "Alamofire",
                       products: [.library(name: "Alamofire",
                                           targets: ["Alamofire"])],
                       targets: [.target(name: "Alamofire",
-                                        path: "Source")],
+                                        path: "Source"),
+                                .testTarget(name: "AlamofireTests",
+                                            dependencies: ["Alamofire"],
+                                            path: "Tests")],
                       swiftLanguageVersions: [.v5])

Source/Request.swift

@@ -1280,7 +1280,7 @@ public class DownloadRequest: Request {
             } else {
                 // Resume to ensure metrics are gathered.
                 task.resume()
-                task.cancel()
+                task.cancel(byProducingResumeData: { _ in })
                 self.underlyingQueue.async { self.didCancelTask(task) }
             }
         }

Source/RequestTaskMap.swift

@@ -110,27 +110,27 @@ struct RequestTaskMap {
         return taskEvents.isEmpty
     }
 
-    mutating func disassociateIfNecessaryAfterGatheringMetricsForTask(_ task: URLSessionTask) {
+    mutating func disassociateIfNecessaryAfterGatheringMetricsForTask(_ task: URLSessionTask) -> Bool {
         guard let events = taskEvents[task] else {
             fatalError("RequestTaskMap consistency error: no events corresponding to task found.")
         }
 
         switch (events.completed, events.metricsGathered) {
         case (_, true): fatalError("RequestTaskMap consistency error: duplicate metricsGatheredForTask call.")
-        case (false, false): taskEvents[task] = (completed: false, metricsGathered: true)
-        case (true, false): self[task] = nil
+        case (false, false): taskEvents[task] = (completed: false, metricsGathered: true); return false
+        case (true, false): self[task] = nil; return true
         }
     }
 
-    mutating func disassociateIfNecessaryAfterCompletingTask(_ task: URLSessionTask) {
+    mutating func disassociateIfNecessaryAfterCompletingTask(_ task: URLSessionTask) -> Bool {
         guard let events = taskEvents[task] else {
             fatalError("RequestTaskMap consistency error: no events corresponding to task found.")
         }
 
         switch (events.completed, events.metricsGathered) {
         case (true, _): fatalError("RequestTaskMap consistency error: duplicate completionReceivedForTask call.")
-        case (false, false): taskEvents[task] = (completed: true, metricsGathered: false)
-        case (false, true): self[task] = nil
+        case (false, false): taskEvents[task] = (completed: true, metricsGathered: false); return false
+        case (false, true): self[task] = nil; return true
         }
     }
 }

Source/Session.swift

@@ -66,6 +66,8 @@ open class Session {
     var requestTaskMap = RequestTaskMap()
     /// Set of currently active `Request`s.
     var activeRequests: Set<Request> = []
+    /// Completion events awaiting `URLSessionTaskMetrics`.
+    var waitingCompletions: [URLSessionTask: () -> Void] = [:]
 
     /// Creates a `Session` from a `URLSession` and other parameters.
     ///
@@ -1017,23 +1019,44 @@ extension Session: RequestDelegate {
 
 extension Session: SessionStateProvider {
     func request(for task: URLSessionTask) -> Request? {
+        dispatchPrecondition(condition: .onQueue(rootQueue))
+
         return requestTaskMap[task]
     }
 
     func didGatherMetricsForTask(_ task: URLSessionTask) {
-        requestTaskMap.disassociateIfNecessaryAfterGatheringMetricsForTask(task)
+        dispatchPrecondition(condition: .onQueue(rootQueue))
+
+        let didDisassociate = requestTaskMap.disassociateIfNecessaryAfterGatheringMetricsForTask(task)
+
+        if didDisassociate {
+            waitingCompletions[task]?()
+            waitingCompletions[task] = nil
+        }
     }
 
-    func didCompleteTask(_ task: URLSessionTask) {
-        requestTaskMap.disassociateIfNecessaryAfterCompletingTask(task)
+    func didCompleteTask(_ task: URLSessionTask, completion: @escaping () -> Void) {
+        dispatchPrecondition(condition: .onQueue(rootQueue))
+
+        let didDisassociate = requestTaskMap.disassociateIfNecessaryAfterCompletingTask(task)
+
+        if didDisassociate {
+            completion()
+        } else {
+            waitingCompletions[task] = completion
+        }
     }
 
     func credential(for task: URLSessionTask, in protectionSpace: URLProtectionSpace) -> URLCredential? {
+        dispatchPrecondition(condition: .onQueue(rootQueue))
+
         return requestTaskMap[task]?.credential ??
             session.configuration.urlCredentialStorage?.defaultCredential(for: protectionSpace)
     }
 
     func cancelRequestsForSessionInvalidation(with error: Error?) {
+        dispatchPrecondition(condition: .onQueue(rootQueue))
+
         requestTaskMap.requests.forEach { $0.finish(error: AFError.sessionInvalidated(error: error)) }
     }
 }

Source/SessionDelegate.swift

@@ -66,7 +66,7 @@ protocol SessionStateProvider: AnyObject {
 
     func request(for task: URLSessionTask) -> Request?
     func didGatherMetricsForTask(_ task: URLSessionTask)
-    func didCompleteTask(_ task: URLSessionTask)
+    func didCompleteTask(_ task: URLSessionTask, completion: @escaping () -> Void)
     func credential(for task: URLSessionTask, in protectionSpace: URLProtectionSpace) -> URLCredential?
     func cancelRequestsForSessionInvalidation(with error: Error?)
 }
@@ -212,9 +212,11 @@ extension SessionDelegate: URLSessionTaskDelegate {
     open func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
         eventMonitor?.urlSession(session, task: task, didCompleteWithError: error)
 
-        stateProvider?.request(for: task)?.didCompleteTask(task, with: error.map { $0.asAFError(or: .sessionTaskFailed(error: $0)) })
+        let request = stateProvider?.request(for: task)
 
-        stateProvider?.didCompleteTask(task)
+        stateProvider?.didCompleteTask(task) {
+            request?.didCompleteTask(task, with: error.map { $0.asAFError(or: .sessionTaskFailed(error: $0)) })
+        }
     }
 
     @available(macOS 10.13, iOS 11.0, tvOS 11.0, watchOS 4.0, *)

Tests/AFError+AlamofireTests.swift

@@ -23,6 +23,7 @@
 //
 
 import Alamofire
+import Foundation
 
 extension AFError {
     // ParameterEncodingFailureReason

Tests/MultipartFormDataTests.swift

@@ -175,6 +175,7 @@ class MultipartFormDataEncodingTestCase: BaseTestCase {
         }
     }
 
+    #if !SWIFT_PACKAGE
     func testEncodingFileBodyPart() {
         // Given
         let multipartFormData = MultipartFormData()
@@ -420,6 +421,7 @@ class MultipartFormDataEncodingTestCase: BaseTestCase {
             XCTAssertEqual(encodedData, expectedData, "data should match expected data")
         }
     }
+    #endif
 }
 
 // MARK: -
@@ -510,6 +512,7 @@ class MultipartFormDataWriteEncodedDataToDiskTestCase: BaseTestCase {
         }
     }
 
+    #if !SWIFT_PACKAGE
     func testWritingEncodedFileBodyPartToDisk() {
         // Given
         let fileURL = temporaryFileURL()
@@ -770,6 +773,7 @@ class MultipartFormDataWriteEncodedDataToDiskTestCase: BaseTestCase {
             XCTFail("file data should not be nil")
         }
     }
+    #endif
 }
 
 // MARK: -

Tests/RequestTests.swift

@@ -127,6 +127,7 @@ final class RequestResponseTestCase: BaseTestCase {
         }
     }
 
+    #if !SWIFT_PACKAGE
     func testPOSTRequestWithBase64EncodedImages() {
         // Given
         let urlString = "https://httpbin.org/post"
@@ -176,6 +177,7 @@ final class RequestResponseTestCase: BaseTestCase {
             XCTFail("form parameter in JSON should not be nil")
         }
     }
+    #endif
 
     // MARK: Queues
 

Tests/ResponseSerializationTests.swift

@@ -667,6 +667,7 @@ final class DecodableResponseSerializerTests: BaseTestCase {
 
 // MARK: -
 
+#if !SWIFT_PACKAGE
 final class DownloadResponseSerializationTestCase: BaseTestCase {
     // MARK: Properties
 
@@ -1059,6 +1060,7 @@ final class DownloadResponseSerializationTestCase: BaseTestCase {
         XCTAssertEqual(result.success as? NSNull, NSNull())
     }
 }
+#endif
 
 final class CustomResponseSerializerTests: BaseTestCase {
     func testThatCustomResponseSerializersCanBeWrittenWithoutCompilerIssues() {

Tests/ServerTrustEvaluatorTests.swift

@@ -26,6 +26,7 @@ import Alamofire
 import Foundation
 import XCTest
 
+#if !SWIFT_PACKAGE
 private struct TestCertificates {
     // Root Certificates
     static let rootCA = TestCertificates.certificate(filename: "alamofire-root-ca")
@@ -1411,3 +1412,4 @@ class ServerTrustPolicyCertificatesInBundleTestCase: ServerTrustPolicyTestCase {
         #endif
     }
 }
+#endif

Tests/TLSEvaluationTests.swift

@@ -26,6 +26,7 @@ import Alamofire
 import Foundation
 import XCTest
 
+#if !SWIFT_PACKAGE
 private struct TestCertificates {
     static let rootCA = TestCertificates.certificate(filename: "expired.badssl.com-root-ca")
     static let intermediateCA1 = TestCertificates.certificate(filename: "expired.badssl.com-intermediate-ca-1")
@@ -553,3 +554,4 @@ final class TLSEvaluationExpiredLeafCertificateTestCase: BaseTestCase {
         XCTAssertNil(error, "error should be nil")
     }
 }
+#endif

Tests/UploadTests.swift

@@ -26,7 +26,8 @@ import Alamofire
 import Foundation
 import XCTest
 
-class UploadFileInitializationTestCase: BaseTestCase {
+#if !SWIFT_PACKAGE
+final class UploadFileInitializationTestCase: BaseTestCase {
     func testUploadClassMethodWithMethodURLAndFile() {
         // Given
         let urlString = "https://httpbin.org/post"
@@ -72,6 +73,7 @@ class UploadFileInitializationTestCase: BaseTestCase {
         XCTAssertNotNil(request.response, "response should not be nil")
     }
 }
+#endif
 
 // MARK: -
 
@@ -122,7 +124,8 @@ class UploadDataInitializationTestCase: BaseTestCase {
 
 // MARK: -
 
-class UploadStreamInitializationTestCase: BaseTestCase {
+#if !SWIFT_PACKAGE
+final class UploadStreamInitializationTestCase: BaseTestCase {
     func testUploadClassMethodWithMethodURLAndStream() {
         // Given
         let urlString = "https://httpbin.org/post"
@@ -170,6 +173,7 @@ class UploadStreamInitializationTestCase: BaseTestCase {
         XCTAssertNotNil(request.response, "response should not be nil, tasks: \(request.tasks)")
     }
 }
+#endif
 
 // MARK: -