Skip to content

Commit

Permalink
prefer NoError/Error over Nil/NotNil
Browse files Browse the repository at this point in the history
  • Loading branch information
trashhalo committed Sep 4, 2020
1 parent 1a04aac commit 203679c
Show file tree
Hide file tree
Showing 36 changed files with 113 additions and 113 deletions.
6 changes: 3 additions & 3 deletions pkg/controller/endpointslice/endpointslice_controller_test.go
Expand Up @@ -98,7 +98,7 @@ func TestSyncServiceNoSelector(t *testing.T) {
})

err := esController.syncService(fmt.Sprintf("%s/%s", ns, serviceName))
assert.Nil(t, err)
assert.NoError(t, err)
assert.Len(t, client.Actions(), 0)
}

Expand All @@ -117,7 +117,7 @@ func TestSyncServicePendingDeletion(t *testing.T) {
})

err := esController.syncService(fmt.Sprintf("%s/%s", ns, serviceName))
assert.Nil(t, err)
assert.NoError(t, err)
assert.Len(t, client.Actions(), 0)
}

Expand Down Expand Up @@ -366,7 +366,7 @@ func TestSyncServiceFull(t *testing.T) {

// run through full sync service loop
err = esController.syncService(fmt.Sprintf("%s/%s", namespace, serviceName))
assert.Nil(t, err)
assert.NoError(t, err)

// last action should be to create endpoint slice
expectActions(t, client.Actions(), 1, "create", "endpointslices")
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/cm/container_manager_linux_test.go
Expand Up @@ -59,7 +59,7 @@ func fakeContainerMgrMountInt() mount.Interface {

func TestCgroupMountValidationSuccess(t *testing.T) {
f, err := validateSystemRequirements(fakeContainerMgrMountInt())
assert.Nil(t, err)
assert.NoError(t, err)
if cgroups.IsCgroup2UnifiedMode() {
assert.True(t, f.cpuHardcapping, "cpu hardcapping is expected to be enabled")
} else {
Expand Down Expand Up @@ -116,7 +116,7 @@ func TestCgroupMountValidationMultipleSubsystem(t *testing.T) {
},
})
_, err := validateSystemRequirements(mountInt)
assert.Nil(t, err)
assert.NoError(t, err)
}

func TestGetCpuWeight(t *testing.T) {
Expand Down
20 changes: 10 additions & 10 deletions pkg/kubelet/dockershim/docker_container_windows_test.go
Expand Up @@ -91,7 +91,7 @@ func TestApplyGMSAConfig(t *testing.T) {
cleanupInfo := &containerCleanupInfo{}
err := applyGMSAConfig(containerConfigWithGMSAAnnotation, createConfig, cleanupInfo)

assert.Nil(t, err)
assert.NoError(t, err)

// the registry key should have been properly created
if assert.Equal(t, 1, len(key.setStringValueArgs)) {
Expand All @@ -114,7 +114,7 @@ func TestApplyGMSAConfig(t *testing.T) {
cleanupInfo := &containerCleanupInfo{}
err := applyGMSAConfig(containerConfigWithGMSAAnnotation, createConfig, cleanupInfo)

assert.Nil(t, err)
assert.NoError(t, err)

if assert.NotNil(t, createConfig.HostConfig) && assert.Equal(t, 1, len(createConfig.HostConfig.SecurityOpt)) {
secOpt := createConfig.HostConfig.SecurityOpt[0]
Expand All @@ -135,15 +135,15 @@ func TestApplyGMSAConfig(t *testing.T) {

err := applyGMSAConfig(containerConfigWithGMSAAnnotation, &dockertypes.ContainerCreateConfig{}, &containerCleanupInfo{})

require.NotNil(t, err)
require.Error(t, err)
assert.Contains(t, err.Error(), "error when generating gMSA registry value name: unable to generate random string")
})
t.Run("if there's an error opening the registry key", func(t *testing.T) {
defer setRegistryCreateKeyFunc(t, &dummyRegistryKey{}, fmt.Errorf("dummy error"))()

err := applyGMSAConfig(containerConfigWithGMSAAnnotation, &dockertypes.ContainerCreateConfig{}, &containerCleanupInfo{})

require.NotNil(t, err)
require.Error(t, err)
assert.Contains(t, err.Error(), "unable to open registry key")
})
t.Run("if there's an error writing to the registry key", func(t *testing.T) {
Expand All @@ -153,7 +153,7 @@ func TestApplyGMSAConfig(t *testing.T) {

err := applyGMSAConfig(containerConfigWithGMSAAnnotation, &dockertypes.ContainerCreateConfig{}, &containerCleanupInfo{})

if assert.NotNil(t, err) {
if assert.Error(t, err) {
assert.Contains(t, err.Error(), "unable to write into registry value")
}
assert.True(t, key.closed)
Expand All @@ -163,7 +163,7 @@ func TestApplyGMSAConfig(t *testing.T) {

err := applyGMSAConfig(&runtimeapi.ContainerConfig{}, createConfig, &containerCleanupInfo{})

assert.Nil(t, err)
assert.NoError(t, err)
assert.Nil(t, createConfig.HostConfig)
})
}
Expand All @@ -178,7 +178,7 @@ func TestRemoveGMSARegistryValue(t *testing.T) {

err := removeGMSARegistryValue(cleanupInfoWithValue)

assert.Nil(t, err)
assert.NoError(t, err)

// the registry key should have been properly deleted
if assert.Equal(t, 1, len(key.deleteValueArgs)) {
Expand All @@ -191,7 +191,7 @@ func TestRemoveGMSARegistryValue(t *testing.T) {

err := removeGMSARegistryValue(cleanupInfoWithValue)

require.NotNil(t, err)
require.Error(t, err)
assert.Contains(t, err.Error(), "unable to open registry key")
})
t.Run("if there's an error deleting from the registry key", func(t *testing.T) {
Expand All @@ -201,7 +201,7 @@ func TestRemoveGMSARegistryValue(t *testing.T) {

err := removeGMSARegistryValue(cleanupInfoWithValue)

if assert.NotNil(t, err) {
if assert.Error(t, err) {
assert.Contains(t, err.Error(), "unable to remove registry value")
}
assert.True(t, key.closed)
Expand All @@ -212,7 +212,7 @@ func TestRemoveGMSARegistryValue(t *testing.T) {

err := removeGMSARegistryValue(&containerCleanupInfo{})

assert.Nil(t, err)
assert.NoError(t, err)
assert.Equal(t, 0, len(key.deleteValueArgs))
})
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubelet/dockershim/security_context_test.go
Expand Up @@ -125,9 +125,9 @@ func TestModifyContainerConfig(t *testing.T) {
dockerCfg := &dockercontainer.Config{}
err := modifyContainerConfig(tc.sc, dockerCfg)
if tc.isErr {
assert.NotNil(t, err)
assert.Error(t, err)
} else {
assert.Nil(t, err)
assert.NoError(t, err)
assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/awsebs/aws_ebs_test.go
Expand Up @@ -411,7 +411,7 @@ func TestGetCandidateZone(t *testing.T) {

for _, test := range tests {
zones, err := getCandidateZones(test.cloud, test.node)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Equal(t, test.expectedZones, zones)
}
}
Expand Down Expand Up @@ -444,7 +444,7 @@ func TestFormatVolumeID(t *testing.T) {
}
for _, test := range tests {
volumeID, err := formatVolumeID(test.volumeIDFromPath)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Equal(t, test.expectedVolumeID, volumeID, test.volumeIDFromPath)
}
}
Expand Up @@ -112,7 +112,7 @@ func TestInternalVersionIsHandlerVersion(t *testing.T) {
i++

_, err := noxuNamespacedResourceClientV1beta2.Patch(context.TODO(), "foo", types.MergePatchType, patch, metav1.PatchOptions{})
assert.NotNil(t, err)
assert.Error(t, err)

// work around "grpc: the client connection is closing" error
// TODO: fix the grpc error
Expand Down
Expand Up @@ -80,47 +80,47 @@ func TestNestedFieldNoCopy(t *testing.T) {
// case 1: field exists and is non-nil
res, exists, err := NestedFieldNoCopy(obj, "a", "b")
assert.True(t, exists)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Equal(t, target, res)
target["foo"] = "baz"
assert.Equal(t, target["foo"], res.(map[string]interface{})["foo"], "result should be a reference to the expected item")

// case 2: field exists and is nil
res, exists, err = NestedFieldNoCopy(obj, "a", "c")
assert.True(t, exists)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Nil(t, res)

// case 3: error traversing obj
res, exists, err = NestedFieldNoCopy(obj, "a", "d", "foo")
assert.False(t, exists)
assert.NotNil(t, err)
assert.Error(t, err)
assert.Nil(t, res)

// case 4: field does not exist
res, exists, err = NestedFieldNoCopy(obj, "a", "g")
assert.False(t, exists)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Nil(t, res)

// case 5: intermediate field does not exist
res, exists, err = NestedFieldNoCopy(obj, "a", "g", "f")
assert.False(t, exists)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Nil(t, res)

// case 6: intermediate field is null
// (background: happens easily in YAML)
res, exists, err = NestedFieldNoCopy(obj, "a", "c", "f")
assert.False(t, exists)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Nil(t, res)

// case 7: array/slice syntax is not supported
// (background: users may expect this to be supported)
res, exists, err = NestedFieldNoCopy(obj, "a", "e[0]")
assert.False(t, exists)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Nil(t, res)
}

Expand All @@ -138,27 +138,27 @@ func TestNestedFieldCopy(t *testing.T) {
// case 1: field exists and is non-nil
res, exists, err := NestedFieldCopy(obj, "a", "b")
assert.True(t, exists)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Equal(t, target, res)
target["foo"] = "baz"
assert.NotEqual(t, target["foo"], res.(map[string]interface{})["foo"], "result should be a copy of the expected item")

// case 2: field exists and is nil
res, exists, err = NestedFieldCopy(obj, "a", "c")
assert.True(t, exists)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Nil(t, res)

// case 3: error traversing obj
res, exists, err = NestedFieldCopy(obj, "a", "d", "foo")
assert.False(t, exists)
assert.NotNil(t, err)
assert.Error(t, err)
assert.Nil(t, res)

// case 4: field does not exist
res, exists, err = NestedFieldCopy(obj, "a", "e")
assert.False(t, exists)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Nil(t, res)
}

Expand Down
2 changes: 1 addition & 1 deletion staging/src/k8s.io/apimachinery/pkg/util/net/http_test.go
Expand Up @@ -428,7 +428,7 @@ func TestConnectWithRedirects(t *testing.T) {
require.NoError(t, err, "unexpected request error")

result, err := ioutil.ReadAll(resp.Body)
assert.Nil(t, err)
assert.NoError(t, err)
require.NoError(t, resp.Body.Close())
if test.expectedRedirects < len(test.redirects) {
// Expect the last redirect to be returned.
Expand Down
6 changes: 3 additions & 3 deletions staging/src/k8s.io/apiserver/pkg/audit/policy/enforce_test.go
Expand Up @@ -82,7 +82,7 @@ func TestEnforcePolicy(t *testing.T) {
objectFuzzer.Fuzz(e)
ev, err := EnforcePolicy(e, tc.level, tc.omitStages)
if omitSet.Has(string(e.Stage)) {
require.Nil(t, err)
require.NoError(t, err)
require.Nil(t, ev)
return
}
Expand Down Expand Up @@ -136,10 +136,10 @@ func TestEnforcePolicy(t *testing.T) {
expected.Level = tc.level
require.Equal(t, expected, ev)
default:
require.NotNil(t, err)
require.Error(t, err)
return
}
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down
10 changes: 5 additions & 5 deletions staging/src/k8s.io/client-go/testing/fixture_test.go
Expand Up @@ -283,7 +283,7 @@ func TestGetWithExactMatch(t *testing.T) {
constructObject := func(s schema.GroupVersionResource, name, namespace string) (*unstructured.Unstructured, schema.GroupVersionResource) {
obj := getArbitraryResource(s, name, namespace)
gvks, _, err := scheme.ObjectKinds(obj)
assert.Nil(t, err)
assert.NoError(t, err)
gvr, _ := meta.UnsafeGuessKindToResource(gvks[0])
return obj, gvr
}
Expand All @@ -298,11 +298,11 @@ func TestGetWithExactMatch(t *testing.T) {

// Exact match
_, err = o.Get(gvr, "", "node")
assert.Nil(t, err)
assert.NoError(t, err)

// Unexpected namespace provided
_, err = o.Get(gvr, "ns", "node")
assert.NotNil(t, err)
assert.Error(t, err)
errNotFound := errors.NewNotFound(gvr.GroupResource(), "node")
assert.EqualError(t, err, errNotFound.Error())

Expand All @@ -314,11 +314,11 @@ func TestGetWithExactMatch(t *testing.T) {

// Exact match
_, err = o.Get(gvr, "default", "pod")
assert.Nil(t, err)
assert.NoError(t, err)

// Missing namespace
_, err = o.Get(gvr, "", "pod")
assert.NotNil(t, err)
assert.Error(t, err)
errNotFound = errors.NewNotFound(gvr.GroupResource(), "pod")
assert.EqualError(t, err, errNotFound.Error())
}
6 changes: 3 additions & 3 deletions staging/src/k8s.io/legacy-cloud-providers/aws/aws_test.go
Expand Up @@ -1764,7 +1764,7 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) {

err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, test.annotations)

require.Nil(t, err)
require.NoError(t, err)
awsServices.elb.(*MockedFakeELB).AssertExpectations(t)
})
}
Expand All @@ -1784,11 +1784,11 @@ func TestEnsureLoadBalancerHealthCheck(t *testing.T) {
// test default HC
elbDesc := &elb.LoadBalancerDescription{LoadBalancerName: &lbName, HealthCheck: defaultHC}
err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, map[string]string{})
assert.Nil(t, err)
assert.NoError(t, err)
// test HC with override
elbDesc = &elb.LoadBalancerDescription{LoadBalancerName: &lbName, HealthCheck: &currentHC}
err = c.ensureLoadBalancerHealthCheck(elbDesc, protocol, port, path, annotations)
assert.Nil(t, err)
assert.NoError(t, err)
})

t.Run("validates resulting expected health check before making an API call", func(t *testing.T) {
Expand Down
Expand Up @@ -221,7 +221,7 @@ func TestCommonAttachDiskWithVMSS(t *testing.T) {
testCloud.DisableAvailabilitySetNodes = true
}
ss, err := newScaleSet(testCloud)
assert.Nil(t, err)
assert.NoError(t, err)
testCloud.VMSet = ss
}

Expand Down
Expand Up @@ -729,5 +729,5 @@ func TestCurrentNodeName(t *testing.T) {
hostname := "testvm"
nodeName, err := cloud.CurrentNodeName(context.Background(), hostname)
assert.Equal(t, types.NodeName(hostname), nodeName)
assert.Nil(t, err)
assert.NoError(t, err)
}
Expand Up @@ -1194,7 +1194,7 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
} else {
assert.Equal(t, test.expectedProbes, probe, "TestCase[%d]: %s", i, test.desc)
assert.Equal(t, test.expectedRules, lbrule, "TestCase[%d]: %s", i, test.desc)
assert.Nil(t, err)
assert.NoError(t, err)
}
}
}
Expand Down
Expand Up @@ -1257,7 +1257,7 @@ func TestReconcileSecurityGroupEtagMismatch(t *testing.T) {

newSG, err := az.reconcileSecurityGroup(testClusterName, &svc1, &lbStatus.Ingress[0].IP, true /* wantLb */)
assert.Nil(t, newSG)
assert.NotNil(t, err)
assert.Error(t, err)

assert.Equal(t, expectedError.Error(), err)
}
Expand Down
Expand Up @@ -102,7 +102,7 @@ func TestVMSSVMCache(t *testing.T) {
vm := expectedVMs[i]
vmName := to.String(vm.OsProfile.ComputerName)
ssName, instanceID, realVM, err := ss.getVmssVM(vmName, azcache.CacheReadTypeDefault)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Equal(t, "vmss", ssName)
assert.Equal(t, to.String(vm.InstanceID), instanceID)
assert.Equal(t, &vm, realVM)
Expand Down

0 comments on commit 203679c

Please sign in to comment.