Skip to content

Commit 337336c

Browse files
twoGiantstwoGiants
committed
refactor: move Validate and ValidateError tests
Now that `Step` implements the `Validatable` interface the tests for the `Step` validation are moved from `task_validation_test.go` to `container_validation_test.go`. The tests cases which test for a valid Step and for validation errors were extracted from `task_validation_test.go`: - `TestTaskSpecValidate` - `TestTaskSpecValidateError` and moved to new test functions in `container_validation_test.go`: - `TestStepValidate` - `TestStepValidateError` Issue #8700. Signed-off-by: Stanislav Jakuschevskij <stas@two-giants.com>
1 parent 7117ec9 commit 337336c

File tree

2 files changed

+136
-114
lines changed

2 files changed

+136
-114
lines changed

‎pkg/apis/pipeline/v1/container_validation_test.go‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"testing"
23+
"time"
2324

2425
"github.com/google/go-cmp/cmp"
2526
"github.com/google/go-cmp/cmp/cmpopts"
@@ -28,6 +29,8 @@ import (
2829
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
2930
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
3031
"github.com/tektoncd/pipeline/test/diff"
32+
corev1 "k8s.io/api/core/v1"
33+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3134
"knative.dev/pkg/apis"
3235
)
3336

@@ -171,6 +174,136 @@ func TestRef_Invalid(t *testing.T) {
171174
}
172175
}
173176

