Skip to content

Commit 6d83259

Browse files
committed
Refactor test infrastructure and consolidate test utilities
Modernize the test infrastructure by consolidating test helpers into a shared testutil package and enhancing connection reliability: - Move test utilities from internal/test to opensearchutil/testutil for broader reusability across the project and external packages - Remove obsolete internal/test/config.go in favor of improved helper functions with better error handling and connection management - Add dynamic field filtering for JSON comparison tests to handle version-specific and environment-dependent OpenSearch responses - Enhance connection robustness with improved readiness checks and health monitoring in opensearchtransport layer - Update all integration tests across opensearchapi, plugins, and transport packages to use the new unified test infrastructure - Add comprehensive documentation and examples for the new test utilities This refactor provides a more maintainable foundation for testing across different OpenSearch versions and environments while reducing code duplication and improving test reliability. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 6643d35 commit 6d83259

58 files changed

Lines changed: 641 additions & 292 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -227,8 +227,8 @@ cluster.scale.3: ## Start full 3-node cluster
227227

228228
cluster.get-cert:
229229
@if [[ -v SECURE_INTEGRATION ]] && [[ $$SECURE_INTEGRATION == "true" ]]; then \
230-
docker cp $$(docker compose --project-directory .ci/opensearch ps --format '{{.Name}}'):/usr/share/opensearch/config/kirk.pem admin.pem && \
231-
docker cp $$(docker compose --project-directory .ci/opensearch ps --format '{{.Name}}'):/usr/share/opensearch/config/kirk-key.pem admin.key; \
230+
docker cp $$(docker compose --project-directory .ci/opensearch ps --format '{{.Name}}' | head -1):/usr/share/opensearch/config/kirk.pem admin.pem && \
231+
docker cp $$(docker compose --project-directory .ci/opensearch ps --format '{{.Name}}' | head -1):/usr/share/opensearch/config/kirk-key.pem admin.key; \
232232
fi
233233

234234

error_test.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func TestError(t *testing.T) {
5050
assert.True(t, resp.IsError())
5151
err := opensearch.ParseError(resp)
5252
var testError *opensearch.StructError
53-
require.True(t, errors.As(err, &testError))
53+
require.ErrorAs(t, err, &testError)
5454
assert.Equal(t, http.StatusBadRequest, testError.Status)
5555
assert.Equal(t, "resource_already_exists_exception", testError.Err.Type)
5656
assert.Equal(t, "index [test/HU2mN_RMRXGcS38j3yV-VQ] already exists", testError.Err.Reason)
@@ -92,7 +92,7 @@ func TestError(t *testing.T) {
9292
assert.True(t, resp.IsError())
9393
err := opensearch.ParseError(resp)
9494
var testError *opensearch.StructError
95-
require.True(t, errors.As(err, &testError))
95+
require.ErrorAs(t, err, &testError)
9696
assert.Equal(t, http.StatusBadRequest, testError.Status)
9797
assert.Equal(t, "illegal_argument_exception", testError.Err.Type)
9898
assert.Equal(t, "composable template [posts] template after composition is invalid", testError.Err.Reason)
@@ -117,14 +117,14 @@ func TestError(t *testing.T) {
117117
}`),
118118
)
119119
body, err := io.ReadAll(reader)
120-
require.Nil(t, err)
120+
require.NoError(t, err)
121121

122122
var errStruct *opensearch.StructError
123123
err = json.Unmarshal(body, &errStruct)
124-
assert.NotNil(t, err)
124+
assert.Error(t, err)
125125

126126
var jsonError *json.UnmarshalTypeError
127-
assert.True(t, errors.As(err, &jsonError))
127+
assert.ErrorAs(t, err, &jsonError)
128128
})
129129
t.Run("string", func(t *testing.T) {
130130
reader := io.NopCloser(
@@ -134,14 +134,14 @@ func TestError(t *testing.T) {
134134
}`),
135135
)
136136
body, err := io.ReadAll(reader)
137-
require.Nil(t, err)
137+
require.NoError(t, err)
138138

139139
var errStruct *opensearch.StructError
140140
err = json.Unmarshal(body, &errStruct)
141-
assert.NotNil(t, err)
141+
assert.Error(t, err)
142142

