Skip to content

Make Nullean.Snapshots available to every test project - #4719

Merged
Mpdreamz merged 5 commits into
mainfrom
test/snapshots-infra
Nov 5, 2026
Merged

Mpdreamz merged 5 commits into
mainfrom
test/snapshots-infra

Conversation

@Mpdreamz

@Mpdreamz Mpdreamz commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

First of a stack that moves the test suite onto Nullean.Snapshots: expected output lives in snapshots (inline in the test when small, .snapshots/ files when large) and is accepted with snap, instead of in chains of Contain assertions. This PR only makes the library available and adds the shared pieces; it converts no tests.

The ApiExplorer pilot (#4654) used Nullean.Snapshots.Html and .TUnit 0.2.0 in one project. Now every unit and integration test project gets 0.4.0 of the library:

  • tests-shared/Elastic.Documentation.Testing (a library, outside tests/ because build/Targets.fs runs dotnet test on every csproj it finds there):
    • DocsHtml.Section() / Body(): the page content without the fixture's leading H1 and the htmx attributes on links. This merges what PrettyHtmlExtensions and HtmlAssertions each did.
    • DiagnosticsSnapshot.ToSnapshotText(): error|warning|hint file:line message, one line each. A clean run is the empty snapshot.
    • NavigationDump.ToText(): the navigation tree as indented text, index pages first, and a child whose Parent is wrong is flagged.
    • MockFileSystemDump.ToText(): every file under a mock-file-system folder, sorted, under === path === headers.
    • A scrubber for temp and repo paths, compiled into each test project (a module initializer in the library would only run once a test touched the assembly).
  • snap as a local dotnet tool.
  • CI: failed runs upload .artifacts/.snapshots/received/ (download it into the same place locally and run dotnet snap review). The build target fails on orphaned snapshots after a unit run.
  • AGENTS.md describes the workflow; the PR review prompt no longer says the repo uses xUnit.

The five TestDiagnosticsCollector copies stay as they are. They differ in lifecycle (Write vs HandleItem), so merging them would change behaviour. DiagnosticsSnapshot works on any of them.

Checked

  • All 18 unit projects pass; the ApiExplorer pilot snapshots pass unchanged on 0.4.0.
  • dotnet run --project build -- unit-test passes, and fails when I plant an orphaned .verified.txt.
  • The helpers have their own tests (SnapshotHelpersTests, inline snapshots). Breaking DocsHtml.Section() makes the HTML one fail.

Not checked

Windows and Linux. The CI artifact step has not run yet.

What CI added

The first CI run failed on Windows in four of the stacked PRs, which is what the path scrubbing here is for. A snapshot that holds a path must read the same on every machine, so:

  • The repository and temp roots are replaced by {repo} and {temp}, and the rest of the path is written with forward slashes ({repo}\docs\index.md on Windows became {repo}/docs/index.md). The path ends at a colon, so file.md:3 keeps its line.
  • A 32-hex-digit run under the temp folder (a Guid-named test folder) becomes {id}.
  • Both are covered by tests, including a Windows-style root that runs on any OS. Removing the separator replacement makes that test fail.

The first version of the temp scrubber swallowed everything after the temp folder, including the file name, so a snapshot could record {temp} and lose the path. One such snapshot is regenerated in the Markdown PR.

All 18 unit projects also pass with CI, GITHUB_ACTIONS and GITHUB_REPOSITORY set and TZ=Asia/Tokyo, which is how I now check each PR locally before pushing.

🤖 Generated with Claude Code

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>
/// under is flagged <c>[wrong parent]</c>; index pages are not checked, because a virtual file's index has a
/// different parent.
/// </summary>
public static string ToText(INavigationItem root)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: P3 (Low)

NavigationDump.ToText is the only new shared helper with no test in SnapshotHelpersTests. Nothing checks that index pages come first, that indentation is right, that generic type names are trimmed, or that the [hidden], [island] and [wrong parent] flags appear. Later PRs in the stack will turn navigation tests into snapshots of this output, so a bug here would end up in accepted snapshots without anyone noticing. In particular, a broken [wrong parent] check would hide the exact regression it exists to catch. Please add a small inline-snapshot test that builds a tree with one mis-parented child.

Generated by Libra. Specialist: libra-complexity. React with 👍 or 👎 to give feedback on this comment.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in #4739: three inline-snapshot tests over small fake navigation types (index first then children, indentation, [hidden] and [island], trimmed generic type names, a mis-parented child flagged [wrong parent], and an index page whose parent differs not being flagged). I mutated NavigationDump seven ways and each fails at least one test, including a [wrong parent] check that is never true.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Docs preview (local build)

Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4719/

Sample changed pages:

Mpdreamz and others added 3 commits November 5, 2026 14:09
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>
…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>
@Mpdreamz
Mpdreamz merged commit 71c99c6 into main Nov 5, 2026
38 checks passed
@Mpdreamz
Mpdreamz deleted the test/snapshots-infra branch November 5, 2026 14:09
@Mpdreamz Mpdreamz mentioned this pull request Nov 5, 2026
Mpdreamz added a commit that referenced this pull request Nov 5, 2026
The review of #4719 pointed out that NavigationDump.ToText was the one shared helper without a test, and that Navigation.Tests snapshots its output, so a bug
in it would be accepted into every one of those snapshots without anyone noticing. A broken [wrong parent] check would hide the regression it
exists to catch.

Three inline-snapshot tests over small fake navigation types: the index page is listed first and the children follow, indented by depth, with
[hidden] and [island] and generic type names trimmed; a child whose Parent is another node is flagged [wrong parent]; an index page whose parent
differs is not.

Each of seven mutations of NavigationDump (no indentation, no wrong-parent flag, no hidden flag, no island flag, untrimmed generic names, index
listed last, index checked for its parent) makes at least one of them fail.

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant