Expand combinatorial testing of HTTP parser (#3166)

* add combinatorial tests for extra trivia and progressively typed code * improve comment parsing; more combinatorial testing * PR feedback * fix for leading request separator; CurrentToken => null instead of throw * remove redundant GetDefaultKernelName logic * update API baseline
dotnet · Sep 13, 2023 · c13d287 · c13d287
1 parent 34cc09c
commit c13d287
Show file tree

Hide file tree

Showing 16 changed files with 425 additions and 459 deletions.
diff --git a/...ive.ApiCompatibility.Tests/ApiCompatibilityTests.Document_api_is_not_changed.approved.txt b/...ive.ApiCompatibility.Tests/ApiCompatibilityTests.Document_api_is_not_changed.approved.txt
@@ -31,7 +31,6 @@ Microsoft.DotNet.Interactive.Documents
     public System.Collections.Generic.IAsyncEnumerable<InteractiveDocument> GetImportsAsync(System.Boolean recursive = False)
     public System.Collections.Generic.IEnumerable<InputField> GetInputFields()
     public System.Collections.Generic.IEnumerable<System.String> GetMagicCommandLines()
-    public System.Boolean TryGetKernelInfosFromMetadata(ref KernelInfoCollection& kernelInfos)
   public class InteractiveDocumentElement
     .ctor()
     .ctor(System.String contents = null, System.String kernelName = null, System.Collections.Generic.IEnumerable<InteractiveDocumentOutputElement> outputs = null)

diff --git a/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs b/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocument.cs
@@ -65,7 +65,7 @@ public async IAsyncEnumerable<InteractiveDocument> GetImportsAsync(bool recursiv
     {
         EnsureImportFieldParserIsInitialized();
 
-        if (!TryGetKernelInfosFromMetadata(out var kernelInfos))
+        if (!TryGetKernelInfosFromMetadata(Metadata, out var kernelInfos))
         {
             kernelInfos = new();
         }
@@ -198,9 +198,9 @@ public static async Task<InteractiveDocument> LoadAsync(
 
     public string? GetDefaultKernelName()
     {
-        if (TryGetKernelInfosFromMetadata(Metadata, out var kernelInfo))
+        if (TryGetKernelInfosFromMetadata(Metadata, out var kernelInfos))
         {
-            return kernelInfo.DefaultKernelName;
+            return kernelInfos.DefaultKernelName;
         }
 
         return null;
@@ -212,28 +212,8 @@ public static async Task<InteractiveDocument> LoadAsync(
         {
             return kernelInfoCollection.DefaultKernelName;
         }
-
-        if (Metadata.TryGetValue("kernelspec", out var kernelspecObj))
-        {
-            if (kernelspecObj is IDictionary<string, object> kernelspecDict)
-            {
-                if (kernelspecDict.TryGetValue("language", out var languageObj) &&
-                    languageObj is string defaultLanguage)
-                {
-                    return defaultLanguage;
-                }
-            }
-        }
-
-        if (kernelInfos.DefaultKernelName is { } defaultFromKernelInfos)
-        {
-            if (kernelInfos.TryGetByAlias(defaultFromKernelInfos, out var info))
-            {
-                return info.Name;
-            }
-        }
-
-        return null;
+
+        return kernelInfos.DefaultKernelName;
     }
 
     internal static void MergeKernelInfos(InteractiveDocument document, KernelInfoCollection kernelInfos)
@@ -259,10 +239,6 @@ internal static void MergeKernelInfos(KernelInfoCollection destination, KernelIn
         destination.AddRange(source.Where(ki => added.Add(ki.Name)));
     }
 
-    public bool TryGetKernelInfosFromMetadata(
-        [NotNullWhen(true)] out KernelInfoCollection? kernelInfos) =>
-        TryGetKernelInfosFromMetadata(Metadata, out kernelInfos);
-
     internal static bool TryGetKernelInfosFromMetadata(
         IDictionary<string, object>? metadata,
         [NotNullWhen(true)] out KernelInfoCollection? kernelInfos)

diff --git a/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocumentElement.cs b/src/Microsoft.DotNet.Interactive.Documents/InteractiveDocumentElement.cs
@@ -6,7 +6,7 @@
 
 namespace Microsoft.DotNet.Interactive.Documents;
 
-public class InteractiveDocumentElement
+public sealed class InteractiveDocumentElement
 {
     [JsonConstructor]
     public InteractiveDocumentElement()

diff --git a/src/Microsoft.DotNet.Interactive.Formatting/PlainTextSummaryFormatter.cs b/src/Microsoft.DotNet.Interactive.Formatting/PlainTextSummaryFormatter.cs
@@ -8,7 +8,6 @@ namespace Microsoft.DotNet.Interactive.Formatting;
 
 public static class PlainTextSummaryFormatter
 {
-    // FIX: (PlainTextSummaryFormatter) rename this
     public const string MimeType = "text/plain+summary";
 
     public static ITypeFormatter GetPreferredFormatterFor(Type type)

diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Body.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Body.cs
@@ -1,7 +1,7 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using System.Linq;
+using System;
 using FluentAssertions;
 using Microsoft.DotNet.Interactive.HttpRequest.Tests.Utility;
 using Xunit;
@@ -12,27 +12,6 @@ public partial class ParserTests
 {
     public class Body
     {
-        [Fact]
-        public void body_separator_is_present()
-        {
-            var result = Parse(
-                """
-            POST https://example.com/comments HTTP/1.1
-            Content-Type: application/xml
-            Authorization: token xxx
-
-            <request>
-                <name>sample</name>
-                <time>Wed, 21 Oct 2015 18:27:50 GMT</time>
-            </request>
-            """);
-
-            var requestNode = result.SyntaxTree.RootNode
-                                    .ChildNodes.Should().ContainSingle<HttpRequestNode>().Which;
-
-            requestNode.BodySeparatorNode.ChildTokens.First().Kind.Should().Be(HttpTokenKind.NewLine);
-        }
-
         [Fact]
         public void body_is_parsed_correctly_when_headers_are_not_present()
         {
@@ -87,5 +66,23 @@ public void multiple_new_lines_before_body_are_parsed_correctly()
                 </request>
                 """);
         }
+
+        [Fact]
+        public void Whitespace_after_headers_is_not_parsed_as_body()
+        {
+            var code = """
+
+                hptps://example.com 
+                Accept: */*
+
+
+
+
+                """;
+
+            var result = Parse(code);
+
+            result.SyntaxTree.RootNode.DescendantNodesAndTokens().Should().NotContain(n => n is HttpBodyNode);
+        }
     }
 }
diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Combinatorial.cs
@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using FluentAssertions;
@@ -13,6 +14,8 @@ namespace Microsoft.DotNet.Interactive.HttpRequest.Tests;
 
 public partial class ParserTests
 {
+    private readonly ITestOutputHelper _output;
+
     public class Combinatorial
     {
         private readonly ITestOutputHelper _output;
@@ -41,6 +44,25 @@ public void Valid_syntax_produces_expected_parse_tree_and_no_diagnostics(ISyntax
             syntaxSpec.Validate(parseResult.SyntaxTree.RootNode.ChildNodes.Single());
         }
 
+        [Theory]
+        [MemberData(nameof(GenerateValidRequestsWithExtraTrivia))]
+        public void Valid_syntax_with_extra_trivia_produces_expected_parse_tree_and_no_diagnostics(ISyntaxSpec syntaxSpec, int index)
+        {
+            var code = syntaxSpec.ToString();
+
+            var parseResult = HttpRequestParser.Parse(code);
+
+            _output.WriteLine($"""
+                === Generation #{index} ===
+
+                {code}
+                """);
+
+            parseResult.GetDiagnostics().Should().BeEmpty();
+
+            syntaxSpec.Validate(parseResult.SyntaxTree.RootNode.ChildNodes.Single());
+        }
+
         [Theory]
         [MemberData(nameof(GenerateInvalidRequests))]
         public void Invalid_syntax_produces_diagnostics(ISyntaxSpec syntaxSpec, int index)
@@ -59,41 +81,83 @@ public void Invalid_syntax_produces_diagnostics(ISyntaxSpec syntaxSpec, int inde
 
             syntaxSpec.Validate(parseResult.SyntaxTree.RootNode.ChildNodes.Single());
         }
-
+
+        [Theory]
+        [MemberData(nameof(GenerateValidRequestsWithExtraTrivia))]
+        public void Code_that_a_user_has_not_finished_typing_round_trips_correctly_and_does_not_throw(ISyntaxSpec syntaxSpec, int index)
+        {
+            var code = syntaxSpec.ToString();
+
+            for (var truncateAfter = 0; truncateAfter < code.Length; truncateAfter++)
+            {
+                var truncatedCode = code[..truncateAfter];
+
+                _output.WriteLine($"""
+                === Generation #{index} truncated after {truncateAfter} characters ===
+
+                {truncatedCode}
+                """);
+
+                Parse(truncatedCode);
+            }
+        }
+
         public static IEnumerable<object[]> GenerateValidRequests()
         {
-            var i = 0;
+            var generationNumber = 0;
 
             foreach (var method in ValidMethods())
             foreach (var url in ValidUrls())
             foreach (var version in ValidVersions())
             foreach (var headerSection in ValidHeaderSections())
             foreach (var bodySection in ValidBodySections())
             {
-                ++i;
+                ++generationNumber;
                 yield return new object[]
                 {
                     new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection),
-                    i
+                    generationNumber
+                };
+            }
+        }
+
+        public static IEnumerable<object[]> GenerateValidRequestsWithExtraTrivia()
+        {
+            var generationNumber = 0;
+
+            foreach (var method in ValidMethods())
+            foreach (var url in ValidUrls())
+            foreach (var version in ValidVersions())
+            foreach (var headerSection in ValidHeaderSections())
+            foreach (var bodySection in ValidBodySections())
+            {
+                ++generationNumber;
+                yield return new object[]
+                {
+                    new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection)
+                    {
+                        ExtraTriviaRandomizer = new Random(1)
+                    },
+                    generationNumber
                 };
             }
         }
 
         public static IEnumerable<object[]> GenerateInvalidRequests()
         {
-            var i = 0;
+            var generationNumber = 0;
 
             foreach (var method in InvalidMethods())
             foreach (var url in ValidUrls())
             foreach (var version in ValidVersions())
             foreach (var headerSection in ValidHeaderSections())
             foreach (var bodySection in ValidBodySections())
             {
-                ++i;
+                ++generationNumber;
                 yield return new object[]
                 {
                     new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection),
-                    i
+                    generationNumber
                 };
             }
 
@@ -103,11 +167,11 @@ public static IEnumerable<object[]> GenerateInvalidRequests()
             foreach (var headerSection in ValidHeaderSections())
             foreach (var bodySection in ValidBodySections())
             {
-                ++i;
+                ++generationNumber;
                 yield return new object[]
                 {
                     new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection),
-                    i
+                    generationNumber
                 };
             }
 
@@ -117,11 +181,11 @@ public static IEnumerable<object[]> GenerateInvalidRequests()
             foreach (var headerSection in ValidHeaderSections())
             foreach (var bodySection in ValidBodySections())
             {
-                ++i;
+                ++generationNumber;
                 yield return new object[]
                 {
                     new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection),
-                    i
+                    generationNumber
                 };
             }
 
@@ -131,11 +195,11 @@ public static IEnumerable<object[]> GenerateInvalidRequests()
             foreach (var headerSection in InvalidHeaderSections())
             foreach (var bodySection in ValidBodySections())
             {
-                ++i;
+                ++generationNumber;
                 yield return new object[]
                 {
                     new HttpRequestNodeSyntaxSpec(method, url, version, headerSection, bodySection),
-                    i
+                    generationNumber
                 };
             }
         }

diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Headers.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.Headers.cs
@@ -1,6 +1,7 @@
-// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
+using System;
 using System.Linq;
 using FluentAssertions;
 using Microsoft.DotNet.Interactive.HttpRequest.Tests.Utility;
@@ -74,6 +75,25 @@ public void header_separator_is_parsed()
                   .Which.Text.Should().Be(":");
         }
 
+        [Fact]
+        public void Comments_can_precede_headers()
+        {
+            var code = """
+                POST https://example.com
+                # this is a comment
+                Accept: */*
+                Accept-Encoding: gzip, deflate, br
+                """;
+
+            var result = Parse(code);
+
+            var requestNode = result.SyntaxTree.RootNode.DescendantNodesAndTokens()
+                                    .Should().ContainSingle<HttpRequestNode>().Which;
+
+            requestNode.DescendantNodesAndTokens().Should().ContainSingle<HttpCommentNode>().Which.Text.Should().Be("# this is a comment");
+            requestNode.ChildNodes.Should().ContainSingle<HttpHeadersNode>();
+        }
+
         [Fact]
         public void headers_are_parsed_correctly()
         {

diff --git a/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.RequestSeparator.cs b/src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.RequestSeparator.cs
@@ -0,0 +1,31 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using FluentAssertions;
+using Microsoft.DotNet.Interactive.HttpRequest.Tests.Utility;
+using Xunit;
+
+namespace Microsoft.DotNet.Interactive.HttpRequest.Tests;
+
+public partial class ParserTests
+{
+    public class RequestSeparator
+    {
+
+        [Fact]
+        public void request_separator_at_start_of_request_is_valid()
+        {
+            var code = """
+                ###
+                GET https://example.com
+                """;
+
+            var result = Parse(code);
+
+            result.SyntaxTree.RootNode.ChildNodes
+                  .Should().ContainSingle<HttpRequestNode>()
+                  .Which.UrlNode.Text.Should().Be("https://example.com");
+        }
+
+    }
+}