Repository navigation
Move six smaller test areas onto Nullean.Snapshots - #4727
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>
EmitterTests checked ~70 Contain/NotContain fragments per output. Each test now snapshots the whole emitted markdown inline. The tests became stricter: every old fact (no raw << , no include::/ifeval::, fences, links) is visible in the literals, and any other output change now fails too. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
ContentStackMappingTests asserted a handful of properties per fixture; each fixture test now snapshots the whole mapped SiteDocument (the two clock-stamped fields are removed first). MappingStructureTests walked the mapping JSON property by property; the five document mappings are now snapshotted whole. The tests became stricter. Dropped the old tautology Body.Should().Be(Body) and if-guarded checks that passed vacuously when the field was absent. Random AutoBogus roundtrips and the small inline-JSON mapper cases stay. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
SearchQueryBuilderTests and ApiPropertySearchTests walked the query JSON with GetProperty chains and Contain/NotContain. The queries and the api.* mapping (lexical and semantic) are now JSON snapshots, so the tests became stricter. The existing one-line expectations are kept as inline JSON snapshots. Dropped an unused 'prefix' argument that both rows set to false. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…formers StreamTransformerTests parsed the transformer output back into events and asserted counts and a few fields. They now snapshot the SSE text the client receives, with generated ids and clock timestamps scrubbed. The tests became stricter: every event, its order and its content is pinned. The three parameterised ConversationStart checks (event present, id, timestamp) collapse into one snapshot per transformer. The multi-line data test asserted HaveCountGreaterThanOrEqualTo(0), which cannot fail; it now pins the output. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
SitemapTests parsed the XML and checked element by element, and checked the PIT search body with Contain. The generated sitemap.xml and the search body are now inline snapshots, so the tests became stricter (namespace, declaration, lastmod format, size, sort and all). The ordering and API-exclusion tests used the current time for lastmod; they use a fixed date now. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
McpServerInstructionTests had ~30 Contain/NotContain checks on the composed instruction text, plus two tests that already compared the whole text with Be. The Contain tests are subsumed by the whole-text comparison, so they are removed and the two exact tests became inline snapshots with the same literals (every fact the Contain/NotContain checks asserted is visible in them). 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
+47
to
+48
| public static SnapshotOptions Scrub() => | ||
| new SnapshotOptions().Scrub(Scrubbers.Guids).Scrub(new Regex(@"""timestamp"":\d{13}"), "\"timestamp\":{now}"); |
There was a problem hiding this comment.
Severity: P3 (Low)
Scrubbers.Guids replaces every GUID, including the conversation id the test controls. The old tests checked that the Agent Builder transformer passed through 360222c5-76aa-405a-8316-703e1061b621 from conversation_id_set, and that the LLM Gateway transformer used the generatedConversationId passed in. Both checks are now lost. If either transformer emitted a fresh random GUID, it would still render as Guid_1 and the snapshots would pass. That is a real regression in what these tests catch, against the PR's claim that the tests got stricter. To fix it, scrub only the generated event id values, or swap the known conversation id for a fixed token before scrubbing GUIDs.
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>
| #pragma warning disable CA2255 // a test assembly is application code | ||
| [ModuleInitializer] | ||
| #pragma warning restore CA2255 | ||
| internal static void PinTimeZone() => Environment.SetEnvironmentVariable("TZ", "UTC"); |
There was a problem hiding this comment.
Severity: P3 (Low)
Setting TZ does not change .NET's local time zone on Windows. On a non-UTC Windows machine, ParseDate still assigns the local offset to the two date-only fixture values, so both release snapshots now fail against the new +00:00 expectations. Normalize these date-only values in the test snapshot preparation using a platform-independent approach rather than relying on the CI runner's time zone.
Generated by Libra. React with 👍 or 👎 to give feedback on this comment.
…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>
The common-behaviour test's two verified files were 230 characters from the repository root. With the runner's checkout folder in front, and the longer received path under .artifacts when a run fails, they passed Windows' 260 characters, and git on the runner could not check the pull request out at all. The method is now NewConversationEmitsConversationStart. The files are renamed, with the same content. 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>
… platform The SSE text is an argument of NewConversationEmitsConversationStart, and the name of its snapshot files carries a hash of the arguments. A checkout on Windows turns this file's line endings into "\r\n", so the text, and with it the hash, differed there: the runner looked for `...AgentBuilderStreamTransformer.ea024bf4.verified.txt` where macOS and Linux write `.084fb69f`. The text is now normalised to "\n". 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/4727/ |
#4732 scoped API navigation search to the reader's version tree. It added the `key` and `trees` fields to the API property mapping and moved the filter that excludes bare /docs roots and hidden documents to wrap the whole query, lexical and semantic. These snapshots caught both, which the old Contain checks would not have. Ten snapshots changed and nothing else in them: two mapping snapshots gain the `key` and `trees` fields (15 lines each), and eight query snapshots lose the same 23-line filter block, which is now applied around the whole query. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The two whole-mapping snapshots (DocumentationDocument and WebsiteSearchDocument) hold the API property mapping too, so #4732's new `key` and `trees` fields show up there as well: the only change in either file. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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 six smaller test areas onto Nullean.Snapshots. One commit each; every commit says its tests became stricter.
EmitterTests: 25 tests, about 97Contain/NotContainchecks replaced by 25 inline snapshots of the whole emitted markdown.ContentStackMappingTestsfixtures became JSON file snapshots of the mapped document (batch_index_date, andlast_updatedwhen the entry has none, are removed because they are the current time). 26MappingStructureTestsproperty walks became 5 whole-mapping snapshots.SearchQueryBuilderTests,ApiPropertySearchTests): the existing expectations are inline JSON snapshots with the same literals; 11 query tests are file snapshots (one file per[Arguments]row); the 7Mapping_*tests collapsed into oneMapping_Api(semantic).StreamTransformerTests: 6 inline SSE text snapshots (GUIDs and clock timestamps scrubbed) and one file snapshot per transformer.SitemapTests: 3 XML and 3 PIT search-body inline snapshots.McpServerInstructionTests: 3Containtests removed, because two exact-text tests already covered them; those two are now inline snapshots.Found by snapshot testing
doc.Body.Should().Be(doc.Body)appeared twice inContentStackMappingTests: a tautology.if (x.TryGetProperty(...)) ... .BeFalse()inMappingStructureTestspassed when the field was absent.BuildApiPropertyQuery_PrefixMatch…test passedprefix: false, so theHaveCount(2)branch never ran.TransformAsyncWithMultilineDataFieldsAccumulatesCorrectlyassertedHaveCountGreaterThanOrEqualTo(0), which cannot fail. The snapshot shows the multi-line data is accumulated (onemessage_chunkwith"multiline") and noconversation_endis emitted.Checked
snap statusclean,snap orphansempty.hierarchy_analyzer(25), boost values (33), a leaked tool-result payload (1), the/docs/api/sitemap exclusion, instruction wording.StreamTransformerTestsran three times with no flakiness.Left as they were
SiteSearch
DocumentSerializationTests(random-data round trips), small mapper tests,ProcessHitand null-return tests. Deploying, OpenApiIndex andDocumentation.Testsare behavioural tests; I only counted theirContainchecks and did not read them.Notes
The lexical and semantic
Mapping_Apisnapshots are byte-identical.MatchInlineSnapshot(options: …, expected: …)puts the literal in theexpected:argument when accepted.Not checked
The CI path (
unit-testtarget) for these projects on their own; other operating systems.What CI added: a product bug
ContentStackMapper.ParseDatereads a date without an offset in the machine's time zone. It callsDateTimeOffset.TryParse(value, out var dto), so for the date-only values in press and product releases ("date": "2023-09-01") the indexedpublished_datecarries the offset of whoever ranessc. My snapshots said+01:00and+02:00(Amsterdam, and daylight saving time in September); a GitHub runner produced+00:00. TwoContentStackMappingTestssnapshots failed for it. A fix would parse withDateTimeStyles.AssumeUniversal; it changes what is indexed whenesscruns in a non-UTC zone, so it is not part of this PR.The test assembly now sets
TZ=UTCwhen it loads, so the verified files read the same everywhere, and the two files are regenerated (the only change in each is the offset). Passes withTZunset,Asia/TokyoandAmerica/Los_Angeles. .NET honoursTZon Linux and macOS; Windows ignores it, but its CI runners are on UTC.🤖 Generated with Claude Code