Introduced cmp.Diff for error comparison

Signed-off-by: tariq-hasan <[email protected]>

pkg/webhook/v1beta1/pod/inject_webhook.go

@@ -49,10 +49,10 @@ import (
 var log = logf.Log.WithName("injector-webhook")
 
 var (
-	errInvalidOwnerAPIVersion = errors.New("invalid owner API version")
-	errInvalidSuggestionName  = errors.New("invalid suggestion name")
-	errPodNotBelongToKatibJob = errors.New("pod does not belong to Katib Job")
-	errNestedObjectNotFound   = errors.New("nested object not found in the cluster")
+	errInvalidOwnerAPIVersion      = errors.New("invalid owner API version")
+	errInvalidSuggestionName       = errors.New("invalid suggestion name")
+	errPodNotBelongToKatibJob      = errors.New("pod does not belong to Katib Job")
+	errFailedToGetTrialTemplateJob = errors.New("unable to get Job in the trialTemplate")
 )
 
 // SidecarInjector injects metrics collect sidecar to the primary pod.
@@ -280,7 +280,7 @@ func (s *SidecarInjector) getKatibJob(object *unstructured.Unstructured, namespa
 			// Nested object namespace must be equal to object namespace
 			err = s.client.Get(context.TODO(), apitypes.NamespacedName{Name: owners[i].Name, Namespace: namespace}, nestedJob)
 			if err != nil {
-				return "", "", fmt.Errorf("%w: %w", errNestedObjectNotFound, err)
+				return "", "", fmt.Errorf("%w: %w", errFailedToGetTrialTemplateJob, err)
 			}
 			// Recursively search for Trial ownership in nested object
 			jobKind, jobName, err = s.getKatibJob(nestedJob, namespace)

pkg/webhook/v1beta1/pod/inject_webhook_test.go

@@ -31,7 +31,6 @@ import (
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
@@ -161,7 +160,16 @@ func TestWrapWorkerContainer(t *testing.T) {
 					},
 				},
 			},
-			PathKind:  common.FileKind,
+			PathKind: common.FileKind,
+			WantPod: &v1.Pod{
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{
+						{
+							Name: "not-primary-container",
+						},
+					},
+				},
+			},
 			WantError: errPrimaryContainerNotFound,
 		},
 		"Container with early stopping command": {
@@ -218,8 +226,8 @@ func TestWrapWorkerContainer(t *testing.T) {
 			if diff := cmp.Diff(tc.WantError, err, cmpopts.EquateErrors()); len(diff) != 0 {
 				t.Errorf("Unexpected error from wrapWorkerContainer (-want,+got):\n%s", diff)
 			}
-			if err == nil && !equality.Semantic.DeepEqual(tc.Pod.Spec.Containers, tc.WantPod.Spec.Containers) {
-				t.Errorf("Unexpected error from wrapWorkerContainer, expected pod: %v, got: %v", tc.WantPod.Spec.Containers, tc.Pod.Spec.Containers)
+			if diff := cmp.Diff(tc.WantPod.Spec.Containers, tc.Pod.Spec.Containers); len(diff) != 0 {
+				t.Errorf("Unexpected pod from wrapWorkerContainer (-want,+got):\n%s", diff)
 			}
 		})
 	}
@@ -636,31 +644,33 @@ func TestMutateMetricsCollectorVolume(t *testing.T) {
 	if diff := cmp.Diff(tc.WantError, err, cmpopts.EquateErrors()); len(diff) != 0 {
 		t.Errorf("Unexpected error from mutateMetricsCollectorVolume (-want,+got):\n%s", diff)
 	}
-	if err == nil && !equality.Semantic.DeepEqual(tc.Pod, tc.WantPod) {
-		t.Errorf("Unexpected error from mutateMetricsCollectorVolume, expected pod: %v, got: %v", tc.WantPod, tc.Pod)
+	if diff := cmp.Diff(tc.WantPod, tc.Pod); len(diff) != 0 {
+		t.Errorf("Unexpected pod from mutateMetricsCollectorVolume (-want,+got):\n%s", diff)
 	}
 }
 
 func TestGetSidecarContainerName(t *testing.T) {
-	cases := []struct {
+	cases := map[string]struct {
 		CollectorKind     common.CollectorKind
 		WantCollectorKind string
 	}{
-		{
+		"Valid run with StdOutCollector": {
 			CollectorKind:     common.StdOutCollector,
 			WantCollectorKind: mccommon.MetricLoggerCollectorContainerName,
 		},
-		{
+		"Valid run with TfEventCollector": {
 			CollectorKind:     common.TfEventCollector,
 			WantCollectorKind: mccommon.MetricCollectorContainerName,
 		},
 	}
 
-	for _, tc := range cases {
-		collectorKind := getSidecarContainerName(tc.CollectorKind)
-		if collectorKind != tc.WantCollectorKind {
-			t.Errorf("Expected Collector Kind: %v, got %v", tc.WantCollectorKind, collectorKind)
-		}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			collectorKind := getSidecarContainerName(tc.CollectorKind)
+			if collectorKind != tc.WantCollectorKind {
+				t.Errorf("Expected Collector Kind: %v, got %v", tc.WantCollectorKind, collectorKind)
+			}
+		})
 	}
 }
 
@@ -880,7 +890,7 @@ func TestGetKatibJob(t *testing.T) {
 					},
 				},
 			},
-			WantError: errNestedObjectNotFound,
+			WantError: errFailedToGetTrialTemplateJob,
 		},
 		"Run when Pod owns Job with invalid API version": {
 			Pod: &v1.Pod{
@@ -986,10 +996,12 @@ func TestIsPrimaryPod(t *testing.T) {
 	}
 
 	for name, tc := range cases {
-		isPrimary := isPrimaryPod(tc.podLabels, tc.primaryPodLabels)
-		if isPrimary != tc.isPrimary {
-			t.Errorf("Case %v. Expected isPrimary %v, got %v", name, tc.isPrimary, isPrimary)
-		}
+		t.Run(name, func(t *testing.T) {
+			isPrimary := isPrimaryPod(tc.podLabels, tc.primaryPodLabels)
+			if isPrimary != tc.isPrimary {
+				t.Errorf("Case %v. Expected isPrimary %v, got %v", name, tc.isPrimary, isPrimary)
+			}
+		})
 	}
 }
 
@@ -1029,10 +1041,12 @@ func TestMutatePodMetadata(t *testing.T) {
 		},
 	}
 
-	for _, tc := range cases {
-		mutatePodMetadata(tc.pod, tc.trial)
-		if diff := cmp.Diff(tc.mutatedPod, tc.pod); len(diff) != 0 {
-			t.Errorf("Unexpected pod from mutatePodMetadata (-want,+got):\n%s", diff)
-		}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			mutatePodMetadata(tc.pod, tc.trial)
+			if diff := cmp.Diff(tc.mutatedPod, tc.pod); len(diff) != 0 {
+				t.Errorf("Unexpected pod from mutatePodMetadata (-want,+got):\n%s", diff)
+			}
+		})
 	}
 }