Repository navigation
Move the Authoring tests onto Nullean.Snapshots - #4722
Merged
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
ConvertsToHtml, ToHtml, ConvertsToPlainText, ConvertsToNewLlm, ConvertsToLlmWithMetadata and ConvertsToJson keep their names and call sites, but now compare through Nullean.Snapshots inline snapshots, so the expected literal is accepted with snap instead of hand-edited. The wrappers forward the caller's file, member and line and name themselves as the method snap looks for in the source (InlineSnapshots.cs). The expected literals passed unchanged except for these, which snap rewrote: - LLM markdown literals started with a blank line that the old comparison trimmed; they are now the trimmed text. - Two ToHtml literals (IncludeBlocks) listed an <h1> that the old BeHtml stripped from both sides; the snapshot no longer has it. - The old text comparison ignored whitespace (DiffPlex default), so indentation mistakes in four literals went unnoticed: mermaid code lost its indent in two, and nested list items and a fenced block in a nested list lose their indentation in the exporter output. The snapshots record the output as it is today and the tests carry a note. These comparisons are therefore stricter than before. ConvertsToJson compares as a tree, so key order no longer matters. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Where a test class asserted a cluster of ConvertsToContainingHtml, ConvertsToContainingRawHtml and DoesNotConvertToContainingHtml fragments on one page, the cluster is now one snapshot of the page content: inline (ConvertsToHtml) up to about 45 lines, a verified .html file (ConvertsToHtmlSnapshot) beyond that, 7 in all. Classes asserting HasError, HasWarning or HasHint now snapshot every diagnostic as 'severity file:line message' (HasDiagnostics), which also absorbs their HasNoErrors and HasNoWarnings. Other HasNoErrors/HasNoWarnings stay. These tests became stricter: they used to look at the first diagnostic's substring or at fragments, and now cover the whole output. Before accepting, every old fragment was checked to appear in the snapshot (whitespace and entities normalised) and every negative fragment to be absent, and every old diagnostic substring to appear in the diagnostic text; the only mismatches were valueless attributes that AngleSharp prints as name="" (alt, open, checked, hidden, data-*). Kept as targeted assertions, because a section snapshot cannot show them: - the newline between <code> elements inside <pre> (ConsoleCodeBlockSegments); - <head> meta tags (ProductsFrontMatter); - the page shell, header, footer and layouts (DesignSystem), whose whole page carries the sprite sheet and the checkout directory name. Dropped the helpers nothing uses any more (ConvertsToContainingHtml, ContainsHtml, DoesNotContainHtml on the result task, the DiffPlex line diff) and the DiffPlex package reference. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
![infra-vault-gh-plugin-prod[bot]](https://avatars.githubusercontent.com/u/6764390?s=60&v=4){kind=small}
Comment on lines
+511
to
+527
| [Test, DisplayName("renders HTML")] | ||
| public async Task RendersHtml() => | ||
| await Docs.ConvertsToHtml( | ||
| """ | ||
| <a class="page-card"> | ||
| <span class="page-card__text"> | ||
| <span class="page-card__title"></span> | ||
| <span class="page-card__description"> | ||
| <p>A description.</p> | ||
| </span> | ||
| </span> | ||
| <svg class="icon icon--sm" xmlns="http://www.w3.org/2000/svg" width="16" height="16" viewBox="0 0 16 16"> | ||
| <path d="M2 7.5h10.793L9.646 4.354l.707-.708L14.707 8l-4.354 4.354-.707-.707L12.793 8.5H2v-1Z"></path> | ||
| </svg> | ||
| </a> | ||
| """ | ||
| ); |
There was a problem hiding this comment.
Severity: P3 (Low)
This fixture (:::{page-card} index.md) is not valid page-card syntax, because the directive needs a [Title](url) argument. It emits page-card requires a markdown link argument, so the new snapshot accepts broken output as the expected design-system card: an <a class="page-card"> with no href and an empty page-card__title. The old fragment checks hid this. The snapshot now makes the broken output the reference, so this test can't catch a regression in a real page card's link or title. Use a valid argument, such as [Admonitions](admonitions.md) with that file added, and snapshot that output instead. Adding HasNoErrors/HasDiagnostics would also catch an invalid fixture.
Generated by Libra. Specialist: libra-complexity. React with 👍 or 👎 to give feedback on this comment.
| { | ||
| [DebuggerStepThrough] | ||
| internal static void HasDiagnostics(string? expected, GeneratorResults results, CallSite site) => | ||
| InlineSnapshots.Text(results.Context.Collector.Diagnostics.ToSnapshotText(), expected, nameof(Scenario.HasDiagnostics), site); |
There was a problem hiding this comment.
Severity: P3 (Low)
Normalize diagnostic path separators before comparing the snapshots. Diagnostics contain native FullName paths, and the shared scrubber replaces only the repository prefix, leaving Windows output such as {repo}\docs\index.md rather than the checked-in {repo}/docs/index.md. The new diagnostics assertions therefore fail on the Windows CI runner even when the diagnostics are otherwise unchanged.
Generated by Libra. React with 👍 or 👎 to give feedback on this comment.
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
The stack was rebased onto fixes for how snapshots scrub paths (Windows separators, random temp folder names). So that the pushed history is not rewritten, this commit has the rebased content and has both the previously pushed tip and the updated base as parents. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The stack was rebased onto fixes for how snapshots scrub paths (Windows separators, random temp folder names). So that the pushed history is not rewritten, this commit has the rebased content and has both the previously pushed tip and the updated base as parents. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ting 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
# Conflicts: # tests/Elastic.Authoring.Tests/Blocks/Hub/GetStarted.cs # tests/Elastic.Authoring.Tests/Elastic.Authoring.Tests.csproj # tests/Elastic.Authoring.Tests/Framework/HtmlAssertions.cs
Contributor
Docs preview (local build)Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4722/ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #4719 and #4720.
Moves Elastic.Authoring.Tests onto Nullean.Snapshots. The expected output now lives in the test as an inline snapshot, or in a
.snapshots/file when it is long.ConvertsToHtml,ToHtml,ConvertsToPlainText,ConvertsToNewLlm,ConvertsToLlmWithMetadata,ConvertsToJson) keep their names and call sites and now compare against an inline snapshot.snap acceptrewrites the literal in the test file.ConvertsToContaining*fragments now have one snapshot of the page content (about 100 inline, 7 files).HasError/HasWarning/HasHintnow snapshot the whole diagnostics list inline.These tests are stricter than before. Each commit says which.
Found by snapshot testing
These look like product bugs. The old assertions hid them; this PR records the current output and does not fix it.
LlmMarkdown/Listsloses the indent of the second nesting level, andCodeblockInListloses the indent of the fenced block inside a list item. The mermaid literals lost their indent too. The old comparison ignored whitespace. There is a note above the two list tests.ImageRefOutOfScopereportsresolved to \Nullean.ScopedFileSystem.ScopedFileInfo``, a type name where a path should be.HintsOnInvalidKeyslistselasticsearchtwice in its valid-keys hint.IncludeBlocksToHtmlliterals contained an<h1>that the old comparison stripped from both sides, so the test never showed it. I removed it from the literals.install-step,Get startedandOuter summarycould match almost anywhere; the page snapshots replace them.ConvertsToJsonnow compares as a tree, so key order no longer matters.Checked
snap statusclean,snap orphansempty.CodeInlinerenderer, an LLM list bullet and aGitCheckoutInformationdefault. Each made snapshots fail with a readable diff. For thelinks.jsonone I only confirmed that it failed, not the diff.name="", plus one whitespace-in-<pre>case, kept as a raw assertion.Left as they were
HasNoErrors/HasNoWarningsin classes that assert no message. The<pre>newline check inConsoleCodeBlockSegments, the<head>meta tags inProductsFrontMatterand the layout tests inDesignSystemDirectives: a whole-page snapshot there is huge and leaks the checkout directory name as the site title.Not checked
That the
modal-XXXXXXXXids in the image snapshots are path-independent. Other operating systems.🤖 Generated with Claude Code