From 62867c6e28548157ab8c60318591d3e26838dda8 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 5 Nov 2026 12:26:24 +0100 Subject: [PATCH 1/9] Wire Nullean.Snapshots into every test project The ApiExplorer pilot (#4654) used Nullean.Snapshots.Html and .TUnit 0.2.0 in one project. This makes the library available to all unit and integration test projects at 0.4.0 and adds the shared pieces the next migration steps build on: - tests-shared/Elastic.Documentation.Testing: DocsHtml (page section without the fixture H1 and htmx attributes), DiagnosticsSnapshot, NavigationDump, MockFileSystemDump. A scrubber for temp and repo paths is compiled into each test project. - The snap CLI as a local tool; CI uploads received snapshots when a test run fails; the build fails on orphaned snapshots after a unit run. - AGENTS.md describes the workflow. The PR review prompt no longer claims the repo uses xUnit. The five TestDiagnosticsCollector copies stay: they differ in lifecycle (Write vs HandleItem), so merging them is a behaviour change. DiagnosticsSnapshot works on any of them. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/ci.yml | 9 +++ .github/workflows/pr-review.yml | 2 +- AGENTS.md | 6 ++ Directory.Packages.props | 8 ++- build/Targets.fs | 6 ++ docs-builder.slnx | 4 ++ dotnet-tools.json | 7 ++ tests-integration/Directory.Build.props | 7 ++ tests-shared/Directory.Build.props | 11 +++ .../DiagnosticsSnapshot.cs | 32 +++++++++ .../Elastic.Documentation.Testing/DocsHtml.cs | 34 +++++++++ .../Elastic.Documentation.Testing.csproj | 28 ++++++++ .../Linked/SnapshotScrubbing.cs | 39 +++++++++++ .../MockFileSystemDump.cs | 35 ++++++++++ .../NavigationDump.cs | 51 ++++++++++++++ tests/Directory.Build.props | 7 ++ .../Elastic.ApiExplorer.Tests.csproj | 2 - .../Testing/SnapshotHelpersTests.cs | 70 +++++++++++++++++++ 18 files changed, 353 insertions(+), 5 deletions(-) create mode 100644 tests-shared/Directory.Build.props create mode 100644 tests-shared/Elastic.Documentation.Testing/DiagnosticsSnapshot.cs create mode 100644 tests-shared/Elastic.Documentation.Testing/DocsHtml.cs create mode 100644 tests-shared/Elastic.Documentation.Testing/Elastic.Documentation.Testing.csproj create mode 100644 tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs create mode 100644 tests-shared/Elastic.Documentation.Testing/MockFileSystemDump.cs create mode 100644 tests-shared/Elastic.Documentation.Testing/NavigationDump.cs create mode 100644 tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 50c34ac2c2..7d4066e86c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -203,6 +203,15 @@ jobs: # --single-target skips the compile dependency; the Compile step above already built everything run: dotnet run --project build -c release -- unit-test --single-target + - name: Upload received snapshots + # A failed snapshot test leaves its received file here. Download it into .artifacts/.snapshots/received/ and run `dotnet snap review`. + if: ${{ failure() && needs.changes.outputs.code == 'true' }} + uses: actions/upload-artifact@v7 + with: + name: received-snapshots-${{ matrix.os }} + path: .artifacts/.snapshots/received/ + if-no-files-found: ignore + - name: Publish AOT if: ${{ needs.changes.outputs.code == 'true' && (github.event_name == 'push' && matrix.os != 'ubuntu-latest') }} # publish containers already validates AOT build run: dotnet run --project build -c release -- publishbinaries diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index 3c884acdc3..314cf54941 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -27,7 +27,7 @@ jobs: validates cross-repo references, and ships as native AOT binaries for CI. ## Language and frameworks - - C# (.NET) with xUnit v3 + AwesomeAssertions for tests. Do NOT suggest TUnit APIs — the migration hasn't happened yet. + - C# (.NET) with TUnit + AwesomeAssertions for tests. Expected output lives in Nullean.Snapshots snapshots (inline literals and `.snapshots/` files); never suggest accepting snapshots in code or CI. - TypeScript/React frontend under src/Elastic.Documentation.Site/. - Test naming convention: Method_Scenario_Expected. diff --git a/AGENTS.md b/AGENTS.md index 0f31e021e8..feb01f4633 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,6 +65,12 @@ Tests live in `tests/` (unit) and `tests-integration/` (integration). - **C#**: TUnit · AwesomeAssertions · FakeItEasy across all test projects. All test projects (unit and integration) use TUnit (wired centrally via `tests/Directory.Build.props` and `tests-integration/Directory.Build.props`; no per-project flag needed). Write new tests using TUnit APIs: `[Test]`, `[Arguments(...)]`, `[MethodDataSource(nameof(X))]`. - **TypeScript**: Vitest +- **Snapshots** (`Nullean.Snapshots`, wired for every test project in `tests/Directory.Build.props`; helpers in `tests-shared/Elastic.Documentation.Testing`): expected output lives in snapshots, not in chains of `Contain` assertions. + - Small values go inline: `x.Should().MatchInlineSnapshot("""...""")`, `MatchInlineHtmlSnapshot`, `MatchInlineJsonSnapshot`. Leave the argument out to write a new one. Large values go to a verified file: `MatchSnapshot()`, `MatchHtmlSnapshot()`, `MatchJsonSnapshot()`, `MatchYamlSnapshot()`. + - Diagnostics: `collector.Diagnostics.ToSnapshotText()` (`DiagnosticsSnapshot`). A clean run is the empty snapshot. + - Navigation trees and generated files: `NavigationDump.ToText(...)`, `MockFileSystemDump.ToText(fs, dir)`. Rendered docs HTML: `DocsHtml.Section()` (the page content, no fixture H1, no htmx attributes) or `DocsHtml.Body()`. + - Accept with the `snap` tool (`dotnet snap status`, `dotnet snap review`, `dotnet snap accept `), or `SNAPSHOT_ACCEPT=1`. Read the diff of the changed `.verified.*` files and inline literals before committing. Nothing is accepted in CI. When a CI test run fails, download the `received-snapshots-*` artifact into `.artifacts/.snapshots/received/` and run `dotnet snap review`. + - Changing a snapshot test's output is a behaviour change. Mutate the code once and confirm the test fails before you commit. - **HTML snapshots** (`Nullean.Snapshots.Html`, used in `Elastic.ApiExplorer.Tests`): when a test would check one rendered block with many `Contain` assertions, compare it to a verified snapshot instead and keep one assertion that states the intent. - Write `html.Should().MatchHtmlSnapshot()`. Narrow it with `new HtmlSnapshotOptions().Select("#some-id")` or `.RemoveAttributes(...)` so unrelated markup and volatile values (bundle names, CI git info) stay out. - Snapshot a partial or one section, not a whole page. Whole-page snapshots break on unrelated changes. diff --git a/Directory.Packages.props b/Directory.Packages.props index 782c52aab0..b1dcf0d2c0 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -127,8 +127,12 @@ - - + + + + + + diff --git a/build/Targets.fs b/build/Targets.fs index a73833190f..432a81e056 100644 --- a/build/Targets.fs +++ b/build/Targets.fs @@ -205,6 +205,12 @@ let private runTests (testSuite: TestSuite) (parsed: ParseResults) = |> Array.toList if not (List.isEmpty failures) then failwithf "Test projects failed:\n%s" (String.concat "\n" failures) + + // Every test project has now run, so the run manifests are complete: a verified snapshot nobody used belongs to a deleted or renamed test. + if testSuite <> Integration then + match exec { exit_code_of "dotnet" "snap" "orphans" } with + | 0 -> () + | _ -> failwith "Orphaned snapshots found. Delete them with `dotnet snap orphans --delete` and commit the removal." let private compressibleExtensions = set [".html"; ".css"; ".js"; ".json"; ".svg"; ".xml"; ".txt"] diff --git a/docs-builder.slnx b/docs-builder.slnx index 47468e9ede..31e5d66687 100644 --- a/docs-builder.slnx +++ b/docs-builder.slnx @@ -90,6 +90,10 @@ + + + + diff --git a/dotnet-tools.json b/dotnet-tools.json index 084330e010..d77fdfb621 100644 --- a/dotnet-tools.json +++ b/dotnet-tools.json @@ -43,6 +43,13 @@ "curb" ], "rollForward": false + }, + "nullean.snapshots.cli": { + "version": "0.4.0", + "commands": [ + "snap" + ], + "rollForward": false } } } diff --git a/tests-integration/Directory.Build.props b/tests-integration/Directory.Build.props index 379438ea3e..1717682543 100644 --- a/tests-integration/Directory.Build.props +++ b/tests-integration/Directory.Build.props @@ -18,6 +18,13 @@ + + + + + + + diff --git a/tests-shared/Directory.Build.props b/tests-shared/Directory.Build.props new file mode 100644 index 0000000000..5b92d69384 --- /dev/null +++ b/tests-shared/Directory.Build.props @@ -0,0 +1,11 @@ + + + + + + CA1822;CA1707;IDE0058 + false + false + + diff --git a/tests-shared/Elastic.Documentation.Testing/DiagnosticsSnapshot.cs b/tests-shared/Elastic.Documentation.Testing/DiagnosticsSnapshot.cs new file mode 100644 index 0000000000..0e8f94f1f2 --- /dev/null +++ b/tests-shared/Elastic.Documentation.Testing/DiagnosticsSnapshot.cs @@ -0,0 +1,32 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Text; +using Elastic.Documentation.Diagnostics; + +namespace Elastic.Documentation.Testing; + +public static class DiagnosticsSnapshot +{ + /// + /// One line per diagnostic, error|warning|hint file:line message, in the order they were written. Empty when + /// there are none, so a clean run reads as MatchInlineSnapshot(""). + /// + public static string ToSnapshotText(this IEnumerable diagnostics) + { + var sb = new StringBuilder(); + foreach (var d in diagnostics) + { + var severity = d.Severity switch + { + Severity.Error => "error", + Severity.Warning => "warning", + _ => "hint" + }; + var position = d.Line is { } line ? $"{d.File}:{line}" : d.File; + sb.Append(severity).Append(' ').Append(position).Append(' ').Append(d.Message).Append('\n'); + } + return sb.ToString(); + } +} diff --git a/tests-shared/Elastic.Documentation.Testing/DocsHtml.cs b/tests-shared/Elastic.Documentation.Testing/DocsHtml.cs new file mode 100644 index 0000000000..961b026997 --- /dev/null +++ b/tests-shared/Elastic.Documentation.Testing/DocsHtml.cs @@ -0,0 +1,34 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using Nullean.Snapshots.Html; + +namespace Elastic.Documentation.Testing; + +/// +/// How docs-builder's tests normalise rendered HTML before comparing or snapshotting it. One place for what +/// Elastic.Markdown.Tests.PrettyHtmlExtensions and Elastic.Authoring.Tests.Framework.HtmlAssertions +/// used to do separately. +/// +public static class DocsHtml +{ + /// htmx navigation attributes on links are plumbing, not content. + private static readonly string[] HtmxAttributes = ["hx-get", "hx-select-oob", "hx-swap", "hx-indicator", "hx-push-url", "preload"]; + + /// The whole body, with htmx attributes removed from links. + public static HtmlSnapshotOptions Body() => new HtmlSnapshotOptions().RemoveAttributesFrom("a", HtmxAttributes); + + /// + /// The rendered page's content section, without the leading H1 the test fixtures wrap every document in + /// (# Test Document), and with htmx attributes removed from links. + /// + public static HtmlSnapshotOptions Section() => + Body() + .Select("section#elastic-docs-v3") + .Transform(root => + { + if (root.FirstElementChild is { TagName: "H1" } h1) + h1.Remove(); + }); +} diff --git a/tests-shared/Elastic.Documentation.Testing/Elastic.Documentation.Testing.csproj b/tests-shared/Elastic.Documentation.Testing/Elastic.Documentation.Testing.csproj new file mode 100644 index 0000000000..ab56af73cd --- /dev/null +++ b/tests-shared/Elastic.Documentation.Testing/Elastic.Documentation.Testing.csproj @@ -0,0 +1,28 @@ + + + + net10.0 + Elastic.Documentation.Testing + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs b/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs new file mode 100644 index 0000000000..6a80bb71fc --- /dev/null +++ b/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs @@ -0,0 +1,39 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; +using Nullean.Snapshots; + +namespace Elastic.Documentation.Testing.Linked; + +/// +/// Scrubbers every snapshot in docs-builder's tests gets: machine-specific paths and git SHAs never belong in a +/// verified file. Runs when any test project referencing this assembly loads it. +/// +internal static class SnapshotScrubbing +{ +#pragma warning disable CA2255 // compiled into each test project, which is application code + [ModuleInitializer] +#pragma warning restore CA2255 + internal static void Register() + { + SnapshotDefaults.AddScrubber( + Scrubbers.Regex( + new Regex(Regex.Escape(Path.GetTempPath().TrimEnd(Path.DirectorySeparatorChar)) + @"[^\s""'<>]*", RegexOptions.Compiled), + "{temp}" + ) + ); + if (FindSolutionRoot() is { } root) + SnapshotDefaults.AddScrubber(Scrubbers.Regex(new Regex(Regex.Escape(root), RegexOptions.Compiled), "{repo}")); + } + + private static string? FindSolutionRoot() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "docs-builder.slnx"))) + dir = dir.Parent; + return dir?.FullName.TrimEnd(Path.DirectorySeparatorChar); + } +} diff --git a/tests-shared/Elastic.Documentation.Testing/MockFileSystemDump.cs b/tests-shared/Elastic.Documentation.Testing/MockFileSystemDump.cs new file mode 100644 index 0000000000..daaf2d8ed6 --- /dev/null +++ b/tests-shared/Elastic.Documentation.Testing/MockFileSystemDump.cs @@ -0,0 +1,35 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.IO.Abstractions; +using System.Text; + +namespace Elastic.Documentation.Testing; + +public static class MockFileSystemDump +{ + /// + /// Every file under , sorted by path, each under an === relative/path === + /// header. Meant for tests that read generated files back from a mock file system. + /// + public static string ToText(IFileSystem fileSystem, string directory, string searchPattern = "*") + { + var root = fileSystem.Path.GetFullPath(directory); + var sb = new StringBuilder(); + var files = fileSystem + .Directory + .EnumerateFiles(root, searchPattern, SearchOption.AllDirectories) + .Select(f => (Full: f, Relative: fileSystem.Path.GetRelativePath(root, f).Replace('\\', '/'))) + .OrderBy(f => f.Relative, StringComparer.Ordinal); + foreach (var (full, relative) in files) + { + sb.Append("=== ").Append(relative).Append(" ===\n"); + var content = fileSystem.File.ReadAllText(full).Replace("\r\n", "\n"); + sb.Append(content); + if (!content.EndsWith('\n')) + sb.Append('\n'); + } + return sb.ToString(); + } +} diff --git a/tests-shared/Elastic.Documentation.Testing/NavigationDump.cs b/tests-shared/Elastic.Documentation.Testing/NavigationDump.cs new file mode 100644 index 0000000000..a70dfa66f6 --- /dev/null +++ b/tests-shared/Elastic.Documentation.Testing/NavigationDump.cs @@ -0,0 +1,51 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Text; +using Elastic.Documentation.Navigation; + +namespace Elastic.Documentation.Testing; + +public static class NavigationDump +{ + /// + /// The navigation tree as indented text, one node per line: Type url "title", with [hidden] and + /// [island] flags. A node lists its index page first (index: Type url "title"), then its children. The + /// index page is where most of the interesting URLs live. A child whose Parent is not the node it is listed + /// under is flagged [wrong parent]; index pages are not checked, because a virtual file's index has a + /// different parent. + /// + public static string ToText(INavigationItem root) + { + var sb = new StringBuilder(); + Write(sb, root, 0, null, null); + return sb.ToString(); + } + + private static void Write(StringBuilder sb, INavigationItem item, int depth, string? label, INavigationItem? container) + { + sb + .Append(' ', depth * 2) + .Append(label) + .Append(item.GetType().Name.Split('`')[0]) + .Append(" ") + .Append(item.Url) + .Append(" \"") + .Append(item.NavigationTitle) + .Append('"'); + if (item.Hidden) + sb.Append(" [hidden]"); + if (item.IsIsland) + sb.Append(" [island]"); + if (container is not null && !ReferenceEquals(item.Parent, container)) + sb.Append(" [wrong parent]"); + sb.Append('\n'); + + if (item is not INodeNavigationItem node) + return; + Write(sb, node.Index, depth + 1, "index: ", null); + foreach (var child in node.NavigationItems) + Write(sb, child, depth + 1, null, item); + } +} diff --git a/tests/Directory.Build.props b/tests/Directory.Build.props index 6ce509c511..b3667da5e1 100644 --- a/tests/Directory.Build.props +++ b/tests/Directory.Build.props @@ -27,5 +27,12 @@ + + + + + + + diff --git a/tests/Elastic.ApiExplorer.Tests/Elastic.ApiExplorer.Tests.csproj b/tests/Elastic.ApiExplorer.Tests/Elastic.ApiExplorer.Tests.csproj index b6138fd950..17b5bb72f8 100644 --- a/tests/Elastic.ApiExplorer.Tests/Elastic.ApiExplorer.Tests.csproj +++ b/tests/Elastic.ApiExplorer.Tests/Elastic.ApiExplorer.Tests.csproj @@ -12,8 +12,6 @@ - - diff --git a/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs b/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs new file mode 100644 index 0000000000..5897c67b4a --- /dev/null +++ b/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs @@ -0,0 +1,70 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.IO.Abstractions.TestingHelpers; +using AwesomeAssertions; +using Elastic.Documentation.Diagnostics; +using Elastic.Documentation.Testing; +using Nullean.Snapshots; +using Nullean.Snapshots.Html; + +namespace Elastic.Documentation.Tests.Testing; + +public class SnapshotHelpersTests +{ + [Test] + public void ToSnapshotText_NoDiagnostics_IsEmpty() => new List().ToSnapshotText().Should().MatchInlineSnapshot(""); + + [Test] + public void ToSnapshotText_MixedSeverities_OneLineEachInOrder() + { + var diagnostics = new List + { + new() { Severity = Severity.Error, File = "a.md", Line = 3, Message = "bad link" }, + new() { Severity = Severity.Warning, File = "b.md", Message = "no line" }, + new() { Severity = Severity.Hint, File = "c.md", Line = 1, Message = "consider" } + }; + diagnostics + .ToSnapshotText() + .Should() + .MatchInlineSnapshot( + """ + error a.md:3 bad link + warning b.md no line + hint c.md:1 consider + """ + ); + } + + [Test] + public void MockFileSystemDump_SortsByPathAndNormalisesLineEndings() + { + var fs = new MockFileSystem(); + fs.AddFile("/out/b/two.md", new MockFileData("two\r\nlines")); + fs.AddFile("/out/a.md", new MockFileData("one\n")); + MockFileSystemDump + .ToText(fs, "/out") + .Should() + .MatchInlineSnapshot( + """ + === a.md === + one + === b/two.md === + two + lines + """ + ); + } + + [Test] + public void DocsHtmlSection_DropsFixtureHeadingAndHtmxAttributes() => + """ +

Test Document

x

nav
+ """.Should().MatchInlineHtmlSnapshot( + """ +

x

+ """, + DocsHtml.Section() + ); +} From a9fd42330390df7a7c5f9ced0b4102e1d1f7f9fa Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 5 Nov 2026 12:29:06 +0100 Subject: [PATCH 2/9] Compare rendered HTML with Nullean.Snapshots.Html ShouldBeHtml and ShouldContainHtml (Elastic.Markdown.Tests) and ConvertsToHtml, ConvertsToContainingHtml and the per-file ToHtml and ContainsHtml (Elastic.Authoring.Tests) keep their names and call sites, and now run on BeHtml and ContainHtml with the shared DocsHtml options. That removes the second copy of the AngleSharp.Diffing diff code and the AngleSharp.Diffing package. DiffPlex stays in Authoring for the plain text assertions. One test passed without testing anything. "marks exactly two options as a fork" gave the bare text `is-fork` to ConvertsToContainingHtml. The old normaliser printed only element children, so a text-only fragment became an empty string and every page contains an empty string. It now uses ConvertsToContainingRawHtml, like the test above it. Checked by corrupting one ShouldContainHtml and one ConvertsToHtml expectation: both fail. Co-Authored-By: Claude Sonnet 5.5 --- Directory.Packages.props | 1 - .../Blocks/Hub/GetStarted.cs | 2 +- .../Elastic.Authoring.Tests.csproj | 1 - .../Framework/HtmlAssertions.cs | 177 ++---------------- .../Elastic.Markdown.Tests.csproj | 2 - .../PrettyHtmlExtensions.cs | 127 +------------ 6 files changed, 22 insertions(+), 288 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index b1dcf0d2c0..0fdb1d5ee6 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -126,7 +126,6 @@ - diff --git a/tests/Elastic.Authoring.Tests/Blocks/Hub/GetStarted.cs b/tests/Elastic.Authoring.Tests/Blocks/Hub/GetStarted.cs index dec67a93fb..d0ccf282e0 100644 --- a/tests/Elastic.Authoring.Tests/Blocks/Hub/GetStarted.cs +++ b/tests/Elastic.Authoring.Tests/Blocks/Hub/GetStarted.cs @@ -164,7 +164,7 @@ public async Task PutsTheFirstStepInItsOwnBox() => await Docs.ConvertsToContainingRawHtml("""
  • """); [Test, DisplayName("marks exactly two options as a fork")] - public async Task MarksExactlyTwoOptionsAsAFork() => await Docs.ConvertsToContainingHtml("""is-fork"""); + public async Task MarksExactlyTwoOptionsAsAFork() => await Docs.ConvertsToContainingRawHtml("""is-fork"""); [Test, DisplayName("has no errors")] public async Task HasNoErrors() => await Docs.HasNoErrors(); diff --git a/tests/Elastic.Authoring.Tests/Elastic.Authoring.Tests.csproj b/tests/Elastic.Authoring.Tests/Elastic.Authoring.Tests.csproj index ee73b53ebd..4e5e05627f 100644 --- a/tests/Elastic.Authoring.Tests/Elastic.Authoring.Tests.csproj +++ b/tests/Elastic.Authoring.Tests/Elastic.Authoring.Tests.csproj @@ -5,7 +5,6 @@ - diff --git a/tests/Elastic.Authoring.Tests/Framework/HtmlAssertions.cs b/tests/Elastic.Authoring.Tests/Framework/HtmlAssertions.cs index 2bf599f62b..7fbebbdf03 100644 --- a/tests/Elastic.Authoring.Tests/Framework/HtmlAssertions.cs +++ b/tests/Elastic.Authoring.Tests/Framework/HtmlAssertions.cs @@ -3,13 +3,12 @@ // See the LICENSE file in the project root for more information using System.Diagnostics; -using AngleSharp.Diffing; -using AngleSharp.Diffing.Core; -using AngleSharp.Html; -using AngleSharp.Html.Parser; +using AwesomeAssertions; using DiffPlex.DiffBuilder; using DiffPlex.DiffBuilder.Model; +using Elastic.Documentation.Testing; using JetBrains.Annotations; +using Nullean.Snapshots.Html; namespace Elastic.Authoring.Tests.Framework; @@ -27,8 +26,7 @@ namespace Elastic.Authoring.Tests.Framework; /// would appear verbatim in every assertion snapshot otherwise. /// /// -/// Produces a humanized diff via that names the diff type -/// (NodeDiff / AttrDiff / MissingNodeDiff etc.) and shows expected vs. actual node text. +/// Compares through Nullean.Snapshots.Html, which reports a structural diff and then a line diff. /// /// /// All assertion methods are synchronous — the async wrapper lives on . @@ -73,173 +71,28 @@ internal static string Diff(string expected, string actual) } // ──────────────────────────────────────────────────────────────────────────────── - // AngleSharp deep HTML diff + // HTML comparison on Nullean.Snapshots.Html // ──────────────────────────────────────────────────────────────────────────────── - internal static string HtmlDiffString(IEnumerable diffs) - { - var formatter = new PrettyMarkupFormatter(); - - string NodeText(ComparisonSource source) - { - using var sw = new StringWriter(); - source.Node.ToHtml(sw, formatter); - return sw.ToString(); - } - - string AttrText(AttributeComparisonSource source) - { - using var sw = new StringWriter(); - source.Attribute.ToHtml(sw, formatter); - return sw.ToString(); - } - - // NodeDiffText and AttrDiffText accept non-nullable (callers pass .Control / .Test directly). - // For Missing*/Unexpected* cases we pass a sentinel "missing" string instead of calling these helpers. - string NodeDiffText(ComparisonSource control, ComparisonSource test) => - $"\nexpected: {NodeText(control)}\nactual: {NodeText(test)}\n"; - - string MissingNodeText(ComparisonSource control) => $"\nexpected: {NodeText(control)}\nactual: missing\n"; - - string UnexpectedNodeText(ComparisonSource test) => $"\nexpected: missing\nactual: {NodeText(test)}\n"; - - string AttrDiffText(AttributeComparisonSource control, AttributeComparisonSource test) => - $"\nexpected: {AttrText(control)}\nactual: {AttrText(test)}\n"; - - string MissingAttrText(AttributeComparisonSource control) => $"\nexpected: {AttrText(control)}\nactual: missing\n"; - - string UnexpectedAttrText(AttributeComparisonSource test) => $"\nexpected: missing\nactual: {AttrText(test)}\n"; - - static string NodeName(ComparisonSource source) => source.Node.NodeType.ToString().ToLowerInvariant(); - - return string.Join('\n', diffs.Select(d => - { - var label = d switch - { - NodeDiff nd when nd.Target == DiffTarget.Text && nd.Control.Path.Equals(nd.Test.Path, StringComparison.Ordinal) => - $"The text in {nd.Control.Path} is different.", - NodeDiff nd when nd.Target == DiffTarget.Text => - $"The expected {NodeName(nd.Control)} at {nd.Control.Path} and the actual {NodeName(nd.Test)} at {nd.Test.Path} is different.", - NodeDiff nd when nd.Control.Path.Equals(nd.Test.Path, StringComparison.Ordinal) => - $"The {NodeName(nd.Control)}s at {nd.Control.Path} are different.", - NodeDiff nd => - $"The expected {NodeName(nd.Control)} at {nd.Control.Path} and the actual {NodeName(nd.Test)} at {nd.Test.Path} are different.", - AttrDiff ad when ad.Control.Path.Equals(ad.Test.Path, StringComparison.Ordinal) => - $"The values of the attributes at {ad.Control.Path} are different.", - AttrDiff ad => $"The value of the attribute {ad.Control.Path} and actual attribute {ad.Test.Path} are different.", - MissingNodeDiff md => $"The {NodeName(md.Control)} at {md.Control.Path} is missing.", - MissingAttrDiff ma => $"The attribute at {ma.Control.Path} is missing.", - UnexpectedNodeDiff ud => $"The {NodeName(ud.Test)} at {ud.Test.Path} was not expected.", - UnexpectedAttrDiff ua => $"The attribute at {ua.Test.Path} was not expected.", - _ => throw new InvalidOperationException($"Unknown diff type detected: {d.GetType()}") - }; - - var detail = d switch - { - NodeDiff nd => NodeDiffText(nd.Control, nd.Test), - AttrDiff ad => AttrDiffText(ad.Control, ad.Test), - MissingNodeDiff md => MissingNodeText(md.Control), - MissingAttrDiff ma => MissingAttrText(ma.Control), - UnexpectedNodeDiff ud => UnexpectedNodeText(ud.Test), - UnexpectedAttrDiff ua => UnexpectedAttrText(ua.Test), - _ => throw new InvalidOperationException($"Unknown diff type detected: {d.GetType()}") - }; - - return label + detail; - })); - } - - // ──────────────────────────────────────────────────────────────────────────────── - // Pretty printer — matches the F# prettyHtml behaviour exactly - // ──────────────────────────────────────────────────────────────────────────────── - - private static string PrettyHtml(string html, string? querySelector) - { - var parser = new HtmlParser(); - var document = parser.ParseDocument(html); - var element = querySelector is null ? document.Body! : document.QuerySelector(querySelector)!; - - // Strip HTMX attributes from links — they are navigation artifacts, not content. - foreach (var link in element.QuerySelectorAll("a")) - { - link.RemoveAttribute("hx-get"); - link.RemoveAttribute("hx-select-oob"); - link.RemoveAttribute("hx-swap"); - link.RemoveAttribute("hx-indicator"); - link.RemoveAttribute("hx-push-url"); - link.RemoveAttribute("preload"); - } - - var formatter = new PrettyMarkupFormatter(); - using var sw = new StringWriter(); - - // Skip the first child if it is an H1 — the fixture wraps Setup.Markdown content in one. - var children = element.Children.ToList(); - for (var i = 0; i < children.Count; i++) - { - if (i == 0 && children[i].TagName == "H1") - continue; - children[i].ToHtml(sw, formatter); - } - - return sw.ToString().TrimStart('\n'); - } - - private static void CreateDiff(string expected, string actual) - { - // Identical pretty-printed HTML has no structural differences, so skip the AngleSharp comparison for the usual passing case. - if (string.Equals(expected, actual, StringComparison.Ordinal)) - return; - - var diffs = DiffBuilder.Compare(expected).WithTest(actual).Build(); - - var deepComparison = HtmlDiffString(diffs); - if (string.IsNullOrEmpty(deepComparison)) - return; - - var textDiff = Diff(expected, actual); - throw new AwesomeAssertions.Execution.AssertionFailedException( - $""" -Html was not equal --- DIFF -- -{textDiff} + /// + /// The rendered page's content section, minus the fixture's leading H1 and the htmx attributes on links. + /// The expected fragment is wrapped in the same section so both sides go through identical normalisation. + /// + private static HtmlSnapshotOptions Options => DocsHtml.Section(); --- Comparison -- -{deepComparison} -""" - ); - } + private static string InSection(string fragment) => $"
    {fragment}
    "; // ──────────────────────────────────────────────────────────────────────────────── // Per-file assertions (called from MarkdownResultTask / Scenario) // ──────────────────────────────────────────────────────────────────────────────── [DebuggerStepThrough] - internal static void ToHtml([LanguageInjection("html")] string expected, MarkdownResult actual) - { - var expectedHtml = PrettyHtml(expected, null); - var actualHtml = PrettyHtml(actual.Html, "section#elastic-docs-v3"); - CreateDiff(expectedHtml, actualHtml); - } + internal static void ToHtml([LanguageInjection("html")] string expected, MarkdownResult actual) => + actual.Html.Should().BeHtml(InSection(expected), Options); [DebuggerStepThrough] - internal static void ContainsHtml([LanguageInjection("html")] string expected, MarkdownResult actual) - { - var prettyExpected = PrettyHtml(expected, null); - var prettyActual = PrettyHtml(actual.Html, "section#elastic-docs-v3"); - - if (!prettyActual.Contains(prettyExpected, StringComparison.Ordinal)) - throw new AwesomeAssertions.Execution.AssertionFailedException( - $""" -Expected html to contain: -{prettyExpected} - -But was not found in: - -{prettyActual} -""" - ); - } + internal static void ContainsHtml([LanguageInjection("html")] string expected, MarkdownResult actual) => + actual.Html.Should().ContainHtml(InSection(expected), Options); [DebuggerStepThrough] internal static void ContainsRawHtml(string expected, MarkdownResult actual) diff --git a/tests/Elastic.Markdown.Tests/Elastic.Markdown.Tests.csproj b/tests/Elastic.Markdown.Tests/Elastic.Markdown.Tests.csproj index a128c14af7..e791376c2a 100644 --- a/tests/Elastic.Markdown.Tests/Elastic.Markdown.Tests.csproj +++ b/tests/Elastic.Markdown.Tests/Elastic.Markdown.Tests.csproj @@ -5,8 +5,6 @@ - - diff --git a/tests/Elastic.Markdown.Tests/PrettyHtmlExtensions.cs b/tests/Elastic.Markdown.Tests/PrettyHtmlExtensions.cs index eb552208b7..29cd3b7467 100644 --- a/tests/Elastic.Markdown.Tests/PrettyHtmlExtensions.cs +++ b/tests/Elastic.Markdown.Tests/PrettyHtmlExtensions.cs @@ -2,142 +2,27 @@ // Elasticsearch B.V licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information -using AngleSharp.Diffing; -using AngleSharp.Html; -using AngleSharp.Html.Parser; using AwesomeAssertions; -using AwesomeAssertions.Execution; -using DiffPlex.DiffBuilder; -using DiffPlex.DiffBuilder.Model; +using Elastic.Documentation.Testing; using JetBrains.Annotations; +using Nullean.Snapshots.Html; namespace Elastic.Markdown.Tests; +/// HTML comparisons on Nullean.Snapshots.Html: structural, so attribute order and whitespace never fail a test. public static class PrettyHtmlExtensions { - public static string PrettyHtml([LanguageInjection("html")] this string html, bool sanitize = true) - { - var parser = new HtmlParser(); - - var document = parser.ParseDocument(html); - var element = document.Body; - if (element is null) - return string.Empty; - - if (sanitize) - { - var links = element.QuerySelectorAll("a"); - foreach (var l in links) - { - l.RemoveAttribute("hx-get"); - l.RemoveAttribute("hx-select-oob"); - l.RemoveAttribute("hx-swap"); - l.RemoveAttribute("hx-indicator"); - l.RemoveAttribute("hx-push-url"); - l.RemoveAttribute("preload"); - } - } - - using var sw = new StringWriter(); - var formatter = new PrettyMarkupFormatter(); - foreach (var c in element.Children) - { - // ReSharper disable once AccessToDisposedClosure - c.ToHtml(sw, formatter); - } - return sw.ToString().TrimStart('\n'); - } + private static HtmlSnapshotOptions Options(bool sanitize) => sanitize ? DocsHtml.Body() : new HtmlSnapshotOptions(); public static void ShouldBeHtml( [LanguageInjection("html")] this string actual, [LanguageInjection("html")] string expected, bool sanitize = true - ) - { - expected = expected.Trim('\n').PrettyHtml(sanitize); - actual = actual.Trim('\n').PrettyHtml(sanitize); + ) => actual.Trim('\n').Should().BeHtml(expected.Trim('\n'), Options(sanitize)); - var diff = DiffBuilder.Compare(actual).WithTest(expected).Build().ToArray(); - - if (diff.Length == 0) - return; - - throw new AssertionFailedException(CreateDiff(actual, expected, sanitize)); - } public static void ShouldContainHtml( [LanguageInjection("html")] this string actual, [LanguageInjection("html")] string expected, bool sanitize = true - ) - { - expected = expected.Trim('\n').PrettyHtml(sanitize); - actual = actual.Trim('\n').PrettyHtml(sanitize); - - var actualCompare = actual.Replace("\t", string.Empty); - var expectedCompare = expected.Replace("\t", string.Empty); - - // we compare over unindented HTML, but if that fails, we rely on the pretty HTML Contain(). - // to throw for improved error messages - if (!actualCompare.Contains(expectedCompare)) - actual.Should().Contain(expected); - } - - public static string CreateDiff(this string actual, string expected, bool sanitize = true) - { - expected = expected.Trim('\n').PrettyHtml(sanitize); - actual = actual.Trim('\n').PrettyHtml(sanitize); - var diffLines = InlineDiffBuilder.Diff(expected, actual).Lines; - - var mutatedCount = diffLines.Count( - l => - l.Type switch - { - ChangeType.Unchanged => false, - ChangeType.Deleted => true, - ChangeType.Inserted => true, - ChangeType.Imaginary => false, - ChangeType.Modified => true, - _ => false - } - ); - if (mutatedCount == 0) - return string.Empty; - - var actualLineLength = actual.Split("\n").Length; - if (mutatedCount >= actualLineLength) - { - return $$""" - Mutations {{mutatedCount}} on all {{actualLineLength}} showing - EXPECTED: - {{expected}} - ACTUAL: - {{actual}} - """; - } - - using var sw = new StringWriter(); - foreach (var l in diffLines) - { - switch (l.Type) - { - case ChangeType.Unchanged: - sw.WriteLine(l.Text); - break; - case ChangeType.Deleted: - sw.WriteLine("- " + l.Text); - break; - case ChangeType.Inserted: - sw.WriteLine("+ " + l.Text); - break; - case ChangeType.Imaginary: - sw.WriteLine("? " + l.Text); - break; - case ChangeType.Modified: - sw.WriteLine("+ " + l.Text); - break; - } - } - - return sw.ToString(); - } + ) => actual.Trim('\n').Should().ContainHtml(expected.Trim('\n'), Options(sanitize)); } From 09f87ca17dbecb8eb3acb0a1b081f5ee985d6905 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 5 Nov 2026 12:34:42 +0100 Subject: [PATCH 3/9] Run the MCP handshake test from an .http file McpHandshakeTests posted one initialize request, parsed the SSE data line by hand and asserted three fields. The request now lives in http/mcp-handshake.http (initialize, then tools/list) and the whole response is the snapshot, so a change to the capabilities, the instructions or any tool definition shows up as a diff. Stricter than before: the full initialize result and the complete tools/list response are compared, not just protocolVersion and serverInfo. The SSE body is quoted text in the HTTP format, so a small scrubber pretty-prints each data: line. The server version is replaced by a placeholder only when it equals the assembly's informational version, which keeps the old check that the wiring reports the right version. Adds the Nullean.Snapshots.Http packages (0.4.0) to central package management and to the two HTTP-facing test projects. Co-Authored-By: Claude Sonnet 5.5 --- Directory.Packages.props | 2 + .../Elastic.Documentation.Api.Tests.csproj | 2 + .../http/mcp-handshake.verified.responses | 246 ++++++++++++++++++ .../Mcp.Remote.Tests/Mcp.Remote.Tests.csproj | 2 + tests/Mcp.Remote.Tests/McpHandshakeTests.cs | 65 ++--- tests/Mcp.Remote.Tests/SseScrubbers.cs | 52 ++++ .../Mcp.Remote.Tests/http/mcp-handshake.http | 24 ++ 7 files changed, 346 insertions(+), 47 deletions(-) create mode 100644 tests/Mcp.Remote.Tests/.snapshots/http/mcp-handshake.verified.responses create mode 100644 tests/Mcp.Remote.Tests/SseScrubbers.cs create mode 100644 tests/Mcp.Remote.Tests/http/mcp-handshake.http diff --git a/Directory.Packages.props b/Directory.Packages.props index 0fdb1d5ee6..342cdfc221 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -129,6 +129,8 @@ + + diff --git a/tests/Elastic.Documentation.Api.Tests/Elastic.Documentation.Api.Tests.csproj b/tests/Elastic.Documentation.Api.Tests/Elastic.Documentation.Api.Tests.csproj index ebfcff86ca..5f45dd079c 100644 --- a/tests/Elastic.Documentation.Api.Tests/Elastic.Documentation.Api.Tests.csproj +++ b/tests/Elastic.Documentation.Api.Tests/Elastic.Documentation.Api.Tests.csproj @@ -12,6 +12,8 @@ + +     diff --git a/tests/Mcp.Remote.Tests/.snapshots/http/mcp-handshake.verified.responses b/tests/Mcp.Remote.Tests/.snapshots/http/mcp-handshake.verified.responses new file mode 100644 index 0000000000..db73575236 --- /dev/null +++ b/tests/Mcp.Remote.Tests/.snapshots/http/mcp-handshake.verified.responses @@ -0,0 +1,246 @@ +## initialize: POST /docs/_mcp +status: 200 OK +content-type: text/event-stream + +> event: message +> data: { +> "result": { +> "protocolVersion": "2025-11-25", +> "capabilities": { +> "logging": {}, +> "tools": {} +> }, +> "serverInfo": { +> "name": "docs-mcp", +> "version": "{server-version}" +> }, +> "instructions": "Use this server to search, retrieve, and analyze Elastic product documentation published at elastic.co/docs. All content covers version 9 and later.\n\n\nUse the server when the user:\n- Wants to find, read, or verify Elastic documentation pages.\n- Needs to check whether a topic is already covered in Elastic documentation.\n- Asks about Elastic documentation structure, coherence, or inconsistencies across pages.\n- References Elastic product names such as Elasticsearch, Kibana, Fleet, APM, Logstash, Beats, Elastic Security, Elastic Observability, or Elastic Cloud.\n\n\n\n- Prefer search_docs over a general web search when looking up Elastic documentation content.\n- Use find_related_docs when exploring what documentation exists around a topic.\n- Use get_document_by_url to retrieve a specific page when the user provides or you already know the URL.\n- Use check_docs_coherence or find_docs_inconsistencies when reviewing or auditing documentation quality.\n" +> }, +> "id": 1, +> "jsonrpc": "2.0" +> } +> +> + +## tools/list: POST /docs/_mcp +status: 200 OK +content-type: text/event-stream + +> event: message +> data: { +> "result": { +> "tools": [ +> { +> "name": "find_docs_inconsistencies", +> "description": "Finds potential inconsistencies across Elastic documentation pages covering the same topic. Use when auditing docs quality, verifying that instructions don't contradict each other, or checking for overlapping content within a product area.", +> "inputSchema": { +> "type": "object", +> "properties": { +> "topic": { +> "description": "Topic or concept to check for inconsistencies", +> "type": "string" +> }, +> "focusArea": { +> "description": "Specific area to focus on (e.g., 'installation', 'configuration')", +> "type": [ +> "string", +> "null" +> ], +> "default": null +> } +> }, +> "required": [ +> "topic" +> ] +> }, +> "annotations": { +> "destructiveHint": false, +> "idempotentHint": true, +> "openWorldHint": false, +> "readOnlyHint": true +> }, +> "execution": { +> "taskSupport": "optional" +> } +> }, +> { +> "name": "check_docs_coherence", +> "description": "Checks how coherently a topic is covered across all Elastic documentation. Use when reviewing documentation quality, auditing coverage of a feature or concept, or checking whether a topic is documented consistently across products and sections.", +> "inputSchema": { +> "type": "object", +> "properties": { +> "topic": { +> "description": "Topic or concept to check coherence for", +> "type": "string" +> }, +> "limit": { +> "description": "Maximum number of documents to analyze (default: 20)", +> "type": "integer", +> "default": 20 +> } +> }, +> "required": [ +> "topic" +> ] +> }, +> "annotations": { +> "destructiveHint": false, +> "idempotentHint": true, +> "openWorldHint": false, +> "readOnlyHint": true +> }, +> "execution": { +> "taskSupport": "optional" +> } +> }, +> { +> "name": "find_related_docs", +> "description": "Finds Elastic documentation pages related to a given topic. Use when exploring what documentation exists around a subject, building context for writing, or discovering related content the user should be aware of.", +> "inputSchema": { +> "type": "object", +> "properties": { +> "topic": { +> "description": "Topic or search terms to find related documents for", +> "type": "string" +> }, +> "limit": { +> "description": "Maximum number of related documents to return (default: 10)", +> "type": "integer", +> "default": 10 +> }, +> "productFilter": { +> "description": "Filter by product ID (e.g., 'elasticsearch', 'kibana')", +> "type": [ +> "string", +> "null" +> ], +> "default": null +> } +> }, +> "required": [ +> "topic" +> ] +> }, +> "annotations": { +> "destructiveHint": false, +> "idempotentHint": true, +> "openWorldHint": false, +> "readOnlyHint": true +> }, +> "execution": { +> "taskSupport": "optional" +> } +> }, +> { +> "name": "get_document_by_url", +> "description": "Retrieves a specific Elastic documentation page by its URL. Use when the user provides a documentation URL, references a known page, or you need the full content and metadata of a specific doc. Returns title, AI summaries, headings, navigation context, and the full body only when includeBody is true. Pass includeBody=true to read the page content; otherwise body is null.", +> "inputSchema": { +> "type": "object", +> "properties": { +> "url": { +> "description": "The URL of the document. Accepts a full URL (e.g. 'https://www.elastic.co/docs/deploy-manage/api-keys') or a path (e.g. '/docs/deploy-manage/api-keys'). Query strings, fragments, and trailing slashes are ignored.", +> "type": "string" +> }, +> "includeBody": { +> "description": "Include the full page body (default: false, so body is null). Set true whenever you need to read the page content.", +> "type": "boolean", +> "default": false +> } +> }, +> "required": [ +> "url" +> ] +> }, +> "annotations": { +> "destructiveHint": false, +> "idempotentHint": true, +> "openWorldHint": false, +> "readOnlyHint": true +> }, +> "execution": { +> "taskSupport": "optional" +> } +> }, +> { +> "name": "search_docs", +> "description": "Searches all published Elastic documentation by meaning. Use when the user asks about Elastic product features, needs to find existing docs pages, verify published content, or research what documentation exists on a topic. Returns relevant documents with AI summaries, relevance scores, and navigation context.", +> "inputSchema": { +> "type": "object", +> "properties": { +> "query": { +> "description": "The search query - can be a question or keywords", +> "type": "string" +> }, +> "pageNumber": { +> "description": "Page number (1-based, default: 1)", +> "type": "integer", +> "default": 1 +> }, +> "pageSize": { +> "description": "Number of results per page (default: 10, max: 50)", +> "type": "integer", +> "default": 10 +> }, +> "productFilter": { +> "description": "Filter by product ID (e.g., 'elasticsearch', 'kibana')", +> "type": [ +> "string", +> "null" +> ], +> "default": null +> }, +> "sectionFilter": { +> "description": "Filter by navigation section (e.g., 'reference', 'getting-started')", +> "type": [ +> "string", +> "null" +> ], +> "default": null +> } +> }, +> "required": [ +> "query" +> ] +> }, +> "annotations": { +> "destructiveHint": false, +> "idempotentHint": true, +> "openWorldHint": false, +> "readOnlyHint": true +> }, +> "execution": { +> "taskSupport": "optional" +> } +> }, +> { +> "name": "analyze_document_structure", +> "description": "Analyzes the structure of a Elastic documentation page. Use when evaluating page quality, checking heading hierarchy, or assessing AI enrichment status. Returns heading count, link count, parent pages, and whether AI summaries are present.", +> "inputSchema": { +> "type": "object", +> "properties": { +> "url": { +> "description": "The URL of the document to analyze. Accepts a full URL (e.g. 'https://www.elastic.co/docs/deploy-manage/api-keys') or a path (e.g. '/docs/deploy-manage/api-keys'). Query strings, fragments, and trailing slashes are ignored.", +> "type": "string" +> } +> }, +> "required": [ +> "url" +> ] +> }, +> "annotations": { +> "destructiveHint": false, +> "idempotentHint": true, +> "openWorldHint": false, +> "readOnlyHint": true +> }, +> "execution": { +> "taskSupport": "optional" +> } +> } +> ] +> }, +> "id": 2, +> "jsonrpc": "2.0" +> } +> +> diff --git a/tests/Mcp.Remote.Tests/Mcp.Remote.Tests.csproj b/tests/Mcp.Remote.Tests/Mcp.Remote.Tests.csproj index 63312acbe6..0f22d678cf 100644 --- a/tests/Mcp.Remote.Tests/Mcp.Remote.Tests.csproj +++ b/tests/Mcp.Remote.Tests/Mcp.Remote.Tests.csproj @@ -10,6 +10,8 @@ + + diff --git a/tests/Mcp.Remote.Tests/McpHandshakeTests.cs b/tests/Mcp.Remote.Tests/McpHandshakeTests.cs index a11520dd74..01623ce387 100644 --- a/tests/Mcp.Remote.Tests/McpHandshakeTests.cs +++ b/tests/Mcp.Remote.Tests/McpHandshakeTests.cs @@ -2,13 +2,13 @@ // Elasticsearch B.V licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information -using System.Net; using System.Reflection; -using System.Text; -using System.Text.Json; using AwesomeAssertions; using Elastic.Documentation.Mcp.Remote; using Microsoft.AspNetCore.Mvc.Testing; +using Nullean.Snapshots; +using Nullean.Snapshots.Http; +using Nullean.Snapshots.TUnit; namespace Mcp.Remote.Tests; @@ -16,12 +16,14 @@ namespace Mcp.Remote.Tests; /// End-to-end tests for the MCP initialize handshake. Verifies that the serverInfo.name, /// serverInfo.version, and protocolVersion fields set in Program.cs are actually returned /// by the server's initialize response — a regression in the wiring would produce wrong -/// metadata while all unit tests still pass. +/// metadata while all unit tests still pass. Each http/*.http file is one test; its responses +/// are in .snapshots/http/*.verified.responses. /// public class McpHandshakeTests { [Test] - public async Task Initialize_PublicProfile_ReturnsCorrectServerInfoAndProtocolVersion() + [HttpFiles("http")] + public async Task Run(SnapshotFile file) { // Program.cs reads MCP_SERVER_PROFILE via Environment.GetEnvironmentVariable before // any DI override can run, so we set and restore the actual process variable to keep @@ -33,56 +35,25 @@ public async Task Initialize_PublicProfile_ReturnsCorrectServerInfoAndProtocolVe using var factory = new WebApplicationFactory(); using var client = factory.CreateClient(); - // The client deliberately sends an older protocol version ("2024-11-05") rather than - // the one the server pins to ("2025-11-25"). If options.ProtocolVersion regresses to - // null the SDK echoes the client value, so this assertion would fail — proving the - // pinning is actually enforced. - const string initRequest = /*lang=json,strict*/ - """ - { - "jsonrpc": "2.0", - "id": 1, - "method": "initialize", - "params": { - "protocolVersion": "2024-11-05", - "capabilities": {}, - "clientInfo": { "name": "test-client", "version": "1.0.0" } - } - } - """; - - using var content = new StringContent(initRequest, Encoding.UTF8, "application/json"); - // MCP HTTP transport requires both JSON and SSE in the Accept header; the server - // responds with SSE framing (text/event-stream). Extract the JSON from the data: line. - using var request = new HttpRequestMessage(HttpMethod.Post, "/docs/_mcp") { Content = content }; - request.Headers.TryAddWithoutValidation("Accept", "application/json, text/event-stream"); - using var response = await client.SendAsync(request, TestContext.Current!.Execution.CancellationToken); - - response.StatusCode.Should().Be(HttpStatusCode.OK); - - var rawBody = await response.Content.ReadAsStringAsync(TestContext.Current!.Execution.CancellationToken); - // SSE format: lines starting with "data: " carry the JSON payload. - var jsonLine = rawBody.Split('\n', StringSplitOptions.RemoveEmptyEntries).FirstOrDefault( - l => l.StartsWith("data:", StringComparison.Ordinal) + var transcript = await HttpFileSnapshot.RunAsync( + file.Path, + new HttpFileSnapshotOptions { Client = client }, + TestContext.Current!.Execution.CancellationToken ); - jsonLine.Should().NotBeNull("response body should contain an SSE data line"); - var json = (jsonLine ?? "data:")["data:".Length..].Trim(); - using var doc = JsonDocument.Parse(json); - var result = doc.RootElement.GetProperty("result"); - result.GetProperty("protocolVersion").GetString().Should().Be("2025-11-25"); - - // Compute the expected version the same way Program.cs does so a regression to the - // default assembly version (1.0.0.0) or the fallback ("0.0.0") fails this assertion. + // Computed the same way Program.cs does, so a regression to the default assembly version (1.0.0.0) + // or the fallback ("0.0.0") leaves a different value in the snapshot instead of the placeholder. var expectedVersion = typeof(Program) .Assembly .GetCustomAttributes() .FirstOrDefault()?.InformationalVersion ?? "0.0.0"; - var serverInfo = result.GetProperty("serverInfo"); - serverInfo.GetProperty("name").GetString().Should().Be(McpServerProfile.Public.ServiceName); - serverInfo.GetProperty("version").GetString().Should().Be(expectedVersion); + transcript.Should().MatchHttpFileSnapshot( + new SnapshotOptions().Scrub(SseScrubbers.PrettyPrintDataLines).Scrub( + text => text.Replace(expectedVersion, "{server-version}", StringComparison.Ordinal) + ) + ); } finally { diff --git a/tests/Mcp.Remote.Tests/SseScrubbers.cs b/tests/Mcp.Remote.Tests/SseScrubbers.cs new file mode 100644 index 0000000000..09d1abc7a0 --- /dev/null +++ b/tests/Mcp.Remote.Tests/SseScrubbers.cs @@ -0,0 +1,52 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Text; +using System.Text.Encodings.Web; +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Text.RegularExpressions; + +namespace Mcp.Remote.Tests; + +/// +/// The HTTP snapshot format writes a text/event-stream body as quoted text, so the JSON-RPC message of a +/// data: line would be one unreadable line. This scrubber pretty-prints it, which keeps a diff readable. +/// It runs on the rendered responses text, where every body line starts with > . +/// +internal static partial class SseScrubbers +{ + private static readonly JsonSerializerOptions Pretty = new() + { + WriteIndented = true, + Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping + }; + + [GeneratedRegex(@"^> data: (?.*)$", RegexOptions.Multiline)] + private static partial Regex DataLine(); + + public static string PrettyPrintDataLines(string text) => + DataLine().Replace(text, match => + { + JsonNode? node; + try + { + node = JsonNode.Parse(match.Groups["json"].Value); + } + catch (JsonException) + { + return match.Value; + } + + var lines = (node?.ToJsonString(Pretty) ?? "null").Replace("\r\n", "\n", StringComparison.Ordinal).Split('\n'); + var result = new StringBuilder(); + for (var i = 0; i < lines.Length; i++) + { + if (i > 0) + _ = result.Append('\n'); + _ = result.Append(i == 0 ? "> data: " : "> ").Append(lines[i]); + } + return result.ToString(); + }); +} diff --git a/tests/Mcp.Remote.Tests/http/mcp-handshake.http b/tests/Mcp.Remote.Tests/http/mcp-handshake.http new file mode 100644 index 0000000000..c1b7ba131c --- /dev/null +++ b/tests/Mcp.Remote.Tests/http/mcp-handshake.http @@ -0,0 +1,24 @@ +### initialize +# The client deliberately asks for an older protocol version than the one the server pins to. If the pinning +# regresses, the SDK echoes the client's version and the snapshot shows it. +POST /docs/_mcp +Content-Type: application/json +Accept: application/json, text/event-stream + +{ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": { "name": "test-client", "version": "1.0.0" } + } +} + +### tools/list +POST /docs/_mcp +Content-Type: application/json +Accept: application/json, text/event-stream + +{ "jsonrpc": "2.0", "id": 2, "method": "tools/list" } From e6fcdee7eb3f17bcf8d380c039144acda41c402d Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 5 Nov 2026 12:38:19 +0100 Subject: [PATCH 4/9] Run the page-feedback endpoint through an .http file and snapshot the stored records http/page-feedback.http sends a valid PUT with an euid cookie (204), an invalid PUT (400) and a DELETE (204) through the shared test host, and the responses are the snapshot. The three tests that checked the PageFeedbackRecord the endpoint handed to the service field by field now compare the whole record as an inline JSON snapshot. The feedback id is replaced by a placeholder only where it equals the id in the request URL, so storing the record under another id still fails. Stricter than before: the reaction-only and multiple-reasons tests used to check only some fields and now check all of them. The other 400/503 tests stay: they check the service was not called, which a response snapshot cannot show. Co-Authored-By: Claude Sonnet 5.5 --- .../http/page-feedback.verified.responses | 8 ++ .../PageFeedbackEndpointTests.cs | 93 ++++++++++++++++--- .../http/page-feedback.http | 27 ++++++ 3 files changed, 115 insertions(+), 13 deletions(-) create mode 100644 tests/Elastic.Documentation.Api.Tests/.snapshots/http/page-feedback.verified.responses create mode 100644 tests/Elastic.Documentation.Api.Tests/http/page-feedback.http diff --git a/tests/Elastic.Documentation.Api.Tests/.snapshots/http/page-feedback.verified.responses b/tests/Elastic.Documentation.Api.Tests/.snapshots/http/page-feedback.verified.responses new file mode 100644 index 0000000000..cca2e1993d --- /dev/null +++ b/tests/Elastic.Documentation.Api.Tests/.snapshots/http/page-feedback.verified.responses @@ -0,0 +1,8 @@ +## valid feedback is stored: PUT /docs/_api/v1/page-feedback/{{$guid}} +status: 204 No Content + +## a thumbs down needs a reason: PUT /docs/_api/v1/page-feedback/{{$guid}} +status: 400 Bad Request + +## feedback is deleted: DELETE /docs/_api/v1/page-feedback/{{$guid}} +status: 204 No Content diff --git a/tests/Elastic.Documentation.Api.Tests/PageFeedbackEndpointTests.cs b/tests/Elastic.Documentation.Api.Tests/PageFeedbackEndpointTests.cs index cea8b059e4..ff127596fc 100644 --- a/tests/Elastic.Documentation.Api.Tests/PageFeedbackEndpointTests.cs +++ b/tests/Elastic.Documentation.Api.Tests/PageFeedbackEndpointTests.cs @@ -8,6 +8,10 @@ using Elastic.Documentation.Api.PageFeedback; using Elastic.Documentation.Api.Tests.Fixtures; using FakeItEasy; +using Nullean.Snapshots; +using Nullean.Snapshots.Http; +using Nullean.Snapshots.Json; +using Nullean.Snapshots.TUnit; namespace Elastic.Documentation.Api.Tests; @@ -30,12 +34,34 @@ public static void DisposeSharedFactory() SharedFactory.Value.Dispose(); } + // The id is replaced by its own text, so a record stored under another id than the request's fails the snapshot. + private static SnapshotOptions ScrubFeedbackId(Guid feedbackId) => + new SnapshotOptions().Scrub(text => text.Replace(feedbackId.ToString(), "{feedback-id}", StringComparison.OrdinalIgnoreCase)); + private static HttpClient CreateClient(IPageFeedbackService feedbackService) { FeedbackService.Current = feedbackService; return SharedFactory.Value.CreateClient(); } + // The requests and the responses they must get are in http/page-feedback.http. The service accepts everything, so + // what the file shows is the endpoint's own validation and status codes. + [Test] + [HttpFiles("http")] + public async Task Run(SnapshotFile file) + { + var feedbackService = A.Fake(); + A.CallTo(() => feedbackService.UpsertFeedbackAsync(A._, A._)).Returns(Task.FromResult(true)); + A.CallTo(() => feedbackService.DeleteFeedbackAsync(A._, A._)).Returns(Task.FromResult(true)); + using var client = CreateClient(feedbackService); + + await HttpFileTest.RunAsync( + file, + new HttpFileSnapshotOptions { Client = client }, + TestContext.Current!.Execution.CancellationToken + ); + } + [Test] public async Task Put_ValidFeedback_RecordsFeedbackWithEuid() { @@ -54,13 +80,23 @@ public async Task Put_ValidFeedback_RecordsFeedbackWithEuid() response.StatusCode.Should().Be(HttpStatusCode.NoContent); recorded.Should().NotBeNull(); - recorded.FeedbackId.Should().Be(feedbackId); - recorded.PageUrl.Should().Be("/docs/test-page"); - recorded.Reaction.Should().Be(PageFeedbackReaction.ThumbsUp); - recorded.Reasons.Should().ContainSingle(r => r == PageFeedbackReason.Accurate); - recorded.ReasonSetVersion.Should().Be(2); - recorded.Comment.Should().Be("Useful page"); - recorded.Euid.Should().Be("test-euid"); + recorded.Should().MatchInlineJsonSnapshot( + """ + { + "FeedbackId": "{feedback-id}", + "PageUrl": "/docs/test-page", + "PageTitle": "Test page", + "Reaction": "thumbsUp", + "Reasons": [ + "accurate" + ], + "ReasonSetVersion": 2, + "Comment": "Useful page", + "Euid": "test-euid" + } + """, + options: ScrubFeedbackId(feedbackId) + ); } [Test] @@ -81,15 +117,28 @@ public async Task Put_ReactionOnly_RecordsFeedbackWithoutDetails() "reaction": "thumbsDown" } """; - using var request = CreateRequest(Guid.NewGuid(), payload); + var feedbackId = Guid.NewGuid(); + using var request = CreateRequest(feedbackId, payload); using var response = await client.SendAsync(request, TestContext.Current!.Execution.CancellationToken); response.StatusCode.Should().Be(HttpStatusCode.NoContent); recorded.Should().NotBeNull(); - recorded.Reasons.Should().BeNull(); - recorded.ReasonSetVersion.Should().BeNull(); - recorded.Comment.Should().BeNull(); + recorded.Should().MatchInlineJsonSnapshot( + """ + { + "FeedbackId": "{feedback-id}", + "PageUrl": "/docs/test-page", + "PageTitle": "Test page", + "Reaction": "thumbsDown", + "Reasons": null, + "ReasonSetVersion": null, + "Comment": null, + "Euid": null + } + """, + options: ScrubFeedbackId(feedbackId) + ); } [Test] @@ -112,13 +161,31 @@ public async Task Put_MultipleReasons_RecordsAllReasons() "reasonSetVersion": 2 } """; - using var request = CreateRequest(Guid.NewGuid(), payload); + var feedbackId = Guid.NewGuid(); + using var request = CreateRequest(feedbackId, payload); using var response = await client.SendAsync(request, TestContext.Current!.Execution.CancellationToken); response.StatusCode.Should().Be(HttpStatusCode.NoContent); recorded.Should().NotBeNull(); - recorded.Reasons.Should().BeEquivalentTo([PageFeedbackReason.Inaccurate, PageFeedbackReason.OutOfDate]); + recorded.Should().MatchInlineJsonSnapshot( + """ + { + "FeedbackId": "{feedback-id}", + "PageUrl": "/docs/test-page", + "PageTitle": "Test page", + "Reaction": "thumbsDown", + "Reasons": [ + "inaccurate", + "outOfDate" + ], + "ReasonSetVersion": 2, + "Comment": null, + "Euid": null + } + """, + options: ScrubFeedbackId(feedbackId) + ); } [Test] diff --git a/tests/Elastic.Documentation.Api.Tests/http/page-feedback.http b/tests/Elastic.Documentation.Api.Tests/http/page-feedback.http new file mode 100644 index 0000000000..2ccc6e7767 --- /dev/null +++ b/tests/Elastic.Documentation.Api.Tests/http/page-feedback.http @@ -0,0 +1,27 @@ +### valid feedback is stored +PUT /docs/_api/v1/page-feedback/{{$guid}} +Content-Type: application/json +Cookie: euid=test-euid + +{ + "pageUrl": "/docs/test-page", + "pageTitle": "Test page", + "reaction": "thumbsUp", + "reasons": ["accurate"], + "reasonSetVersion": 2, + "comment": " Useful page " +} + +### a thumbs down needs a reason +PUT /docs/_api/v1/page-feedback/{{$guid}} +Content-Type: application/json + +{ + "pageUrl": "/docs/test-page", + "pageTitle": "Test page", + "reaction": "thumbsDown", + "comment": "Missing a required reason" +} + +### feedback is deleted +DELETE /docs/_api/v1/page-feedback/{{$guid}} From c9f98aaf7c3f8da838f4461ef41cc874cba5962c Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 5 Nov 2026 13:43:51 +0100 Subject: [PATCH 5/9] Run the URL normaliser tests with the unit tests DocumentUrlNormalizerTests needs no Elasticsearch, but it sat in the integration project because DocumentGateway.NormalizeUrl is internal and only that project could see it. The Mcp.Remote assembly now also shows its internals to Mcp.Remote.Tests, so the test moves to the unit project and runs in CI, which the integration tests do not. Co-Authored-By: Claude Sonnet 5.5 --- .../Elastic.Documentation.Mcp.Remote.csproj | 3 +++ .../Mcp.Remote.Tests}/DocumentUrlNormalizerTests.cs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) rename {tests-integration/Mcp.Remote.IntegrationTests => tests/Mcp.Remote.Tests}/DocumentUrlNormalizerTests.cs (97%) diff --git a/src/api/Elastic.Documentation.Mcp.Remote/Elastic.Documentation.Mcp.Remote.csproj b/src/api/Elastic.Documentation.Mcp.Remote/Elastic.Documentation.Mcp.Remote.csproj index f5a5b5bd5d..869bddc313 100644 --- a/src/api/Elastic.Documentation.Mcp.Remote/Elastic.Documentation.Mcp.Remote.csproj +++ b/src/api/Elastic.Documentation.Mcp.Remote/Elastic.Documentation.Mcp.Remote.csproj @@ -29,6 +29,9 @@ <_Parameter1>Mcp.Remote.IntegrationTests + + <_Parameter1>Mcp.Remote.Tests + diff --git a/tests-integration/Mcp.Remote.IntegrationTests/DocumentUrlNormalizerTests.cs b/tests/Mcp.Remote.Tests/DocumentUrlNormalizerTests.cs similarity index 97% rename from tests-integration/Mcp.Remote.IntegrationTests/DocumentUrlNormalizerTests.cs rename to tests/Mcp.Remote.Tests/DocumentUrlNormalizerTests.cs index 5d303f0348..87d9854843 100644 --- a/tests-integration/Mcp.Remote.IntegrationTests/DocumentUrlNormalizerTests.cs +++ b/tests/Mcp.Remote.Tests/DocumentUrlNormalizerTests.cs @@ -5,7 +5,7 @@ using AwesomeAssertions; using Elastic.Documentation.Mcp.Remote.Gateways; -namespace Mcp.Remote.IntegrationTests; +namespace Mcp.Remote.Tests; /// /// Unit tests for . From e3ff2669b3ae29739d5ece8b4ae66ebed0ace856 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 5 Nov 2026 14:09:32 +0100 Subject: [PATCH 6/9] Scrub paths the same way on every operating system The repository and temp folder scrubbers replaced only the root, so a Windows run printed {repo}\docs\index.md where the verified text says {repo}/docs/index.md, and about 160 diagnostics snapshots failed there. The path that follows the root is now written with forward slashes. The temp scrubber also swallowed everything up to the next whitespace, including the :line after a file name. A path now ends at a colon too, and the repository root is tried before the temp root because the repository can sit under the temp folder. The doc comment claimed it scrubbed git SHAs; it does not, and no longer says so. Tested with a Windows-style root, which also runs on Linux and macOS. Removing the separator replacement makes that test fail. Co-Authored-By: Claude Sonnet 5.5 --- .../Linked/SnapshotScrubbing.cs | 29 ++++++++++++------- .../Testing/SnapshotHelpersTests.cs | 15 ++++++++++ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs b/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs index 6a80bb71fc..44132cd7bd 100644 --- a/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs +++ b/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs @@ -9,8 +9,9 @@ namespace Elastic.Documentation.Testing.Linked; /// -/// Scrubbers every snapshot in docs-builder's tests gets: machine-specific paths and git SHAs never belong in a -/// verified file. Runs when any test project referencing this assembly loads it. +/// Scrubbers every snapshot in docs-builder's tests gets: the repository root and the temp folder are replaced by +/// {repo} and {temp}, and the rest of such a path is written with forward slashes, so a verified file +/// reads the same on every machine and operating system. Compiled into each test project. /// internal static class SnapshotScrubbing { @@ -19,14 +20,22 @@ internal static class SnapshotScrubbing #pragma warning restore CA2255 internal static void Register() { - SnapshotDefaults.AddScrubber( - Scrubbers.Regex( - new Regex(Regex.Escape(Path.GetTempPath().TrimEnd(Path.DirectorySeparatorChar)) + @"[^\s""'<>]*", RegexOptions.Compiled), - "{temp}" - ) - ); + // the more specific root first: the repository may sit under the temp folder if (FindSolutionRoot() is { } root) - SnapshotDefaults.AddScrubber(Scrubbers.Regex(new Regex(Regex.Escape(root), RegexOptions.Compiled), "{repo}")); + SnapshotDefaults.AddScrubber(Under(root, "{repo}")); + SnapshotDefaults.AddScrubber(Under(Path.GetTempPath(), "{temp}")); + } + + /// + /// Replaces with and writes the path that follows it with + /// forward slashes. The path ends at whitespace, a quote, an angle bracket or a colon, so file.md:3 keeps its line. + /// + internal static Func Under(string root, string token) + { + root = root.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); + var roots = new[] { root, root.Replace('\\', '/') }.Distinct().Select(Regex.Escape); + var pattern = new Regex($"(?:{string.Join('|', roots)})(?[^\\s\"'<>:*?|]*)", RegexOptions.Compiled); + return text => pattern.Replace(text, m => token + m.Groups["rest"].Value.Replace('\\', '/')); } private static string? FindSolutionRoot() @@ -34,6 +43,6 @@ internal static void Register() var dir = new DirectoryInfo(AppContext.BaseDirectory); while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "docs-builder.slnx"))) dir = dir.Parent; - return dir?.FullName.TrimEnd(Path.DirectorySeparatorChar); + return dir?.FullName; } } diff --git a/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs b/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs index 5897c67b4a..bef3f63064 100644 --- a/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs +++ b/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs @@ -6,6 +6,7 @@ using AwesomeAssertions; using Elastic.Documentation.Diagnostics; using Elastic.Documentation.Testing; +using Elastic.Documentation.Testing.Linked; using Nullean.Snapshots; using Nullean.Snapshots.Html; @@ -67,4 +68,18 @@ public void DocsHtmlSection_DropsFixtureHeadingAndHtmxAttributes() => """, DocsHtml.Section() ); + + [Test] + public void PathScrubber_WindowsRoot_WritesTheRestWithForwardSlashesAndKeepsTheLine() => + SnapshotScrubbing + .Under(@"D:\a\docs-builder\docs-builder", "{repo}")(@"error D:\a\docs-builder\docs-builder\docs\index.md:3 bad link") + .Should() + .MatchInlineSnapshot("error {repo}/docs/index.md:3 bad link"); + + [Test] + public void PathScrubber_UnixRootInsideText_LeavesOtherPathsAlone() => + SnapshotScrubbing + .Under("/work/repo/", "{repo}")("see /work/repo/docs/a.md and /elsewhere/b.md") + .Should() + .MatchInlineSnapshot("see {repo}/docs/a.md and /elsewhere/b.md"); } From 12c95f57cc576994e27cfe2084ffb1e61dc50507 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 5 Nov 2026 14:12:40 +0100 Subject: [PATCH 7/9] Scrub random folder names under the temp directory A test that works in a Guid-named folder in the temp directory printed that name in its diagnostics, so a snapshot of them could not be stable. The temp scrubber now replaces a run of 32 hex digits in the path with {id}. Co-Authored-By: Claude Sonnet 5.5 --- .../Linked/SnapshotScrubbing.cs | 14 +++++++++++--- .../Testing/SnapshotHelpersTests.cs | 9 +++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs b/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs index 44132cd7bd..7c97d0d9b1 100644 --- a/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs +++ b/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs @@ -23,21 +23,29 @@ internal static void Register() // the more specific root first: the repository may sit under the temp folder if (FindSolutionRoot() is { } root) SnapshotDefaults.AddScrubber(Under(root, "{repo}")); - SnapshotDefaults.AddScrubber(Under(Path.GetTempPath(), "{temp}")); + // a test folder in the temp directory is usually named after a fresh Guid; that name is not part of what is verified + SnapshotDefaults.AddScrubber(Under(Path.GetTempPath(), "{temp}", RandomName)); } /// /// Replaces with and writes the path that follows it with /// forward slashes. The path ends at whitespace, a quote, an angle bracket or a colon, so file.md:3 keeps its line. /// - internal static Func Under(string root, string token) + internal static Func Under(string root, string token, Regex? randomName = null) { root = root.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); var roots = new[] { root, root.Replace('\\', '/') }.Distinct().Select(Regex.Escape); var pattern = new Regex($"(?:{string.Join('|', roots)})(?[^\\s\"'<>:*?|]*)", RegexOptions.Compiled); - return text => pattern.Replace(text, m => token + m.Groups["rest"].Value.Replace('\\', '/')); + return text => pattern.Replace(text, m => + { + var rest = m.Groups["rest"].Value.Replace('\\', '/'); + return token + (randomName is null ? rest : randomName.Replace(rest, "{id}")); + }); } + /// A run of 32 hex digits: a Guid.ToString("N") used as a folder name. + private static readonly Regex RandomName = new("[0-9a-fA-F]{32}", RegexOptions.Compiled); + private static string? FindSolutionRoot() { var dir = new DirectoryInfo(AppContext.BaseDirectory); diff --git a/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs b/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs index bef3f63064..21b3ae758a 100644 --- a/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs +++ b/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs @@ -82,4 +82,13 @@ public void PathScrubber_UnixRootInsideText_LeavesOtherPathsAlone() => .Under("/work/repo/", "{repo}")("see /work/repo/docs/a.md and /elsewhere/b.md") .Should() .MatchInlineSnapshot("see {repo}/docs/a.md and /elsewhere/b.md"); + + [Test] + public void TempScrubber_GuidFolderName_IsReplacedSoTheSnapshotIsStable() => + SnapshotScrubbing + .Under("/tmp", "{temp}", new System.Text.RegularExpressions.Regex("[0-9a-fA-F]{32}"))( + "error /tmp/diff-validate-test-1d248de3468f4d58849398ff8b320802/docs/redirects.yml Missing" + ) + .Should() + .MatchInlineSnapshot("error {temp}/diff-validate-test-{id}/docs/redirects.yml Missing"); } From 021c3c0f9588533105b2572ba69aacd38ce1360f Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 5 Nov 2026 14:28:53 +0100 Subject: [PATCH 8/9] Make diagnostics snapshots and snapshot paths the same on every operating system CI on Windows failed five of the stacked PRs for reasons a laptop never shows: - DiagnosticsSnapshot now writes paths with forward slashes and drops a leading drive letter, so `C:\docs\toc.yml`, `C:/docs/toc.yml` and `/docs/toc.yml` read the same, and `testing\req.md` inside a message reads `testing/req.md`. - ToSnapshotText(firstLineOnly: true) keeps only the first line of each message, for diagnostics that carry an exception: its stack trace has source line numbers and the exception type differs per platform. - SnapshotPathLengthTests fails when a verified snapshot's path, with the received file's longer folder and the CI checkout root, would pass Windows' 260 characters. git on the runner cannot check such a file out, so the whole job failed before any test ran. Each has a test. Lowering the length limit makes the guard fail. Co-Authored-By: Claude Sonnet 5.5 --- .../DiagnosticsSnapshot.cs | 29 +++++++-- .../Testing/SnapshotHelpersTests.cs | 43 +++++++++++++ .../Testing/SnapshotPathLengthTests.cs | 60 +++++++++++++++++++ 3 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 tests/Elastic.Documentation.Tests/Testing/SnapshotPathLengthTests.cs diff --git a/tests-shared/Elastic.Documentation.Testing/DiagnosticsSnapshot.cs b/tests-shared/Elastic.Documentation.Testing/DiagnosticsSnapshot.cs index 0e8f94f1f2..f53161eea3 100644 --- a/tests-shared/Elastic.Documentation.Testing/DiagnosticsSnapshot.cs +++ b/tests-shared/Elastic.Documentation.Testing/DiagnosticsSnapshot.cs @@ -3,17 +3,28 @@ // See the LICENSE file in the project root for more information using System.Text; +using System.Text.RegularExpressions; using Elastic.Documentation.Diagnostics; namespace Elastic.Documentation.Testing; -public static class DiagnosticsSnapshot +public static partial class DiagnosticsSnapshot { /// /// One line per diagnostic, error|warning|hint file:line message, in the order they were written. Empty when /// there are none, so a clean run reads as MatchInlineSnapshot(""). + /// + /// Paths read the same on every operating system: backslashes become forward slashes and a leading drive letter is + /// dropped, so C:\docs\toc.yml, C:/docs/toc.yml and /docs/toc.yml are the same text. A message that + /// really contains a backslash reads with a slash, on Linux too. + /// /// - public static string ToSnapshotText(this IEnumerable diagnostics) + /// What a test's collector holds. + /// + /// Keep only the first line of each message. For diagnostics that carry an exception, whose stack trace (frames and + /// source line numbers) is no part of what a test verifies, and whose type and text can differ per platform. + /// + public static string ToSnapshotText(this IEnumerable diagnostics, bool firstLineOnly = false) { var sb = new StringBuilder(); foreach (var d in diagnostics) @@ -24,9 +35,19 @@ public static string ToSnapshotText(this IEnumerable diagnostics) Severity.Warning => "warning", _ => "hint" }; - var position = d.Line is { } line ? $"{d.File}:{line}" : d.File; - sb.Append(severity).Append(' ').Append(position).Append(' ').Append(d.Message).Append('\n'); + var file = Normalise(d.File); + var position = d.Line is { } line ? $"{file}:{line}" : file; + var message = d.Message ?? string.Empty; + if (firstLineOnly) + message = message.Split('\n')[0].TrimEnd('\r'); + _ = sb.Append(severity).Append(' ').Append(position).Append(' ').Append(Normalise(message)).Append('\n'); } return sb.ToString(); } + + private static string Normalise(string text) => DriveLetter().Replace(text.Replace('\\', '/'), string.Empty); + + // "C:/docs/..." at the start of a path; not the "s:" of "https://", which a letter or digit precedes + [GeneratedRegex("(? ) .Should() .MatchInlineSnapshot("error {temp}/diff-validate-test-{id}/docs/redirects.yml Missing"); + + [Test] + public void ToSnapshotText_WindowsPaths_ReadLikeUnixPaths() + { + var diagnostics = new List + { + new() + { + Severity = Severity.Error, + File = @"C:\docs\section-b\toc.yml", + Message = @"`sub-requirements2` does not exist in testing\req.md." + }, + new() { Severity = Severity.Error, File = "C:/docs/docset.yml", Line = 4, Message = "see https://elastic.co/docs and D:/x" }, + new() { Severity = Severity.Warning, File = @"/docs\docset.yml", Message = "mixed" } + }; + diagnostics + .ToSnapshotText() + .Should() + .MatchInlineSnapshot( + """ + error /docs/section-b/toc.yml `sub-requirements2` does not exist in testing/req.md. + error /docs/docset.yml:4 see https://elastic.co/docs and /x + warning /docs/docset.yml mixed + """ + ); + } + + [Test] + public void ToSnapshotText_FirstLineOnly_DropsTheStackTrace() + { + var diagnostics = new List + { + new() + { + Severity = Severity.Error, + File = "index.md", + Line = 3, + Message = + "registry could not be read\nSystem.IO.FileNotFoundException: Could not find file 'x'.\n at Some.Method() in /src/File.cs:line 209" + } + }; + diagnostics.ToSnapshotText(firstLineOnly: true).Should().MatchInlineSnapshot("error index.md:3 registry could not be read"); + } } diff --git a/tests/Elastic.Documentation.Tests/Testing/SnapshotPathLengthTests.cs b/tests/Elastic.Documentation.Tests/Testing/SnapshotPathLengthTests.cs new file mode 100644 index 0000000000..647cddfb22 --- /dev/null +++ b/tests/Elastic.Documentation.Tests/Testing/SnapshotPathLengthTests.cs @@ -0,0 +1,60 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using AwesomeAssertions; + +namespace Elastic.Documentation.Tests.Testing; + +/// +/// A verified snapshot is named after its test class and method, and a failed run writes the received file under +/// .artifacts/.snapshots/received/{Project}/, a longer path. Windows stops at 260 characters, and git on the CI runner +/// cannot even check out a file that is too long, so the whole job fails before any test runs. +/// +public class SnapshotPathLengthTests +{ + // "D:\a\docs-builder\docs-builder\" is where a GitHub runner puts the checkout. + private const int CheckoutRootLength = 31; + private const string ReceivedFolder = ".artifacts/.snapshots/received/"; + // Windows' MAX_PATH is 260 including the terminating character; keep a margin. + private const int MaxLength = 250; + + [Test] + public void VerifiedSnapshots_FitInAWindowsPath_EvenWhenTheReceivedFileIsWritten() + { + var root = FindRepositoryRoot(); + var tooLong = new List(); + foreach (var testsFolder in new[] { "tests", "tests-integration" }) + { + var folder = Path.Combine(root, testsFolder); + if (!Directory.Exists(folder)) + continue; + foreach (var project in Directory.EnumerateDirectories(folder)) + { + var snapshots = Path.Combine(project, ".snapshots"); + if (!Directory.Exists(snapshots)) + continue; + foreach (var file in Directory.EnumerateFiles(snapshots, "*", SearchOption.AllDirectories)) + { + var relative = Path.GetRelativePath(snapshots, file).Replace('\\', '/'); + var length = CheckoutRootLength + ReceivedFolder.Length + Path.GetFileName(project).Length + 1 + relative.Length; + if (length > MaxLength) + tooLong.Add($"{length} {Path.GetRelativePath(root, file).Replace('\\', '/')}"); + } + } + } + + tooLong.Should().BeEmpty( + "a snapshot path over {0} characters cannot be checked out on Windows; shorten the test's name or its class", + MaxLength + ); + } + + private static string FindRepositoryRoot() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "docs-builder.slnx"))) + dir = dir.Parent; + return dir?.FullName ?? throw new InvalidOperationException("docs-builder.slnx not found above the test binaries"); + } +} From 2b0c0d96482ca8657090d80cb9c58fe6556356eb Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Thu, 5 Nov 2026 14:50:06 +0100 Subject: [PATCH 9/9] Match the repository root in the form DiagnosticsSnapshot writes it DiagnosticsSnapshot drops a leading drive letter and writes forward slashes, so on Windows D:\a\docs-builder\docs-builder\docs\index.md reaches the {repo} scrubber as /a/docs-builder/docs-builder/docs/index.md, which it no longer recognised. Every diagnostics snapshot that names a file failed on the Windows runner. The scrubber now also matches the root without its drive letter. Removing that variant makes the new test fail. Co-Authored-By: Claude Sonnet 5.5 --- .../Linked/SnapshotScrubbing.cs | 7 ++++++- .../Testing/SnapshotHelpersTests.cs | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs b/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs index 7c97d0d9b1..2a0413a362 100644 --- a/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs +++ b/tests-shared/Elastic.Documentation.Testing/Linked/SnapshotScrubbing.cs @@ -34,7 +34,10 @@ internal static void Register() internal static Func Under(string root, string token, Regex? randomName = null) { root = root.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); - var roots = new[] { root, root.Replace('\\', '/') }.Distinct().Select(Regex.Escape); + // DiagnosticsSnapshot writes a path with forward slashes and without its drive letter, so the root has to match that way too + var forward = root.Replace('\\', '/'); + var withoutDrive = DriveLetter.Replace(forward, string.Empty); + var roots = new[] { root, forward, withoutDrive }.Where(r => r.Length > 1).Distinct().Select(Regex.Escape); var pattern = new Regex($"(?:{string.Join('|', roots)})(?[^\\s\"'<>:*?|]*)", RegexOptions.Compiled); return text => pattern.Replace(text, m => { @@ -43,6 +46,8 @@ internal static Func Under(string root, string token, Regex? ran }); } + private static readonly Regex DriveLetter = new("^[A-Za-z]:", RegexOptions.Compiled); + /// A run of 32 hex digits: a Guid.ToString("N") used as a folder name. private static readonly Regex RandomName = new("[0-9a-fA-F]{32}", RegexOptions.Compiled); diff --git a/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs b/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs index ca2e42c247..c89933d40b 100644 --- a/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs +++ b/tests/Elastic.Documentation.Tests/Testing/SnapshotHelpersTests.cs @@ -134,4 +134,13 @@ public void ToSnapshotText_FirstLineOnly_DropsTheStackTrace() }; diagnostics.ToSnapshotText(firstLineOnly: true).Should().MatchInlineSnapshot("error index.md:3 registry could not be read"); } + + [Test] + public void PathScrubber_RootAfterDiagnosticsSnapshotNormalisedIt_StillReadsAsRepo() + { + // what a Windows run writes for D:\a\docs-builder\docs-builder\docs\index.md once the drive letter and backslashes are gone + var scrub = SnapshotScrubbing.Under(@"D:\a\docs-builder\docs-builder", "{repo}"); + scrub("error /a/docs-builder/docs-builder/docs/index.md:3 bad").Should().MatchInlineSnapshot("error {repo}/docs/index.md:3 bad"); + scrub(@"error D:\a\docs-builder\docs-builder\docs\index.md:3 bad").Should().MatchInlineSnapshot("error {repo}/docs/index.md:3 bad"); + } }