From 2ce0784368fbe81b39fe5b57bcd06aa24e331ce8 Mon Sep 17 00:00:00 2001 From: Aaron McTavish Date: Mon, 20 Feb 2017 08:02:50 +0000 Subject: [PATCH 1/2] Improve error messages and recovery suggestions Resolves #35 `Improve Error Explanation for Rate Limit`. - Adds checks for rate limiting and uses new, specific error - Refactors response validation to static methods in an extension of `ResponseHandler`, thus providing a default implementation to all response handlers - Refactors the deserialisation of JSON in `JSONResponseHandler` into a separate method to improve readability - Updates `GitHubScannerProtocolError` to inherit from `LocalizedError` and provides a default implementation for `CustomStringConvertible` - Adds `errorDescription` and `recoverySuggestion` to all error types - Moves `NetworkError` and `ScanOptionsError` into own files - Expands test suite for error handling - Refactors accessing the `HTTPURLResponse` next link header into an extension property to improve readability of `JSONResponseHandler` and to help maintain single responsibility principle --- .../Extension/HTTPURLResponse+Additions.swift | 18 +- .../GitHubKit/Networking/NetworkClient.swift | 9 +- .../GitHubKit/Networking/NetworkError.swift | 55 +++++ .../Networking/ResponseHandler.swift | 82 ++++++- .../JSONResponseHandler.swift | 45 ++-- .../Command/ScanOptions.swift | 32 --- .../Command/ScanOptionsError.swift | 85 +++++++ Sources/GitHubScannerKit/Errors.swift | 34 ++- .../Networking/ResponseHandlerTests.swift | 225 +++++++++--------- .../JSONResponseHandlerTests.swift | 133 +++++++++++ .../Command/ScanOptionsErrorTests.swift | 96 ++++++++ 11 files changed, 636 insertions(+), 178 deletions(-) create mode 100644 Sources/GitHubKit/Networking/NetworkError.swift create mode 100644 Sources/GitHubScannerKit/Command/ScanOptionsError.swift create mode 100644 Tests/GitHubKitTests/Networking/ResponseHandlers/JSONResponseHandlerTests.swift create mode 100644 Tests/GitHubScannerKitTests/Command/ScanOptionsErrorTests.swift diff --git a/Sources/GitHubKit/Extension/HTTPURLResponse+Additions.swift b/Sources/GitHubKit/Extension/HTTPURLResponse+Additions.swift index f22cc38..f30dbff 100644 --- a/Sources/GitHubKit/Extension/HTTPURLResponse+Additions.swift +++ b/Sources/GitHubKit/Extension/HTTPURLResponse+Additions.swift @@ -3,7 +3,7 @@ // GitHub Scanner // // Created by Aaron McTavish on 10/02/2017. -// Copyright © 2016 ustwo Fampany Ltd. All rights reserved. +// Copyright © 2017 ustwo Fampany Ltd. All rights reserved. // import Foundation @@ -14,6 +14,17 @@ import Foundation extension HTTPURLResponse { + // MARK: - Types + + private struct Constants { + static let nextLinkHeader = "next" + static let urlHeaderValue = "url" + } + + + // MARK: - Properties + + /// Parses the link headers from the `HTTPURLResponse` and returns a dictionary. public var links: [String: [String: String]] { var result = [String: [String: String]]() @@ -57,4 +68,9 @@ extension HTTPURLResponse { return result } + /// Returns the 'next' link header, if it exists. + public var nextLink: String? { + return links[Constants.nextLinkHeader]?[Constants.urlHeaderValue] + } + } diff --git a/Sources/GitHubKit/Networking/NetworkClient.swift b/Sources/GitHubKit/Networking/NetworkClient.swift index 00edb78..4597a68 100644 --- a/Sources/GitHubKit/Networking/NetworkClient.swift +++ b/Sources/GitHubKit/Networking/NetworkClient.swift @@ -3,7 +3,7 @@ // GitHub Scanner // // Created by Aaron McTavish on 09/02/2017. -// Copyright © 2016 ustwo Fampany Ltd. All rights reserved. +// Copyright © 2017 ustwo Fampany Ltd. All rights reserved. // @@ -12,13 +12,6 @@ import Foundation -public enum NetworkError: Error { - case failedRequest(status: Int) - case invalidJSON - case unknown(error: Error?) -} - - public final class NetworkClient { diff --git a/Sources/GitHubKit/Networking/NetworkError.swift b/Sources/GitHubKit/Networking/NetworkError.swift new file mode 100644 index 0000000..b1d6a49 --- /dev/null +++ b/Sources/GitHubKit/Networking/NetworkError.swift @@ -0,0 +1,55 @@ +// +// NetworkError.swift +// GitHub Scanner +// +// Created by Aaron McTavish on 20/02/2017. +// Copyright © 2017 ustwo Fampany Ltd. All rights reserved. +// + + +import Foundation + + +public enum NetworkError: LocalizedError { + case failedRequest(status: Int) + case invalidJSON + case rateLimited + case unauthorized + case unknown(error: Error?) +} + + +extension NetworkError { + + + public var errorDescription: String? { + switch self { + case let .failedRequest(status): + return "Failed Request. Status Code: \(status)" + case .invalidJSON: + return "Invalid JSON returned from the server" + case .rateLimited: + return "Exceed rate limit for requests" + case .unauthorized: + return "Not authorized" + case let .unknown(error): + if let localError = error as? LocalizedError, + let description = localError.errorDescription { + + return "Unknown Error: " + description + } else { + return "Unknown Error" + } + } + } + + public var recoverySuggestion: String? { + switch self { + case .rateLimited, .unauthorized: + return "Use the '--oauth' flag and supply an access token" + case .failedRequest, .invalidJSON, .unknown: + return nil + } + } + +} diff --git a/Sources/GitHubKit/Networking/ResponseHandler.swift b/Sources/GitHubKit/Networking/ResponseHandler.swift index 1eb9816..9d858ef 100644 --- a/Sources/GitHubKit/Networking/ResponseHandler.swift +++ b/Sources/GitHubKit/Networking/ResponseHandler.swift @@ -3,13 +3,22 @@ // GitHub Scanner // // Created by Aaron McTavish on 10/02/2017. -// Copyright © 2016 ustwo Fampany Ltd. All rights reserved. +// Copyright © 2017 ustwo Fampany Ltd. All rights reserved. // // swiftlint:disable large_tuple import Foundation +import Result + + +public typealias NetworkValidationResult = Result<(Data, HTTPURLResponse), NetworkError> + + +public enum ResponseHandlers { + static let `default` = JSONResponseHandler() +} public protocol ResponseHandler { @@ -24,6 +33,73 @@ public protocol ResponseHandler { } -public enum ResponseHandlers { - static let `default` = JSONResponseHandler() +extension ResponseHandler { + + + /// Checks the response's status code to ensure it is in the 200 range. + /// + /// - Parameter response: The `HTTPURLResponse` to validate. + /// - Returns: Whether or not the response is valid. + public static func isValidResponseStatus(_ response: HTTPURLResponse) -> Bool { + return 200..<300 ~= response.statusCode + } + + /// Checks the response to see if it failed due to rate limiting. + /// + /// - Parameter response: The `HTTPURLResponse` to validate. + /// - Returns: Whether or not the response has been rate limited. + public static func isRateLimitedResponse(_ response: HTTPURLResponse) -> Bool { + if [401, 403].contains(response.statusCode), + let rateLimitString = response.allHeaderFields["X-RateLimit-Remaining"] as? String, + let rateLimit = Int(rateLimitString), + rateLimit == 0 { + + return true + } + + return false + } + + /// Checks the response to see if it failed due to not having sufficient authorization on the request. + /// + /// - Parameter response: The `HTTPURLResponse` to validate. + /// - Returns: Whether or not the response failed due to being unauthorized. + /// + /// - Note: Should check `ResponseHandler.isRateLimitedResponse(_:)` before calling this method, + /// as being rate limited could also return a 401 status code. + public static func isUnauthorized(_ response: HTTPURLResponse) -> Bool { + return response.statusCode == 401 + } + + /// Validates a network response. + /// + /// - Parameters: + /// - data: `Data` from the body of the response. + /// - response: `URLResponse` to validate. + /// - error: `Error` from making the network request. + /// - Returns: If the validation is successful, returns the `data` unwrapped and the response + /// as an `HTTPURLResponse`. If the validation is unsuccessful, returns the `NetworkError` + /// representing the issue. + /// + /// - Note: Checks for a valid status code, rate limiting error, and authorization. + public static func validateResponse(data: Data?, response: URLResponse?, error: Error?) -> NetworkValidationResult { + guard let httpResponse = response as? HTTPURLResponse, + let data = data else { + + return .failure(NetworkError.unknown(error: error)) + } + + guard Self.isValidResponseStatus(httpResponse) else { + if Self.isRateLimitedResponse(httpResponse) { + return .failure(NetworkError.rateLimited) + } else if Self.isUnauthorized(httpResponse) { + return .failure(NetworkError.unauthorized) + } + + return .failure(NetworkError.failedRequest(status: httpResponse.statusCode)) + } + + return .success((data, httpResponse)) + } + } diff --git a/Sources/GitHubKit/Networking/ResponseHandlers/JSONResponseHandler.swift b/Sources/GitHubKit/Networking/ResponseHandlers/JSONResponseHandler.swift index 67f0786..ea56261 100644 --- a/Sources/GitHubKit/Networking/ResponseHandlers/JSONResponseHandler.swift +++ b/Sources/GitHubKit/Networking/ResponseHandlers/JSONResponseHandler.swift @@ -11,10 +11,14 @@ import Foundation +import Result public final class JSONResponseHandler: ResponseHandler { + + // MARK: - ResponseHandler + public func process(data: Data?, response: URLResponse?, error: Error?, @@ -22,29 +26,42 @@ public final class JSONResponseHandler: ResponseHandler { _ linkHeader: String?, _ error: NetworkError?) -> Void)?) { - guard let httpResponse = response as? HTTPURLResponse, - let data = data else { + let validationResult = JSONResponseHandler.validateResponse(data: data, + response: response, + error: error) - completion?(nil, nil, NetworkError.unknown(error: error)) + switch validationResult { + case let .success(body, httpResponse): + let deserializationResult: Result = deserializeJSON(data: body) + + switch deserializationResult { + case let .success(result): + completion?(result, httpResponse.nextLink, nil) return - } - guard 200..<300 ~= httpResponse.statusCode else { - completion?(nil, nil, NetworkError.failedRequest(status: httpResponse.statusCode)) + case let .failure(deserializationError): + completion?(nil, nil, deserializationError) + return + } + + case let .failure(validationError): + completion?(nil, nil, validationError) return } + } - if let json = try? JSONSerialization.jsonObject(with: data), - let result = Output(json: json) { + /// Deserializes the JSON into a model, if possible. + /// + /// - Parameter data: The JSON `Data` to deserialize. + /// - Returns: If success, returns the deserialized model. Otherwise, returns a `NetworkError`. + func deserializeJSON(data: Data) -> Result { + guard let json = try? JSONSerialization.jsonObject(with: data), + let result = Output(json: json) else { - let links = httpResponse.links - let link = links["next"]?["url"] - completion?(result, link, nil) - return + return .failure(NetworkError.invalidJSON) } - completion?(nil, nil, NetworkError.invalidJSON) - return + return .success(result) } } diff --git a/Sources/GitHubScannerKit/Command/ScanOptions.swift b/Sources/GitHubScannerKit/Command/ScanOptions.swift index a0fb176..2412da1 100644 --- a/Sources/GitHubScannerKit/Command/ScanOptions.swift +++ b/Sources/GitHubScannerKit/Command/ScanOptions.swift @@ -12,38 +12,6 @@ import GitHubKit import Result -public enum ScanOptionsError: GitHubScannerProtocolError { - case invalidCategory(value: String) - case invalidRepositoryType(value: String) - case missingAuthorization - case missingOwner - case unknown(error: Error) -} - - -extension ScanOptionsError: Equatable { - - public static func == (lhs: ScanOptionsError, rhs: ScanOptionsError) -> Bool { - switch (lhs, rhs) { - case (let .invalidCategory(lhsValue), let .invalidCategory(rhsValue)): - return lhsValue == rhsValue - - case (let .invalidRepositoryType(lhsValue), let .invalidRepositoryType(rhsValue)): - return lhsValue == rhsValue - - case (.missingAuthorization, .missingAuthorization), - (.missingOwner, .missingOwner): - - return true - - default: - return false - } - } - -} - - public struct ScanOptions: OptionsProtocol { diff --git a/Sources/GitHubScannerKit/Command/ScanOptionsError.swift b/Sources/GitHubScannerKit/Command/ScanOptionsError.swift new file mode 100644 index 0000000..5712f24 --- /dev/null +++ b/Sources/GitHubScannerKit/Command/ScanOptionsError.swift @@ -0,0 +1,85 @@ +// +// ScanOptionsError.swift +// GitHub Scanner +// +// Created by Aaron McTavish on 17/02/2017. +// Copyright © 2017 ustwo Fampany Ltd. All rights reserved. +// + + +import Foundation +import GitHubKit + + +public enum ScanOptionsError: GitHubScannerProtocolError { + case invalidCategory(value: String) + case invalidRepositoryType(value: String) + case missingAuthorization + case missingOwner + case unknown(error: Error) +} + + +extension ScanOptionsError { + + public var errorDescription: String? { + switch self { + case let .invalidCategory(value): + return "Invalid Category: " + value + case let .invalidRepositoryType(value): + return "Invalid Repository Type: " + value + case .missingAuthorization: + return "Missing Authorization" + case .missingOwner: + return "Missing Repository Owner" + case let .unknown(error): + if let localError = error as? LocalizedError, + let description = localError.errorDescription { + + return "Unknown Error: " + description + } else { + return "Unknown Error" + } + } + } + + public var recoverySuggestion: String? { + switch self { + case .invalidCategory: + return "Valid categories are: \(ScanCategory.allValuesList)" + case .missingAuthorization: + return "Use the '--oauth' flag and supply an access token" + case .missingOwner: + return "Supply an owner name (organization or user) as the second argument" + case .invalidRepositoryType, .unknown: + return nil + } + } + +} + + +extension ScanOptionsError: Equatable { + + public static func == (lhs: ScanOptionsError, rhs: ScanOptionsError) -> Bool { + switch (lhs, rhs) { + case (let .invalidCategory(lhsValue), let .invalidCategory(rhsValue)): + return lhsValue == rhsValue + + case (let .invalidRepositoryType(lhsValue), let .invalidRepositoryType(rhsValue)): + return lhsValue == rhsValue + + case (.missingAuthorization, .missingAuthorization), + (.missingOwner, .missingOwner): + + return true + + case (let .unknown(lhsValue), let .unknown(rhsValue)): + return lhsValue.localizedDescription == rhsValue.localizedDescription + + default: + return false + } + } + +} diff --git a/Sources/GitHubScannerKit/Errors.swift b/Sources/GitHubScannerKit/Errors.swift index d8ceef6..fbef0fc 100644 --- a/Sources/GitHubScannerKit/Errors.swift +++ b/Sources/GitHubScannerKit/Errors.swift @@ -3,18 +3,48 @@ // GitHub Scanner // // Created by Aaron McTavish on 09/02/2017. -// Copyright © 2016 ustwo Fampany Ltd. All rights reserved. +// Copyright © 2017 ustwo Fampany Ltd. All rights reserved. // + +import Foundation import GitHubKit -public protocol GitHubScannerProtocolError: Error {} +public protocol GitHubScannerProtocolError: LocalizedError, CustomStringConvertible {} + + +extension GitHubScannerProtocolError { + + public var description: String { + guard let errorDescription = errorDescription else { + return "" + } + + return errorDescription + ". " + (recoverySuggestion ?? "") + } + +} public enum GitHubScannerError: GitHubScannerProtocolError { case `internal`(GitHubScannerProtocolError) case external(Error) + + public var errorDescription: String? { + switch self { + case let .internal(error): + return error.description + case let .external(error): + if let localError = error as? LocalizedError, + let description = localError.errorDescription { + + return "External Error: " + description + } else { + return "External Error" + } + } + } } diff --git a/Tests/GitHubKitTests/Networking/ResponseHandlerTests.swift b/Tests/GitHubKitTests/Networking/ResponseHandlerTests.swift index c59b888..44a0c34 100644 --- a/Tests/GitHubKitTests/Networking/ResponseHandlerTests.swift +++ b/Tests/GitHubKitTests/Networking/ResponseHandlerTests.swift @@ -7,128 +7,93 @@ // -// swiftlint:disable force_unwrapping force_try +// swiftlint:disable force_unwrapping force_try large_tuple import Foundation @testable import GitHubKit import XCTest -final class JSONResponseHandlerTests: XCTestCase { +typealias NetworkErrorAssertion = (NetworkError) -> Void - // MARK: - Types - - typealias RepositoryCompletionHandler = (ArrayFoo?, String?, NetworkError?) -> Void - +struct MockResponseHandler: ResponseHandler { - // MARK: - Properties - - let handler = JSONResponseHandler() - let defaultResponse = HTTPURLResponse(url: URL(string: "http://foo.com")!, - statusCode: 200, - httpVersion: "HTTP/1.1", - headerFields: [String: String]()) + func process(data: Data?, + response: URLResponse?, + error: Error?, + completion: ((_ result: Output?, + _ linkHeader: String?, + _ error: NetworkError?) -> Void)?) { + completion?(nil, nil, nil) + } - // MARK: - Test Success +} - func testProcessSuccess() { - // Given - let expectedID = 1234 - let expectedURL = URL(string: "http://foo.com")! - let expectedName = "foo" - let repositoryJSON: [[String: Any]] = [["id": expectedID, - "html_url": expectedURL.absoluteString, - "name": expectedName]] - let repositoryData = try! JSONSerialization.data(withJSONObject: repositoryJSON) +final class ResponseHandlerTests: XCTestCase { - // When - let completionHandler: RepositoryCompletionHandler = { repository, link, error in - guard error == nil else { - XCTFail("Expected no error but found error: \(error)") - return - } - guard let repositories = repository, - let responseRepository = repositories.first else { - - XCTFail("Expected success repository deserialization, but found nil.") - return - } + // MARK: - Types - XCTAssertEqual(expectedID, - responseRepository.identifier, - "Expected id: \(expectedID) but found \(responseRepository.identifier)") + typealias RepositoryCompletionHandler = (ArrayFoo?, String?, NetworkError?) -> Void - XCTAssertEqual(expectedURL, - responseRepository.htmlURL, - "Expected html_url: \(expectedURL.absoluteString) " + - "but found \(responseRepository.htmlURL.absoluteString)") - XCTAssertEqual(expectedName, - responseRepository.name, - "Expected name: \(expectedName) but found \(responseRepository.name)") - } + // MARK: - Properties - // Then - assertHandlerProcess(data: repositoryData, - response: defaultResponse, - error: nil, - completion: completionHandler) - } + let handler = MockResponseHandler() + let defaultResponse = HTTPURLResponse(url: URL(string: "http://foo.com")!, + statusCode: 200, + httpVersion: "HTTP/1.1", + headerFields: [String: String]()) - // MARK: - Test Failure + // MARK: - Test Validation func testNoDataFailure() { - // When - let completionHandler: RepositoryCompletionHandler = { repository, link, error in - guard let responseError = error, - case .unknown = responseError else { - - XCTFail("Expected unknown but found error: \(error)") - return + // Then + let assertion: NetworkErrorAssertion = { error in + guard case .unknown = error else { + XCTFail("Expected unknown but found error: \(error)") + return } } - // Then - assertHandlerProcess(data: nil, - response: defaultResponse, - error: nil, - completion: completionHandler) + assertValidationFailure(data: nil, + response: defaultResponse, + error: nil, + errorAssertion: assertion) } func testNoResponseFailure() { - // When - let completionHandler: RepositoryCompletionHandler = { repository, link, error in - guard let responseError = error, - case .unknown = responseError else { - - XCTFail("Expected unknown but found error: \(error)") - return + // Then + let assertion: NetworkErrorAssertion = { error in + guard case .unknown = error else { + XCTFail("Expected unknown but found error: \(error)") + return } } - // Then - assertHandlerProcess(data: Data(), - response: nil, - error: nil, - completion: completionHandler) + assertValidationFailure(data: Data(), + response: nil, + error: nil, + errorAssertion: assertion) } func testFailedStatusCode() { // Given let expectedStatusCode = 404 + let response = HTTPURLResponse(url: URL(string: "http://foo.com")!, + statusCode: expectedStatusCode, + httpVersion: "HTTP/1.1", + headerFields: [String: String]()) - // When - let completionHandler: RepositoryCompletionHandler = { repository, link, error in - guard let responseError = error, - case .failedRequest(let statusCode) = responseError else { - - XCTFail("Expected failedRequest but found error: \(error)") - return + // Then + let assertion: NetworkErrorAssertion = { error in + guard case .failedRequest(let statusCode) = error else { + XCTFail("Expected failedRequest but found error: \(error)") + return } XCTAssertEqual(statusCode, @@ -136,59 +101,83 @@ final class JSONResponseHandlerTests: XCTestCase { "Expected statusCode: \(expectedStatusCode) but found \(statusCode)") } - // Then - assertHandlerProcess(data: Data(), - response: HTTPURLResponse(url: URL(string: "http://foo.com")!, - statusCode: expectedStatusCode, - httpVersion: "HTTP/1.1", - headerFields: [String: String]()), - error: nil, - completion: completionHandler) + assertValidationFailure(data: Data(), + response: response, + error: nil, + errorAssertion: assertion) } - func testInvalidJSONFailure() { + func testRateLimitedFailure() { // Given - let badRepositoryJSON: [[String: Any]] = [["id": 1234]] - let badRepositoryData = try! JSONSerialization.data(withJSONObject: badRepositoryJSON) - - // When - let completionHandler: RepositoryCompletionHandler = { repository, link, error in - guard let responseError = error, - case .invalidJSON = responseError else { + let response = HTTPURLResponse(url: URL(string: "http://foo.com")!, + statusCode: 401, + httpVersion: "HTTP/1.1", + headerFields: ["X-RateLimit-Remaining": "0"]) - XCTFail("Expected invalidJSON but found error: \(error)") - return + // Then + let assertion: NetworkErrorAssertion = { error in + guard case .rateLimited = error else { + XCTFail("Expected rateLimited but found error: \(error)") + return } } + assertValidationFailure(data: Data(), + response: response, + error: nil, + errorAssertion: assertion) + } + + func testUnauthorizedFailure() { + // Given + let response = HTTPURLResponse(url: URL(string: "http://foo.com")!, + statusCode: 401, + httpVersion: "HTTP/1.1", + headerFields: [String: String]()) + // Then - assertHandlerProcess(data: badRepositoryData, - response: defaultResponse, - error: nil, - completion: completionHandler) + let assertion: NetworkErrorAssertion = { error in + guard case .unauthorized = error else { + XCTFail("Expected unauthorized but found error: \(error)") + return + } + } + + assertValidationFailure(data: Data(), + response: response, + error: nil, + errorAssertion: assertion) } // MARK: - Convenience - private func assertHandlerProcess(data: Data?, - response: URLResponse?, - error: Error?, - completion: @escaping RepositoryCompletionHandler) { + private func assertValidationFailure(data: Data?, + response: URLResponse?, + error: Error?, + errorAssertion: @escaping NetworkErrorAssertion) { - let completionExpectation = expectation(description:"completionExpectation") - let expectationHandler: RepositoryCompletionHandler = { repository, link, error in + let assertionExpectation = expectation(description:"assertionExpectation") + let expectationHandler: NetworkErrorAssertion = { error in defer { - completionExpectation.fulfill() + assertionExpectation.fulfill() } - completion(repository, link, error) + errorAssertion(error) } - handler.process(data: data, - response: response, - error: error, - completion: expectationHandler) + // When + let validationResult = MockResponseHandler.validateResponse(data: data, + response: response, + error: error) + + // Then + switch validationResult { + case .success: + XCTFail("Expected validation to fail, but succeeded.") + case let .failure(validationError): + expectationHandler(validationError) + } waitForExpectations(timeout: 2.0) XCTAssertTrue(true) diff --git a/Tests/GitHubKitTests/Networking/ResponseHandlers/JSONResponseHandlerTests.swift b/Tests/GitHubKitTests/Networking/ResponseHandlers/JSONResponseHandlerTests.swift new file mode 100644 index 0000000..d7b92dc --- /dev/null +++ b/Tests/GitHubKitTests/Networking/ResponseHandlers/JSONResponseHandlerTests.swift @@ -0,0 +1,133 @@ +// +// JSONResponseHandlerTests.swift +// GitHub Scanner +// +// Created by Aaron McTavish on 19/02/2017. +// Copyright © 2017 ustwo Fampany Ltd. All rights reserved. +// + + +// swiftlint:disable force_unwrapping force_try + +import Foundation +@testable import GitHubKit +import XCTest + + +final class JSONResponseHandlerTests: XCTestCase { + + + // MARK: - Types + + typealias RepositoryCompletionHandler = (ArrayFoo?, String?, NetworkError?) -> Void + + + // MARK: - Properties + + let handler = JSONResponseHandler() + let defaultResponse = HTTPURLResponse(url: URL(string: "http://foo.com")!, + statusCode: 200, + httpVersion: "HTTP/1.1", + headerFields: [String: String]()) + + + // MARK: - Test Success + + func testProcessSuccess() { + // Given + let expectedID = 1234 + let expectedURL = URL(string: "http://foo.com")! + let expectedName = "foo" + + let repositoryJSON: [[String: Any]] = [["id": expectedID, + "html_url": expectedURL.absoluteString, + "name": expectedName]] + let repositoryData = try! JSONSerialization.data(withJSONObject: repositoryJSON) + + // When + let completionHandler: RepositoryCompletionHandler = { repository, link, error in + guard error == nil else { + XCTFail("Expected no error but found error: \(error)") + return + } + + guard let repositories = repository, + let responseRepository = repositories.first else { + + XCTFail("Expected success repository deserialization, but found nil.") + return + } + + XCTAssertEqual(expectedID, + responseRepository.identifier, + "Expected id: \(expectedID) but found \(responseRepository.identifier)") + + XCTAssertEqual(expectedURL, + responseRepository.htmlURL, + "Expected html_url: \(expectedURL.absoluteString) " + + "but found \(responseRepository.htmlURL.absoluteString)") + + XCTAssertEqual(expectedName, + responseRepository.name, + "Expected name: \(expectedName) but found \(responseRepository.name)") + } + + // Then + assertHandlerProcess(data: repositoryData, + response: defaultResponse, + error: nil, + completion: completionHandler) + } + + + // MARK: - Test Failure + + func testInvalidJSONFailure() { + // Given + let badRepositoryJSON: [[String: Any]] = [["id": 1234]] + let badRepositoryData = try! JSONSerialization.data(withJSONObject: badRepositoryJSON) + + // When + let completionHandler: RepositoryCompletionHandler = { repository, link, error in + guard let responseError = error, + case .invalidJSON = responseError else { + + XCTFail("Expected invalidJSON but found error: \(error)") + return + } + } + + // Then + assertHandlerProcess(data: badRepositoryData, + response: defaultResponse, + error: nil, + completion: completionHandler) + } + + + // MARK: - Convenience + + private func assertHandlerProcess(data: Data?, + response: URLResponse?, + error: Error?, + completion: @escaping RepositoryCompletionHandler) { + + let completionExpectation = expectation(description:"completionExpectation") + let expectationHandler: RepositoryCompletionHandler = { repository, link, error in + defer { + completionExpectation.fulfill() + } + + completion(repository, link, error) + } + + handler.process(data: data, + response: response, + error: error, + completion: expectationHandler) + + waitForExpectations(timeout: 2.0) + XCTAssertTrue(true) + } + +} diff --git a/Tests/GitHubScannerKitTests/Command/ScanOptionsErrorTests.swift b/Tests/GitHubScannerKitTests/Command/ScanOptionsErrorTests.swift new file mode 100644 index 0000000..ff9d918 --- /dev/null +++ b/Tests/GitHubScannerKitTests/Command/ScanOptionsErrorTests.swift @@ -0,0 +1,96 @@ +// +// ScanOptionsErrorTests.swift +// GitHub Scanner +// +// Created by Aaron McTavish on 19/02/2017. +// Copyright © 2017 ustwo Fampany Ltd. All rights reserved. +// + + +import Foundation +@testable import GitHubScannerKit +import XCTest + + +final class ScanOptionsErrorTests: XCTestCase { + + + // MARK: - Test Equatable + + func testInvalidCategory_Equal() { + // Given + let error = ScanOptionsError.invalidCategory(value: "Foo") + + // Then + XCTAssertEqual(error, error) + } + + func testInvalidRepositoryType_Equal() { + // Given + let error = ScanOptionsError.invalidRepositoryType(value: "Foo") + + // Then + XCTAssertEqual(error, error) + } + + func testMissingAuthorization_Equal() { + // Given + let error = ScanOptionsError.missingAuthorization + + // Then + XCTAssertEqual(error, error) + } + + func testMissingOwner_Equal() { + // Given + let error = ScanOptionsError.missingOwner + + // Then + XCTAssertEqual(error, error) + } + + func testUnknown_Equal() { + // Given + let error = ScanOptionsError.unknown(error: ScanOptionsError.missingAuthorization) + + // Then + XCTAssertEqual(error, error) + } + + func testInvalidCategory_NotEqual() { + // Given + let error = ScanOptionsError.invalidCategory(value: "Foo") + let error2 = ScanOptionsError.invalidCategory(value: "Bar") + + // Then + XCTAssertNotEqual(error, error2) + } + + func testInvalidRepositoryType_NotEqual() { + // Given + let error = ScanOptionsError.invalidRepositoryType(value: "Foo") + let error2 = ScanOptionsError.invalidRepositoryType(value: "Bar") + + // Then + XCTAssertNotEqual(error, error2) + } + + func testUnknown_NotEqual() { + // Given + let error = ScanOptionsError.unknown(error: ScanOptionsError.missingAuthorization) + let error2 = ScanOptionsError.unknown(error: ScanOptionsError.missingOwner) + + // Then + XCTAssertNotEqual(error, error2) + } + + func testDifferentErrors_NotEqual() { + // Given + let error = ScanOptionsError.missingAuthorization + let error2 = ScanOptionsError.missingOwner + + // Then + XCTAssertNotEqual(error, error2) + } + +} From 2847e891579f9ed8b687a7f26a56f7c526f954d1 Mon Sep 17 00:00:00 2001 From: Aaron McTavish Date: Mon, 20 Feb 2017 09:40:24 +0000 Subject: [PATCH 2/2] Fix indendentation of `if` statement Per code review by @arnau --- Sources/GitHubKit/Networking/NetworkError.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/GitHubKit/Networking/NetworkError.swift b/Sources/GitHubKit/Networking/NetworkError.swift index b1d6a49..675a9c2 100644 --- a/Sources/GitHubKit/Networking/NetworkError.swift +++ b/Sources/GitHubKit/Networking/NetworkError.swift @@ -33,7 +33,7 @@ extension NetworkError { case .unauthorized: return "Not authorized" case let .unknown(error): - if let localError = error as? LocalizedError, + if let localError = error as? LocalizedError, let description = localError.errorDescription { return "Unknown Error: " + description