Skip to content

Commit

Permalink
Testable NetworkRequest (#404)
Browse files Browse the repository at this point in the history
* Implement TestableNetworkRequest using @testable import for access to NetworkRequest

Override Equatable and Hashable implementations for use in testing comparisons

* Refactor NetworkRequestHelper data properties to use new TestableNetworkRequest as keys

Refactor usages to use TestableNetworkRequest methods
Rename setResponseFor to addResponseFor to more accurately reflect what the method does

* Refactor Mock and RealNetworkService based on changes to NetworkServiceHelper

* Refactor UpstreamIntegrationTests based on changes to RealNetworkService

* Add convenience init to TestableNetworkRequest and update usages

* Update implementation to use lowercased string comparison and hash for alignment

* Reorder operations to avoid race condition in MockNetworkService

By doing the countdown AFTER notifying the completion handler, awaits on the expected network request can properly gate the rest of the test case logic (for example, if the test case resets the mock network service, there isn't a race condition between the reset and the get mock response, due to prematurely ungated await)

* Remove unused NetworkService isCustomEquals method

* Update docs to reflect actual method name

* Align the logical implementations of isEqual and hash

* Refactor networkResponses data struct to be 1:1 request to response

Refactor related logic for getting and setting, and usage sites

* Add method and class docs for TestableNetworkRequest

* Remove defaultMockResponse property, and place it inline with only place it is used

* Update method doc for getResponseFor
  • Loading branch information
timkimadobe authored Sep 21, 2023
1 parent c3ce8a6 commit 285aeb9
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 67 deletions.
6 changes: 6 additions & 0 deletions AEPEdge.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
2EDD05CA286EAA7000229CB2 /* Preview Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = D4D5B5852432977F00CAB6E4 /* Preview Assets.xcassets */; };
2EDD05CB286EAA7000229CB2 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = D4D5B5822432977F00CAB6E4 /* Assets.xcassets */; };
423B5033D43ADB49049383FB /* Pods_FunctionalTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 0B13510405D14B13EFD1B58A /* Pods_FunctionalTests.framework */; };
4CA0B8E12ABB936B007E5D45 /* TestableNetworkRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CA0B8E02ABB936B007E5D45 /* TestableNetworkRequest.swift */; };
4CA0B8E22ABB936B007E5D45 /* TestableNetworkRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CA0B8E02ABB936B007E5D45 /* TestableNetworkRequest.swift */; };
4CBEE5CA29D637170084BC50 /* TestUtils.swift in Sources */ = {isa = PBXBuildFile; fileRef = BF947FA1244587520057A6CC /* TestUtils.swift */; };
4CBEE5CB29D637170084BC50 /* EventHub+Test.swift in Sources */ = {isa = PBXBuildFile; fileRef = D49A628E250AE4F500B7C4A3 /* EventHub+Test.swift */; };
4CBEE5CC29D637170084BC50 /* UserDefaults+Test.swift in Sources */ = {isa = PBXBuildFile; fileRef = D49A6293250AEEF000B7C4A3 /* UserDefaults+Test.swift */; };
Expand Down Expand Up @@ -257,6 +259,7 @@
240E3C9824EF357100D44993 /* MockDataStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MockDataStore.swift; sourceTree = "<group>"; };
2EDD05D2286EAA7000229CB2 /* TestApptvOS.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = TestApptvOS.app; sourceTree = BUILT_PRODUCTS_DIR; };
316EF062B0B622235F6BB384 /* Pods_TestAppiOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_TestAppiOS.framework; sourceTree = BUILT_PRODUCTS_DIR; };
4CA0B8E02ABB936B007E5D45 /* TestableNetworkRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestableNetworkRequest.swift; sourceTree = "<group>"; };
4CBEE5E029D637170084BC50 /* UpstreamIntegrationTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = UpstreamIntegrationTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
4CBEE5E229D637E90084BC50 /* UpstreamIntegrationTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UpstreamIntegrationTests.swift; sourceTree = "<group>"; };
4CBEE5EA29D63B180084BC50 /* EnumUtils.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EnumUtils.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -557,6 +560,7 @@
children = (
4CBEE5FF2A02E2A90084BC50 /* XCTestCase+AnyCodableAsserts.swift */,
4CBEE5F929FB19600084BC50 /* AnyCodable+Array.swift */,
4CA0B8E02ABB936B007E5D45 /* TestableNetworkRequest.swift */,
D44BBB74248DC54F00775DAC /* CountDownLatch.swift */,
21888AF22555F9FC005677ED /* FileManager+Testable.swift */,
D4655401246CD0B000A150E2 /* InstrumentedExtension.swift */,
Expand Down Expand Up @@ -1307,6 +1311,7 @@
4CBEE5FD29FB47270084BC50 /* TestConstants.swift in Sources */,
4CBEE5FB29FB47170084BC50 /* TestBase.swift in Sources */,
4CBEE5EC29D648F90084BC50 /* FileManager+Testable.swift in Sources */,
4CA0B8E22ABB936B007E5D45 /* TestableNetworkRequest.swift in Sources */,
4CBEE5EB29D63B180084BC50 /* EnumUtils.swift in Sources */,
4CBEE6002A02E2A90084BC50 /* XCTestCase+AnyCodableAsserts.swift in Sources */,
4CBEE6022A02E2B70084BC50 /* AnyCodableAssertsTests.swift in Sources */,
Expand Down Expand Up @@ -1411,6 +1416,7 @@
files = (
4CC5831A2A9559BB004B1169 /* AnyCodable+Array.swift in Sources */,
4CC583192A955886004B1169 /* XCTestCase+AnyCodableAsserts.swift in Sources */,
4CA0B8E12ABB936B007E5D45 /* TestableNetworkRequest.swift in Sources */,
D45F2E7825EDE0D6000AC350 /* MockHitProcessor.swift in Sources */,
D44BBB75248DC54F00775DAC /* CountDownLatch.swift in Sources */,
D4A473CB2485A8F300D31710 /* TestUtils.swift in Sources */,
Expand Down
33 changes: 18 additions & 15 deletions Tests/TestUtils/MockNetworkService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,28 +20,31 @@ class MockNetworkService: Networking {
private var responseDelay: UInt32 = 0

func connectAsync(networkRequest: NetworkRequest, completionHandler: ((HttpConnection) -> Void)? = nil) {
helper.recordSentNetworkRequest(networkRequest)
self.helper.countDownExpected(networkRequest: networkRequest)
guard let unwrappedCompletionHandler = completionHandler else { return }

if self.responseDelay > 0 {
sleep(self.responseDelay)
}

if let response = self.getMockResponsesFor(networkRequest: networkRequest).first {
unwrappedCompletionHandler(response)
if let response = self.getMockResponseFor(networkRequest: networkRequest) {
completionHandler?(response)
} else {
// Default mock response
unwrappedCompletionHandler(
completionHandler?(
HttpConnection(
data: "".data(using: .utf8),
response: HTTPURLResponse(url: networkRequest.url,
statusCode: 200,
httpVersion: nil,
headerFields: nil),
error: nil)
response: HTTPURLResponse(
url: networkRequest.url,
statusCode: 200,
httpVersion: nil,
headerFields: nil
),
error: nil
)
)
}
// Do these countdown after notifying completion handler to avoid prematurely ungating awaits
// before required network logic finishes
helper.recordSentNetworkRequest(networkRequest)
helper.countDownExpected(networkRequest: networkRequest)
}

func reset() {
Expand All @@ -67,7 +70,7 @@ class MockNetworkService: Networking {
guard let networkRequest = NetworkRequest(urlString: url, httpMethod: httpMethod) else {
return
}
helper.setResponseFor(networkRequest: networkRequest, responseConnection: responseConnection)
setMockResponseFor(networkRequest: networkRequest, responseConnection: responseConnection)
}

// MARK: - Passthrough for shared helper APIs
Expand Down Expand Up @@ -95,7 +98,7 @@ class MockNetworkService: Networking {

// MARK: - Private helpers
// MARK: Network request response helpers
private func getMockResponsesFor(networkRequest: NetworkRequest) -> [HttpConnection] {
return helper.getResponsesFor(networkRequest: networkRequest)
private func getMockResponseFor(networkRequest: NetworkRequest) -> HttpConnection? {
return helper.getResponseFor(networkRequest: networkRequest)
}
}
55 changes: 24 additions & 31 deletions Tests/TestUtils/NetworkRequestHelper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,20 @@ import XCTest
/// - ``MockNetworkService``
/// - ``RealNetworkService``
class NetworkRequestHelper {
private var sentNetworkRequests: [NetworkRequest: [NetworkRequest]] = [:]
private var sentNetworkRequests: [TestableNetworkRequest: [NetworkRequest]] = [:]
/// Matches sent `NetworkRequest`s with their corresponding `HttpConnection` response.
private(set) var networkResponses: [NetworkRequest: HttpConnection] = [:]
private var expectedNetworkRequests: [NetworkRequest: CountDownLatch] = [:]
private(set) var networkResponses: [TestableNetworkRequest: HttpConnection] = [:]
private var expectedNetworkRequests: [TestableNetworkRequest: CountDownLatch] = [:]

func recordSentNetworkRequest(_ networkRequest: NetworkRequest) {
TestBase.log("Received connectAsync to URL \(networkRequest.url.absoluteString) and HTTPMethod \(networkRequest.httpMethod.toString())")
let testableNetworkRequest = TestableNetworkRequest(from: networkRequest)
if let equalNetworkRequest = sentNetworkRequests.first(where: { key, _ in
networkRequest.isCustomEqual(key)
key == testableNetworkRequest
}) {
sentNetworkRequests[equalNetworkRequest.key]!.append(networkRequest)
sentNetworkRequests[equalNetworkRequest.key]?.append(networkRequest)
} else {
sentNetworkRequests[networkRequest] = [networkRequest]
sentNetworkRequests[testableNetworkRequest] = [networkRequest]
}
}

Expand All @@ -45,7 +46,7 @@ class NetworkRequestHelper {

func countDownExpected(networkRequest: NetworkRequest) {
for expectedNetworkRequest in expectedNetworkRequests {
if networkRequest.isCustomEqual(expectedNetworkRequest.key) {
if expectedNetworkRequest.key == TestableNetworkRequest(from: networkRequest) {
expectedNetworkRequest.value.countDown()
}
}
Expand All @@ -59,7 +60,7 @@ class NetworkRequestHelper {
/// 2. Order of the deadline timer application is important
private func awaitFor(networkRequest: NetworkRequest, timeout: TimeInterval) -> DispatchTimeoutResult? {
for expectedNetworkRequest in expectedNetworkRequests {
if networkRequest.isCustomEqual(expectedNetworkRequest.key) {
if expectedNetworkRequest.key == TestableNetworkRequest(from: networkRequest) {
return expectedNetworkRequest.value.await(timeout: timeout)
}
}
Expand All @@ -70,7 +71,7 @@ class NetworkRequestHelper {
/// Returns all of the original outgoing `NetworkRequest`s satisfying `NetworkRequest.isCustomEqual(_:)`.
func getSentNetworkRequestsMatching(networkRequest: NetworkRequest) -> [NetworkRequest] {
for request in sentNetworkRequests {
if networkRequest.isCustomEqual(request.key) {
if request.key == TestableNetworkRequest(from: networkRequest) {
return request.value
}
}
Expand All @@ -81,16 +82,16 @@ class NetworkRequestHelper {
// MARK: - Network response helpers
/// Sets the `HttpConnection` response connection for a given `NetworkRequest`
func setResponseFor(networkRequest: NetworkRequest, responseConnection: HttpConnection?) {
networkResponses[networkRequest] = responseConnection
let testableNetworkRequest = TestableNetworkRequest(from: networkRequest)
networkResponses[testableNetworkRequest] = responseConnection
}

/// Gets all network responses for `NetworkRequest`s matching the given `NetworkRequest`
/// Returns the network response associated with the given network request.
///
/// See:
func getResponsesFor(networkRequest: NetworkRequest) -> [HttpConnection] {
return networkResponses
.filter { networkRequest.isCustomEqual($0.key) }
.map { $0.value }
/// - Parameter networkRequest: The `NetworkRequest` for which the response should be retrieved.
/// - Returns: The `HttpConnection` response associated with the provided `NetworkRequest`, or `nil` if no response was found.
func getResponseFor(networkRequest: NetworkRequest) -> HttpConnection? {
return networkResponses[TestableNetworkRequest(from: networkRequest)]
}

// MARK: Assertion helpers
Expand All @@ -106,15 +107,15 @@ class NetworkRequestHelper {
return
}

expectedNetworkRequests[networkRequest] = CountDownLatch(expectedCount)
expectedNetworkRequests[TestableNetworkRequest(from: networkRequest)] = CountDownLatch(expectedCount)
}

/// For all previously set expections, asserts that the correct number of network requests were sent.
/// - See also:
/// - `setExpectationNetworkRequest(url:httpMethod:)`
/// - SeeAlso:
/// - ``setExpectationForNetworkRequest(url:httpMethod:)``
func assertAllNetworkRequestExpectations(file: StaticString = #file, line: UInt = #line) {
guard !expectedNetworkRequests.isEmpty else {
assertionFailure("There are no network request expectations set, use this API after calling setExpectationNetworkRequest")
assertionFailure("There are no network request expectations set, use this API after calling setExpectationForNetworkRequest")
return
}

Expand All @@ -128,14 +129,14 @@ class NetworkRequestHelper {
}

/// Returns the `NetworkRequest`(s) sent through the Core NetworkService, or empty if none was found.
/// Use this API after calling `setExpectationNetworkRequest(url:httpMethod:count:)` to wait for the right amount of time
/// Use this API after calling `setExpectationForNetworkRequest(networkRequest:expectedCount:file:line:)` to wait for expectations.
/// - Parameters:
/// - url: The URL for which to retrieved the network requests sent, should be a valid URL
/// - httpMethod: the `HttpMethod` for which to retrieve the network requests, along with the `url`
/// - timeout: how long should this method wait for the expected network requests, in seconds; by default it waits up to 1 second
/// - Returns: list of network requests with the provided `url` and `httpMethod`, or empty if none was dispatched
/// - See also:
/// - setExpectationNetworkRequest(url:httpMethod:)
/// - SeeAlso:
/// - ``setExpectationForNetworkRequest(networkRequest:expectedCount:file:line:)``
func getNetworkRequestsWith(url: String, httpMethod: HttpMethod, timeout: TimeInterval = TestConstants.Defaults.WAIT_NETWORK_REQUEST_TIMEOUT, file: StaticString = #file, line: UInt = #line) -> [NetworkRequest] {
guard let networkRequest = NetworkRequest(urlString: url, httpMethod: httpMethod) else {
return []
Expand Down Expand Up @@ -180,14 +181,6 @@ extension NetworkRequest {
self.init(url: url, httpMethod: httpMethod)
}

/// Custom equals compare based on host, scheme and URL path. Query params are not taken into consideration.
func isCustomEqual(_ other: NetworkRequest) -> Bool { // Maybe isCustomEqual?
return self.url.host?.lowercased() == other.url.host?.lowercased()
&& self.url.scheme?.lowercased() == other.url.scheme?.lowercased()
&& self.url.path.lowercased() == other.url.path.lowercased()
&& self.httpMethod.rawValue == other.httpMethod.rawValue
}

/// Converts the `connectPayload` into a flattened dictionary containing its data.
/// This API fails the assertion if the request body cannot be parsed as JSON.
/// - Returns: The JSON request body represented as a flattened dictionary
Expand Down
4 changes: 2 additions & 2 deletions Tests/TestUtils/RealNetworkService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ class RealNetworkService: NetworkService {
})
}

func getResponsesFor(networkRequest: NetworkRequest, timeout: TimeInterval = TestConstants.Defaults.WAIT_NETWORK_REQUEST_TIMEOUT, file: StaticString = #file, line: UInt = #line) -> [HttpConnection] {
func getResponseFor(networkRequest: NetworkRequest, timeout: TimeInterval = TestConstants.Defaults.WAIT_NETWORK_REQUEST_TIMEOUT, file: StaticString = #file, line: UInt = #line) -> HttpConnection? {
helper.awaitRequest(networkRequest, timeout: timeout, file: file, line: line)
return helper.getResponsesFor(networkRequest: networkRequest)
return helper.getResponseFor(networkRequest: networkRequest)
}

// MARK: - Passthrough for shared helper APIs
Expand Down
73 changes: 73 additions & 0 deletions Tests/TestUtils/TestableNetworkRequest.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
//
// Copyright 2023 Adobe. All rights reserved.
// This file is licensed to you under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License. You may obtain a copy
// of the License at http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software distributed under
// the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
// OF ANY KIND, either express or implied. See the License for the specific language
// governing permissions and limitations under the License.
//


import Foundation
@testable import AEPServices

/// `TestableNetworkRequest` is a specialized subclass of `NetworkRequest` for use in testing scenarios.
/// It provides custom, overriding logic for the `Equatable` and `Hashable` protocols, and is meant for direct use as keys
/// in collections that rely on the previously mentioned protocols for uniqueness (dictionaries, sets, etc.).
class TestableNetworkRequest: NetworkRequest {
/// Construct from existing `NetworkRequest` instance
convenience init(from networkRequest: NetworkRequest) {
self.init(url: networkRequest.url,
httpMethod: networkRequest.httpMethod,
connectPayloadData: networkRequest.connectPayload,
httpHeaders: networkRequest.httpHeaders,
connectTimeout: networkRequest.connectTimeout,
readTimeout: networkRequest.readTimeout)
}

// Note that the Equatable and Hashable conformance logic needs to align exactly for it to work as expected
// in the case of dictionary keys. Lowercased is used because across current test cases it has the same
// properties as case insensitive compare, and is straightforward to implement for isEqual and hash. However,
// if there are new cases where lowercased does not satisfy the property of case insensitive compare, this logic
// will need to be updated accordingly to handle that case.

// MARK: - Equatable (ObjC) conformance
/// Determines equality by comparing the URL's scheme, host, path, and HTTP method, while excluding query parameters
/// (and any other NetworkRequest properties).
///
/// Note that host and scheme use `String.lowercased()` to perform case insensitive comparison.
///
/// - Parameter object: The object to be compared with the current instance.
/// - Returns: A boolean value indicating whether the given object is equal to the current instance.
override func isEqual(_ object: Any?) -> Bool {
guard let other = object as? NetworkRequest else {
return false
}

return url.host?.lowercased() == other.url.host?.lowercased()
&& url.scheme?.lowercased() == other.url.scheme?.lowercased()
&& url.path == other.url.path
&& httpMethod.rawValue == other.httpMethod.rawValue
}

// MARK: - Hashable (ObjC) conformance
/// Determines the hash value by combining the URL's scheme, host, path, and HTTP method, while excluding query parameters
/// (and any other NetworkRequest properties).
///
/// Note that host and scheme use `String.lowercased()` to perform case insensitive combination.
public override var hash: Int {
var hasher = Hasher()
if let scheme = url.scheme {
hasher.combine(scheme.lowercased())
}
if let host = url.host {
hasher.combine(host.lowercased())
}
hasher.combine(url.path)
hasher.combine(httpMethod.rawValue)
return hasher.finalize()
}
}
Loading

0 comments on commit 285aeb9

Please # to comment.