Repository navigation
Move the navigation and configuration tests onto Nullean.Snapshots - #4726
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>
The isolated and assembler navigation tests found nodes with ElementAt/First and asserted Url, type and child count one node at a time, so a wrong URL elsewhere in the tree went unnoticed (several only checked StartWith on a prefix). They now snapshot the whole tree inline as indented text. The shared NavigationDump leaves out each node's index page, where most URLs live, so NavigationTreeDump in this project adds an index line per node and flags a child whose Parent is not the node it is listed under. These tests became stricter: every URL, title, type, hidden/island flag and child count of the tree is now pinned, which includes every fact the removed assertions checked. Tests about behaviour (HomeProvider changes, island rendering, parent and root identity) are untouched. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The secondary nav, nav tree, table of contents, footer, docs bar and nav side tests checked fragments with Contain/NotContain and IndexOf ordering. They now write the rendered component to a verified HTML file (htmx attributes on links removed), so an unexpected extra, missing or reordered element shows in the diff. These tests became stricter. Before accepting, every Contain/NotContain literal of the old assertions was checked against the snapshot text with a script (the scoped negatives, such as 'no chevron inside the page link' and 'no <a> before the title', were checked by reading the HTML). Several tests rendered the same input and asserted different fragments of it (top nav bar: 5 tests into 1, built-in bar: 2 into 1, mobile drawer: 2 into 1, docs bar: 4 into 1), which would have produced identical snapshot files. They are merged into one test per render, so the test count drops by 8 with no coverage lost. Left as Contain checks: the landing page and style guide pages (whole pages of several hundred lines), the head asset ordering tests and the examples' generic invariants. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…contents Error and warning tests asserted a count (Errors == 1, Warnings == 1) or that one message contained a substring, so a test named EmitsError_WhenProductMissing passed on any single error. They now snapshot every diagnostic as 'severity file:line message' (UnknownYamlKeyTests, the API configuration errors, the CTA default errors, the Storybook registry warning, the docset/toc unknown key errors). The Column assertions stay as explicit checks because the snapshot text has no column. The parameterised repository-form error uses a file snapshot, as its output differs per case. DocumentationSetFileTests, SiteNavigationFileTests and CreateNavigationFileTests walked deserialised refs element by element, or checked the generated navigation.yml with Contain/NotContain. They now snapshot the TOC (TocDump), the site navigation file (SiteNavDump) and the generated YAML text. These tests became stricter: each fact the removed assertions checked (types, paths, hidden/island flags, titles, child counts, removed private entries) is visible in the snapshots. Default-value and scalar property tests, the Arguments tables, CsCheck properties and Throw assertions are untouched. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
NavigationDump now prints each node's index page before its children and flags a child whose Parent is not the node it is listed under. That is what Navigation.Tests had in a local NavigationTreeDump, so the local copy goes and the snapshots stay as they were. 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}
| collector.Errors.Should().Be(1); | ||
| config.ApiConfigurations.Should().BeNull(); | ||
| // the output differs per case, so it is a file snapshot (keyed by the arguments) rather than a shared inline literal | ||
| collector.Captured.ToSnapshotText().Should().MatchSnapshot(); |
There was a problem hiding this comment.
Severity: P3 (Low)
EmitsError_WhenRepositoryNotInOrgSlashRepoForm no longer asserts config.ApiConfigurations.Should().BeNull(), and config is now unused. If a regression kept the API configuration despite the invalid repository: value, this test would still pass. Every sibling EmitsError_* test kept its config assertion next to the new snapshot, so this one is weaker than before. Add the assertion back after MatchSnapshot().
Generated by Libra. Specialist: libra-complexity. React with 👍 or 👎 to give feedback on this comment.
| .Should() | ||
| .MatchInlineSnapshot( | ||
| "error /docs/docset.yml:2 Unknown key 'totally_unknown_key'. Valid keys: api, branding, codex, cross_links, cta, default_cta, description, dev_docs, display_name, exclude, extensions, features, …." | ||
| ); |
There was a problem hiding this comment.
Severity: P3 (Low)
Normalize the mock filesystem's absolute paths before comparing these diagnostic snapshots. On Windows, /docs/docset.yml resolves to a drive-qualified path such as C:/docs/docset.yml, but this expectation requires /docs/docset.yml. The existing scrubbers only cover repository and temporary paths, so these tests fail in the Windows CI job despite producing the correct diagnostics.
Generated by Libra. React with 👍 or 👎 to give feedback on this comment.
| .IndexOf("Your Privacy Choices", StringComparison.Ordinal) | ||
| .Should() | ||
| .BeLessThan(preferencesLink.IndexOf("<svg", StringComparison.Ordinal)); | ||
| html.Should().MatchHtmlSnapshot(DocsHtml.Body()); |
There was a problem hiding this comment.
Severity: P3 (Low)
Scrub or otherwise stabilize the copyright year before snapshotting the entire footer. _AssemblerFooter renders DateTime.Today.Year, while the verified snapshot fixes it to 2026, so this previously date-independent test will fail when the calendar year changes without any rendering regression.
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>
Contributor
Docs preview (local build)Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4726/ |
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 Navigation.Tests and Elastic.Documentation.Configuration.Tests onto Nullean.Snapshots.
Navigation.Tests (1,197 tests, 8 fewer because tests that rendered the same input are merged)
Url.Should().Be,BeOfType,NavigationItems.ElementAt,HaveCount) now compare an inline snapshot ofNavigationDump.ToText(). The dump lists each node's index page, where most URLs live, and flags a child whoseParentis wrong.Configuration.Tests (867 tests)
UnknownYamlKeyTests,ApiConfigurationTests, CTA and storybook errors), deserialised table-of-contents trees, the generatednavigation.ymltext.ApiConfigurationTestsuses file snapshots.These tests are stricter; each commit says so. Before accepting a tree snapshot I read it against every
Url, type and count the old test asserted.Found by snapshot testing
SiteDocumentationSetsTests.DocumentationSetNavigationHasCorrectStructureassertedx.NavigationTitle.Should().Be(x.NavigationTitle): a tautology.EmitsError_*tests inApiConfigurationTestsonly counted one error, so any single error passed.StartWith(prefix), orContain(d => Severity == Error && Message.Contains(...)).ComplexSiteNavigationTests.FolderNavigationWithinNestedTocsHasCorrectPathPrefixthe context is not an assembler context, and the nestedpath_prefixvalues are ignored: both TOCs resolve to/platform/cloud. The old prefix assertions could not tell. The snapshot shows it as it is.Checked
snap statusclean,snap orphansempty.FolderNavigation.Urlwith/xfailed 35 Navigation tests; renaming thenav-tree__chevronclass failed the HTML snapshots; rewording an API error, changing a TOC parent path and forcing the site-navislandparse to false failed 12 Configuration tests.Contain/NotContainliteral in the HTML tests against the snapshot text.Left as they were
Codex, Island, Constructor, FolderSortOrder, FolderExclude, PhysicalDocset,
NavigationRenderModelandNavigationCurrentMarkertests (cheap precise checks). The landing page and StyleGuide tests, whose whole-page snapshots would be 400-700 lines. The[Arguments]tables, CsCheck properties,Throwassertions and default-value checks.Not checked
Windows: mock file system paths such as
/docs/docset.ymlappear unscrubbed, so a Windows run would print drive-letter paths.DiagnosticsSnapshothas no column, so column assertions stay explicit.🤖 Generated with Claude Code