Skip to content

Commit 34dfe0a

Browse files
cbandycbandy
committed
Another test helper for metav1.Status.Details
Every call to "require.StatusError" was followed by reaching into its Details field. This eliminates the check for nil that entailed.
1 parent 46e6493 commit 34dfe0a

5 files changed

Lines changed: 73 additions & 80 deletions

File tree

‎internal/testing/require/errors.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,25 @@ import (
1616
// StatusError returns the [metav1.Status] within err's tree.
1717
// It calls t.Fatal when err is nil or there is no status.
1818
func StatusError(t testing.TB, err error) metav1.Status {
19-
status, ok := err.(apierrors.APIStatus)
19+
t.Helper()
2020

21+
status, ok := err.(apierrors.APIStatus)
2122
assert.Assert(t, ok || errors.As(err, &status),
2223
"%T does not implement %T", err, status)
2324

2425
return status.Status()
2526
}
2627

28+
// StatusErrorDetails returns the details of [metav1.Status] within err's tree.
29+
// It calls t.Fatal when err is nil, there is no status, or its Details field is nil.
30+
func StatusErrorDetails(t testing.TB, err error) metav1.StatusDetails {
31+
t.Helper()
32+
33+
status := StatusError(t, err)
34+
assert.Assert(t, status.Details != nil)
35+
return *status.Details
36+
}
37+
2738
// Value returns v or panics when err is not nil.
2839
func Value[T any](v T, err error) T {
2940
if err != nil {

‎internal/testing/validation/pgadmin_test.go‎

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,14 @@ func TestPGAdminDataVolume(t *testing.T) {
4747
assert.ErrorContains(t, err, "dataVolumeClaimSpec")
4848
assert.ErrorContains(t, err, "Required")
4949

50-
status := require.StatusError(t, err)
51-
assert.Assert(t, status.Details != nil)
52-
assert.Assert(t, cmp.Len(status.Details.Causes, 2))
50+
details := require.StatusErrorDetails(t, err)
51+
assert.Assert(t, cmp.Len(details.Causes, 2))
5352

54-
assert.Equal(t, status.Details.Causes[0].Field, "spec.dataVolumeClaimSpec")
55-
assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, "Required"))
53+
assert.Equal(t, details.Causes[0].Field, "spec.dataVolumeClaimSpec")
54+
assert.Assert(t, cmp.Contains(details.Causes[0].Message, "Required"))
5655

57-
assert.Equal(t, string(status.Details.Causes[1].Type), "FieldValueInvalid")
58-
assert.Assert(t, cmp.Contains(status.Details.Causes[1].Message, "rules were not checked"))
56+
assert.Equal(t, string(details.Causes[1].Type), "FieldValueInvalid")
57+
assert.Assert(t, cmp.Contains(details.Causes[1].Message, "rules were not checked"))
5958
})
6059

