Merge pull request #1590 from marquiz/devel/validation-assert

apis/nfd/validate: use testify/assert for checking test results
kubernetes-sigs · Mar 12, 2024 · ebd19fe · ebd19fe
2 parents 8553265 + 1f3b9cc
commit ebd19fe
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 47 deletions.
diff --git a/pkg/apis/nfd/validate/validate.go b/pkg/apis/nfd/validate/validate.go
@@ -115,7 +115,7 @@ func Label(key, value string) error {
 
 	// Validate label value
 	if err := k8svalidation.IsValidLabelValue(value); len(err) > 0 {
-		return fmt.Errorf("invalid labelvalue %q: %s", value, strings.Join(err, "; "))
+		return fmt.Errorf("invalid value %q: %s", value, strings.Join(err, "; "))
 	}
 
 	return nil
@@ -156,7 +156,7 @@ func Annotation(key, value string) error {
 
 	// Validate annotation value
 	if errs := k8svalidation.IsValidLabelValue(value); len(errs) > 0 {
-		return fmt.Errorf("invalid annotation value %q: %s", value, strings.Join(errs, "; "))
+		return fmt.Errorf("invalid value %q: %s", value, strings.Join(errs, "; "))
 	}
 
 	return nil
@@ -236,10 +236,10 @@ func ExtendedResource(key, value string) error {
 		}
 	}
 
-	// Static Value (Pre-Defined at the NodeFeatureRule)
+	// Validate extended resource value
 	_, err := k8sQuantity.ParseQuantity(value)
 	if err != nil {
-		return fmt.Errorf("invalid value %s (from %s): %w", value, value, err)
+		return fmt.Errorf("invalid value %q: %w", value, err)
 	}
 
 	return nil

diff --git a/pkg/apis/nfd/validate/validate_test.go b/pkg/apis/nfd/validate/validate_test.go
@@ -1,9 +1,9 @@
 package validate
 
 import (
-	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -12,8 +12,7 @@ func TestAnnotation(t *testing.T) {
 		name  string
 		key   string
 		value string
-		want  error
-		fail  bool
+		want  interface{}
 	}{
 		{
 			name:  "Valid annotation",
@@ -31,26 +30,23 @@ func TestAnnotation(t *testing.T) {
 			name:  "Invalid annotation value",
 			key:   "feature.node.kubernetes.io/feature",
 			value: "invalid value",
-			want:  fmt.Errorf("invalid value \"invalid value\": value must be a valid label value"),
-			fail:  true,
+			want:  "invalid value \"invalid value\": ",
 		},
 		{
 			name:  "Denied annotation key",
 			key:   "kubernetes.io/denied",
 			value: "true",
 			want:  ErrNSNotAllowed,
-			fail:  true,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got := Annotation(tt.key, tt.value)
-			if got != tt.want {
-				if tt.fail {
-					return
-				}
-				t.Errorf("Annotation() = %v, want %v", got, tt.want)
+			err := Annotation(tt.key, tt.value)
+			if str, ok := tt.want.(string); ok {
+				assert.ErrorContains(t, err, str)
+			} else {
+				assert.Equal(t, tt.want, err)
 			}
 		})
 	}
@@ -112,9 +108,7 @@ func TestTaint(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := Taint(tt.taint)
-			if got != tt.want {
-				t.Errorf("Taint() = %v, want %v", got, tt.want)
-			}
+			assert.Equal(t, tt.want, got)
 		})
 	}
 }
@@ -124,61 +118,53 @@ func TestLabel(t *testing.T) {
 		name  string
 		key   string
 		value string
-		want  error
-		fail  bool
+		want  interface{}
 	}{
 		{
 			name:  "Valid label",
 			key:   "feature.node.kubernetes.io/label",
 			value: "true",
 			want:  nil,
-			fail:  false,
 		},
 		{
 			name:  "Valid vendor label",
 			key:   "vendor.io/label",
 			value: "true",
 			want:  nil,
-			fail:  false,
 		},
 		{
 			name:  "Denied label with prefix",
 			key:   "kubernetes.io/label",
 			value: "true",
 			want:  ErrNSNotAllowed,
-			fail:  true,
 		},
 		{
 			name:  "Invalid label key",
 			key:   "invalid-key",
 			value: "true",
-			want:  ErrNSNotAllowed,
-			fail:  true,
+			want:  ErrUnprefixedKeysNotAllowed,
 		},
 		{
 			name:  "Invalid label value",
 			key:   "feature.node.kubernetes.io/label",
 			value: "invalid value",
-			want:  fmt.Errorf("invalid value \"invalid value\": value must be a valid label value"),
-			fail:  true,
+			want:  "invalid value \"invalid value\": ",
 		},
 		{
 			name:  "Valid value label",
 			key:   "feature.node.kubernetes.io/label",
 			value: "true",
 			want:  nil,
-			fail:  false,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			err := Label(tt.key, tt.value)
-			if err != tt.want {
-				if tt.fail {
-					return
-				}
-				t.Errorf("Label() = %v, want %v", err, tt.want)
+			if str, ok := tt.want.(string); ok {
+				assert.ErrorContains(t, err, str)
+			} else {
+				assert.Equal(t, tt.want, err)
 			}
 		})
 	}
@@ -189,47 +175,41 @@ func TestExtendedResource(t *testing.T) {
 		name  string
 		key   string
 		value string
-		want  error
-		fail  bool
+		want  interface{}
 	}{
 		{
 			name:  "Valid extended resource",
 			key:   "feature.node.kubernetes.io/extended-resource",
 			value: "123",
 			want:  nil,
-			fail:  false,
 		},
 		{
 			name:  "Invalid extended resource key",
 			key:   "invalid-key",
 			value: "123",
-			want:  ErrNSNotAllowed,
-			fail:  true,
+			want:  ErrUnprefixedKeysNotAllowed,
 		},
 		{
 			name:  "Invalid extended resource value",
 			key:   "feature.node.kubernetes.io/extended-resource",
 			value: "invalid value",
-			want:  fmt.Errorf("invalid value \"invalid value\": value must be a valid label value"),
-			fail:  true,
+			want:  "invalid value \"invalid value\": ",
 		},
 		{
 			name:  "Denied extended resource key",
 			key:   "kubernetes.io/extended-resource",
 			value: "123",
 			want:  ErrNSNotAllowed,
-			fail:  true,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			err := ExtendedResource(tt.key, tt.value)
-			if err != tt.want {
-				if tt.fail {
-					return
-				}
-				t.Errorf("ExtendedResource() = %v, want %v", err, tt.want)
+			if str, ok := tt.want.(string); ok {
+				assert.ErrorContains(t, err, str)
+			} else {
+				assert.Equal(t, tt.want, err)
 			}
 		})
 	}