Repository navigation
Move the changelog generated-output tests onto Nullean.Snapshots - #4725
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 render tests read index.md, highlights.md and the other generated files back from the mock file system and checked a few Contain and NotContain fragments. Each one now snapshots every file the service wrote (MockFileSystemDump), inline when the output is short and as a verified file otherwise, so a change to any generated line shows up as a diff and the absence of a file is part of the expectation. The tests became stricter: before accepting, every old Contain and NotContain fragment was checked against the received output, and the ordering checks in the GFM tests (sections, highlights first) are covered by the snapshot. Warnings and errors the tests looked for with Diagnostics.Should().Contain(...) are inline diagnostics snapshots now, so any other diagnostic would fail them too. Guids in the paths of the mock file system are numbered by a scrubber registered for this project. The invalid-bundle test accepted either of two messages; it now asserts the single error that is actually produced. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The create tests read the YAML file the service wrote and checked a few Contain fragments (title, type, a PR url...). Each one now snapshots the whole file inline, and the multi-file tests snapshot every entry under its file name. The snapshots are text, not YAML trees, so the quoting and key order of the written YAML stay pinned (several old checks were about exactly that, e.g. feature-id: "feature:foo"). Every changelog file starts with the same 78 lines documenting the fields; ChangelogEntryYaml.WithoutSchemaComments strips them so the snapshots are about the entry, and the Unpack test keeps its check that the block is there. The bundle's release-date is scrubbed. Warnings and errors that were looked up with Diagnostics.Should().Contain are inline diagnostics snapshots, so the exact message and any additional diagnostic is now part of the expectation. The tests became stricter: before accepting, every old Contain and NotContain fragment was checked against the received output. One old check was vacuous: the multiple-PRs-with-failing-fetch test accepted either PR url and only one is written; the snapshot pins the first. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The bundling tests read the bundle (or amend sidecar) the service wrote and checked a few Contain fragments, or split the text to count entries. Each one now snapshots the whole file, inline up to 35 lines and as a verified file beyond that. Errors and warnings looked up with Diagnostics.Should().Contain(...) are inline diagnostics snapshots. The tests became stricter: before accepting, every old Contain and NotContain fragment was checked against the received output (top-level product checks that applied only to the part before entries: still hold, the snapshot shows the entries with their own products). The snapshots also pin the deprecation and repository-name warnings several profile tests used to ignore. A bundle is stamped with the day it was created, so the project scrubs a release-date that equals today (a release-date a test sets stays visible). Also two comments describing the create tests are put back. The two checksum comparison tests (two bundles from the same data) and the calls that need two outputs in one test are left as they were. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The four CDN amend tests checked a few Contain fragments of the sidecar that ReadSingleAmendAsync returns. They snapshot it inline now. The tests became stricter; the old fragments (exclude-entries, the entry names, the title) were checked against the received output first. 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}
|
|
||
| // Description should be hidden | ||
| asciidocContent.Should().NotContain("This description should be hidden in asciidoc format when --no-descriptions is used."); | ||
| * Test feature for asciidoc. {es-pull}300[#300] |
There was a problem hiding this comment.
Severity: P3 (Low)
This inline asciidoc snapshot needs a trailing space on the entry line, which the renderer emits after [#300]. .editorconfig sets trim_trailing_whitespace=true for *.cs, so any editor that follows it removes that space the next time someone saves the file. The test then fails with nothing visible to explain it. OutputFormatTests.cs:169 and HighlightsRenderTests.cs:186,195 have the same problem. Put these asciidoc outputs in .verified.txt files (other asciidoc tests in this PR already do), or make the trailing whitespace visible, for example by scrubbing it to a marker.
Generated by Libra. Specialist: libra-complexity. 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>
WithoutSchemaComments looks for the blank line that ends the field documentation, "\n\n". The writer uses the platform's line ending, so on Windows it is "\r\n\r\n", the header stayed in, and 161 tests failed there. The helper now normalises line endings first. The new test runs both endings and fails for "\r\n" without the fix. 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>
A bundle records the SHA-1 of each entry file it was made from. The test inputs are raw string literals and the product writes files with the
platform's line ending, so on Windows the same entry has other bytes and another checksum: 111 bundle snapshots failed there on the checksum line
alone. The value is now replaced by {sha1} in the snapshots. What a checksum must be is still checked by the tests that compute the expected one
from the file content.
All 112 changed snapshots differ only in their checksum lines (reviewed before accepting).
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
| entries: | ||
| - file: | ||
| name: 1755268140-resolved-feature.yaml | ||
| checksum: {sha1} |
There was a problem hiding this comment.
Severity: P3 (Low)
Retain an unscrubbed checksum equality assertion in the checksum-specific tests. This snapshot now accepts any 40-character hexadecimal checksum, so AmendBundle_WithComments_ProducesNormalizedChecksum would pass if the amend service started hashing the raw, comment-bearing YAML instead of normalized YAML. The computed expectedChecksum is never asserted; the same gap exists in BundleChangelogs_WithComments_ProducesNormalizedChecksum. Compare the extracted checksum with expectedChecksum before snapshot scrubbing.
Generated by Libra. React with 👍 or 👎 to give feedback on this comment.
Contributor
Docs preview (local build)Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4725/ |
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 the generated-output tests in Elastic.Changelog.Tests (
Render/,Create/and the bundling tests) onto Nullean.Snapshots..md/.yamlback from the mock file system and checked several fragments now has one snapshot of what was written (MockFileSystemDumpfor multi-file output).Contain-style diagnostics checks became inline diagnostics snapshots. They now also pin warnings the old tests ignored, such as the profileoutput_productsdeprecation.WithoutSchemaComments()strips the 78-line field-documentation header every created changelog starts with; theUnpacktest still asserts it is there.ChangelogEntryYamlandSnapshotSetup(a GUID scrubber and one that replacesrelease-date: <today>with{today}; a release date a test sets on purpose stays visible).These tests are stricter; each commit says so. 1,728 tests, all of them still there.
Found by snapshot testing
CreateChangelog_WithMultiplePrsButPrFetchFails_GeneratesBasicChangelogsaccepted either PR URL. Only the first (12345) is written, though a comment claimed the last one wins. The snapshot pins the first.RenderChangelogs_WithInvalidBundleStructure_ReturnsErroraccepted either "No changelog entries to render" or "Failed to deserialize". It now asserts the single real error. It is not a snapshot because the message carries a SharpYaml stack trace with line numbers.NotContain "No --title option provided", someWarnings > 0). They are now an empty or exact diagnostics snapshot.ExtractionDisabled…create tests writeimpact: ''andaction: ''for non-breaking entries. The snapshots now pin that.Things to know when reviewing
output_productsdeprecation warning now fails many bundle tests.release-date.git diff -- '*.cs'as well as the.verified.txtfiles. A few explanatory comments in the bundle tests went with the assertions they described (for example "Entries are always inlined…").Checked
snap statusclean,snap orphansempty.IndexMarkdownRendererfailed 25 render tests; changing a TODO comment and a validator message failed 2 create tests; renaming thegit_refproperty failed 4 bundle tests (-git_ref/+git_reff). The CDN amend tests got no mutation.Contain/NotContainfragment and diagnostics assertion against the received output. Non-literal fragments I read by eye.Left as they were
The checksum comparison tests, the amend and backfill property checks, Evaluation and Uploading (FakeItEasy), tests that count or name files, and the other classes with diagnostics
Containchecks (ChangelogConfigurationTests,GitHubCommitRangeServiceTests,ChangelogRemoveTests,ReleaseNotesPageParserTests, …), which would make a follow-up.Not checked
CI and other operating systems. Two verified files have trailing spaces in asciidoc output, which git may warn about.
What CI added
75 bundle snapshots failed on every CI platform. Tests that do not inject an
IEnvironmentVariableslet the bundle service read the process environment, and it resolves the repository fromGITHUB_REPOSITORY. On GitHub Actions that iselastic/docs-builder, so the bundles readrepo: docs-builderthere, and on a laptop they carried the warning about an unresolved repository. The oldContainchecks never saw the difference. The test assembly now clears the variable when it loads; tests that need it pass it in throughGithubRepositoryEnvironmentas before.Reproduced locally with
GITHUB_REPOSITORY=elastic/docs-builder dotnet test(75 failures), and all 1,728 tests pass with it set after the change.🤖 Generated with Claude Code