test: Close the clients cmd/osgen's generated tests and test helpers construct - #1049
Merged
ryanyuan merged 4 commits intoAug 7, 2026
Merged
Conversation
The round-trip test template built a client per subtest and never released it. opensearchtransport.New starts a cluster-health refresher and a node-stats poller unconditionally (healthCheckRate derives from the server core count and is always positive; NodeStatsInterval: 0 means auto-derive, not disabled), and opensearch.NewClient adds a node-discovery goroutine, so every subtest left those goroutines running for the rest of the test binary's life and kept requesting the subtest's httptest.Server after the test body returned. Register a t.Cleanup on the resource owner in both subtests of both flavors: the wrapped opensearch.Client for plugin packages (the plugin client type has no Close), the generated client itself for opensearchapi. The cleanup is registered after the require.NoError guarding construction, so a failed construction never gets one. Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Output of `make gen` after the round-trip template change. No hand edits. Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
The round-trip template was one of four places the generated tests build a client; the three shared test-helper constructors were left untouched, so the sweep only reached the subtests. opensearchtransport.New falls back to context.Background() when cfg.Context is nil, so plugintest.CreateFailingClient, which passes a bare opensearch.Config, left its pollers running for the whole test binary. plugintest.NewClient and osapitest.CreateFailingClient both inherit t.Context(), so their pollers stopped when the test did, but neither closed the transport's idle connections. All three now register a t.Cleanup that closes the client they return. Together they cover 806 call sites: 313 plugintest.NewClient, 312 plugintest.CreateFailingClient, and 181 osapitest.CreateFailingClient. opensearch.NewClient is never cached -- the refcounted cache serves NewDefaultClient only -- so each helper call owns a fresh transport and closing it cannot affect another test. No call site closes the returned client itself. osapitest/helper.go is hand-written and belonged with the earlier transport-leak work; the fix lands here because that branch missed it. Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
Output of `make gen` after the plugin test-helper template change. No hand edits. Adds 50 t.Cleanup calls across the 25 generated plugins/*/internal/*test/helpers_gen.go files. Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
ryanyuan
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1049 +/- ##
==========================================
- Coverage 61.31% 61.30% -0.01%
==========================================
Files 666 666
Lines 59841 59841
==========================================
- Hits 36689 36687 -2
- Misses 21245 21247 +2
Partials 1907 1907
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ryanyuan
approved these changes
Aug 7, 2026
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
2 participants
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.
Description
cmd/osgen's round-trip test template built a client per subtest and never released it, so each of the 960 subtests acrossopensearchapi/andplugins/left transport goroutines running for the rest of the test binary's life.opensearchtransport.Newstarts both background pollers unconditionally:healthCheckRatederives from the server core count and is always positiveNodeStatsInterval: 0means auto-derive, not disabledopensearch.NewClientadds a node-discovery goroutine on top. The pollers kept requesting the subtest'shttptest.Serverafter the test body returned, which also madeServer.Closeblock on in-flight requests.opensearch.NewClientis never cached (the refcounted cache servesNewDefaultClientonly), so each helper call owns a fresh transport and closing it cannot affect another test. No call site closes the returned client itself.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.