6160
t.Run("AccessModes", func(t *testing.T) {
@@ -150,11 +149,10 @@ func TestPGAdminInstrumentation(t *testing.T) {
150149
assert.ErrorContains(t, err, "minRecords")
151150
assert.ErrorContains(t, err, "maxDelay")
152151

153-
status := require.StatusError(t, err)
154-
assert.Assert(t, status.Details != nil)
155-
assert.Assert(t, cmp.Len(status.Details.Causes, 1))
152+
details := require.StatusErrorDetails(t, err)
153+
assert.Assert(t, cmp.Len(details.Causes, 1))
156154

157-
for _, cause := range status.Details.Causes {
155+
for _, cause := range details.Causes {
158156
assert.Equal(t, cause.Field, "spec.instrumentation.logs.batches")
159157
assert.Assert(t, cmp.Contains(cause.Message, "disable batching"))
160158
assert.Assert(t, cmp.Contains(cause.Message, "minRecords and maxDelay must be zero"))
@@ -176,11 +174,10 @@ func TestPGAdminInstrumentation(t *testing.T) {
176174
assert.ErrorContains(t, err, "maxDelay")
177175
assert.ErrorContains(t, err, "5m")
178176

179-
status := require.StatusError(t, err)
180-
assert.Assert(t, status.Details != nil)
181-
assert.Assert(t, cmp.Len(status.Details.Causes, 1))
177+
details := require.StatusErrorDetails(t, err)
178+
assert.Assert(t, cmp.Len(details.Causes, 1))
182179

183-
for _, cause := range status.Details.Causes {
180+
for _, cause := range details.Causes {
184181
assert.Equal(t, cause.Field, "spec.instrumentation.logs.batches.maxDelay")
185182
}
186183
})
@@ -200,11 +197,10 @@ func TestPGAdminInstrumentation(t *testing.T) {
200197
assert.ErrorContains(t, err, "maxRecords")
201198
assert.ErrorContains(t, err, "greater than or equal to 1")
202199

203-
status := require.StatusError(t, err)
204-
assert.Assert(t, status.Details != nil)
205-
assert.Assert(t, cmp.Len(status.Details.Causes, 2))
200+
details := require.StatusErrorDetails(t, err)
201+
assert.Assert(t, cmp.Len(details.Causes, 2))
206202

207-
for _, cause := range status.Details.Causes {
203+
for _, cause := range details.Causes {
208204
switch cause.Field {
209205
case "spec.instrumentation.logs.batches.maxRecords":
210206
assert.Assert(t, cmp.Contains(cause.Message, "0"))
@@ -233,11 +229,10 @@ func TestPGAdminInstrumentation(t *testing.T) {
233229
assert.ErrorContains(t, err, "minRecords")
234230
assert.ErrorContains(t, err, "maxRecords")
235231

236-
status := require.StatusError(t, err)
237-
assert.Assert(t, status.Details != nil)
238-
assert.Assert(t, cmp.Len(status.Details.Causes, 1))
232+
details := require.StatusErrorDetails(t, err)
233+
assert.Assert(t, cmp.Len(details.Causes, 1))
239234

240-
for _, cause := range status.Details.Causes {
235+
for _, cause := range details.Causes {
241236
assert.Equal(t, cause.Field, "spec.instrumentation.logs.batches")
242237
assert.Assert(t, cmp.Contains(cause.Message, "minRecords cannot be larger than maxRecords"))
243238
}
@@ -260,11 +255,10 @@ func TestPGAdminInstrumentation(t *testing.T) {
260255
assert.ErrorContains(t, err, "hour|day|week")
261256
assert.ErrorContains(t, err, "one hour")
262257

263-
status := require.StatusError(t, err)
264-
assert.Assert(t, status.Details != nil)
265-
assert.Assert(t, cmp.Len(status.Details.Causes, 2))
258+
details := require.StatusErrorDetails(t, err)
259+
assert.Assert(t, cmp.Len(details.Causes, 2))
266260

267-
for _, cause := range status.Details.Causes {
261+
for _, cause := range details.Causes {
268262
assert.Equal(t, cause.Field, "spec.instrumentation.logs.retentionPeriod")
269263
}
270264

‎internal/testing/validation/postgrescluster/postgres_authentication_test.go‎

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,10 @@ func testPostgresAuthenticationCommon(t *testing.T, cc client.Client, base unstr
9999
err := cc.Create(ctx, cluster, client.DryRunAll)
100100
assert.Assert(t, apierrors.IsInvalid(err))
101101

102-
status := require.StatusError(t, err)
103-
assert.Assert(t, status.Details != nil)
104-
assert.Assert(t, cmp.Len(status.Details.Causes, 2))
102+
details := require.StatusErrorDetails(t, err)
103+
assert.Assert(t, cmp.Len(details.Causes, 2))
105104

106-
for i, cause := range status.Details.Causes {
105+
for i, cause := range details.Causes {
107106
assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i))
108107
assert.Assert(t, cmp.Contains(cause.Message, "cannot be combined"))
109108
}
@@ -122,11 +121,10 @@ func testPostgresAuthenticationCommon(t *testing.T, cc client.Client, base unstr
122121
err := cc.Create(ctx, cluster, client.DryRunAll)
123122
assert.Assert(t, apierrors.IsInvalid(err))
124123

125-
status := require.StatusError(t, err)
126-
assert.Assert(t, status.Details != nil)
127-
assert.Assert(t, cmp.Len(status.Details.Causes, 3))
124+
details := require.StatusErrorDetails(t, err)
125+
assert.Assert(t, cmp.Len(details.Causes, 3))
128126

129-
for i, cause := range status.Details.Causes {
127+
for i, cause := range details.Causes {
130128
assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d].hba", i))
131129
assert.Assert(t, cmp.Contains(cause.Message, "cannot include"))
132130
}
@@ -145,11 +143,10 @@ func testPostgresAuthenticationCommon(t *testing.T, cc client.Client, base unstr
145143
err := cc.Create(ctx, cluster, client.DryRunAll)
146144
assert.Assert(t, apierrors.IsInvalid(err))
147145

148-
status := require.StatusError(t, err)
149-
assert.Assert(t, status.Details != nil)
150-
assert.Assert(t, cmp.Len(status.Details.Causes, 3))
146+
details := require.StatusErrorDetails(t, err)
147+
assert.Assert(t, cmp.Len(details.Causes, 3))
151148

152-
for i, cause := range status.Details.Causes {
149+
for i, cause := range details.Causes {
153150
assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d].method", i))
154151
assert.Assert(t, cmp.Contains(cause.Message, "unsafe"))
155152
}
@@ -169,11 +166,10 @@ func testPostgresAuthenticationCommon(t *testing.T, cc client.Client, base unstr
169166
err := cc.Create(ctx, cluster, client.DryRunAll)
170167
assert.Assert(t, apierrors.IsInvalid(err))
171168

172-
status := require.StatusError(t, err)
173-
assert.Assert(t, status.Details != nil)
174-
assert.Assert(t, cmp.Len(status.Details.Causes, 3))
169+
details := require.StatusErrorDetails(t, err)
170+
assert.Assert(t, cmp.Len(details.Causes, 3))
175171

176-
for i, cause := range status.Details.Causes {
172+
for i, cause := range details.Causes {
177173
assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i), "%#v", cause)
178174
assert.Assert(t, cmp.Contains(cause.Message, `"ldap" method requires`))
179175
}
@@ -205,11 +201,10 @@ func testPostgresAuthenticationCommon(t *testing.T, cc client.Client, base unstr
205201
err := cc.Create(ctx, cluster, client.DryRunAll)
206202
assert.Assert(t, apierrors.IsInvalid(err))
207203

208-
status := require.StatusError(t, err)
209-
assert.Assert(t, status.Details != nil)
210-
assert.Assert(t, cmp.Len(status.Details.Causes, 2))
204+
details := require.StatusErrorDetails(t, err)
205+
assert.Assert(t, cmp.Len(details.Causes, 2))
211206

212-
for i, cause := range status.Details.Causes {
207+
for i, cause := range details.Causes {
213208
assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i), "%#v", cause)
214209
assert.Assert(t, cmp.Regexp(`cannot use .+? options with .+? options`, cause.Message))
215210
}
@@ -246,11 +241,10 @@ func testPostgresAuthenticationCommon(t *testing.T, cc client.Client, base unstr
246241
err := cc.Create(ctx, cluster, client.DryRunAll)
247242
assert.Assert(t, apierrors.IsInvalid(err))
248243

249-
status := require.StatusError(t, err)
250-
assert.Assert(t, status.Details != nil)
251-
assert.Assert(t, cmp.Len(status.Details.Causes, 5))
244+
details := require.StatusErrorDetails(t, err)
245+
assert.Assert(t, cmp.Len(details.Causes, 5))
252246

253-
for i, cause := range status.Details.Causes {
247+
for i, cause := range details.Causes {
254248
assert.Equal(t, cause.Field, fmt.Sprintf("spec.authentication.rules[%d]", i), "%#v", cause)
255249
assert.Assert(t, cmp.Contains(cause.Message, `"radius" method requires`))
256250
}

‎internal/testing/validation/postgrescluster/postgres_config_test.go‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,12 @@ func testPostgresConfigParametersCommon(t *testing.T, cc client.Client, base uns
131131
err := cc.Create(ctx, cluster, client.DryRunAll)
132132
assert.Assert(t, apierrors.IsInvalid(err))
133133

134-
status := require.StatusError(t, err)
135-
assert.Assert(t, status.Details != nil)
136-
assert.Assert(t, cmp.Len(status.Details.Causes, 1))
134+
details := require.StatusErrorDetails(t, err)
135+
assert.Assert(t, cmp.Len(details.Causes, 1))
137136

138137
// TODO(k8s-1.30) TODO(validation): Move the parameter name from the message to the field path.
139-
assert.Equal(t, status.Details.Causes[0].Field, "spec.config.parameters")
140-
assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, tt.key))
138+
assert.Equal(t, details.Causes[0].Field, "spec.config.parameters")
139+
assert.Assert(t, cmp.Contains(details.Causes[0].Message, tt.key))
141140
})
142141
}
143142
})
@@ -180,14 +179,13 @@ func testPostgresConfigParametersCommon(t *testing.T, cc client.Client, base uns
180179
} else {
181180
assert.Assert(t, apierrors.IsInvalid(err))
182181

183-
status := require.StatusError(t, err)
184-
assert.Assert(t, status.Details != nil)
185-
assert.Assert(t, cmp.Len(status.Details.Causes, 1))
182+
details := require.StatusErrorDetails(t, err)
183+
assert.Assert(t, cmp.Len(details.Causes, 1))
186184

187185
// TODO(k8s-1.30) TODO(validation): Move the parameter name from the message to the field path.
188-
assert.Equal(t, status.Details.Causes[0].Field, "spec.config.parameters")
189-
assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, tt.key))
190-
assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, tt.message))
186+
assert.Equal(t, details.Causes[0].Field, "spec.config.parameters")
187+
assert.Assert(t, cmp.Contains(details.Causes[0].Message, tt.key))
188+
assert.Assert(t, cmp.Contains(details.Causes[0].Message, tt.message))
191189
}
192190
})
193191
}
@@ -256,11 +254,10 @@ func testPostgresConfigParametersCommon(t *testing.T, cc client.Client, base uns
256254
assert.Assert(t, apierrors.IsInvalid(err))
257255
assert.ErrorContains(t, err, `"replica" or higher`)
258256

259-
status := require.StatusError(t, err)
260-
assert.Assert(t, status.Details != nil)
261-
assert.Assert(t, cmp.Len(status.Details.Causes, 1))
262-
assert.Equal(t, status.Details.Causes[0].Field, "spec.config.parameters")
263-
assert.Assert(t, cmp.Contains(status.Details.Causes[0].Message, "wal_level"))
257+
details := require.StatusErrorDetails(t, err)
258+
assert.Assert(t, cmp.Len(details.Causes, 1))
259+
assert.Equal(t, details.Causes[0].Field, "spec.config.parameters")
260+
assert.Assert(t, cmp.Contains(details.Causes[0].Message, "wal_level"))
264261
})
265262
})
266263

‎internal/testing/validation/postgrescluster/postgres_users_test.go‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,10 @@ func testPostgresUserOptionsCommon(t *testing.T, cc client.Client, base unstruct
103103
assert.Assert(t, apierrors.IsInvalid(err))
104104
assert.ErrorContains(t, err, "cannot contain comments")
105105

106-
status := require.StatusError(t, err)
107-
assert.Assert(t, status.Details != nil)
108-
assert.Assert(t, cmp.Len(status.Details.Causes, 3))
106+
details := require.StatusErrorDetails(t, err)
107+
assert.Assert(t, cmp.Len(details.Causes, 3))
109108

110-
for i, cause := range status.Details.Causes {
109+
for i, cause := range details.Causes {
111110
assert.Equal(t, cause.Field, fmt.Sprintf("spec.users[%d].options", i))
112111
assert.Assert(t, cmp.Contains(cause.Message, "cannot contain comments"))
113112
}
@@ -126,11 +125,10 @@ func testPostgresUserOptionsCommon(t *testing.T, cc client.Client, base unstruct
126125
assert.Assert(t, apierrors.IsInvalid(err))
127126
assert.ErrorContains(t, err, "cannot assign password")
128127

129-
status := require.StatusError(t, err)
130-
assert.Assert(t, status.Details != nil)
131-
assert.Assert(t, cmp.Len(status.Details.Causes, 2))
128+
details := require.StatusErrorDetails(t, err)
129+
assert.Assert(t, cmp.Len(details.Causes, 2))
132130

133-
for i, cause := range status.Details.Causes {
131+
for i, cause := range details.Causes {
134132
assert.Equal(t, cause.Field, fmt.Sprintf("spec.users[%d].options", i))
135133
assert.Assert(t, cmp.Contains(cause.Message, "cannot assign password"))
136134
}
@@ -148,10 +146,9 @@ func testPostgresUserOptionsCommon(t *testing.T, cc client.Client, base unstruct
148146
assert.Assert(t, apierrors.IsInvalid(err))
149147
assert.ErrorContains(t, err, "should match")
150148

151-
status := require.StatusError(t, err)
152-
assert.Assert(t, status.Details != nil)
153-
assert.Assert(t, cmp.Len(status.Details.Causes, 1))
154-
assert.Equal(t, status.Details.Causes[0].Field, "spec.users[0].options")
149+
details := require.StatusErrorDetails(t, err)
150+
assert.Assert(t, cmp.Len(details.Causes, 1))
151+
assert.Equal(t, details.Causes[0].Field, "spec.users[0].options")
155152
})
156153

157154
t.Run("Valid", func(t *testing.T) {

0 commit comments

Comments
 (0)