Reduce flakiness of LargeUpload tests (#3036)

* Move h2spec tests to own project

KestrelHttpServer.sln

@@ -134,6 +134,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Kestrel.Transport.Sockets.B
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Kestrel.Transport.Libuv.BindTests", "test\Kestrel.Transport.Libuv.BindTests\Kestrel.Transport.Libuv.BindTests.csproj", "{FB9C6B61-0A7B-4FFA-B772-A754316B262E}"
 EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Kestrel.H2Spec.FunctionalTests", "test\Kestrel.H2Spec.FunctionalTests\Kestrel.H2Spec.FunctionalTests.csproj", "{C4123E55-5760-4557-B89B-39E1258FD7F9}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -420,6 +422,18 @@ Global
 		{FB9C6B61-0A7B-4FFA-B772-A754316B262E}.Release|x64.Build.0 = Release|Any CPU
 		{FB9C6B61-0A7B-4FFA-B772-A754316B262E}.Release|x86.ActiveCfg = Release|Any CPU
 		{FB9C6B61-0A7B-4FFA-B772-A754316B262E}.Release|x86.Build.0 = Release|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|x64.Build.0 = Debug|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Debug|x86.Build.0 = Debug|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|Any CPU.Build.0 = Release|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|x64.ActiveCfg = Release|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|x64.Build.0 = Release|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|x86.ActiveCfg = Release|Any CPU
+		{C4123E55-5760-4557-B89B-39E1258FD7F9}.Release|x86.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -450,6 +464,7 @@ Global
 		{B5422347-E919-431D-9EF2-C352FFE4D6C1} = {D3273454-EA07-41D2-BF0B-FCC3675C2483}
 		{9254C3EB-196B-402F-A059-34FEA6140500} = {D3273454-EA07-41D2-BF0B-FCC3675C2483}
 		{FB9C6B61-0A7B-4FFA-B772-A754316B262E} = {D3273454-EA07-41D2-BF0B-FCC3675C2483}
+		{C4123E55-5760-4557-B89B-39E1258FD7F9} = {D3273454-EA07-41D2-BF0B-FCC3675C2483}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {2D10D020-6770-47CA-BB8D-2C23FE3AE071}

test/Kestrel.H2Spec.FunctionalTests/H2SpecCommands.cs

@@ -10,7 +10,7 @@ using System.Runtime.InteropServices;
 using System.Xml;
 using Microsoft.Extensions.Logging;
 
-namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
+namespace H2Spec.FunctionalTests
 {
     public static class H2SpecCommands
     {

test/Kestrel.H2Spec.FunctionalTests/H2SpecTests.cs

@@ -1,33 +1,35 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-#if NETCOREAPP2_2
+using System;
-
 using System.IO;
 using System.Linq;
 using System.Net;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.Hosting.Server.Features;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Core;
-using Microsoft.AspNetCore.Testing;
 using Microsoft.AspNetCore.Testing.xunit;
+using Microsoft.Extensions.Logging.Testing;
 using Xunit;
 using Xunit.Abstractions;
 
-namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2
+namespace H2Spec.FunctionalTests
 {
     [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "Missing SslStream ALPN support: https://github.com/dotnet/corefx/issues/30492")]
     [MinimumOSVersion(OperatingSystems.Windows, WindowsVersions.Win81,
         SkipReason = "Missing Windows ALPN support: https://en.wikipedia.org/wiki/Application-Layer_Protocol_Negotiation#Support")]
-    public class H2SpecTests : TestApplicationErrorLoggerLoggedTest
+    public class H2SpecTests : LoggedTest
     {
+        private static readonly string _testCertPath = Path.Combine(Directory.GetCurrentDirectory(), "shared", "TestCertificates", "testCert.pfx");
+
         [ConditionalTheory]
         [MemberData(nameof(H2SpecTestCases))]
         public async Task RunIndividualTestCase(H2SpecTestCase testCase)
         {
-            var hostBuilder = TransportSelector.GetWebHostBuilder()
+            var hostBuilder = new WebHostBuilder()
                 .UseKestrel(options =>
                 {
                     options.Listen(IPAddress.Loopback, 0, listenOptions =>
@@ -35,7 +37,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2
                         listenOptions.Protocols = HttpProtocols.Http2;
                         if (testCase.Https)
                         {
-                            listenOptions.UseHttps(TestResources.TestCertificatePath, "testPassword");
+                            listenOptions.UseHttps(_testCertPath, "testPassword");
                         }
                     });
                 })
@@ -46,7 +48,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2
             {
                 await host.StartAsync();
 
-                H2SpecCommands.RunTest(testCase.Id, host.GetPort(), testCase.Https, Logger);
+                H2SpecCommands.RunTest(testCase.Id, GetPort(host), testCase.Https, Logger);
             }
         }
 
@@ -129,9 +131,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2
                 await context.Response.WriteAsync("Hello World");
             });
         }
