From 0bbae2f6a9c26c5fca05dea1ec59c2571104c361 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 18 Jan 2022 12:06:36 +1300 Subject: [PATCH 1/3] Fix flakey HTTP/2 tests --- .../Http2/Http2ConnectionTests.cs | 19 +++++++++++-------- .../Http2/Http2TestBase.cs | 11 ++++++----- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index 1e3ae7b852bc..dc82dbf73ac0 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -3695,10 +3695,13 @@ public async Task GOAWAY_Received_ConnectionStops() await WaitForConnectionStopAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); } - [Fact] - [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/39520")] - public async Task GOAWAY_Received_SetsConnectionStateToClosingAndWaitForAllStreamsToComplete() + public static readonly IEnumerable DummyData = Enumerable.Range(0, 5000).Select(i => new object[] { i }).ToArray(); + + [Theory] + [MemberData(nameof(DummyData))] + public async Task GOAWAY_Received_SetsConnectionStateToClosingAndWaitForAllStreamsToComplete(int i) { + Console.WriteLine(i); await InitializeConnectionAsync(_echoApplication); // Start some streams @@ -5219,7 +5222,7 @@ await WaitForConnectionErrorAsync( [Fact] public async Task StartConnection_SendPreface_ReturnSettings() { - await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + InitializeConnectionWithoutPreface(_noopApplication); await SendAsync(Http2Connection.ClientPreface); @@ -5234,7 +5237,7 @@ await ExpectAsync(Http2FrameType.SETTINGS, [Fact] public async Task StartConnection_SendHttp1xRequest_ReturnHttp11Status400() { - await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + InitializeConnectionWithoutPreface(_noopApplication); await SendAsync(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n")); @@ -5247,7 +5250,7 @@ public async Task StartConnection_SendHttp1xRequest_ReturnHttp11Status400() [Fact] public async Task StartConnection_SendHttp1xRequest_ExceedsRequestLineLimit_ProtocolError() { - await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + InitializeConnectionWithoutPreface(_noopApplication); await SendAsync(Encoding.ASCII.GetBytes($"GET /{new string('a', _connection.Limits.MaxRequestLineSize)} HTTP/1.1\r\n")); @@ -5267,7 +5270,7 @@ public async Task StartTlsConnection_SendHttp1xRequest_NoError() tlsHandshakeMock.SetupGet(m => m.Protocol).Returns(SslProtocols.Tls12); _connection.ConnectionFeatures.Set(tlsHandshakeMock.Object); - await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + InitializeConnectionWithoutPreface(_noopApplication); await SendAsync(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\n")); @@ -5277,7 +5280,7 @@ public async Task StartTlsConnection_SendHttp1xRequest_NoError() [Fact] public async Task StartConnection_SendNothing_NoError() { - await InitializeConnectionWithoutPrefaceAsync(_noopApplication); + InitializeConnectionWithoutPreface(_noopApplication); await StopConnectionAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index dd7d062f5810..5c223b11c107 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -470,7 +470,7 @@ protected void CreateConnection() _timeoutControl.Initialize(_serviceContext.SystemClock.UtcNow.Ticks); } - protected async Task InitializeConnectionWithoutPrefaceAsync(RequestDelegate application) + protected void InitializeConnectionWithoutPreface(RequestDelegate application) { if (_connection == null) { @@ -493,14 +493,15 @@ async Task CompletePipeOnTaskCompletion() } _connectionTask = CompletePipeOnTaskCompletion(); - - // Lose xUnit's AsyncTestSyncContext so middleware always runs inline for better determinism. - await ThreadPoolAwaitable.Instance; } protected async Task InitializeConnectionAsync(RequestDelegate application, int expectedSettingsCount = 3, bool expectedWindowUpdate = true) { - await InitializeConnectionWithoutPrefaceAsync(application); + InitializeConnectionWithoutPreface(application); + + // Lose xUnit's AsyncTestSyncContext so middleware always runs inline for better determinism. + await ThreadPoolAwaitable.Instance; + await SendPreambleAsync(); await SendSettingsAsync(); From fb45912ca1bdeb8c8ca7d62aa97e4c0559061d1c Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 18 Jan 2022 12:08:45 +1300 Subject: [PATCH 2/3] Clean up --- .../InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index dc82dbf73ac0..714f65cecfdd 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -3697,11 +3697,10 @@ public async Task GOAWAY_Received_ConnectionStops() public static readonly IEnumerable DummyData = Enumerable.Range(0, 5000).Select(i => new object[] { i }).ToArray(); - [Theory] - [MemberData(nameof(DummyData))] - public async Task GOAWAY_Received_SetsConnectionStateToClosingAndWaitForAllStreamsToComplete(int i) + [Fact] + [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/39520")] + public async Task GOAWAY_Received_SetsConnectionStateToClosingAndWaitForAllStreamsToComplete() { - Console.WriteLine(i); await InitializeConnectionAsync(_echoApplication); // Start some streams From 51dafb15e8bee2a39566ad670876d1058f3d5508 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 18 Jan 2022 12:09:26 +1300 Subject: [PATCH 3/3] Clean up --- .../test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs index 714f65cecfdd..ecbe3071bf35 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs @@ -3695,8 +3695,6 @@ public async Task GOAWAY_Received_ConnectionStops() await WaitForConnectionStopAsync(expectedLastStreamId: 0, ignoreNonGoAwayFrames: false); } - public static readonly IEnumerable DummyData = Enumerable.Range(0, 5000).Select(i => new object[] { i }).ToArray(); - [Fact] [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/39520")] public async Task GOAWAY_Received_SetsConnectionStateToClosingAndWaitForAllStreamsToComplete()