143143
var errStr *opensearch.StringError
144-
require.True(t, errors.As(err, &errStr))
144+
require.ErrorAs(t, err, &errStr)
145145
})
146146
})
147147
})
@@ -159,7 +159,7 @@ func TestError(t *testing.T) {
159159
assert.True(t, resp.IsError())
160160
err := opensearch.ParseError(resp)
161161
var testError *opensearch.StringError
162-
require.True(t, errors.As(err, &testError))
162+
require.ErrorAs(t, err, &testError)
163163
assert.Equal(t, http.StatusMethodNotAllowed, testError.Status)
164164
assert.Contains(t, testError.Err, "Incorrect HTTP method for uri")
165165
_ = fmt.Sprintf("%s", testError)
@@ -175,7 +175,7 @@ func TestError(t *testing.T) {
175175
assert.True(t, resp.IsError())
176176
err := opensearch.ParseError(resp)
177177
var testError *opensearch.StringError
178-
require.True(t, errors.As(err, &testError))
178+
require.ErrorAs(t, err, &testError)
179179
assert.Equal(t, http.StatusNotFound, testError.Status)
180180
assert.Contains(t, testError.Err, "{\"_index\":\"index\",\"_id\":\"2\",\"matched\":false}")
181181
_ = fmt.Sprintf("%s", testError)
@@ -193,7 +193,7 @@ func TestError(t *testing.T) {
193193
assert.True(t, resp.IsError())
194194
err := opensearch.ParseError(resp)
195195
var testError *opensearch.Error
196-
require.True(t, errors.As(err, &testError))
196+
require.ErrorAs(t, err, &testError)
197197
assert.Contains(t, testError.Err, "no handler found for uri [/_plugins/_security/xxx] and method [GET]")
198198
_ = fmt.Sprintf("%s", testError)
199199
})
@@ -214,7 +214,7 @@ func TestError(t *testing.T) {
214214
assert.True(t, resp.IsError())
215215
err := opensearch.ParseError(resp)
216216
var testError *opensearch.ReasonError
217-
require.True(t, errors.As(err, &testError))
217+
require.ErrorAs(t, err, &testError)
218218
assert.Equal(t, "error", testError.Status)
219219
assert.Contains(t, testError.Reason, "Invalid configuration")
220220
_ = fmt.Sprintf("%s", testError)
@@ -230,7 +230,7 @@ func TestError(t *testing.T) {
230230
assert.True(t, resp.IsError())
231231
err := opensearch.ParseError(resp)
232232
var testError *opensearch.MessageError
233-
require.True(t, errors.As(err, &testError))
233+
require.ErrorAs(t, err, &testError)
234234
assert.Equal(t, "BAD_REQUEST", testError.Status)
235235
assert.Contains(t, testError.Message, "Wrong request body")
236236
_ = fmt.Sprintf("%s", testError)
@@ -277,7 +277,7 @@ func TestError(t *testing.T) {
277277
t.Run(tt.Name, func(t *testing.T) {
278278
err := opensearch.ParseError(tt.Resp)
279279
for _, wantedError := range tt.WantedErrors {
280-
assert.True(t, errors.Is(err, wantedError))
280+
assert.ErrorIs(t, err, wantedError)
281281
}
282282
})
283283
}
@@ -289,7 +289,7 @@ func TestError(t *testing.T) {
289289
}
290290
assert.True(t, resp.IsError())
291291
err := opensearch.ParseError(resp)
292-
assert.True(t, errors.Is(err, opensearch.ErrJSONUnmarshalBody))
292+
assert.ErrorIs(t, err, opensearch.ErrJSONUnmarshalBody)
293293
})
294294
t.Run("too many requests", func(t *testing.T) {
295295
resp := &opensearch.Response{
@@ -298,7 +298,7 @@ func TestError(t *testing.T) {
298298
}
299299
assert.True(t, resp.IsError())
300300
err := opensearch.ParseError(resp)
301-
assert.True(t, errors.Is(err, opensearch.ErrJSONUnmarshalBody))
301+
assert.ErrorIs(t, err, opensearch.ErrJSONUnmarshalBody)
302302
})
303303
})
304304
}

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
github.com/aws/aws-sdk-go-v2/config v1.32.7
99
github.com/stretchr/testify v1.11.1
1010
github.com/wI2L/jsondiff v0.7.0
11+
golang.org/x/mod v0.32.0
1112
)
1213

