Skip to content

Flaky test 'HTTPClientTests.testFileDownload' #347

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.

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

Open
Lukasa opened this issue Mar 16, 2021 · 0 comments
Open

Flaky test 'HTTPClientTests.testFileDownload' #347

Lukasa opened this issue Mar 16, 2021 · 0 comments
Labels
area/testing Improvements to tests. kind/bug Feature doesn't work as expected.

Comments

@Lukasa
Copy link
Collaborator

Lukasa commented Mar 16, 2021

Hit on #346. Output:

Test Case 'HTTPClientTests.testFileDownload' started at 2021-03-16 07:45:24.473
/code/Tests/AsyncHTTPClientTests/HTTPClientTests.swift:505: error: HTTPClientTests.testFileDownload : XCTAssertEqual failed: ("Optional(50)") is not equal to ("Optional(45)") - 
Test Case 'HTTPClientTests.testFileDownload' failed (0.779 seconds)

This test as crafted is inherently racy. The code in question is here:

let delegate = try FileDownloadDelegate(path: path)
let progress = try self.defaultClient.execute(
request: request,
delegate: delegate
)
.wait()
try XCTAssertEqual(50, TemporaryFileHelpers.fileSize(path: path))
return progress

This code assumes that once the task future completes the data will all be written to disk. However, FileDownloadDelegate does not guarantee this, and the delegate protocol we have provided makes it impossible to guarantee it. While the delegate protocol will exert backpressure on the response, it cannot prevent multiple events occurring at the same time. This means it is possible to see a situation where the same call to Channel.read triggers both .body and .end. If that happens, the task promise will complete but we may not have performed the final write and file close yet.

This is revealed in this test by the fact that the checking of the file on disk revealed fewer than 50 bytes written, but the progress structure accounts for all 50. They'll get there eventually, but we can't guarantee they're there right away.

The solution to this issue likely involves adding a promise to FileDownloadDelegate that will be fulfilled when the FD is closed, such that we know no further I/O to the file is forthcoming.

@Lukasa Lukasa added the kind/bug Feature doesn't work as expected. label Mar 16, 2021
@weissi weissi added the area/testing Improvements to tests. label Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Improvements to tests. kind/bug Feature doesn't work as expected.
Projects
None yet
Development

No branches or pull requests

2 participants