Skip to content

Commit

Permalink
CacheableNetworkOperation: fixed race condition in new test (#1932)
Browse files Browse the repository at this point in the history
The test introduced by #1926 passed locally, but sometimes failed in CI
due to a race condition.

What we're testing is that the second request reuses the callback of the
first one, **if called while the first one is still in progress**.
However, nothing in the test ensures that's the case.
This introduces an optional delay in `MockHTTPClient` so we can ensure
that.

Another reason that test was failing randomly is because we had another
test modifying global state, which interferes with every other test. We
run tests in parallel, so it was making the new test that requires
`debug` logs to fail randomly, if this test happened to run before.

_Note: because of the nature of these 2 tests, they don't actually wait
for the whole response to finish, so they still pass in ~0.006s_
  • Loading branch information
NachoSoto authored Sep 23, 2022
1 parent 06b8009 commit 68d0a00
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 21 deletions.
29 changes: 21 additions & 8 deletions Tests/UnitTests/Mocks/MockHTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,27 @@ class MockHTTPClient: HTTPClient {
struct Response {

let response: HTTPResponse<Data>.Result
let delay: DispatchTimeInterval

private init(response: HTTPResponse<Data>.Result) {
private init(response: HTTPResponse<Data>.Result, delay: DispatchTimeInterval) {
self.response = response
self.delay = delay
}

init(statusCode: HTTPStatusCode, response: [String: Any] = [:]) {
init(
statusCode: HTTPStatusCode,
response: [String: Any] = [:],
delay: DispatchTimeInterval = .never
) {
// swiftlint:disable:next force_try
let data = try! JSONSerialization.data(withJSONObject: response)

self.init(response: .success(.init(statusCode: statusCode, body: data)))
self.init(response: .success(.init(statusCode: statusCode, body: data)),
delay: delay)
}

init(error: NetworkError) {
self.init(response: .failure(error))
init(error: NetworkError, delay: DispatchTimeInterval = .never) {
self.init(response: .failure(error), delay: delay)
}

}
Expand Down Expand Up @@ -71,9 +78,15 @@ class MockHTTPClient: HTTPClient {
let mock = self.mocks[request.path] ?? .init(statusCode: .success)

if let completionHandler = completionHandler {
completionHandler(
mock.response.parseResponse()
)
let response: HTTPResponse<Value>.Result = mock.response.parseResponse()

if mock.delay != .never {
DispatchQueue.main.asyncAfter(deadline: .now() + mock.delay) {
completionHandler(response)
}
} else {
completionHandler(response)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ class BackendGetOfferingsTests: BaseBackendTests {
func testGetOfferingsCachesForSameUserID() {
self.httpClient.mock(
requestPath: .getOfferings(appUserID: Self.userID),
response: .init(statusCode: .success, response: Self.noOfferingsResponse as [String: Any])
response: .init(statusCode: .success,
response: Self.noOfferingsResponse as [String: Any],
delay: .seconds(2))
)
self.offerings.getOfferings(appUserID: Self.userID, withRandomDelay: false) { _ in }
self.offerings.getOfferings(appUserID: Self.userID, withRandomDelay: false) { _ in }
Expand All @@ -73,7 +75,9 @@ class BackendGetOfferingsTests: BaseBackendTests {

self.httpClient.mock(
requestPath: .getOfferings(appUserID: Self.userID),
response: .init(statusCode: .success, response: Self.noOfferingsResponse as [String: Any])
response: .init(statusCode: .success,
response: Self.noOfferingsResponse as [String: Any],
delay: .seconds(2))
)
self.offerings.getOfferings(appUserID: Self.userID, withRandomDelay: false) { _ in }
self.offerings.getOfferings(appUserID: Self.userID, withRandomDelay: false) { _ in }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,17 +209,6 @@ class PurchasesConfiguringTests: BasePurchasesTests {
expect(SystemInfo.serverHostURL) == defaultHostURL
}

@available(*, deprecated) // Ignore deprecation warnings
func testSetDebugLogsEnabledSetsTheCorrectValue() {
Logger.logLevel = .warn

Purchases.debugLogsEnabled = true
expect(Logger.logLevel) == .debug

Purchases.debugLogsEnabled = false
expect(Logger.logLevel) == .info
}

func testIsAnonymous() {
setupAnonPurchases()
expect(self.purchases.isAnonymous).to(beTrue())
Expand Down
3 changes: 3 additions & 0 deletions fastlane/Fastfile
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ platform :ios do
number_of_retries: 5,
result_bundle: true,
testplan: "CI-AllTests",
configuration: 'Debug',
output_directory: "fastlane/test_output/xctest/ios"
)
end
Expand All @@ -144,6 +145,7 @@ platform :ios do
number_of_retries: 5,
result_bundle: true,
testplan: "CI-AllTests",
configuration: 'Debug',
output_directory: "fastlane/test_output/xctest/tvos"
)
end
Expand Down Expand Up @@ -342,6 +344,7 @@ platform :ios do
number_of_retries: 3,
result_bundle: true,
testplan: "CI-BackendIntegration",
configuration: 'Debug',
output_directory: "fastlane/test_output/xctest/ios"
)
end
Expand Down

0 comments on commit 68d0a00

Please sign in to comment.