Repository navigation
Run the MCP and page-feedback tests from .http files - #4728
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>
McpHandshakeTests posted one initialize request, parsed the SSE data line by hand and asserted three fields. The request now lives in http/mcp-handshake.http (initialize, then tools/list) and the whole response is the snapshot, so a change to the capabilities, the instructions or any tool definition shows up as a diff. Stricter than before: the full initialize result and the complete tools/list response are compared, not just protocolVersion and serverInfo. The SSE body is quoted text in the HTTP format, so a small scrubber pretty-prints each data: line. The server version is replaced by a placeholder only when it equals the assembly's informational version, which keeps the old check that the wiring reports the right version. Adds the Nullean.Snapshots.Http packages (0.4.0) to central package management and to the two HTTP-facing test projects. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… stored records http/page-feedback.http sends a valid PUT with an euid cookie (204), an invalid PUT (400) and a DELETE (204) through the shared test host, and the responses are the snapshot. The three tests that checked the PageFeedbackRecord the endpoint handed to the service field by field now compare the whole record as an inline JSON snapshot. The feedback id is replaced by a placeholder only where it equals the id in the request URL, so storing the record under another id still fails. Stricter than before: the reaction-only and multiple-reasons tests used to check only some fields and now check all of them. The other 400/503 tests stay: they check the service was not called, which a response snapshot cannot show. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
DocumentUrlNormalizerTests needs no Elasticsearch, but it sat in the integration project because DocumentGateway.NormalizeUrl is internal and only that project could see it. The Mcp.Remote assembly now also shows its internals to Mcp.Remote.Tests, so the test moves to the unit project and runs in CI, which the integration tests do not. 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}
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>
# Conflicts: # Directory.Packages.props
Contributor
Docs preview (local build)Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4728/ |
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.
McpHandshakeTestsbecomeshttp/mcp-handshake.http(initialize, thentools/list), run with[HttpFiles]againstWebApplicationFactory<Program>and snapshotted. It keeps theMCP_SERVER_PROFILEsetup. The server version is scrubbed only where it equals the assembly's informational version, so the old "wiring reports the right version" check survives. It is stricter: the full initialize result andtools/listare compared. A smallSseScrubberspretty-prints thedata:JSON in the event stream so diffs read.http/page-feedback.httpcovers PUT valid (204), PUT with a missing reason (400) and DELETE (204), run through the shared factory. The three tests that checked the storedPageFeedbackRecordfield by field now use an inline JSON snapshot of the whole record, with the feedback id scrubbed by its own value so a wrong id still fails. Stricter forReactionOnlyandMultipleReasons, which checked only some fields.DocumentUrlNormalizerTestsmoves from the integration project toMcp.Remote.Tests. It needs no Elasticsearch, butDocumentGateway.NormalizeUrlis internal and only the integration project could see it. The onesrc/change is a secondInternalsVisibleToentry; the test now runs in CI, which the integration tests do not.Nullean.Snapshots.Httpand.Http.Assertions0.4.0.Checked
snap statusclean,snap orphansempty. Both integration projects still build.NoContenttoOkfailed with$[0].status expected "204 No Content" actual "200 OK".Not converted
Elastic.Documentation.IntegrationTests(Aspire smoke tests) no longer exists since Remove the Aspire AppHost and the end-to-end tests that need it #4689.Mcp.Remote.IntegrationTeststool-JSON snapshots and aSearch.IntegrationTestsrelevance report need a live Elasticsearch and index. I did not write verified files by hand, and a top-5 URL report changes on every reindex, so it belongs in a scheduled job. Neither integration project was run.AskAiGatewayStreamingTests,EuidEnrichmentTestsand the search tests are untouched.Library gaps hit here
The event-stream body is stored as one unreadable line (hence the scrubber), and HTTP snapshots take only text scrubbers, no
IgnorePaths. Both are filed against the library.🤖 Generated with Claude Code