refactor validate task name

Introducing a function receiver to validate pipeline task name and moving
the tests to appropriate test file.

pkg/apis/pipeline/v1beta1/pipeline_types.go

@@ -17,9 +17,13 @@ limitations under the License.
 package v1beta1
 
 import (
+	"fmt"
+
 	"github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/validation"
+	"knative.dev/pkg/apis"
 )
 
 // +genclient
@@ -169,6 +173,18 @@ func (pt PipelineTask) HashKey() string {
 	return pt.Name
 }
 
+func (pt PipelineTask) ValidateName() *apis.FieldError {
+	if err := validation.IsDNS1123Label(pt.Name); len(err) > 0 {
+		return &apis.FieldError{
+			Message: fmt.Sprintf("invalid value %q", pt.Name),
+			Paths:   []string{"name"},
+			Details: "Pipeline Task name must be a valid DNS Label." +
+				"For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
+		}
+	}
+	return nil
+}
+
 func (pt PipelineTask) Deps() []string {
 	deps := []string{}
 

pkg/apis/pipeline/v1beta1/pipeline_types_test.go

@@ -19,11 +19,11 @@ package v1beta1
 import (
 	"testing"
 
-	"github.com/tektoncd/pipeline/test/diff"
-
 	"github.com/google/go-cmp/cmp"
-
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/tektoncd/pipeline/test/diff"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"knative.dev/pkg/apis"
 )
 
 func TestPipelineTaskList_Names(t *testing.T) {
@@ -40,6 +40,47 @@ func TestPipelineTaskList_Names(t *testing.T) {
 	}
 }
 
+func TestPipelineTask_ValidateName(t *testing.T) {
+	pipelineTasks := []struct {
+		name    string
+		task    PipelineTask
+		message string
+	}{{
+		name:    "pipeline task with empty task name",
+		task:    PipelineTask{Name: ""},
+		message: `invalid value ""`,
+	}, {
+		name:    "pipeline task with invalid task name",
+		task:    PipelineTask{Name: "_foo"},
+		message: `invalid value "_foo"`,
+	}, {
+		name:    "pipeline task with invalid task name (camel case)",
+		task:    PipelineTask{Name: "fooTask"},
+		message: `invalid value "fooTask"`,
+	}}
+
+	// expected error if a task name is not valid
+	expectedError := apis.FieldError{
+		Paths: []string{"name"},
+		Details: "Pipeline Task name must be a valid DNS Label." +
+			"For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
+	}
+
+	for _, tc := range pipelineTasks {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.task.ValidateName()
+			if err == nil {
+				t.Error("PipelineTask.ValidateName() did not return error for invalid pipeline task name")
+			}
+			// error message changes for each test as it includes the task name in the message
+			expectedError.Message = tc.message
+			if d := cmp.Diff(expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
+				t.Errorf("PipelineTask.ValidateName() errors diff %s", diff.PrintWantGot(d))
+			}
+		})
+	}
+}
+
 func TestPipelineTaskList_Deps(t *testing.T) {
 	pipelines := []struct {
 		name         string

pkg/apis/pipeline/v1beta1/pipeline_validation.go

@@ -89,21 +89,9 @@ func ValidatePipelineTasks(ctx context.Context, tasks []PipelineTask, finalTasks
 	return errs
 }
 
-func validatePipelineTaskName(name string) *apis.FieldError {
-	if err := validation.IsDNS1123Label(name); len(err) > 0 {
-		return &apis.FieldError{
-			Message: fmt.Sprintf("invalid value %q", name),
-			Paths:   []string{"name"},
-			Details: "Pipeline Task name must be a valid DNS Label." +
-				"For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
-		}
-	}
-	return nil
-}
-
 func validatePipelineTask(ctx context.Context, t PipelineTask, taskNames sets.String) *apis.FieldError {
 	cfg := config.FromContextOrDefaults(ctx)
-	errs := validatePipelineTaskName(t.Name)
+	errs := t.ValidateName()
 
 	hasTaskRef := t.TaskRef != nil
 	hasTaskSpec := t.TaskSpec != nil

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go

@@ -821,33 +821,6 @@ func TestValidatePipelineTasks_Failure(t *testing.T) {
 			Message: `expected exactly one, got both`,
 			Paths:   []string{"tasks[1].name"},
 		},
-	}, {
-		name:  "pipeline task with empty task name",
-		tasks: []PipelineTask{{Name: "", TaskRef: &TaskRef{Name: "foo-task"}}},
-		expectedError: apis.FieldError{
-			Message: `invalid value ""`,
-			Paths:   []string{"tasks[0].name"},
-			Details: "Pipeline Task name must be a valid DNS Label." +
-				"For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
-		},
-	}, {
-		name:  "pipeline task with invalid task name",
-		tasks: []PipelineTask{{Name: "_foo", TaskRef: &TaskRef{Name: "foo-task"}}},
-		expectedError: apis.FieldError{
-			Message: `invalid value "_foo"`,
-			Paths:   []string{"tasks[0].name"},
-			Details: "Pipeline Task name must be a valid DNS Label." +
-				"For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
-		},
-	}, {
-		name:  "pipeline task with invalid task name (camel case)",
-		tasks: []PipelineTask{{Name: "fooTask", TaskRef: &TaskRef{Name: "foo-task"}}},
-		expectedError: apis.FieldError{
-			Message: `invalid value "fooTask"`,
-			Paths:   []string{"tasks[0].name"},
-			Details: "Pipeline Task name must be a valid DNS Label." +
-				"For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
-		},
 	}, {
 		name:  "pipeline task with invalid taskref name",
 		tasks: []PipelineTask{{Name: "foo", TaskRef: &TaskRef{Name: "_foo-task"}}},