+
+        private static int GetPort(IWebHost host)
+        {
+            return host.ServerFeatures.Get<IServerAddressesFeature>().Addresses
+                .Select(a => new Uri(a))
+                .First()
+                .Port;
+        }
     }
 }
-#elif NET461 // HTTP/2 is not supported
-#else
-#error TFMs need updating
-#endif

test/Kestrel.H2Spec.FunctionalTests/Kestrel.H2Spec.FunctionalTests.csproj

@@ -0,0 +1,28 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <AssemblyName>H2Spec.FunctionalTests</AssemblyName>
+    <RootNamespace>H2Spec.FunctionalTests</RootNamespace>
+    <!-- h2spec doesn't run on the desktop framework where ALPN isn't available. -->
+    <!-- DeveloperBuildTestTfms is StandardTestTfms minus desktop. -->
+    <TargetFrameworks>$(DeveloperBuildTestTfms)</TargetFrameworks>
+    <ServerGarbageCollection>true</ServerGarbageCollection>
+    <TestGroupName>H2Spec.FunctionalTests</TestGroupName>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <Content Include="..\shared\TestCertificates\*.pfx" LinkBase="shared\TestCertificates" CopyToOutputDirectory="PreserveNewest" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\..\src\Kestrel\Kestrel.csproj" />
+    <ProjectReference Include="..\..\src\Kestrel.Https\Kestrel.Https.csproj" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.AspNetCore.Testing" Version="$(MicrosoftAspNetCoreTestingPackageVersion)" />
+    <PackageReference Include="Microsoft.Extensions.Logging.Testing" Version="$(MicrosoftExtensionsLoggingTestingPackageVersion)" />
+    <PackageReference Include="Microsoft.Internal.AspNetCore.H2Spec.All" Version="$(MicrosoftInternalAspNetCoreH2SpecAllPackageVersion)" />
+  </ItemGroup>
+
+</Project>

test/Kestrel.Transport.FunctionalTests/MaxRequestBufferSizeTests.cs

@@ -51,7 +51,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
 
         private static readonly string[] _requestLines = new[]
         {
-            "POST / HTTP/1.0\r\n",
+            "POST / HTTP/1.1\r\n",
+            "Host: \r\n",
             $"Content-Length: {_dataLength}\r\n",
             "\r\n"
         };
@@ -60,11 +61,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
         {
             get
             {
+                var totalHeaderSize = 0;
+
+                for (var i = 1; i < _requestLines.Length - 1; i++)
+                {
+                    totalHeaderSize += _requestLines[i].Length;
+                }
+
                 var maxRequestBufferSizeValues = new Tuple<long?, bool>[] {
-                    // Smallest buffer that can hold a test request line without causing
+                    // Smallest buffer that can hold the test request headers without causing
                     // the server to hang waiting for the end of the request line or
                     // a header line.
-                    Tuple.Create((long?)(_requestLines.Max(line => line.Length)), true),
+                    Tuple.Create((long?)totalHeaderSize, true),
 
                     // Small buffer, but large enough to hold all request headers.
                     Tuple.Create((long?)16 * 1024, true),
@@ -194,11 +202,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         startReadingRequestBody.TrySetResult(null);
                     }
 
-                    using (var reader = new StreamReader(stream, Encoding.ASCII))
+                    await AssertStreamContains(stream, $"bytesRead: {data.Length}");
-                    {
-                        var response = reader.ReadToEnd();
-                        Assert.Contains($"bytesRead: {data.Length}", response);
-                    }
                 }
             }
 
@@ -374,5 +378,38 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                 }
             }
         }
+
+        // THIS IS NOT GENERAL PURPOSE. If the initial characters could repeat, this is broken. However, since we're
+        // looking for /bytesWritten: \d+/ and the initial "b" cannot occur elsewhere in the pattern, this works.
+        private static async Task AssertStreamContains(Stream stream, string expectedSubstring)
+        {
+            var expectedBytes = Encoding.ASCII.GetBytes(expectedSubstring);
+            var exptectedLength = expectedBytes.Length;
+            var responseBuffer = new byte[exptectedLength];
+
+            var matchedChars = 0;
+
+            while (matchedChars < exptectedLength)
+            {
+                var count = await stream.ReadAsync(responseBuffer, 0, exptectedLength - matchedChars).DefaultTimeout();
+
+                if (count == 0)
+                {
+                    Assert.True(false, "Stream completed without expected substring.");
+                }
+
+                for (var i = 0; i < count && matchedChars < exptectedLength; i++)
+                {
+                    if (responseBuffer[i] == expectedBytes[matchedChars])
+                    {
+                        matchedChars++;
+                    }
+                    else
+                    {
+                        matchedChars = 0;
+                    }
+                }
+            }
+        }
     }
 }

