Skip to content

Commit f79bccf

Browse files
danwinshipdanwinship
committed
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).
1 parent 76f1684 commit f79bccf

File tree

2 files changed

+119
-94
lines changed

2 files changed

+119
-94
lines changed

‎pkg/apis/core/validation/validation_test.go‎

Lines changed: 83 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -9951,16 +9951,18 @@ func TestValidatePodDNSConfig(t *testing.T) {
99519951
}
99529952

99539953
for _, tc := range testCases {
9954-
if tc.dnsPolicy == nil {
9955-
tc.dnsPolicy = &testDNSClusterFirst
9956-
}
9954+
t.Run("", func(t *testing.T) {
9955+
if tc.dnsPolicy == nil {
9956+
tc.dnsPolicy = &testDNSClusterFirst
9957+
}
99579958

9958-
errs := validatePodDNSConfig(tc.dnsConfig, tc.dnsPolicy, field.NewPath("dnsConfig"), tc.opts)
9959-
if len(errs) != 0 && !tc.expectedError {
9960-
t.Errorf("%v: validatePodDNSConfig(%v) = %v, want nil", tc.desc, tc.dnsConfig, errs)
9961-
} else if len(errs) == 0 && tc.expectedError {
9962-
t.Errorf("%v: validatePodDNSConfig(%v) = nil, want error", tc.desc, tc.dnsConfig)
9963-
}
9959+
errs := validatePodDNSConfig(tc.dnsConfig, tc.dnsPolicy, field.NewPath("dnsConfig"), tc.opts)
9960+
if len(errs) != 0 && !tc.expectedError {
9961+
t.Errorf("%v: validatePodDNSConfig(%v) = %v, want nil", tc.desc, tc.dnsConfig, errs)
9962+
} else if len(errs) == 0 && tc.expectedError {
9963+
t.Errorf("%v: validatePodDNSConfig(%v) = nil, want error", tc.desc, tc.dnsConfig)
9964+
}
9965+
})
99649966
}
99659967
}
99669968

@@ -10304,12 +10306,14 @@ func TestValidatePodSpec(t *testing.T) {
1030410306
),
1030510307
}
1030610308
for k, v := range failureCases {
10307-
opts := PodValidationOptions{
10308-
ResourceIsPod: true,
10309-
}
10310-
if errs := ValidatePodSpec(&v.Spec, nil, field.NewPath("field"), opts); len(errs) == 0 {
10311-
t.Errorf("expected failure for %q", k)
10312-
}
10309+
t.Run(k, func(t *testing.T) {
10310+
opts := PodValidationOptions{
10311+
ResourceIsPod: true,
10312+
}
10313+
if errs := ValidatePodSpec(&v.Spec, nil, field.NewPath("field"), opts); len(errs) == 0 {
10314+
t.Errorf("expected failure")
10315+
}
10316+
})
1031310317
}
1031410318
}
1031510319

