Skip to content
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

Jump to bottom

Fix flaky test TransactionTests.testCancelAsyncRequest #707

Merged
merged 8 commits into from
Oct 4, 2023
Merged

Fix flaky test TransactionTests.testCancelAsyncRequest #707

merged 8 commits into from
Oct 4, 2023

Conversation

dnadoba
Copy link
Collaborator

@dnadoba dnadoba commented Aug 14, 2023

Resolves #706.
The continuation is resumed with the cancelation error before we cancel the scheduled request.

async-http-client/Sources/AsyncHTTPClient/AsyncAwait/Transaction.swift

Lines 290 to 294 in 62c06d4

case .failResponseHead(let continuation, let error, let scheduler, let executor, let bodyStreamContinuation):
continuation.resume(throwing: error)
bodyStreamContinuation?.resume(throwing: error)
scheduler?.cancelRequest(self) // NOTE: scheduler and executor are exclusive here
executor?.cancelRequest(self)

Therefore this test is inherently flaky. The fix waits up to one second for the scheduled request to be canceled.

@dnadoba
Fix flaky test TransactionTests.testCancelAsyncRequest
448264c 
Resolves swift-server#706.
The continuation is resumed with the cancelation error before we cancel the scheduled request.
https://github.com/swift-server/async-http-client/blob/62c06d47c8d21c91335e9f8998589e4ce31411e6/Sources/AsyncHTTPClient/AsyncAwait/Transaction.swift#L290C10-L294

Therefore this test is inherently flaky. The fix waits up to one second for the scheduled request to be canceled.
@dnadoba dnadoba added the semver/none For PRs that when merged do not need a bump in version number. label Aug 14, 2023
@dnadoba
Copy link
Collaborator Author

dnadoba commented Aug 14, 2023

5.6 fails with

[505/505] Testing AsyncHTTPClientTests.NoBytesSentOverBodyLimitTests/testNoBytesSentOverBodyLimit

Test Suite 'Selected tests' started at 2023-08-14 19:20:37.322
Test Suite 'TransactionTests' started at 2023-08-14 19:20:37.325
Test Case 'TransactionTests.testCancelAsyncRequest' started at 2023-08-14 19:20:37.325

Exited with signal code 4
1
Build step 'Execute shell' marked build as failure

not sure what is happening.

@yim-lee
Copy link
Contributor

yim-lee commented Oct 4, 2023

CI cleanup might have caused the doc check failure? @swift-server-bot test this please

@dnadoba dnadoba merged commit 4c07d3b into swift-server:main Oct 4, 2023
@dnadoba dnadoba deleted the dn-fix-flaky-test-test-cancel-async-request branch October 4, 2023 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lukasa Lukasa Lukasa approved these changes

Assignees
No one assigned
Labels
semver/none For PRs that when merged do not need a bump in version number.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

flaky test: TransactionTests.testCancelAsyncRequest
3 participants
@dnadoba @yim-lee @Lukasa