Skip to content

[Infra] Reduce test flakes due to async variable assignment #13610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 9, 2024
Merged

[Infra] Reduce test flakes due to async variable assignment #13610

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b30e87
[Infra] Reduce test flakes due to async variable assignment
ncooke3 Sep 9, 2024
c8b8d9f
Update FirebaseAuth/Tests/Unit/AuthBackendRPCImplentationTests.swift
ncooke3 Sep 9, 2024
e05bece
Update FirebaseAuth/Tests/Unit/Fakes/FakeBackendRPCIssuer.swift
ncooke3 Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions FirebaseAuth/Tests/Unit/AuthBackendRPCImplentationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,12 +590,10 @@ class AuthBackendRPCImplementationTests: RPCBaseTests {
try self.rpcIssuer.respond(withJSON: [:])
}
_ = try? await rpcImplementation.call(with: request)
// Make sure completeRequest updates.
usleep(10000)
Copy link
Member Author

@ncooke3 ncooke3 Sep 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this leads to flakes on slower CI machines where this is not a long enough wait.


// Then
let expectedHeader = HeartbeatLoggingTestUtils.nonEmptyHeartbeatsPayload.headerValue()
let completeRequest = try XCTUnwrap(rpcIssuer.completeRequest)
let completeRequest = await rpcIssuer.completeRequest.value
let headerValue = completeRequest.value(forHTTPHeaderField: "X-Firebase-Client")
XCTAssertEqual(headerValue, expectedHeader)
}
Expand All @@ -619,10 +617,8 @@ class AuthBackendRPCImplementationTests: RPCBaseTests {
try self.rpcIssuer.respond(withJSON: [:])
}
_ = try? await rpcImplementation.call(with: request)
// Make sure completeRequest updates.
usleep(10000)

let completeRequest = try XCTUnwrap(rpcIssuer.completeRequest)
let completeRequest = await rpcIssuer.completeRequest.value
let headerValue = completeRequest.value(forHTTPHeaderField: "X-Firebase-AppCheck")
XCTAssertEqual(headerValue, fakeAppCheck.fakeAppCheckToken)
}
Expand Down Expand Up @@ -651,11 +647,9 @@ class AuthBackendRPCImplementationTests: RPCBaseTests {
try self.rpcIssuer.respond(withJSON: [:])
}
_ = try? await rpcImplementation.call(with: request)
// Make sure completRequest updates.
usleep(10000)

// Then
let completeRequest = try XCTUnwrap(rpcIssuer.completeRequest)
let completeRequest = await rpcIssuer.completeRequest.value
XCTAssertNil(completeRequest.value(forHTTPHeaderField: "X-Firebase-Client"))
}
#endif // COCOAPODS || SWIFT_PACKAGE
Expand Down
12 changes: 6 additions & 6 deletions FirebaseAuth/Tests/Unit/Fakes/FakeBackendRPCIssuer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class FakeBackendRPCIssuer: NSObject, AuthBackendRPCIssuer {
/** @property completeRequest
@brief The last request to be processed by the backend.
*/
var completeRequest: URLRequest?
var completeRequest: Task<URLRequest, Never>!
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the tests, it wouldn't make sense to access this property before doing _ = try? await rpcImplementation.call(with: request), so this is implicitly unwrapped.


/** @var _handler
@brief A block we must invoke when @c respondWithError or @c respondWithJSON are called.
Expand Down Expand Up @@ -148,12 +148,12 @@ class FakeBackendRPCIssuer: NSObject, AuthBackendRPCIssuer {
requestData = body
// Use the real implementation so that the complete request can
// be verified during testing.
Task {
self.completeRequest = await AuthBackend.request(withURL: requestURL!,
contentType: contentType,
requestConfiguration: request
.requestConfiguration())
completeRequest = Task {
await AuthBackend.request(withURL: requestURL!,
Comment on lines +151 to +152
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before, assigning this property in the task led to it being resolved later than the method returns, causing for the need to sleep since it was assigned as a side effect of the _ = try? await rpcImplementation.call(with: request) call. Now, the task is synchronously assigned to it's resolved value can be async-ly waited on later.

contentType: contentType,
requestConfiguration: request.requestConfiguration())
}

decodedRequest = try? JSONSerialization.jsonObject(with: body) as? [String: Any]
}
if let respondBlock {
Expand Down
Loading