validation unit test cleanups

Fix some incorrect test case names.

Use t.Run() in a few more places (to facilitate using
SetFeatureGateDuringTest later).

Clarify TestPodIPsValidation/TestHostIPsValidation (and fix
weird indentation).
This commit is contained in:
Dan Winship 2025-02-27 09:32:52 -05:00
parent 76f1684117
commit f79bccf4d9
2 changed files with 119 additions and 94 deletions

View File

@ -9951,6 +9951,7 @@ func TestValidatePodDNSConfig(t *testing.T) {
}
for _, tc := range testCases {
t.Run("", func(t *testing.T) {
if tc.dnsPolicy == nil {
tc.dnsPolicy = &testDNSClusterFirst
}
@ -9961,6 +9962,7 @@ func TestValidatePodDNSConfig(t *testing.T) {
} else if len(errs) == 0 && tc.expectedError {
t.Errorf("%v: validatePodDNSConfig(%v) = nil, want error", tc.desc, tc.dnsConfig)
}
})
}
}
@ -10304,12 +10306,14 @@ func TestValidatePodSpec(t *testing.T) {
),
}
for k, v := range failureCases {
t.Run(k, func(t *testing.T) {
opts := PodValidationOptions{
ResourceIsPod: true,
}
if errs := ValidatePodSpec(&v.Spec, nil, field.NewPath("field"), opts); len(errs) == 0 {
t.Errorf("expected failure for %q", k)
t.Errorf("expected failure")
}
})
}
}
@ -15302,35 +15306,35 @@ func TestValidateServiceCreate(t *testing.T) {
// numErrs: 1,
numErrs: 0,
}, {
name: "invalid publicIPs localhost",
name: "invalid externalIPs localhost",
tweakSvc: func(s *core.Service) {
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.ExternalIPs = []string{"127.0.0.1"}
},
numErrs: 1,
}, {
name: "invalid publicIPs unspecified",
name: "invalid externalIPs unspecified",
tweakSvc: func(s *core.Service) {
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.ExternalIPs = []string{"0.0.0.0"}
},
numErrs: 1,
}, {
name: "invalid publicIPs loopback",
name: "invalid externalIPs loopback",
tweakSvc: func(s *core.Service) {
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.ExternalIPs = []string{"127.0.0.1"}
},
numErrs: 1,
}, {
name: "invalid publicIPs host",
name: "invalid externalIPs host",
tweakSvc: func(s *core.Service) {
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.ExternalIPs = []string{"myhost.mydomain"}
},
numErrs: 1,
}, {
name: "valid publicIPs",
name: "valid externalIPs",
tweakSvc: func(s *core.Service) {
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
s.Spec.ExternalIPs = []string{"1.2.3.4"}
@ -16989,9 +16993,11 @@ func TestValidateNode(t *testing.T) {
},
}
for _, successCase := range successCases {
t.Run("", func(t *testing.T) {
if errs := ValidateNode(&successCase); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
})
}
errorCases := map[string]core.Node{
@ -17195,9 +17201,10 @@ func TestValidateNode(t *testing.T) {
},
}
for k, v := range errorCases {
t.Run(k, func(t *testing.T) {
errs := ValidateNode(&v)
if len(errs) == 0 {
t.Errorf("expected failure for %s", k)
t.Errorf("expected failure")
}
for i := range errs {
field := errs[i].Field
@ -17215,10 +17222,11 @@ func TestValidateNode(t *testing.T) {
}
if val, ok := expectedFields[field]; ok {
if !val {
t.Errorf("%s: missing prefix for: %v", k, errs[i])
t.Errorf("missing prefix for: %v", errs[i])
}
}
}
})
}
}
@ -22825,10 +22833,14 @@ func makePod(podName string, podNamespace string, podIPs []core.PodIP) core.Pod
}
}
func TestPodIPsValidation(t *testing.T) {
// We test updating every pod in testCases to every other pod in testCases.
// expectError is true if we expect an error when updating *to* that pod.
testCases := []struct {
pod core.Pod
expectError bool
}{{
}{
{
expectError: false,
pod: makePod("nil-ips", "ns", nil),
}, {
@ -22887,10 +22899,10 @@ func TestPodIPsValidation(t *testing.T) {
errs := ValidatePodStatusUpdate(newPod, oldPod, PodValidationOptions{})
if len(errs) == 0 && testCase.expectError {
t.Fatalf("expected failure for %s, but there were none", testCase.pod.Name)
t.Fatalf("expected failure updating from %s, but there were none", oldTestCase.pod.Name)
}
if len(errs) != 0 && !testCase.expectError {
t.Fatalf("expected success for %s, but there were errors: %v", testCase.pod.Name, errs)
t.Fatalf("expected success updating from %s, but there were errors: %v", oldTestCase.pod.Name, errs)
}
}
})
@ -22921,6 +22933,9 @@ func makePodWithHostIPs(podName string, podNamespace string, hostIPs []core.Host
}
func TestHostIPsValidation(t *testing.T) {
// We test updating every pod in testCases to every other pod in testCases.
// expectError is true if we expect an error when updating *to* that pod.
testCases := []struct {
pod core.Pod
expectError bool
@ -22952,7 +22967,7 @@ func TestHostIPsValidation(t *testing.T) {
/* failure cases start here */
{
expectError: true,
pod: makePodWithHostIPs("invalid-pod-ip", "ns", []core.HostIP{{IP: "this-is-not-an-ip"}}),
pod: makePodWithHostIPs("invalid-host-ip", "ns", []core.HostIP{{IP: "this-is-not-an-ip"}}),
},
{
expectError: true,
@ -22994,10 +23009,10 @@ func TestHostIPsValidation(t *testing.T) {
errs := ValidatePodStatusUpdate(newPod, oldPod, PodValidationOptions{})
if len(errs) == 0 && testCase.expectError {
t.Fatalf("expected failure for %s, but there were none", testCase.pod.Name)
t.Fatalf("expected failure updating from %s, but there were none", oldTestCase.pod.Name)
}
if len(errs) != 0 && !testCase.expectError {
t.Fatalf("expected success for %s, but there were errors: %v", testCase.pod.Name, errs)
t.Fatalf("expected success updating from %s, but there were errors: %v", oldTestCase.pod.Name, errs)
}
}
})

View File

@ -249,10 +249,12 @@ func TestValidateNetworkPolicy(t *testing.T) {
// Success cases are expected to pass validation.
for k, v := range successCases {
for _, v := range successCases {
t.Run("", func(t *testing.T) {
if errs := ValidateNetworkPolicy(v, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) != 0 {
t.Errorf("Expected success for the success validation test number %d, got %v", k, errs)
t.Errorf("Expected success, got %v", errs)
}
})
}
invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"}
@ -367,9 +369,11 @@ func TestValidateNetworkPolicy(t *testing.T) {
// Error cases are not expected to pass validation.
for testName, networkPolicy := range errorCases {
t.Run(testName, func(t *testing.T) {
if errs := ValidateNetworkPolicy(networkPolicy, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) == 0 {
t.Errorf("Expected failure for test: %s", testName)
t.Errorf("Expected failure")
}
})
}
}
@ -420,11 +424,13 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
}
for testName, successCase := range successCases {
t.Run(testName, func(t *testing.T) {
successCase.old.ObjectMeta.ResourceVersion = "1"
successCase.update.ObjectMeta.ResourceVersion = "1"
if errs := ValidateNetworkPolicyUpdate(&successCase.update, &successCase.old, NetworkPolicyValidationOptions{false}); len(errs) != 0 {
t.Errorf("expected success (%s): %v", testName, errs)
t.Errorf("expected success, but got %v", errs)
}
})
}
errorCases := map[string]npUpdateTest{
@ -447,11 +453,13 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
}
for testName, errorCase := range errorCases {
t.Run(testName, func(t *testing.T) {
errorCase.old.ObjectMeta.ResourceVersion = "1"
errorCase.update.ObjectMeta.ResourceVersion = "1"
if errs := ValidateNetworkPolicyUpdate(&errorCase.update, &errorCase.old, NetworkPolicyValidationOptions{false}); len(errs) == 0 {
t.Errorf("expected failure: %s", testName)
t.Errorf("expected failure")
}
})
}
}
@ -1824,9 +1832,10 @@ func TestValidateIngressStatusUpdate(t *testing.T) {
"status.loadBalancer.ingress[0].hostname: Invalid value": invalidHostname,
}
for k, v := range errorCases {
t.Run(k, func(t *testing.T) {
errs := ValidateIngressStatusUpdate(&v, &oldValue)
if len(errs) == 0 {
t.Errorf("expected failure for %s", k)
t.Errorf("expected failure")
} else {
s := strings.Split(k, ":")
err := errs[0]
@ -1834,6 +1843,7 @@ func TestValidateIngressStatusUpdate(t *testing.T) {
t.Errorf("unexpected error: %q, expected: %q", err, k)
}
}
})
}
}