Repository navigation
Make Nullean.Snapshots available to every test project - #4719
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>
| /// 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Docs preview (local build)Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4719/ Sample changed pages: |
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>
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>
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 withsnap, instead of in chains ofContainassertions. This PR only makes the library available and adds the shared pieces; it converts no tests.The ApiExplorer pilot (#4654) used
Nullean.Snapshots.Htmland.TUnit0.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, outsidetests/becausebuild/Targets.fsrunsdotnet teston 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 whatPrettyHtmlExtensionsandHtmlAssertionseach 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 whoseParentis wrong is flagged.MockFileSystemDump.ToText(): every file under a mock-file-system folder, sorted, under=== path ===headers.snapas a local dotnet tool..artifacts/.snapshots/received/(download it into the same place locally and rundotnet snap review). The build target fails on orphaned snapshots after a unit run.AGENTS.mddescribes the workflow; the PR review prompt no longer says the repo uses xUnit.The five
TestDiagnosticsCollectorcopies stay as they are. They differ in lifecycle (WritevsHandleItem), so merging them would change behaviour.DiagnosticsSnapshotworks on any of them.Checked
dotnet run --project build -- unit-testpasses, and fails when I plant an orphaned.verified.txt.SnapshotHelpersTests, inline snapshots). BreakingDocsHtml.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:
{repo}and{temp}, and the rest of the path is written with forward slashes ({repo}\docs\index.mdon Windows became{repo}/docs/index.md). The path ends at a colon, sofile.md:3keeps its line.{id}.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_ACTIONSandGITHUB_REPOSITORYset andTZ=Asia/Tokyo, which is how I now check each PR locally before pushing.🤖 Generated with Claude Code