1314
require (

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY=
5252
github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28=
5353
github.com/wI2L/jsondiff v0.7.0 h1:1lH1G37GhBPqCfp/lrs91rf/2j3DktX6qYAKZkLuCQQ=
5454
github.com/wI2L/jsondiff v0.7.0/go.mod h1:KAEIojdQq66oJiHhDyQez2x+sRit0vIzC9KeK0yizxM=
55+
golang.org/x/mod v0.32.0 h1:9F4d3PHLljb6x//jOyokMv3eX+YDeepZSEo3mFJy93c=
56+
golang.org/x/mod v0.32.0/go.mod h1:SgipZ/3h2Ci89DlEtEXWUk/HteuRin+HHhN+WbNhguU=
5557
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
5658
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
5759
gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=

internal/test/config.go

Lines changed: 0 additions & 72 deletions
This file was deleted.

internal/test/doc.go

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,65 @@
44
// this file be licensed under the Apache-2.0 license or a
55
// compatible open source license.
66

7-
// Package ostest provides OpenSearch testing utilities and infrastructure.
8-
// Integration-specific functionality is available only when building with integration tags.
7+
// Package ostest provides internal integration test utilities for the OpenSearch Go client.
8+
//
9+
// This package contains complex test orchestration, OpenSearch-specific utilities,
10+
// and internal integration test helpers. It builds upon the basic utilities
11+
// provided by opensearchutil/testutil but adds OpenSearch-aware functionality.
12+
//
13+
// # Design Principles
14+
//
15+
// Functions in this package follow the same design principles as testutil:
16+
// - MUST take `t *testing.T` as the first parameter when applicable
17+
// - MUST call `t.Helper()` as the first statement for test helpers
18+
// - SHOULD delegate to testutil for basic utilities rather than duplicating them
19+
//
20+
// # When to Add Functions Here
21+
//
22+
// Add functions to this package when:
23+
// - The utility requires complex OpenSearch cluster orchestration
24+
// - The functionality is specific to internal integration testing
25+
// - The utility needs to perform readiness checks or cluster health validation
26+
// - The functionality involves OpenSearch version-specific logic
27+
// - The utility is only needed by internal test packages
28+
//
29+
// # Key Functionality
30+
//
31+
// This package provides:
32+
// - NewClient() - Creates OpenSearch clients with automatic readiness checking
33+
// - Cluster health and readiness validation
34+
// - JSON comparison utilities for response validation
35+
// - OpenSearchTestSuite for structured test organization
36+
// - Version-specific test utilities and skip logic
37+
// - Complex client configuration scenarios
38+
//
39+
// # Relationship to opensearchutil/testutil
40+
//
41+
// This package imports and uses opensearchutil/testutil for basic utilities:
42+
// - Uses testutil.ClientConfig(t) for basic client configuration
43+
// - Uses testutil.IsSecure(t) for security checks
44+
// - Uses testutil.GetPassword(t) for authentication
45+
// - Provides enhanced functionality on top of these basics
46+
//
47+
// Basic utilities should be added to testutil, while OpenSearch-specific
48+
// orchestration should be added here.
49+
//
50+
// # Examples
51+
//
52+
// // Create client with automatic readiness checking
53+
// client, err := ostest.NewClient(t)
54+
// require.NoError(t, err)
55+
//
56+
// // Use test suite with version checking
57+
// type MyTestSuite struct {
58+
// ostest.OpenSearchTestSuite
59+
// }
60+
//
61+
// func (s *MyTestSuite) TestFeature() {
62+
// s.SkipIfBelowVersion(2, 4, "FeatureName")
63+
// // test implementation
64+
// }
65+
//
66+
// // Validate JSON response completeness
67+
// ostest.CompareRawJSONwithParsedJSON(t, resp, resp.Inspect().Response)
968
package ostest

0 commit comments

Comments
 (0)