177+
func TestStepValidate(t *testing.T) {
178+
tests := []struct {
179+
name string
180+
Step v1.Step
181+
}{{
182+
name: "unnamed step",
183+
Step: v1.Step{
184+
Image: "myimage",
185+
},
186+
}, {
187+
name: "valid step with script",
188+
Step: v1.Step{
189+
Image: "my-image",
190+
Script: `
191+
#!/usr/bin/env bash
192+
hello world`,
193+
},
194+
}, {
195+
name: "valid step with script and args",
196+
Step: v1.Step{
197+
Image: "my-image",
198+
Args: []string{"arg"},
199+
Script: `
200+
#!/usr/bin/env bash
201+
hello $1`,
202+
},
203+
}, {
204+
name: "valid step with volumeMount under /tekton/home",
205+
Step: v1.Step{
206+
Image: "myimage",
207+
VolumeMounts: []corev1.VolumeMount{{
208+
Name: "foo",
209+
MountPath: "/tekton/home",
210+
}},
211+
},
212+
}}
213+
for _, st := range tests {
214+
t.Run(st.name, func(t *testing.T) {
215+
ctx := cfgtesting.EnableAlphaAPIFields(context.Background())
216+
if err := st.Step.Validate(ctx); err != nil {
217+
t.Errorf("Step.Validate() = %v", err)
218+
}
219+
})
220+
}
221+
}
222+
223+
func TestStepValidateError(t *testing.T) {
224+
tests := []struct {
225+
name string
226+
Step v1.Step
227+
expectedError apis.FieldError
228+
}{{
229+
name: "invalid step with missing Image",
230+
Step: v1.Step{},
231+
expectedError: apis.FieldError{
232+
Message: "missing field(s)",
233+
Paths: []string{"Image"},
234+
},
235+
}, {
236+
name: "invalid step name",
237+
Step: v1.Step{
238+
Name: "replaceImage",
239+
Image: "myimage",
240+
},
241+
expectedError: apis.FieldError{
242+
Message: `invalid value "replaceImage"`,
243+
Paths: []string{"name"},
244+
Details: "Task step name must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
245+
},
246+
}, {
247+
name: "step with script and command",
248+
Step: v1.Step{
249+
Image: "myimage",
250+
Command: []string{"command"},
251+
Script: "script",
252+
},
253+
expectedError: apis.FieldError{
254+
Message: "script cannot be used with command",
255+
Paths: []string{"script"},
256+
},
257+
}, {
258+
name: "step volume mounts under /tekton/",
259+
Step: v1.Step{
260+
Image: "myimage",
261+
VolumeMounts: []corev1.VolumeMount{{
262+
Name: "foo",
263+
MountPath: "/tekton/foo",
264+
}},
265+
},
266+
expectedError: apis.FieldError{
267+
Message: `volumeMount cannot be mounted under /tekton/ (volumeMount "foo" mounted at "/tekton/foo")`,
268+
Paths: []string{"volumeMounts[0].mountPath"},
269+
},
270+
}, {
271+
name: "step volume mount name starts with tekton-internal-",
272+
Step: v1.Step{
273+
Image: "myimage",
274+
VolumeMounts: []corev1.VolumeMount{{
275+
Name: "tekton-internal-foo",
276+
MountPath: "/this/is/fine",
277+
}},
278+
},
279+
expectedError: apis.FieldError{
280+
Message: `volumeMount name "tekton-internal-foo" cannot start with "tekton-internal-"`,
281+
Paths: []string{"volumeMounts[0].name"},
282+
},
283+
}, {
284+
name: "negative timeout string",
285+
Step: v1.Step{
286+
Timeout: &metav1.Duration{Duration: -10 * time.Second},
287+
},
288+
expectedError: apis.FieldError{
289+
Message: "invalid value: -10s",
290+
Paths: []string{"negative timeout"},
291+
},
292+
}}
293+
for _, st := range tests {
294+
t.Run(st.name, func(t *testing.T) {
295+
ctx := cfgtesting.EnableAlphaAPIFields(context.Background())
296+
err := st.Step.Validate(ctx)
297+
if err == nil {
298+
t.Fatalf("Expected an error, got nothing for %v", st.Step)
299+
}
300+
if d := cmp.Diff(st.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
301+
t.Errorf("Step.Validate() errors diff %s", diff.PrintWantGot(d))
302+
}
303+
})
304+
}
305+
}
306+
174307
func TestSidecarValidate(t *testing.T) {
175308
tests := []struct {
176309
name string

‎pkg/apis/pipeline/v1/task_validation_test.go‎

Lines changed: 3 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"errors"
1919
"fmt"
2020
"testing"
21-
"time"
2221

2322
"github.com/google/go-cmp/cmp"
2423
"github.com/google/go-cmp/cmp/cmpopts"
@@ -38,11 +37,6 @@ var validSteps = []v1.Step{{
3837
Image: "myimage",
3938
}}
4039

41-
var invalidSteps = []v1.Step{{
42-
Name: "replaceImage",
43-
Image: "myimage",
44-
}}
45-
4640
func TestTaskValidate(t *testing.T) {
4741
tests := []struct {
4842
name string
@@ -161,15 +155,6 @@ func TestTaskSpecValidate(t *testing.T) {
161155
name string
162156
fields fields
163157
}{{
164-
name: "unnamed steps",
165-
fields: fields{
166-
Steps: []v1.Step{{
167-
Image: "myimage",
168-
}, {
169-
Image: "myotherimage",
170-
}},
171-
},
172-
}, {
173158
name: "valid params type implied",
174159
fields: fields{
175160
Params: []v1.ParamSpec{{
@@ -323,16 +308,6 @@ func TestTaskSpecValidate(t *testing.T) {
323308
Image: "some-image",
324309
},
325310
},
326-
}, {
327-
name: "valid step with script",
328-
fields: fields{
329-
Steps: []v1.Step{{
330-
Image: "my-image",
331-
Script: `
332-
#!/usr/bin/env bash
333-
hello world`,
334-
}},
335-
},
336311
}, {
337312
name: "step template included in validation with stepaction",
338313
fields: fields{
@@ -383,28 +358,6 @@ func TestTaskSpecValidate(t *testing.T) {
383358
hello $(params.baz)`,
384359
}},
385360
},
386-
}, {
387-
name: "valid step with script and args",
388-
fields: fields{
389-
Steps: []v1.Step{{
390-
Image: "my-image",
391-
Args: []string{"arg"},
392-
Script: `
393-
#!/usr/bin/env bash
394-
hello $1`,
395-
}},
396-
},
397-
}, {
398-
name: "valid step with volumeMount under /tekton/home",
399-
fields: fields{
400-
Steps: []v1.Step{{
401-
Image: "myimage",
402-
VolumeMounts: []corev1.VolumeMount{{
403-
Name: "foo",
404-
MountPath: "/tekton/home",
405-
}},
406-
}},
407-
},
408361
}, {
409362
name: "valid workspace",
410363
fields: fields{
@@ -1037,16 +990,6 @@ func TestTaskSpecValidateError(t *testing.T) {
1037990
Message: "missing field(s)",
1038991
Paths: []string{"steps"},
1039992
},
1040-
}, {
1041-
name: "invalid step name",
1042-
fields: fields{
1043-
Steps: invalidSteps,
1044-
},
1045-
expectedError: apis.FieldError{
1046-
Message: `invalid value "replaceImage"`,
1047-
Paths: []string{"steps[0].name"},
1048-
Details: "Task step name must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
1049-
},
1050993
}, {
1051994
name: "duplicate step name",
1052995
fields: fields{
@@ -1245,49 +1188,6 @@ func TestTaskSpecValidateError(t *testing.T) {
12451188
Message: `multiple volumes with same name "workspace"`,
12461189
Paths: []string{"volumes[1].name"},
12471190
},
1248-
}, {
1249-
name: "step with script and command",
1250-
fields: fields{
1251-
Steps: []v1.Step{{
1252-
Image: "myimage",
1253-
Command: []string{"command"},
1254-
Script: "script",
1255-
}},
1256-
},
1257-
expectedError: apis.FieldError{
1258-
Message: "script cannot be used with command",
1259-
Paths: []string{"steps[0].script"},
1260-
},
1261-
}, {
1262-
name: "step volume mounts under /tekton/",
1263-
fields: fields{
1264-
Steps: []v1.Step{{
1265-
Image: "myimage",
1266-
VolumeMounts: []corev1.VolumeMount{{
1267-
Name: "foo",
1268-
MountPath: "/tekton/foo",
1269-
}},
1270-
}},
1271-
},
1272-
expectedError: apis.FieldError{
1273-
Message: `volumeMount cannot be mounted under /tekton/ (volumeMount "foo" mounted at "/tekton/foo")`,
1274-
Paths: []string{"steps[0].volumeMounts[0].mountPath"},
1275-
},
1276-
}, {
1277-
name: "step volume mount name starts with tekton-internal-",
1278-
fields: fields{
1279-
Steps: []v1.Step{{
1280-
Image: "myimage",
1281-
VolumeMounts: []corev1.VolumeMount{{
1282-
Name: "tekton-internal-foo",
1283-
MountPath: "/this/is/fine",
1284-
}},
1285-
}},
1286-
},
1287-
expectedError: apis.FieldError{
1288-
Message: `volumeMount name "tekton-internal-foo" cannot start with "tekton-internal-"`,
1289-
Paths: []string{"steps[0].volumeMounts[0].name"},
1290-
},
12911191
}, {
12921192
name: "declared workspaces names are not unique",
12931193
fields: fields{
@@ -1397,7 +1297,7 @@ func TestTaskSpecValidateError(t *testing.T) {
13971297
Paths: []string{"workspaces[0].mountpath"},
13981298
},
13991299
}, {
1400-
name: "result name not validate",
1300+
name: "result name not valid",
14011301
fields: fields{
14021302
Steps: validSteps,
14031303
Results: []v1.TaskResult{{
@@ -1411,7 +1311,7 @@ func TestTaskSpecValidateError(t *testing.T) {
14111311
Details: "Name must consist of alphanumeric characters, '-', '_', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my-name', or 'my_name', regex used for validation is '^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$')",
14121312
},
14131313
}, {
1414-
name: "result type not validate",
1314+
name: "result type not valid",
14151315
fields: fields{
14161316
Steps: validSteps,
14171317
Results: []v1.TaskResult{{
@@ -1426,7 +1326,7 @@ func TestTaskSpecValidateError(t *testing.T) {
14261326
Details: "type must be string",
14271327
},
14281328
}, {
1429-
name: "context not validate",
1329+
name: "context not valid",
14301330
fields: fields{
14311331
Steps: []v1.Step{{
14321332
Image: "my-image",
@@ -1440,17 +1340,6 @@ func TestTaskSpecValidateError(t *testing.T) {
14401340
Message: `non-existent variable in "\n\t\t\t\t#!/usr/bin/env bash\n\t\t\t\thello \"$(context.task.missing)\""`,
14411341
Paths: []string{"steps[0].script"},
14421342
},
1443-
}, {
1444-
name: "negative timeout string",
1445-
fields: fields{
1446-
Steps: []v1.Step{{
1447-
Timeout: &metav1.Duration{Duration: -10 * time.Second},
1448-
}},
1449-
},
1450-
expectedError: apis.FieldError{
1451-
Message: "invalid value: -10s",
1452-
Paths: []string{"steps[0].negative timeout"},
1453-
},
14541343
}}
14551344
for _, tt := range tests {
14561345
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)