test/Kestrel.Transport.FunctionalTests/RequestTests.cs

@@ -53,7 +53,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
         // https://github.com/aspnet/KestrelHttpServer/issues/520#issuecomment-188591242
         // will be lost.
         [InlineData((long)int.MaxValue + 1, false)]
-        public void LargeUpload(long contentLength, bool checkBytes)
+        public async Task LargeUpload(long contentLength, bool checkBytes)
         {
             const int bufferLength = 1024 * 1024;
             Assert.True(contentLength % bufferLength == 0, $"{nameof(contentLength)} sent must be evenly divisible by {bufferLength}.");
@@ -89,7 +89,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                             total += received;
                         }
 
-                        await context.Response.WriteAsync(total.ToString(CultureInfo.InvariantCulture));
+                        await context.Response.WriteAsync($"bytesRead: {total.ToString()}");
                     });
                 });
 
@@ -100,8 +100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                 using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp))
                 {
                     socket.Connect(new IPEndPoint(IPAddress.Loopback, host.GetPort()));
-                    socket.Send(Encoding.ASCII.GetBytes("POST / HTTP/1.0\r\n"));
+                    socket.Send(Encoding.ASCII.GetBytes("POST / HTTP/1.1\r\nHost: \r\n"));
-                    Thread.Sleep(5000);
                     socket.Send(Encoding.ASCII.GetBytes($"Content-Length: {contentLength}\r\n\r\n"));
 
                     var contentBytes = new byte[bufferLength];
@@ -119,15 +118,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         socket.Send(contentBytes);
                     }
 
-                    var response = new StringBuilder();
+                    using (var stream = new NetworkStream(socket))
-                    var responseBytes = new byte[4096];
-                    var received = 0;
-                    while ((received = socket.Receive(responseBytes)) > 0)
                     {
-                        response.Append(Encoding.ASCII.GetString(responseBytes, 0, received));
+                        await AssertStreamContains(stream, $"bytesRead: {contentLength}");
                     }
-
-                    Assert.Contains(contentLength.ToString(CultureInfo.InvariantCulture), response.ToString());
                 }
             }
         }
@@ -906,5 +900,38 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                 Assert.NotEmpty(facts["RemotePort"].Value<string>());
             }
         }
+
+        // THIS IS NOT GENERAL PURPOSE. If the initial characters could repeat, this is broken. However, since we're
+        // looking for /bytesWritten: \d+/ and the initial "b" cannot occur elsewhere in the pattern, this works.
+        private static async Task AssertStreamContains(Stream stream, string expectedSubstring)
+        {
+            var expectedBytes = Encoding.ASCII.GetBytes(expectedSubstring);
+            var exptectedLength = expectedBytes.Length;
+            var responseBuffer = new byte[exptectedLength];
+
+            var matchedChars = 0;
+
+            while (matchedChars < exptectedLength)
+            {
+                var count = await stream.ReadAsync(responseBuffer, 0, exptectedLength - matchedChars).DefaultTimeout();
+
+                if (count == 0)
+                {
+                    Assert.True(false, "Stream completed without expected substring.");
+                }
+
+                for (var i = 0; i < count && matchedChars < exptectedLength; i++)
+                {
+                    if (responseBuffer[i] == expectedBytes[matchedChars])
+                    {
+                        matchedChars++;
+                    }
+                    else
+                    {
+                        matchedChars = 0;
+                    }
+                }
+            }
+        }
     }
 }

test/Kestrel.Transport.Libuv.FunctionalTests/Kestrel.Transport.Libuv.FunctionalTests.csproj

@@ -11,7 +11,6 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <Compile Include="..\Kestrel.Transport.FunctionalTests\**\*.cs" />
     <Compile Include="..\shared\*.cs" LinkBase="shared" />
     <Compile Include="..\shared\TransportTestHelpers\*.cs" LinkBase="shared\TransportTestHelpers" />
     <Content Include="..\shared\TestCertificates\*.pfx" LinkBase="shared\TestCertificates" CopyToOutputDirectory="PreserveNewest" />