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.

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

flaky test: NIOSSLIntegrationTest.testNewCallbackCanDelayHandshake #197

Open
weissi opened this issue Mar 3, 2020 · 15 comments
Open

flaky test: NIOSSLIntegrationTest.testNewCallbackCanDelayHandshake #197

weissi opened this issue Mar 3, 2020 · 15 comments
Labels
area/testing Improvements to tests.

Comments

@weissi
Copy link
Member

weissi commented Mar 3, 2020

we were stuck for 17 minutes in NIOSSLIntegrationTest.testNewCallbackCanDelayHandshake:

21:56:36 Test Case 'NIOSSLIntegrationTest.testNewCallbackCanDelayHandshake' started at 2020-03-03 21:56:36.961
22:13:09 Build timed out (after 20 minutes). Marking the build as failed.
@weissi weissi added the area/testing Improvements to tests. label Mar 3, 2020
@weissi
Copy link
Member Author

weissi commented Mar 3, 2020

(https://ci.swiftserver.group/job/swift-nio-ssl2-swift50-prb/196/console)

@agnosticdev
Copy link
Contributor

Thoughts on using an XCTestExpectation for this to make sure if it hangs it results in a failure?
Something like:

let completionExpectation = XCTestExpectation(description: "CompletionPromiseExpectation")

completionPromiseFiredLock.withLock {
    XCTAssertFalse(completionPromiseFired)
    completionExpectation.fulfill()
}

// Ok, allow the handshake to run.
handshakeCompletePromise!.succeed(.certificateVerified)

let newBuffer = try completionPromise.futureResult.wait()
XCTAssertTrue(completionPromiseFired)
XCTAssertEqual(newBuffer, originalBuffer)
wait(for: [completionExpectation], timeout: 5.0)

@Lukasa
Copy link
Contributor

Lukasa commented Mar 26, 2020

That works, but doesn't really address the flakiness of the test. In practice I mostly don't mind it if a hang results in failure, because in either case we really do have to fix this issue.

@weissi
Copy link
Member Author

weissi commented Mar 26, 2020

Yeah, I think hangs are a good enough signal too. There's no way we could catch all things that could possibly hang so if we were to start using XCTestExpectations, hanging tests now either hang or they fail the expectations, I'd rather have them just hang because it's easier to code and we have one well-defined failure mode.

@agnosticdev
Copy link
Contributor

Sure, that all makes sense. I will sideline this and keep an eye out for anything I see that may be the root cause of this issue. Thanks.

@glbrntt
Copy link
Contributor

glbrntt commented Sep 30, 2020

@PeterAdams-A
Copy link
Contributor

08:57:08 Test Case 'NIOSSLIntegrationTest.testNewCallbackCanDelayHandshake' started at 2020-10-19 07:57:08.018
09:25:41 Build timed out (after 30 minutes). Marking the build as failed.

@weissi
Copy link
Member Author

weissi commented May 6, 2021

hit again in CI in #293

@Lukasa
Copy link
Contributor

Lukasa commented May 12, 2021

Hit again in CI in #295.

@Lukasa
Copy link
Contributor

Lukasa commented May 13, 2021

Hit again in CI on nightly in #299.

@Lukasa
Copy link
Contributor

Lukasa commented May 13, 2021

Actually hit repeatedly there, a few times in 5.0 as well. Seems like this is manifesting more under load. I've done some previous glancing at this in the past and was never able to diagnose anything, seems like I may need to do a more thorough investigation.

@Lukasa
Copy link
Contributor

Lukasa commented Jun 15, 2021

Hit again on CI in nightly on #300.

@glbrntt
Copy link
Contributor

glbrntt commented Nov 25, 2021

Hit again in #333 on 5.3

@fabianfett
Copy link
Member

@glbrntt
Copy link
Contributor

glbrntt commented May 4, 2022

Hit on 5.6 in #365

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Improvements to tests.
Projects
None yet
Development

No branches or pull requests

6 participants