From 2f3b56540152cc57e3ed3130f1982608f3648506 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 11 Jun 2018 16:43:33 -0700 Subject: [PATCH 1/3] Minimize blocking threads to improve test reliability --- .../Kestrel.Core.Tests/HttpConnectionTests.cs | 26 ++++---- test/Kestrel.Core.Tests/MessageBodyTests.cs | 6 +- .../ChunkedResponseTests.cs | 6 +- .../RequestBodyTimeoutTests.cs | 61 ++++++++++++++----- test/Kestrel.FunctionalTests/RequestTests.cs | 12 ++-- test/Kestrel.FunctionalTests/ResponseTests.cs | 43 +++++++------ 6 files changed, 92 insertions(+), 62 deletions(-) diff --git a/test/Kestrel.Core.Tests/HttpConnectionTests.cs b/test/Kestrel.Core.Tests/HttpConnectionTests.cs index 091b3cadea..c8dbf28ba0 100644 --- a/test/Kestrel.Core.Tests/HttpConnectionTests.cs +++ b/test/Kestrel.Core.Tests/HttpConnectionTests.cs @@ -418,10 +418,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests } [Fact] - public void WriteTimingAbortsConnectionWhenWriteDoesNotCompleteWithMinimumDataRate() + public async Task WriteTimingAbortsConnectionWhenWriteDoesNotCompleteWithMinimumDataRate() { var systemClock = new MockSystemClock(); - var aborted = new ManualResetEventSlim(); + var aborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); @@ -434,7 +434,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _httpConnection.Http1Connection.Reset(); _httpConnection.Http1Connection.RequestAborted.Register(() => { - aborted.Set(); + aborted.SetResult(null); }); // Initialize timestamp @@ -448,15 +448,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _httpConnection.Tick(systemClock.UtcNow); Assert.True(_httpConnection.RequestTimedOut); - Assert.True(aborted.Wait(TimeSpan.FromSeconds(10))); + await aborted.Task.DefaultTimeout(); } [Fact] - public void WriteTimingAbortsConnectionWhenSmallWriteDoesNotCompleteWithinGracePeriod() + public async Task WriteTimingAbortsConnectionWhenSmallWriteDoesNotCompleteWithinGracePeriod() { var systemClock = new MockSystemClock(); var minResponseDataRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(5)); - var aborted = new ManualResetEventSlim(); + var aborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = minResponseDataRate; _httpConnectionContext.ServiceContext.SystemClock = systemClock; @@ -468,7 +468,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _httpConnection.Http1Connection.Reset(); _httpConnection.Http1Connection.RequestAborted.Register(() => { - aborted.Set(); + aborted.SetResult(null); }); // Initialize timestamp @@ -490,14 +490,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _httpConnection.Tick(systemClock.UtcNow); Assert.True(_httpConnection.RequestTimedOut); - Assert.True(aborted.Wait(TimeSpan.FromSeconds(10))); + await aborted.Task.DefaultTimeout(); } [Fact] - public void WriteTimingTimeoutPushedOnConcurrentWrite() + public async Task WriteTimingTimeoutPushedOnConcurrentWrite() { var systemClock = new MockSystemClock(); - var aborted = new ManualResetEventSlim(); + var aborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2)); @@ -510,7 +510,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _httpConnection.Http1Connection.Reset(); _httpConnection.Http1Connection.RequestAborted.Register(() => { - aborted.Set(); + aborted.SetResult(null); }); // Initialize timestamp @@ -537,7 +537,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests _httpConnection.Tick(systemClock.UtcNow); Assert.True(_httpConnection.RequestTimedOut); - Assert.True(aborted.Wait(TimeSpan.FromSeconds(10))); + await aborted.Task.DefaultTimeout(); } [Fact] @@ -547,7 +547,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests var minResponseDataRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(5)); var numWrites = 5; var writeSize = 100; - var aborted = new TaskCompletionSource(); + var aborted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = minResponseDataRate; _httpConnectionContext.ServiceContext.SystemClock = systemClock; diff --git a/test/Kestrel.Core.Tests/MessageBodyTests.cs b/test/Kestrel.Core.Tests/MessageBodyTests.cs index 703808a400..5f006b12e3 100644 --- a/test/Kestrel.Core.Tests/MessageBodyTests.cs +++ b/test/Kestrel.Core.Tests/MessageBodyTests.cs @@ -687,11 +687,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { using (var input = new TestInput()) { - var logEvent = new ManualResetEventSlim(); + var logEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var mockLogger = new Mock(); mockLogger .Setup(logger => logger.RequestBodyDone("ConnectionId", "RequestId")) - .Callback(() => logEvent.Set()); + .Callback(() => logEvent.SetResult(null)); input.Http1Connection.ServiceContext.Log = mockLogger.Object; input.Http1Connection.ConnectionIdFeature = "ConnectionId"; input.Http1Connection.TraceIdentifier = "RequestId"; @@ -706,7 +706,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests input.Fin(); - Assert.True(logEvent.Wait(TestConstants.DefaultTimeout)); + await logEvent.Task.DefaultTimeout(); await body.StopAsync(); } diff --git a/test/Kestrel.FunctionalTests/ChunkedResponseTests.cs b/test/Kestrel.FunctionalTests/ChunkedResponseTests.cs index c9a7e70747..6cb1a9e439 100644 --- a/test/Kestrel.FunctionalTests/ChunkedResponseTests.cs +++ b/test/Kestrel.FunctionalTests/ChunkedResponseTests.cs @@ -350,7 +350,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { var testContext = new TestServiceContext(LoggerFactory); - var flushWh = new ManualResetEventSlim(); + var flushWh = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using (var server = new TestServer(async httpContext => { @@ -358,7 +358,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello "), 0, 6); // Don't complete response until client has received the first chunk. - flushWh.Wait(); + await flushWh.Task.DefaultTimeout(); await response.Body.WriteAsync(Encoding.ASCII.GetBytes("World!"), 0, 6); }, testContext, listenOptions)) @@ -379,7 +379,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests "Hello ", ""); - flushWh.Set(); + flushWh.SetResult(null); await connection.ReceiveEnd( "6", diff --git a/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs b/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs index 0ae0e9f0ce..9d292e53fc 100644 --- a/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs +++ b/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs @@ -30,15 +30,39 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests DateHeaderValueManager = new DateHeaderValueManager(systemClock) }; - var appRunningEvent = new ManualResetEventSlim(); + var appRunningEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using (var server = new TestServer(context => { context.Features.Get().MinDataRate = new MinDataRate(bytesPerSecond: 1, gracePeriod: gracePeriod); - appRunningEvent.Set(); - return context.Request.Body.ReadAsync(new byte[1], 0, 1); + // The server must call Request.Body.ReadAsync() *before* the test sets systemClock.UtcNow (which is triggered by the + // server calling appRunningEvent.SetResult(null)). If systemClock.UtcNow is set first, it's possible for the test to fail + // due to the following race condition: + // + // 1. [test] systemClock.UtcNow += gracePeriod + TimeSpan.FromSeconds(1); + // 2. [server] Heartbeat._timer is triggered, which calls HttpConnection.Tick() + // 3. [server] HttpConnection.Tick() calls HttpConnection.CheckForReadDataRateTimeout() + // 4. [server] HttpConnection.CheckForReadDataRateTimeout() is a no-op, since _readTimingEnabled is false, + // since Request.Body.ReadAsync() has not been called yet + // 5. [server] HttpConnection.Tick() sets _lastTimestamp = timestamp + // 6. [server] Request.Body.ReadAsync() is called + // 6. [test] systemClock.UtcNow is never updated again, so server timestamp is never updated, + // so HttpConnection.CheckForReadDataRateTimeout() is always a no-op until test fails + // + // This is a pretty tight race, since the once-per-second Heartbeat._timer needs to fire between the test updating + // systemClock.UtcNow and the server calling Request.Body.ReadAsync(). But it happened often enough to cause + // test flakiness in our CI (https://github.com/aspnet/KestrelHttpServer/issues/2539). + // + // For verification, I was able to induce the race by adding a sleep in the RequestDelegate: + // appRunningEvent.SetResult(null); + // Thread.Sleep(5000); + // return context.Request.Body.ReadAsync(new byte[1], 0, 1); + + var readTask = context.Request.Body.ReadAsync(new byte[1], 0, 1); + appRunningEvent.SetResult(null); + return readTask; }, serviceContext)) { using (var connection = server.CreateConnection()) @@ -50,7 +74,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests "", ""); - Assert.True(appRunningEvent.Wait(TestConstants.DefaultTimeout)); + await appRunningEvent.Task.DefaultTimeout(); systemClock.UtcNow += gracePeriod + TimeSpan.FromSeconds(1); await connection.Receive( @@ -77,13 +101,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests DateHeaderValueManager = new DateHeaderValueManager(systemClock), }; - var appRunningEvent = new ManualResetEventSlim(); + var appRunningEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using (var server = new TestServer(context => { context.Features.Get().MinDataRate = null; - appRunningEvent.Set(); + appRunningEvent.SetResult(null); return Task.CompletedTask; }, serviceContext)) { @@ -96,7 +120,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests "", ""); - Assert.True(appRunningEvent.Wait(TestConstants.DefaultTimeout)); + await appRunningEvent.Task.DefaultTimeout(); await connection.Receive( "HTTP/1.1 200 OK", @@ -115,7 +139,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests Assert.Contains(TestSink.Writes, w => w.EventId.Id == 33 && w.LogLevel == LogLevel.Information); } - [Fact(Skip="https://github.com/aspnet/KestrelHttpServer/issues/2464")] + [Fact] public async Task ConnectionClosedEvenIfAppSwallowsException() { var gracePeriod = TimeSpan.FromSeconds(5); @@ -126,23 +150,30 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests DateHeaderValueManager = new DateHeaderValueManager(systemClock) }; - var appRunningEvent = new ManualResetEventSlim(); - var exceptionSwallowedEvent = new ManualResetEventSlim(); + var appRunningTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var exceptionSwallowedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using (var server = new TestServer(async context => { context.Features.Get().MinDataRate = new MinDataRate(bytesPerSecond: 1, gracePeriod: gracePeriod); - appRunningEvent.Set(); + // See comment in RequestTimesOutWhenRequestBodyNotReceivedAtSpecifiedMinimumRate for + // why we call ReadAsync before setting the appRunningEvent. + var readTask = context.Request.Body.ReadAsync(new byte[1], 0, 1); + appRunningTcs.SetResult(null); try { - await context.Request.Body.ReadAsync(new byte[1], 0, 1); + await readTask; } catch (BadHttpRequestException ex) when (ex.StatusCode == 408) { - exceptionSwallowedEvent.Set(); + exceptionSwallowedTcs.SetResult(null); + } + catch (Exception ex) + { + exceptionSwallowedTcs.SetException(ex); } var response = "hello, world"; @@ -159,9 +190,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests "", ""); - Assert.True(appRunningEvent.Wait(TestConstants.DefaultTimeout), "AppRunningEvent timed out."); + await appRunningTcs.Task.DefaultTimeout(); systemClock.UtcNow += gracePeriod + TimeSpan.FromSeconds(1); - Assert.True(exceptionSwallowedEvent.Wait(TestConstants.DefaultTimeout), "ExceptionSwallowedEvent timed out."); + await exceptionSwallowedTcs.Task.DefaultTimeout(); await connection.Receive( "HTTP/1.1 200 OK", diff --git a/test/Kestrel.FunctionalTests/RequestTests.cs b/test/Kestrel.FunctionalTests/RequestTests.cs index eac3aaa369..32dfac74fc 100644 --- a/test/Kestrel.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.FunctionalTests/RequestTests.cs @@ -1696,8 +1696,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests [Fact] public async Task DoesNotEnforceRequestBodyMinimumDataRateOnUpgradedRequest() { - var appEvent = new ManualResetEventSlim(); - var delayEvent = new ManualResetEventSlim(); + var appEvent = new TaskCompletionSource(); + var delayEvent = new TaskCompletionSource(); var serviceContext = new TestServiceContext(LoggerFactory) { SystemClock = new SystemClock() @@ -1710,12 +1710,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests using (var stream = await context.Features.Get().UpgradeAsync()) { - appEvent.Set(); + appEvent.SetResult(null); // Read once to go through one set of TryPauseTimingReads()/TryResumeTimingReads() calls await stream.ReadAsync(new byte[1], 0, 1); - delayEvent.Wait(); + await delayEvent.Task.DefaultTimeout(); // Read again to check that the connection is still alive await stream.ReadAsync(new byte[1], 0, 1); @@ -1735,11 +1735,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests "", "a"); - Assert.True(appEvent.Wait(TestConstants.DefaultTimeout)); + await appEvent.Task.DefaultTimeout(); await Task.Delay(TimeSpan.FromSeconds(5)); - delayEvent.Set(); + delayEvent.SetResult(null); await connection.Send("b"); diff --git a/test/Kestrel.FunctionalTests/ResponseTests.cs b/test/Kestrel.FunctionalTests/ResponseTests.cs index 9660dca2f3..2f22bc1f67 100644 --- a/test/Kestrel.FunctionalTests/ResponseTests.cs +++ b/test/Kestrel.FunctionalTests/ResponseTests.cs @@ -1116,12 +1116,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var flushed = new SemaphoreSlim(0, 1); var serviceContext = new TestServiceContext(LoggerFactory) { ServerOptions = { AllowSynchronousIO = true } }; - using (var server = new TestServer(httpContext => + using (var server = new TestServer(async httpContext => { httpContext.Response.ContentLength = 12; httpContext.Response.Body.Write(Encoding.ASCII.GetBytes("hello, world"), 0, 12); - flushed.Wait(); - return Task.CompletedTask; + await flushed.WaitAsync(); }, serviceContext)) { using (var connection = server.CreateConnection()) @@ -1152,7 +1151,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests { httpContext.Response.ContentLength = 12; await httpContext.Response.WriteAsync(""); - flushed.Wait(); + await flushed.WaitAsync(); await httpContext.Response.WriteAsync("hello, world"); }, new TestServiceContext(LoggerFactory))) { @@ -1180,23 +1179,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests [Fact] public async Task WriteAfterConnectionCloseNoops() { - var connectionClosed = new ManualResetEventSlim(); - var requestStarted = new ManualResetEventSlim(); - var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var connectionClosed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var requestStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var appCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using (var server = new TestServer(async httpContext => { try { - requestStarted.Set(); - connectionClosed.Wait(); + requestStarted.SetResult(null); + await connectionClosed.Task.DefaultTimeout(); httpContext.Response.ContentLength = 12; await httpContext.Response.WriteAsync("hello, world"); - tcs.TrySetResult(null); + appCompleted.TrySetResult(null); } catch (Exception ex) { - tcs.TrySetException(ex); + appCompleted.TrySetException(ex); } }, new TestServiceContext(LoggerFactory))) { @@ -1208,14 +1207,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests "", ""); - requestStarted.Wait(); + await requestStarted.Task.DefaultTimeout(); connection.Shutdown(SocketShutdown.Send); await connection.WaitForConnectionClose().DefaultTimeout(); } - connectionClosed.Set(); + connectionClosed.SetResult(null); - await tcs.Task.DefaultTimeout(); + await appCompleted.Task.DefaultTimeout(); } } @@ -2280,19 +2279,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests var largeString = new string('a', maxBytesPreCompleted + 1); var writeTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var requestAbortedWh = new ManualResetEventSlim(); - var requestStartWh = new ManualResetEventSlim(); + var requestAbortedWh = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var requestStartWh = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); using (var server = new TestServer(async httpContext => { - requestStartWh.Set(); + requestStartWh.SetResult(null); var response = httpContext.Response; var request = httpContext.Request; var lifetime = httpContext.Features.Get(); - lifetime.RequestAborted.Register(() => requestAbortedWh.Set()); - Assert.True(requestAbortedWh.Wait(TestConstants.DefaultTimeout)); + lifetime.RequestAborted.Register(() => requestAbortedWh.SetResult(null)); + await requestAbortedWh.Task.DefaultTimeout(); try { @@ -2316,15 +2315,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests "", ""); - Assert.True(requestStartWh.Wait(TestConstants.DefaultTimeout)); + await requestStartWh.Task.DefaultTimeout(); } // Write failed - can throw TaskCanceledException or OperationCanceledException, - // dependending on how far the canceled write goes. + // depending on how far the canceled write goes. await Assert.ThrowsAnyAsync(async () => await writeTcs.Task).DefaultTimeout(); // RequestAborted tripped - Assert.True(requestAbortedWh.Wait(TestConstants.DefaultTimeout)); + await requestAbortedWh.Task.DefaultTimeout(); } } From 5ba327faa1306a3977acb58511728daedc31d243 Mon Sep 17 00:00:00 2001 From: John Luo Date: Tue, 11 Sep 2018 09:59:54 -0700 Subject: [PATCH 2/3] Relax connection stop checks in tests to reduce flakiness --- test/Kestrel.FunctionalTests/RequestTests.cs | 4 ++-- test/Kestrel.FunctionalTests/ResponseTests.cs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/Kestrel.FunctionalTests/RequestTests.cs b/test/Kestrel.FunctionalTests/RequestTests.cs index 32dfac74fc..d0fe276273 100644 --- a/test/Kestrel.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.FunctionalTests/RequestTests.cs @@ -1302,7 +1302,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await appFuncCompleted.Task.DefaultTimeout(); } - mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); + mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.AtMostOnce()); } [Theory] @@ -1357,7 +1357,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await Assert.ThrowsAnyAsync(() => readTcs.Task).DefaultTimeout(); } - mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); + mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.AtMostOnce()); } [Theory] diff --git a/test/Kestrel.FunctionalTests/ResponseTests.cs b/test/Kestrel.FunctionalTests/ResponseTests.cs index 2f22bc1f67..5763a54a5b 100644 --- a/test/Kestrel.FunctionalTests/ResponseTests.cs +++ b/test/Kestrel.FunctionalTests/ResponseTests.cs @@ -2427,7 +2427,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await Assert.ThrowsAnyAsync(() => writeTcs.Task).DefaultTimeout(); } - mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); + mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.AtMostOnce()); Assert.True(requestAborted); } @@ -3174,7 +3174,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await appFuncCompleted.Task.DefaultTimeout(); mockKestrelTrace.Verify(t => t.ResponseMininumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); - mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); + mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.AtMostOnce()); Assert.False(requestAborted); } } @@ -3249,7 +3249,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests await AssertStreamCompleted(connection.Reader.BaseStream, minTotalOutputSize, targetBytesPerSecond); mockKestrelTrace.Verify(t => t.ResponseMininumDataRateNotSatisfied(It.IsAny(), It.IsAny()), Times.Never()); - mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.Once()); + mockKestrelTrace.Verify(t => t.ConnectionStop(It.IsAny()), Times.AtMostOnce()); Assert.False(requestAborted); } } From e5b2b680e07ec6142bfd0f2ca39592f1acf85b2f Mon Sep 17 00:00:00 2001 From: John Luo Date: Fri, 7 Sep 2018 18:18:02 -0700 Subject: [PATCH 3/3] Fix flaky test by ignoring indeterminant response --- test/Kestrel.FunctionalTests/ChunkedRequestTests.cs | 2 +- test/shared/TestConnection.cs | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/test/Kestrel.FunctionalTests/ChunkedRequestTests.cs b/test/Kestrel.FunctionalTests/ChunkedRequestTests.cs index 10b68fdf45..ecbb6f23b2 100644 --- a/test/Kestrel.FunctionalTests/ChunkedRequestTests.cs +++ b/test/Kestrel.FunctionalTests/ChunkedRequestTests.cs @@ -678,7 +678,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests connection.Socket.Shutdown(SocketShutdown.Send); - await connection.ReceiveEnd(); + await connection.ReceiveEnd(ignoreResponse: true); var badReqEx = await exTcs.Task.TimeoutAfter(TestConstants.DefaultTimeout); Assert.Equal(RequestRejectionReason.UnexpectedEndOfRequestContent, badReqEx.Reason); diff --git a/test/shared/TestConnection.cs b/test/shared/TestConnection.cs index 2d760a5bc8..6469dd2d19 100644 --- a/test/shared/TestConnection.cs +++ b/test/shared/TestConnection.cs @@ -149,14 +149,20 @@ namespace Microsoft.AspNetCore.Testing Assert.Equal(expected, new string(actual, 0, offset)); } - public async Task ReceiveEnd(params string[] lines) + public Task ReceiveEnd(params string[] lines) + => ReceiveEnd(false, lines); + + public async Task ReceiveEnd(bool ignoreResponse, params string[] lines) { await Receive(lines).ConfigureAwait(false); _socket.Shutdown(SocketShutdown.Send); var ch = new char[128]; var count = await _reader.ReadAsync(ch, 0, 128).TimeoutAfter(Timeout).ConfigureAwait(false); - var text = new string(ch, 0, count); - Assert.Equal("", text); + if (!ignoreResponse) + { + var text = new string(ch, 0, count); + Assert.Equal("", text); + } } public async Task ReceiveForcedEnd(params string[] lines)