Skip to content

Fix flaky HTTPClientTests.testResponseDelayGet() test #584

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

Merged
merged 3 commits into from
Apr 22, 2022

Conversation

dnadoba
Copy link
Collaborator

@dnadoba dnadoba commented Apr 21, 2022

Motivation

We observed a test failure on our CI:

/async-http-client/Tests/AsyncHTTPClientTests/HTTPClientTests.swift: error: HTTPClientTests.testResponseDelayGet : XCTAssertGreaterThan failed: ("1.9851820468902588") is not greater than ("2.0") -
Test Case 'HTTPClientTests.testResponseDelayGet' failed (1.989 seconds)

Modification

I could not reproduce the failure myself but giving this a bit more leeway does not hurt. To measure the time difference, we now use NIODeadline which internally uses nanoseconds since boot to measure the time instead of Date. Date uses the system time which may change during execution due to time sync with a time server and is therefore not best suited to measure elapsed time.

Result

Immune to system time sync and slightly higher tolerance agains inaccurate timer implementations.

@dnadoba dnadoba added the semver/none No version bump required. label Apr 21, 2022
@fabianfett
Copy link
Member

It seems that we need a code format run

if `numberFormatting` is disable, it will allows us to add `_` in number literals when appropriate
Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@dnadoba
Copy link
Collaborator Author

dnadoba commented Apr 22, 2022

5.7 & nightly fail because of sendable related errors.

@dnadoba dnadoba merged commit 3725095 into swift-server:main Apr 22, 2022
@dnadoba dnadoba deleted the dn-fix-flaky-test branch April 22, 2022 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants