Skip to content

Commit 5d564d2

Browse files
mogsiemogsie
committed
🐛 fix: Enhance the webhook test template
The webhook scaffold had some oddly indented regions, resulting in the generated source code having a mix of tabs and spaces, and was generally looking quite ugly. The gomega asserions are too chatty; errors, and return values are checked after first being assigned as variables in scope. This is not needed, and Gomega checks all of this for us: Expect(someFunc()).Error().To(HaveOccurred()) checks both the error, but also that the other return values are zero or nil. Expect(someFunc()).To(ContainSubstring("X")) checks both the return value and verifies that the error is nil. - Indentation - simplify gomega assertions
1 parent 571a948 commit 5d564d2

File tree

19 files changed

+177
-277
lines changed

19 files changed

+177
-277
lines changed

‎docs/book/src/cronjob-tutorial/testdata/project/api/v1/cronjob_webhook_test.go‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -104,32 +104,28 @@ var _ = Describe("CronJob Webhook", func() {
104104
Context("When creating or updating CronJob under Validating Webhook", func() {
105105
It("Should deny creation if the name is too long", func() {
106106
obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long"
107-
warnings, err := validator.ValidateCreate(ctx, obj)
108-
Expect(err).To(HaveOccurred(), "Expected name validation to fail for a too-long name")
109-
Expect(warnings).To(BeNil())
110-
Expect(err.Error()).To(ContainSubstring("must be no more than 52 characters"))
107+
Expect(validator.ValidateCreate(ctx, obj)).Error().To(
108+
MatchError(ContainSubstring("must be no more than 52 characters")),
109+
"Expected name validation to fail for a too-long name")
111110
})
112111

113112
It("Should admit creation if the name is valid", func() {
114113
obj.ObjectMeta.Name = "valid-cronjob-name"
115-
warnings, err := validator.ValidateCreate(ctx, obj)
116-
Expect(err).NotTo(HaveOccurred(), "Expected name validation to pass for a valid name")
117-
Expect(warnings).To(BeNil())
114+
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
115+
"Expected name validation to pass for a valid name")
118116
})
119117

120118
It("Should deny creation if the schedule is invalid", func() {
121119
obj.Spec.Schedule = "invalid-cron-schedule"
122-
warnings, err := validator.ValidateCreate(ctx, obj)
123-
Expect(err).To(HaveOccurred(), "Expected spec validation to fail for an invalid schedule")
124-
Expect(warnings).To(BeNil())
125-
Expect(err.Error()).To(ContainSubstring("Expected exactly 5 fields, found 1: invalid-cron-schedule"))
120+
Expect(validator.ValidateCreate(ctx, obj)).Error().To(
121+
MatchError(ContainSubstring("Expected exactly 5 fields, found 1: invalid-cron-schedule")),
122+
"Expected spec validation to fail for an invalid schedule")
126123
})
127124

128125
It("Should admit creation if the schedule is valid", func() {
129126
obj.Spec.Schedule = "*/5 * * * *"
130-
warnings, err := validator.ValidateCreate(ctx, obj)
131-
Expect(err).NotTo(HaveOccurred(), "Expected spec validation to pass for a valid schedule")
132-
Expect(warnings).To(BeNil())
127+
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
128+
"Expected spec validation to pass for a valid schedule")
133129
})
134130

135131
It("Should deny update if both name and spec are invalid", func() {
@@ -141,9 +137,8 @@ var _ = Describe("CronJob Webhook", func() {
141137
obj.Spec.Schedule = "invalid-cron-schedule"
142138

143139
By("validating an update")
144-
warnings, err := validator.ValidateUpdate(ctx, oldObj, obj)
145-
Expect(err).To(HaveOccurred(), "Expected validation to fail for both name and spec")
146-
Expect(warnings).To(BeNil())
140+
Expect(validator.ValidateUpdate(ctx, oldObj, obj)).Error().To(HaveOccurred(),
141+
"Expected validation to fail for both name and spec")
147142
})
148143

149144
It("Should admit update if both name and spec are valid", func() {
@@ -155,9 +150,8 @@ var _ = Describe("CronJob Webhook", func() {
155150
obj.Spec.Schedule = "0 0 * * *"
156151

157152
By("validating an update")
158-
warnings, err := validator.ValidateUpdate(ctx, oldObj, obj)
159-
Expect(err).NotTo(HaveOccurred(), "Expected validation to pass for a valid update")
160-
Expect(warnings).To(BeNil())
153+
Expect(validator.ValidateUpdate(ctx, oldObj, obj)).To(BeNil(),
154+
"Expected validation to pass for a valid update")
161155
})
162156
})
163157

‎docs/book/src/multiversion-tutorial/testdata/project/api/v1/cronjob_webhook_test.go‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -104,32 +104,28 @@ var _ = Describe("CronJob Webhook", func() {
104104
Context("When creating or updating CronJob under Validating Webhook", func() {
105105
It("Should deny creation if the name is too long", func() {
106106
obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long"
107-
warnings, err := validator.ValidateCreate(ctx, obj)
108-
Expect(err).To(HaveOccurred(), "Expected name validation to fail for a too-long name")
109-
Expect(warnings).To(BeNil())
110-
Expect(err.Error()).To(ContainSubstring("must be no more than 52 characters"))
107+
Expect(validator.ValidateCreate(ctx, obj)).Error().To(
108+
MatchError(ContainSubstring("must be no more than 52 characters")),
109+
"Expected name validation to fail for a too-long name")
111110
})
112111

113112
It("Should admit creation if the name is valid", func() {
114113
obj.ObjectMeta.Name = "valid-cronjob-name"
115-
warnings, err := validator.ValidateCreate(ctx, obj)
116-
Expect(err).NotTo(HaveOccurred(), "Expected name validation to pass for a valid name")
117-
Expect(warnings).To(BeNil())
114+
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
115+
"Expected name validation to pass for a valid name")
118116
})
119117

120118
It("Should deny creation if the schedule is invalid", func() {
121119
obj.Spec.Schedule = "invalid-cron-schedule"
122-
warnings, err := validator.ValidateCreate(ctx, obj)
123-
Expect(err).To(HaveOccurred(), "Expected spec validation to fail for an invalid schedule")
124-
Expect(warnings).To(BeNil())
125-
Expect(err.Error()).To(ContainSubstring("Expected exactly 5 fields, found 1: invalid-cron-schedule"))
120+
Expect(validator.ValidateCreate(ctx, obj)).Error().To(
121+
MatchError(ContainSubstring("Expected exactly 5 fields, found 1: invalid-cron-schedule")),
122+
"Expected spec validation to fail for an invalid schedule")
126123
})
127124

128125
It("Should admit creation if the schedule is valid", func() {
129126
obj.Spec.Schedule = "*/5 * * * *"
130-
warnings, err := validator.ValidateCreate(ctx, obj)
131-
Expect(err).NotTo(HaveOccurred(), "Expected spec validation to pass for a valid schedule")
132-
Expect(warnings).To(BeNil())
127+
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
128+
"Expected spec validation to pass for a valid schedule")
133129
})
134130

135131
It("Should deny update if both name and spec are invalid", func() {
@@ -141,9 +137,8 @@ var _ = Describe("CronJob Webhook", func() {
141137
obj.Spec.Schedule = "invalid-cron-schedule"
142138

143139
By("validating an update")
144-
warnings, err := validator.ValidateUpdate(ctx, oldObj, obj)
145-
Expect(err).To(HaveOccurred(), "Expected validation to fail for both name and spec")
146-
Expect(warnings).To(BeNil())
140+
Expect(validator.ValidateUpdate(ctx, oldObj, obj)).Error().To(HaveOccurred(),
141+
"Expected validation to fail for both name and spec")
147142
})
148143

149144
It("Should admit update if both name and spec are valid", func() {
@@ -155,9 +150,8 @@ var _ = Describe("CronJob Webhook", func() {
155150
obj.Spec.Schedule = "0 0 * * *"
156151

157152
By("validating an update")
158-
warnings, err := validator.ValidateUpdate(ctx, oldObj, obj)
159-
Expect(err).NotTo(HaveOccurred(), "Expected validation to pass for a valid update")
160-
Expect(warnings).To(BeNil())
153+
Expect(validator.ValidateUpdate(ctx, oldObj, obj)).To(BeNil(),
154+
"Expected validation to pass for a valid update")
161155
})
162156
})
163157

‎docs/book/src/multiversion-tutorial/testdata/project/api/v2/cronjob_webhook_test.go‎

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,8 @@ var _ = Describe("CronJob Webhook", func() {
4343
// Example:
4444
// It("Should apply defaults when a required field is empty", func() {
4545
// By("simulating a scenario where defaults should be applied")
46-
// obj.SomeFieldWithDefault = ""
47-
// err := obj.Default(ctx)
48-
// Expect(err).NotTo(HaveOccurred())
46+
// obj.SomeFieldWithDefault = ""
47+
// Expect(obj.Default(ctx)).To(Succeed())
4948
// Expect(obj.SomeFieldWithDefault).To(Equal("default_value"))
5049
// })
5150
})
@@ -54,40 +53,33 @@ var _ = Describe("CronJob Webhook", func() {
5453
// TODO (user): Add logic for validating webhooks
5554
// Example:
5655
// It("Should deny creation if a required field is missing", func() {
57-
// By("simulating an invalid creation scenario")
56+
// By("simulating an invalid creation scenario")
5857
// obj.SomeRequiredField = ""
59-
// warnings, err := obj.ValidateCreate(ctx)
60-
// Expect(err).To(HaveOccurred())
61-
// Expect(warnings).To(BeNil())
58+
// Expect(obj.ValidateCreate(ctx)).Error().To(HaveOccurred())
6259
// })
6360
//
6461
// It("Should admit creation if all required fields are present", func() {
65-
// By("simulating an invalid creation scenario")
62+
// By("simulating an invalid creation scenario")
6663
// obj.SomeRequiredField = "valid_value"
67-
// warnings, err := obj.ValidateCreate(ctx)
68-
// Expect(err).NotTo(HaveOccurred())
69-
// Expect(warnings).To(BeNil())
64+
// Expect(obj.ValidateCreate(ctx)).To(BeNil())
7065
// })
7166
//
7267
// It("Should validate updates correctly", func() {
7368
// By("simulating a valid update scenario")
74-
// oldObj := &Captain{SomeRequiredField: "valid_value"}
75-
// obj.SomeRequiredField = "updated_value"
76-
// warnings, err := obj.ValidateUpdate(ctx, oldObj)
77-
// Expect(err).NotTo(HaveOccurred())
78-
// Expect(warnings).To(BeNil())
69+
// oldObj := &Captain{SomeRequiredField: "valid_value"}
70+
// obj.SomeRequiredField = "updated_value"
71+
// Expect(obj.ValidateUpdate(ctx, oldObj)).To(BeNil())
7972
// })
8073
})
8174

8275
Context("When creating CronJob under Conversion Webhook", func() {
83-
It("Should convert the object correctly", func() {
84-
// TODO (user): Add logic to convert the object to the desired version and verify the conversion
85-
// Example:
86-
// convertedObj := &CronJob{}
87-
// err := obj.ConvertTo(convertedObj)
88-
// Expect(err).NotTo(HaveOccurred())
89-
// Expect(convertedObj).ToNot(BeNil())
90-
})
76+
// TODO (user): Add logic to convert the object to the desired version and verify the conversion
77+
// Example:
78+
// It("Should convert the object correctly", func() {
79+
// convertedObj := &CronJob{}
80+
// Expect(obj.ConvertTo(convertedObj)).To(Succeed())
81+
// Expect(convertedObj).ToNot(BeNil())
82+
// })
9183
})
9284

9385
})

‎hack/docs/internal/cronjob-tutorial/webhook_implementation.go‎

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,8 @@ const webhookTestCreateDefaultingFragment = `// TODO (user): Add logic for defau
185185
// Example:
186186
// It("Should apply defaults when a required field is empty", func() {
187187
// By("simulating a scenario where defaults should be applied")
188-
// obj.SomeFieldWithDefault = ""
189-
// err := obj.Default(ctx)
190-
// Expect(err).NotTo(HaveOccurred())
188+
// obj.SomeFieldWithDefault = ""
189+
// Expect(obj.Default(ctx)).To(Succeed())
191190
// Expect(obj.SomeFieldWithDefault).To(Equal("default_value"))
192191
// })`
193192

@@ -231,58 +230,48 @@ const webhookTestCreateDefaultingReplaceFragment = `It("Should apply defaults wh
231230
const webhookTestingValidatingTodoFragment = `// TODO (user): Add logic for validating webhooks
232231
// Example:
233232
// It("Should deny creation if a required field is missing", func() {
234-
// By("simulating an invalid creation scenario")
233+
// By("simulating an invalid creation scenario")
235234
// obj.SomeRequiredField = ""
236-
// warnings, err := obj.ValidateCreate(ctx)
237-
// Expect(err).To(HaveOccurred())
238-
// Expect(warnings).To(BeNil())
235+
// Expect(obj.ValidateCreate(ctx)).Error().To(HaveOccurred())
239236
// })
240237
//
241238
// It("Should admit creation if all required fields are present", func() {
242-
// By("simulating an invalid creation scenario")
239+
// By("simulating an invalid creation scenario")
243240
// obj.SomeRequiredField = "valid_value"
244-
// warnings, err := obj.ValidateCreate(ctx)
245-
// Expect(err).NotTo(HaveOccurred())
246-
// Expect(warnings).To(BeNil())
241+
// Expect(obj.ValidateCreate(ctx)).To(BeNil())
247242
// })
248243
//
249244
// It("Should validate updates correctly", func() {
250245
// By("simulating a valid update scenario")
251-
// oldObj := &Captain{SomeRequiredField: "valid_value"}
252-
// obj.SomeRequiredField = "updated_value"
253-
// warnings, err := obj.ValidateUpdate(ctx, oldObj)
254-
// Expect(err).NotTo(HaveOccurred())
255-
// Expect(warnings).To(BeNil())
246+
// oldObj := &Captain{SomeRequiredField: "valid_value"}
247+
// obj.SomeRequiredField = "updated_value"
248+
// Expect(obj.ValidateUpdate(ctx, oldObj)).To(BeNil())
256249
// })`
257250

258251
const webhookTestingValidatingExampleFragment = `It("Should deny creation if the name is too long", func() {
259252
obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long"
260-
warnings, err := validator.ValidateCreate(ctx, obj)
261-
Expect(err).To(HaveOccurred(), "Expected name validation to fail for a too-long name")
262-
Expect(warnings).To(BeNil())
263-
Expect(err.Error()).To(ContainSubstring("must be no more than 52 characters"))
253+
Expect(validator.ValidateCreate(ctx, obj)).Error().To(
254+
MatchError(ContainSubstring("must be no more than 52 characters")),
255+
"Expected name validation to fail for a too-long name")
264256
})
265257
266258
It("Should admit creation if the name is valid", func() {
267259
obj.ObjectMeta.Name = "valid-cronjob-name"
268-
warnings, err := validator.ValidateCreate(ctx, obj)
269-
Expect(err).NotTo(HaveOccurred(), "Expected name validation to pass for a valid name")
270-
Expect(warnings).To(BeNil())
260+
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
261+
"Expected name validation to pass for a valid name")
271262
})
272263
273264
It("Should deny creation if the schedule is invalid", func() {
274265
obj.Spec.Schedule = "invalid-cron-schedule"
275-
warnings, err := validator.ValidateCreate(ctx, obj)
276-
Expect(err).To(HaveOccurred(), "Expected spec validation to fail for an invalid schedule")
277-
Expect(warnings).To(BeNil())
278-
Expect(err.Error()).To(ContainSubstring("Expected exactly 5 fields, found 1: invalid-cron-schedule"))
266+
Expect(validator.ValidateCreate(ctx, obj)).Error().To(
267+
MatchError(ContainSubstring("Expected exactly 5 fields, found 1: invalid-cron-schedule")),
268+
"Expected spec validation to fail for an invalid schedule")
279269
})
280270
281271
It("Should admit creation if the schedule is valid", func() {
282272
obj.Spec.Schedule = "*/5 * * * *"
283-
warnings, err := validator.ValidateCreate(ctx, obj)
284-
Expect(err).NotTo(HaveOccurred(), "Expected spec validation to pass for a valid schedule")
285-
Expect(warnings).To(BeNil())
273+
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
274+
"Expected spec validation to pass for a valid schedule")
286275
})
287276
288277
It("Should deny update if both name and spec are invalid", func() {
@@ -294,9 +283,8 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the
294283
obj.Spec.Schedule = "invalid-cron-schedule"
295284
296285
By("validating an update")
297-
warnings, err := validator.ValidateUpdate(ctx, oldObj, obj)
298-
Expect(err).To(HaveOccurred(), "Expected validation to fail for both name and spec")
299-
Expect(warnings).To(BeNil())
286+
Expect(validator.ValidateUpdate(ctx, oldObj, obj)).Error().To(HaveOccurred(),
287+
"Expected validation to fail for both name and spec")
300288
})
301289
302290
It("Should admit update if both name and spec are valid", func() {
@@ -308,9 +296,8 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the
308296
obj.Spec.Schedule = "0 0 * * *"
309297
310298
By("validating an update")
311-
warnings, err := validator.ValidateUpdate(ctx, oldObj, obj)
312-
Expect(err).NotTo(HaveOccurred(), "Expected validation to pass for a valid update")
313-
Expect(warnings).To(BeNil())
299+
Expect(validator.ValidateUpdate(ctx, oldObj, obj)).To(BeNil(),
300+
"Expected validation to pass for a valid update")
314301
})`
315302

316303
const webhookTestsBeforeEachOriginal = `obj = &CronJob{}

‎pkg/plugins/golang/v4/scaffolds/internal/templates/api/webhook_test_template.go‎

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ var _ = Describe("{{ .Resource.Kind }} Webhook", func() {
8989
BeforeEach(func() {
9090
obj = &{{ .Resource.Kind }}{}
9191
Expect(obj).NotTo(BeNil(), "Expected obj to be initialized")
92-
92+
9393
// TODO (user): Add any setup logic common to all tests
9494
})
9595
@@ -103,14 +103,13 @@ var _ = Describe("{{ .Resource.Kind }} Webhook", func() {
103103

104104
const conversionWebhookTestTemplate = `
105105
Context("When creating {{ .Resource.Kind }} under Conversion Webhook", func() {
106-
It("Should convert the object correctly", func() {
107-
// TODO (user): Add logic to convert the object to the desired version and verify the conversion
108-
// Example:
109-
// convertedObj := &{{ .Resource.Kind }}{}
110-
// err := obj.ConvertTo(convertedObj)
111-
// Expect(err).NotTo(HaveOccurred())
112-
// Expect(convertedObj).ToNot(BeNil())
113-
})
106+
// TODO (user): Add logic to convert the object to the desired version and verify the conversion
107+
// Example:
108+
// It("Should convert the object correctly", func() {
109+
// convertedObj := &{{ .Resource.Kind }}{}
110+
// Expect(obj.ConvertTo(convertedObj)).To(Succeed())
111+
// Expect(convertedObj).ToNot(BeNil())
112+
// })
114113
})
115114
`
116115

@@ -119,28 +118,22 @@ Context("When creating or updating {{ .Resource.Kind }} under Validating Webhook
119118
// TODO (user): Add logic for validating webhooks
120119
// Example:
121120
// It("Should deny creation if a required field is missing", func() {
122-
// By("simulating an invalid creation scenario")
121+
// By("simulating an invalid creation scenario")
123122
// obj.SomeRequiredField = ""
124-
// warnings, err := obj.ValidateCreate(ctx)
125-
// Expect(err).To(HaveOccurred())
126-
// Expect(warnings).To(BeNil())
123+
// Expect(obj.ValidateCreate(ctx)).Error().To(HaveOccurred())
127124
// })
128125
//
129126
// It("Should admit creation if all required fields are present", func() {
130-
// By("simulating an invalid creation scenario")
127+
// By("simulating an invalid creation scenario")
131128
// obj.SomeRequiredField = "valid_value"
132-
// warnings, err := obj.ValidateCreate(ctx)
133-
// Expect(err).NotTo(HaveOccurred())
134-
// Expect(warnings).To(BeNil())
129+
// Expect(obj.ValidateCreate(ctx)).To(BeNil())
135130
// })
136131
//
137132
// It("Should validate updates correctly", func() {
138133
// By("simulating a valid update scenario")
139-
// oldObj := &Captain{SomeRequiredField: "valid_value"}
140-
// obj.SomeRequiredField = "updated_value"
141-
// warnings, err := obj.ValidateUpdate(ctx, oldObj)
142-
// Expect(err).NotTo(HaveOccurred())
143-
// Expect(warnings).To(BeNil())
134+
// oldObj := &Captain{SomeRequiredField: "valid_value"}
135+
// obj.SomeRequiredField = "updated_value"
136+
// Expect(obj.ValidateUpdate(ctx, oldObj)).To(BeNil())
144137
// })
145138
})
146139
`
@@ -151,9 +144,8 @@ Context("When creating {{ .Resource.Kind }} under Defaulting Webhook", func() {
151144
// Example:
152145
// It("Should apply defaults when a required field is empty", func() {
153146
// By("simulating a scenario where defaults should be applied")
154-
// obj.SomeFieldWithDefault = ""
155-
// err := obj.Default(ctx)
156-
// Expect(err).NotTo(HaveOccurred())
147+
// obj.SomeFieldWithDefault = ""
148+
// Expect(obj.Default(ctx)).To(Succeed())
157149
// Expect(obj.SomeFieldWithDefault).To(Equal("default_value"))
158150
// })
159151
})

0 commit comments

Comments
 (0)