Repository navigation
Share the helpers that ApiExplorer tests kept copying - #4660
Merged
Merged
Conversation
Six test files each declared a private StubHandler. Nine files carried their own copy of the inline-spec loader, two of them with a generator-from-spec helper, and 14 BuildContext constructions in 11 files differed only in the collector. These now live in TestHelpers.CreateBuildContext, TestHelpers.CreateScopedBuildContext, TestSpecs.LoadValidSpecAsync, TestSpecs.CreateGeneratorWithSpecAsync and one shared StubHandler. The test count is unchanged at 1407. 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}
ApiExplorerFixture.DisposeAsync now calls GC.SuppressFinalize, which clears the CA1816 warning in the test build. KibanaApiMarkdownNavigationTests builds its context with a real FileSystem, the same one the shared helper creates, so it now uses TestHelpers.CreateBuildContext too. The usings that these helper changes left unused are removed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
akira28
added a commit
that referenced
this pull request
Nov 3, 2026
The shared test helpers PR (#4660) makes the same DisposeAsync change. Keeping it in both PRs made them conflict in ApiExplorerFixture.cs, so this PR no longer touches that method. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
TestHelpers.CreateBuildContext now takes an optional build type and URL path prefix and sets them while it constructs the context, which is the only way to set those init-only properties. ApiLayoutVersionDropdownTests was the last file that built the same context by hand, and it now calls the helper. The other callers get the build context defaults, so their behaviour is unchanged. 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/4660/ |
reakaleek
approved these changes
Nov 3, 2026
akira28
enabled auto-merge (squash)
November 3, 2026 11:00
akira28
disabled auto-merge
November 3, 2026 11:04
Mpdreamz
pushed a commit
that referenced
this pull request
Nov 4, 2026
…#4654) * Pilot Verify snapshot tests in Elastic.ApiExplorer.Tests Replace eight HTML Contain checks in the heading-level test with one Verify snapshot, keeping the heading-order assertion. The five operation rendering tests now share RenderAsync from ApiExplorerFixture instead of five private copies. Verify.TUnit 31.28.0 is the newest release that needs a TUnit.Core no newer than the repo's TUnit 1.65.0. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Snapshot only the operation content in the Verify pilot The full-page snapshot embeds the frontend bundle name and the CI git branch, commit and ref, so it failed on every build that differed from the one that recorded it. The test now snapshots the main content element, which holds none of those values. The asset-hash and year scrubbers have nothing left to scrub and are removed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Move the Verify pilot to Verify.TUnit 32.0.1 and fix CA1816 Verify.TUnit 32.0.1 is the newest release before the Verify 33 sponsorship check, and it works with the TUnit 1.78.0 now on main. ApiExplorerFixture.DisposeAsync now calls GC.SuppressFinalize, which clears the CA1816 warning that the PR's diff surfaced on that line. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Pin Verify.TUnit to 32.0.0, the last release before the Verify fee Verify's maintenance fee covers every release published after 1 September 2026. Verify.TUnit 32.0.0 was published on 2026-08-26 and 32.0.1 on 2026-09-12, so 32.0.1 is covered. 32.0.0 needs TUnit.Core 1.65.63 or later, which the TUnit 1.78.0 on main satisfies. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Leave the CA1816 fix in ApiExplorerFixture to the helpers PR The shared test helpers PR (#4660) makes the same DisposeAsync change. Keeping it in both PRs made them conflict in ApiExplorerFixture.cs, so this PR no longer touches that method. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Switch the snapshot pilot from Verify to Nullean.Snapshots Nullean.Snapshots is MIT licensed, so the pilot no longer depends on Verify's maintenance fee. It is built on AwesomeAssertions and has a TUnit adapter, and its dependency floors match the versions this repo already pins. The pilot test now calls MatchHtmlSnapshot with a Select on the operation content, which replaces the manual AngleSharp extraction and the module initializer. Verified snapshots live under .snapshots, and received files go to .artifacts, which is already ignored, so the *.received.* rule is gone. The packages are 0.1.0-alpha.1. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Match the snapshot folder at any depth in .gitattributes The pattern .snapshots/** only matches at the repository root, so git applied no line-ending rule to tests/Elastic.ApiExplorer.Tests/.snapshots and a Windows checkout with core.autocrlf could convert the verified file to CRLF. **/.snapshots/** matches the folder wherever a test project keeps it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * Update Nullean.Snapshots to 0.2.0 and drop the hash from the snapshot name Nullean.Snapshots 0.2.0 leaves fixtures injected with ClassDataSource out of the arguments hash, which fixes the report from this pilot (nullean/snapshots#13). The pilot test now writes OperationHeadingLevelTests/Render_SearchOperation_UsesOneH1ThenH2SectionsAndH3Nested.verified.html with no change to the test class. The snapshot content is byte for byte identical, so the file is renamed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> --------- 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.
The API Explorer tests now share one
StubHandler, one build-context helper and one inline-spec loader instead of copying them file by file. The change removes about 210 net lines and leaves the test count at 1,407.Affects: Internals only
Prompt summary: The tests keep growing, so the goal is to cut duplication without changing what they check. This PR is the first API Explorer refactor: it consolidates the repeated helpers and does not touch the Verify snapshot work in #4654.
Why
Six test files each declare a private
StubHandler. Nine files carry their own copy of the code that loads an inline OpenAPI spec, and 16BuildContextconstructions in 13 files differ only in the collector, the build type or the URL path prefix. A fix to any of these has to be repeated in every copy.What
One stub HTTP handler
StubHandleris now a single internal class. It answers from the supplied responder and records the requested paths, which the two files that asserted on paths already did. It skips recording when the request URI is relative instead of throwing.Build contexts
TestHelpers.CreateBuildContext(collector?, buildType, urlPathPrefix)replaces the repeated construction rooted at the working directory. The build type and prefix areinit-only, so the helper sets them while it builds the context, which letsApiLayoutVersionDropdownTestsuse it too.TestHelpers.CreateScopedBuildContextreplaces the identical four-argumentCreateContextin the multi-version and current-spec-resolution tests. It keeps its ownDocumentationFileSystem.Resolveoverload, so it behaves as before.Spec loading
TestSpecs.LoadSpecAsyncreplaces six hand-written copies, in five files, of the write-to-temp-file, load, delete pattern.LoadValidSpecAsynckeeps the stricter behaviour of the in-memory loaders, which throw on parse errors.CreateGeneratorWithSpecAsyncreplaces the two near-identicalCreateGeneratorWithSpechelpers.Warning fix
ApiExplorerFixture.DisposeAsyncnow callsGC.SuppressFinalize, which clears the CA1816 warning from the test build. #4654 makes the same change and adds methods right after it, so whichever of the two merges second needs a one-hunk conflict resolution in that file. The files the helpers touched also lose theusinglines they no longer need.Out of scope: Tests that read real spec files from disk and
ReaderTestskeep their direct reader calls.Verify
dotnet test --project tests/Elastic.ApiExplorer.Tests ./build.sh lintThe test run reports 1,407 tests: 1,406 passed and 1 skipped, the same as
main.