Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Fix races in some EngineTests. #1257

Merged
merged 1 commit into from
Dec 13, 2016
Merged

Fix races in some EngineTests. #1257

merged 1 commit into from
Dec 13, 2016

Conversation

cesarblum
Copy link
Contributor

Uncaught test failure while I was working on #1218.

In OnStartingCallbacksAreCalledInLastInFirstOutOrder and OnCompletedCallbacksAreCalledInLastInFirstOutOrder, we need to wait externally for all callbacks to be called.

Previously this worked fine because by the time we received the FIN from Kestrel, we could be sure those callbacks had been called, since the connection shut down gracefully. Now, however, the client FIN aborts the request, and depending on the timing and the order that things run, the test might see the connection being closed by the server before OnStarting/OnCompleted callbacks were called (because the abort runs concurrent to request processing).

@halter73 @natemcmaster

@cesarblum
Copy link
Contributor Author

Fixed a similar race in ResponseTests.WhenAppWritesLessThanContentLengthErrorLogged.

@muratg
Copy link
Contributor

muratg commented Dec 13, 2016

:shipit:

@@ -909,7 +919,7 @@ public Task ResponseStatusCodeSetBeforeHttpContextDisposedRequestMalformedReadIg
"POST / HTTP/1.1",
"Transfer-Encoding: chunked",
"",
"wrong");
"gg");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated but piggybacking this here. Test can fail if Send is still sending bytes but the server has already closed the connection because of the bad chunk length.

@cesarblum cesarblum merged commit 4485bfe into dev Dec 13, 2016
@cesarblum cesarblum deleted the cesarbs/fix-test-race branch December 13, 2016 21:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants