refactor: move Step onError and API version 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`.

`TestStepOnError` is moved.

`TestIncompatibleAPIVersions` is broken down and the Step fields
related tests i.e. "windows script support", "stdout stream support" and
"stderr stream support" are moved to a new test
`TestStepIncompatibleAPIVersions`.

Issue #8700.

Signed-off-by: Stanislav Jakuschevskij <[email protected]>

Author: twoGiants

pkg/apis/pipeline/v1/container_validation_test.go

@@ -171,6 +171,148 @@ func TestRef_Invalid(t *testing.T) {
 	}
 }
 
+func TestStepOnError(t *testing.T) {
+	tests := []struct {
+		name          string
+		params        []v1.ParamSpec
+		step          v1.Step
+		expectedError *apis.FieldError
+	}{{
+		name: "valid step - valid onError usage - set to continue",
+		step: v1.Step{
+			OnError: v1.Continue,
+			Image:   "image",
+			Args:    []string{"arg"},
+		},
+	}, {
+		name: "valid step - valid onError usage - set to stopAndFail",
+		step: v1.Step{
+			OnError: v1.StopAndFail,
+			Image:   "image",
+			Args:    []string{"arg"},
+		},
+	}, {
+		name: "valid step - valid onError usage - set to a task parameter",
+		params: []v1.ParamSpec{{
+			Name:    "CONTINUE",
+			Default: &v1.ParamValue{Type: v1.ParamTypeString, StringVal: string(v1.Continue)},
+		}},
+		step: v1.Step{
+			OnError: "$(params.CONTINUE)",
+			Image:   "image",
+			Args:    []string{"arg"},
+		},
+	}, {
+		name: "invalid step - onError set to invalid value",
+		step: v1.Step{
+			OnError: "onError",
+			Image:   "image",
+			Args:    []string{"arg"},
+		},
+		expectedError: &apis.FieldError{
+			Message: `invalid value: "onError"`,
+			Paths:   []string{"onError"},
+			Details: `Task step onError must be either "continue" or "stopAndFail"`,
+		},
+	}}
+	for _, st := range tests {
+		t.Run(st.name, func(t *testing.T) {
+			ctx := context.Background()
+			err := st.step.Validate(ctx)
+			if st.expectedError == nil && err != nil {
+				t.Errorf("No error expected from Step.Validate() but got = %v", err)
+			} else if st.expectedError != nil {
+				if err == nil {
+					t.Errorf("Expected error from Step.Validate() = %v, but got none", st.expectedError)
+				} else if d := cmp.Diff(st.expectedError.Error(), err.Error()); d != "" {
+					t.Errorf("returned error from Step.Validate() does not match with the expected error: %s", diff.PrintWantGot(d))
+				}
+			}
+		})
+	}
+}
+
+// TestStepIncompatibleAPIVersions exercises validation of fields in a Step
+// that require a specific feature gate version in order to work.
+func TestStepIncompatibleAPIVersions(t *testing.T) {
+	versions := []string{"alpha", "beta", "stable"}
+	isStricterThen := func(first, second string) bool {
+		// assume values are in order alpha (less strict), beta, stable (strictest)
+		// return true if first is stricter then second
+		switch first {
+		case second, "alpha":
+			return false
+		case "stable":
+			return true
+		default:
+			// first is beta, true is second is alpha, false is second is stable
+			return second == "alpha"
+		}
+	}
+
+	for _, st := range []struct {
+		name            string
+		requiredVersion string
+		step            v1.Step
+	}{
+		{
+			name:            "windows script support requires alpha",
+			requiredVersion: "alpha",
+			step: v1.Step{
+				Image: "my-image",
+				Script: `
+				#!win powershell -File
+				script-1`,
+			},
+		}, {
+			name:            "stdout stream support requires alpha",
+			requiredVersion: "alpha",
+			step: v1.Step{
+				Image: "foo",
+				StdoutConfig: &v1.StepOutputConfig{
+					Path: "/tmp/stdout.txt",
+				},
+			},
+		}, {
+			name:            "stderr stream support requires alpha",
+			requiredVersion: "alpha",
+			step: v1.Step{
+				Image: "foo",
+				StderrConfig: &v1.StepOutputConfig{
+					Path: "/tmp/stderr.txt",
+				},
+			},
+		},
+	} {
+		for _, version := range versions {
+			testName := fmt.Sprintf("(using %s) %s", version, st.name)
+			t.Run(testName, func(t *testing.T) {
+				ctx := context.Background()
+				if version == "alpha" {
+					ctx = cfgtesting.EnableAlphaAPIFields(ctx)
+				}
+				if version == "beta" {
+					ctx = cfgtesting.EnableBetaAPIFields(ctx)
+				}
+				if version == "stable" {
+					ctx = cfgtesting.EnableStableAPIFields(ctx)
+				}
+				err := st.step.Validate(ctx)
+
+				// If the configured version is stricter than the required one, we expect an error
+				if isStricterThen(version, st.requiredVersion) && err == nil {
+					t.Fatalf("no error received even though version required is %q while feature gate is %q", st.requiredVersion, version)
+				}
+
+				// If the configured version is more permissive than the required one, we expect no error
+				if isStricterThen(st.requiredVersion, version) && err != nil {
+					t.Fatalf("error received despite required version and feature gate matching %q: %v", version, err)
+				}
+			})
+		}
+	}
+}
+
 func TestSidecarValidate(t *testing.T) {
 	tests := []struct {
 		name    string

pkg/apis/pipeline/v1/task_validation_test.go

@@ -2142,72 +2142,6 @@ func TestStepAndSidecarWorkspacesErrors(t *testing.T) {
 	}
 }
 
-func TestStepOnError(t *testing.T) {
-	tests := []struct {
-		name          string
-		params        []v1.ParamSpec
-		steps         []v1.Step
-		expectedError *apis.FieldError
-	}{{
-		name: "valid step - valid onError usage - set to continue",
-		steps: []v1.Step{{
-			OnError: v1.Continue,
-			Image:   "image",
-			Args:    []string{"arg"},
-		}},
-	}, {
-		name: "valid step - valid onError usage - set to stopAndFail",
-		steps: []v1.Step{{
-			OnError: v1.StopAndFail,
-			Image:   "image",
-			Args:    []string{"arg"},
-		}},
-	}, {
-		name: "valid step - valid onError usage - set to a task parameter",
-		params: []v1.ParamSpec{{
-			Name:    "CONTINUE",
-			Default: &v1.ParamValue{Type: v1.ParamTypeString, StringVal: string(v1.Continue)},
-		}},
-		steps: []v1.Step{{
-			OnError: "$(params.CONTINUE)",
-			Image:   "image",
-			Args:    []string{"arg"},
-		}},
-	}, {
-		name: "invalid step - onError set to invalid value",
-		steps: []v1.Step{{
-			OnError: "onError",
-			Image:   "image",
-			Args:    []string{"arg"},
-		}},
-		expectedError: &apis.FieldError{
-			Message: `invalid value: "onError"`,
-			Paths:   []string{"steps[0].onError"},
-			Details: `Task step onError must be either "continue" or "stopAndFail"`,
-		},
-	}}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			ts := &v1.TaskSpec{
-				Params: tt.params,
-				Steps:  tt.steps,
-			}
-			ctx := context.Background()
-			ts.SetDefaults(ctx)
-			err := ts.Validate(ctx)
-			if tt.expectedError == nil && err != nil {
-				t.Errorf("No error expected from TaskSpec.Validate() but got = %v", err)
-			} else if tt.expectedError != nil {
-				if err == nil {
-					t.Errorf("Expected error from TaskSpec.Validate() = %v, but got none", tt.expectedError)
-				} else if d := cmp.Diff(tt.expectedError.Error(), err.Error()); d != "" {
-					t.Errorf("returned error from TaskSpec.Validate() does not match with the expected error: %s", diff.PrintWantGot(d))
-				}
-			}
-		})
-	}
-}
-
 // TestIncompatibleAPIVersions exercises validation of fields that
 // require a specific feature gate version in order to work.
 func TestIncompatibleAPIVersions(t *testing.T) {
@@ -2262,39 +2196,6 @@ func TestIncompatibleAPIVersions(t *testing.T) {
 					}},
 				}},
 			},
-		}, {
-			name:            "windows script support requires alpha",
-			requiredVersion: "alpha",
-			spec: v1.TaskSpec{
-				Steps: []v1.Step{{
-					Image: "my-image",
-					Script: `
-				#!win powershell -File
-				script-1`,
-				}},
-			},
-		}, {
-			name:            "stdout stream support requires alpha",
-			requiredVersion: "alpha",
-			spec: v1.TaskSpec{
-				Steps: []v1.Step{{
-					Image: "foo",
-					StdoutConfig: &v1.StepOutputConfig{
-						Path: "/tmp/stdout.txt",
-					},
-				}},
-			},
-		}, {
-			name:            "stderr stream support requires alpha",
-			requiredVersion: "alpha",
-			spec: v1.TaskSpec{
-				Steps: []v1.Step{{
-					Image: "foo",
-					StderrConfig: &v1.StepOutputConfig{
-						Path: "/tmp/stderr.txt",
-					},
-				}},
-			},
 		},
 	} {
 		for _, version := range versions {