@@ -15302,35 +15306,35 @@ func TestValidateServiceCreate(t *testing.T) {
1530215306
// numErrs: 1,
1530315307
numErrs: 0,
1530415308
}, {
15305-
name: "invalid publicIPs localhost",
15309+
name: "invalid externalIPs localhost",
1530615310
tweakSvc: func(s *core.Service) {
1530715311
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
1530815312
s.Spec.ExternalIPs = []string{"127.0.0.1"}
1530915313
},
1531015314
numErrs: 1,
1531115315
}, {
15312-
name: "invalid publicIPs unspecified",
15316+
name: "invalid externalIPs unspecified",
1531315317
tweakSvc: func(s *core.Service) {
1531415318
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
1531515319
s.Spec.ExternalIPs = []string{"0.0.0.0"}
1531615320
},
1531715321
numErrs: 1,
1531815322
}, {
15319-
name: "invalid publicIPs loopback",
15323+
name: "invalid externalIPs loopback",
1532015324
tweakSvc: func(s *core.Service) {
1532115325
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
1532215326
s.Spec.ExternalIPs = []string{"127.0.0.1"}
1532315327
},
1532415328
numErrs: 1,
1532515329
}, {
15326-
name: "invalid publicIPs host",
15330+
name: "invalid externalIPs host",
1532715331
tweakSvc: func(s *core.Service) {
1532815332
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
1532915333
s.Spec.ExternalIPs = []string{"myhost.mydomain"}
1533015334
},
1533115335
numErrs: 1,
1533215336
}, {
15333-
name: "valid publicIPs",
15337+
name: "valid externalIPs",
1533415338
tweakSvc: func(s *core.Service) {
1533515339
s.Spec.ExternalTrafficPolicy = core.ServiceExternalTrafficPolicyCluster
1533615340
s.Spec.ExternalIPs = []string{"1.2.3.4"}
@@ -16989,9 +16993,11 @@ func TestValidateNode(t *testing.T) {
1698916993
},
1699016994
}
1699116995
for _, successCase := range successCases {
16992-
if errs := ValidateNode(&successCase); len(errs) != 0 {
16993-
t.Errorf("expected success: %v", errs)
16994-
}
16996+
t.Run("", func(t *testing.T) {
16997+
if errs := ValidateNode(&successCase); len(errs) != 0 {
16998+
t.Errorf("expected success: %v", errs)
16999+
}
17000+
})
1699517001
}
1699617002

1699717003
errorCases := map[string]core.Node{
@@ -17195,30 +17201,32 @@ func TestValidateNode(t *testing.T) {
1719517201
},
1719617202
}
1719717203
for k, v := range errorCases {
17198-
errs := ValidateNode(&v)
17199-
if len(errs) == 0 {
17200-
t.Errorf("expected failure for %s", k)
17201-
}
17202-
for i := range errs {
17203-
field := errs[i].Field
17204-
expectedFields := map[string]bool{
17205-
"metadata.name": true,
17206-
"metadata.labels": true,
17207-
"metadata.annotations": true,
17208-
"metadata.namespace": true,
17209-
"spec.externalID": true,
17210-
"spec.taints[0].key": true,
17211-
"spec.taints[0].value": true,
17212-
"spec.taints[0].effect": true,
17213-
"metadata.annotations.scheduler.alpha.kubernetes.io/preferAvoidPods[0].PodSignature": true,
17214-
"metadata.annotations.scheduler.alpha.kubernetes.io/preferAvoidPods[0].PodSignature.PodController.Controller": true,
17204+
t.Run(k, func(t *testing.T) {
17205+
errs := ValidateNode(&v)
17206+
if len(errs) == 0 {
17207+
t.Errorf("expected failure")
1721517208
}
17216-
if val, ok := expectedFields[field]; ok {
17217-
if !val {
17218-
t.Errorf("%s: missing prefix for: %v", k, errs[i])
17209+
for i := range errs {
17210+
field := errs[i].Field
17211+
expectedFields := map[string]bool{
17212+
"metadata.name": true,
17213+
"metadata.labels": true,
17214+
"metadata.annotations": true,
17215+
"metadata.namespace": true,
17216+
"spec.externalID": true,
17217+
"spec.taints[0].key": true,
17218+
"spec.taints[0].value": true,
17219+
"spec.taints[0].effect": true,
17220+
"metadata.annotations.scheduler.alpha.kubernetes.io/preferAvoidPods[0].PodSignature": true,
17221+
"metadata.annotations.scheduler.alpha.kubernetes.io/preferAvoidPods[0].PodSignature.PodController.Controller": true,
17222+
}
17223+
if val, ok := expectedFields[field]; ok {
17224+
if !val {
17225+
t.Errorf("missing prefix for: %v", errs[i])
17226+
}
1721917227
}
1722017228
}
17221-
}
17229+
})
1722217230
}
1722317231
}
1722417232

@@ -22825,28 +22833,32 @@ func makePod(podName string, podNamespace string, podIPs []core.PodIP) core.Pod
2282522833
}
2282622834
}
2282722835
func TestPodIPsValidation(t *testing.T) {
22836+
// We test updating every pod in testCases to every other pod in testCases.
22837+
// expectError is true if we expect an error when updating *to* that pod.
22838+
2282822839
testCases := []struct {
2282922840
pod core.Pod
2283022841
expectError bool
22831-
}{{
22832-
expectError: false,
22833-
pod: makePod("nil-ips", "ns", nil),
22834-
}, {
22835-
expectError: false,
22836-
pod: makePod("empty-podips-list", "ns", []core.PodIP{}),
22837-
}, {
22838-
expectError: false,
22839-
pod: makePod("single-ip-family-6", "ns", []core.PodIP{{IP: "::1"}}),
22840-
}, {
22841-
expectError: false,
22842-
pod: makePod("single-ip-family-4", "ns", []core.PodIP{{IP: "1.1.1.1"}}),
22843-
}, {
22844-
expectError: false,
22845-
pod: makePod("dual-stack-4-6", "ns", []core.PodIP{{IP: "1.1.1.1"}, {IP: "::1"}}),
22846-
}, {
22847-
expectError: false,
22848-
pod: makePod("dual-stack-6-4", "ns", []core.PodIP{{IP: "::1"}, {IP: "1.1.1.1"}}),
22849-
},
22842+
}{
22843+
{
22844+
expectError: false,
22845+
pod: makePod("nil-ips", "ns", nil),
22846+
}, {
22847+
expectError: false,
22848+
pod: makePod("empty-podips-list", "ns", []core.PodIP{}),
22849+
}, {
22850+
expectError: false,
22851+
pod: makePod("single-ip-family-6", "ns", []core.PodIP{{IP: "::1"}}),
22852+
}, {
22853+
expectError: false,
22854+
pod: makePod("single-ip-family-4", "ns", []core.PodIP{{IP: "1.1.1.1"}}),
22855+
}, {
22856+
expectError: false,
22857+
pod: makePod("dual-stack-4-6", "ns", []core.PodIP{{IP: "1.1.1.1"}, {IP: "::1"}}),
22858+
}, {
22859+
expectError: false,
22860+
pod: makePod("dual-stack-6-4", "ns", []core.PodIP{{IP: "::1"}, {IP: "1.1.1.1"}}),
22861+
},
2285022862
/* failure cases start here */
2285122863
{
2285222864
expectError: true,
@@ -22887,10 +22899,10 @@ func TestPodIPsValidation(t *testing.T) {
2288722899
errs := ValidatePodStatusUpdate(newPod, oldPod, PodValidationOptions{})
2288822900

2288922901
if len(errs) == 0 && testCase.expectError {
22890-
t.Fatalf("expected failure for %s, but there were none", testCase.pod.Name)
22902+
t.Fatalf("expected failure updating from %s, but there were none", oldTestCase.pod.Name)
2289122903
}
2289222904
if len(errs) != 0 && !testCase.expectError {
22893-
t.Fatalf("expected success for %s, but there were errors: %v", testCase.pod.Name, errs)
22905+
t.Fatalf("expected success updating from %s, but there were errors: %v", oldTestCase.pod.Name, errs)
2289422906
}
2289522907
}
2289622908
})
@@ -22921,6 +22933,9 @@ func makePodWithHostIPs(podName string, podNamespace string, hostIPs []core.Host
2292122933
}
2292222934

2292322935
func TestHostIPsValidation(t *testing.T) {
22936+
// We test updating every pod in testCases to every other pod in testCases.
22937+
// expectError is true if we expect an error when updating *to* that pod.
22938+
2292422939
testCases := []struct {
2292522940
pod core.Pod
2292622941
expectError bool
@@ -22952,7 +22967,7 @@ func TestHostIPsValidation(t *testing.T) {
2295222967
/* failure cases start here */
2295322968
{
2295422969
expectError: true,
22955-
pod: makePodWithHostIPs("invalid-pod-ip", "ns", []core.HostIP{{IP: "this-is-not-an-ip"}}),
22970+
pod: makePodWithHostIPs("invalid-host-ip", "ns", []core.HostIP{{IP: "this-is-not-an-ip"}}),
2295622971
},
2295722972
{
2295822973
expectError: true,
@@ -22994,10 +23009,10 @@ func TestHostIPsValidation(t *testing.T) {
2299423009
errs := ValidatePodStatusUpdate(newPod, oldPod, PodValidationOptions{})
2299523010

2299623011
if len(errs) == 0 && testCase.expectError {
22997-
t.Fatalf("expected failure for %s, but there were none", testCase.pod.Name)
23012+
t.Fatalf("expected failure updating from %s, but there were none", oldTestCase.pod.Name)
2299823013
}
2299923014
if len(errs) != 0 && !testCase.expectError {
23000-
t.Fatalf("expected success for %s, but there were errors: %v", testCase.pod.Name, errs)
23015+
t.Fatalf("expected success updating from %s, but there were errors: %v", oldTestCase.pod.Name, errs)
2300123016
}
2300223017
}
2300323018
})

‎pkg/apis/networking/validation/validation_test.go‎

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,12 @@ func TestValidateNetworkPolicy(t *testing.T) {
249249

250250
// Success cases are expected to pass validation.
251251

252-
for k, v := range successCases {
253-
if errs := ValidateNetworkPolicy(v, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) != 0 {
254-
t.Errorf("Expected success for the success validation test number %d, got %v", k, errs)
255-
}
252+
for _, v := range successCases {
253+
t.Run("", func(t *testing.T) {
254+
if errs := ValidateNetworkPolicy(v, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) != 0 {
255+
t.Errorf("Expected success, got %v", errs)
256+
}
257+
})
256258
}
257259

258260
invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"}
@@ -367,9 +369,11 @@ func TestValidateNetworkPolicy(t *testing.T) {
367369

368370
// Error cases are not expected to pass validation.
369371
for testName, networkPolicy := range errorCases {
370-
if errs := ValidateNetworkPolicy(networkPolicy, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) == 0 {
371-
t.Errorf("Expected failure for test: %s", testName)
372-
}
372+
t.Run(testName, func(t *testing.T) {
373+
if errs := ValidateNetworkPolicy(networkPolicy, NetworkPolicyValidationOptions{AllowInvalidLabelValueInSelector: true}); len(errs) == 0 {
374+
t.Errorf("Expected failure")
375+
}
376+
})
373377
}
374378
}
375379

@@ -420,11 +424,13 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
420424
}
421425

422426
for testName, successCase := range successCases {
423-
successCase.old.ObjectMeta.ResourceVersion = "1"
424-
successCase.update.ObjectMeta.ResourceVersion = "1"
425-
if errs := ValidateNetworkPolicyUpdate(&successCase.update, &successCase.old, NetworkPolicyValidationOptions{false}); len(errs) != 0 {
426-
t.Errorf("expected success (%s): %v", testName, errs)
427-
}
427+
t.Run(testName, func(t *testing.T) {
428+
successCase.old.ObjectMeta.ResourceVersion = "1"
429+
successCase.update.ObjectMeta.ResourceVersion = "1"
430+
if errs := ValidateNetworkPolicyUpdate(&successCase.update, &successCase.old, NetworkPolicyValidationOptions{false}); len(errs) != 0 {
431+
t.Errorf("expected success, but got %v", errs)
432+
}
433+
})
428434
}
429435

430436
errorCases := map[string]npUpdateTest{
@@ -447,11 +453,13 @@ func TestValidateNetworkPolicyUpdate(t *testing.T) {
447453
}
448454

449455
for testName, errorCase := range errorCases {
450-
errorCase.old.ObjectMeta.ResourceVersion = "1"
451-
errorCase.update.ObjectMeta.ResourceVersion = "1"
452-
if errs := ValidateNetworkPolicyUpdate(&errorCase.update, &errorCase.old, NetworkPolicyValidationOptions{false}); len(errs) == 0 {
453-
t.Errorf("expected failure: %s", testName)
454-
}
456+
t.Run(testName, func(t *testing.T) {
457+
errorCase.old.ObjectMeta.ResourceVersion = "1"
458+
errorCase.update.ObjectMeta.ResourceVersion = "1"
459+
if errs := ValidateNetworkPolicyUpdate(&errorCase.update, &errorCase.old, NetworkPolicyValidationOptions{false}); len(errs) == 0 {
460+
t.Errorf("expected failure")
461+
}
462+
})
455463
}
456464
}
457465

@@ -1824,16 +1832,18 @@ func TestValidateIngressStatusUpdate(t *testing.T) {
18241832
"status.loadBalancer.ingress[0].hostname: Invalid value": invalidHostname,
18251833
}
18261834
for k, v := range errorCases {
1827-
errs := ValidateIngressStatusUpdate(&v, &oldValue)
1828-
if len(errs) == 0 {
1829-
t.Errorf("expected failure for %s", k)
1830-
} else {
1831-
s := strings.Split(k, ":")
1832-
err := errs[0]
1833-
if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
1834-
t.Errorf("unexpected error: %q, expected: %q", err, k)
1835+
t.Run(k, func(t *testing.T) {
1836+
errs := ValidateIngressStatusUpdate(&v, &oldValue)
1837+
if len(errs) == 0 {
1838+
t.Errorf("expected failure")
1839+
} else {
1840+
s := strings.Split(k, ":")
1841+
err := errs[0]
1842+
if err.Field != s[0] || !strings.Contains(err.Error(), s[1]) {
1843+
t.Errorf("unexpected error: %q, expected: %q", err, k)
1844+
}
18351845
}
1836-
}
1846+
})
18371847
}
18381848
}
18391849

0 commit comments

Comments
 (0)