Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 lint: enable ginkgolinter and fix findings #2282

Merged
merged 1 commit into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ linters:
- errchkjson
- exportloopref
- gci
# - ginkgolinter
- ginkgolinter
- goconst
- gocritic
- godot
Expand Down Expand Up @@ -172,6 +172,9 @@ linters-settings:
- name: bool-literal-in-expr
- name: constant-logical-expr

ginkgolinter:
forbid-focus-container: true

chrischdi marked this conversation as resolved.
Show resolved Hide resolved
issues:
max-same-issues: 0
max-issues-per-linter: 0
Expand Down
6 changes: 3 additions & 3 deletions controllers/clustermodule_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func TestReconciler_Reconcile(t *testing.T) {
// if cluster module creation fails due to resource pool owner incompatibility, vSphereCluster object is set to Ready
haveError: false,
customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(0))
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.BeEmpty())
g.Expect(conditions.Has(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
g.Expect(conditions.IsFalse(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition)).To(gomega.BeTrue())
g.Expect(conditions.Get(ctx.VSphereCluster, infrav1.ClusterModulesAvailableCondition).Message).To(gomega.ContainSubstring("kcp"))
Expand Down Expand Up @@ -387,7 +387,7 @@ func TestReconciler_Reconcile(t *testing.T) {
svc.On("Remove", mock.Anything, mdUUID).Return(nil)
},
customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(0))
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.BeEmpty())
},
},
{
Expand All @@ -399,7 +399,7 @@ func TestReconciler_Reconcile(t *testing.T) {
},
clusterModules: []infrav1.ClusterModule{},
customAssert: func(g *gomega.WithT, ctx *context.ClusterContext) {
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.HaveLen(0))
g.Expect(ctx.VSphereCluster.Spec.ClusterModules).To(gomega.BeEmpty())
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions controllers/serviceaccount_controller_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func assertRoleWithGetPVC(ctx *helpers.UnitTestContextForController, ctrlClient
}
err := ctrlClient.List(ctx, &roleList, opts)
Expect(err).ShouldNot(HaveOccurred())
Expect(len(roleList.Items)).To(Equal(1))
Expect(roleList.Items).To(HaveLen(1))
Expect(roleList.Items[0].Name).To(Equal(name))
Expect(roleList.Items[0].Rules).To(Equal([]rbacv1.PolicyRule{
{
Expand All @@ -147,7 +147,7 @@ func assertRoleBinding(_ *helpers.UnitTestContextForController, ctrlClient clien
}
err := ctrlClient.List(goctx.TODO(), &roleBindingList, opts)
Expect(err).ShouldNot(HaveOccurred())
Expect(len(roleBindingList.Items)).To(Equal(1))
Expect(roleBindingList.Items).To(HaveLen(1))
Expect(roleBindingList.Items[0].Name).To(Equal(name))
Expect(roleBindingList.Items[0].RoleRef).To(Equal(rbacv1.RoleRef{
Name: name,
Expand Down
6 changes: 3 additions & 3 deletions controllers/vmware/vspherecluster_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ var _ = Describe("Cluster Controller Tests", func() {

It("should not find FailureDomains", func() {
fds, err := reconciler.getFailureDomains(ctx)
Expect(err).To(BeNil())
Expect(fds).Should(HaveLen(0))
Expect(err).ToNot(HaveOccurred())
Expect(fds).Should(BeEmpty())
})

It("should find FailureDomains", func() {
Expand All @@ -148,7 +148,7 @@ var _ = Describe("Cluster Controller Tests", func() {
}

fds, err := reconciler.getFailureDomains(ctx)
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(fds).NotTo(BeNil())
Expect(fds).Should(HaveLen(3))
})
Expand Down
6 changes: 3 additions & 3 deletions controllers/vspheredeploymentzone_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ func TestVSphereDeploymentZoneReconciler_ReconcileDelete(t *testing.T) {
reconciler := vsphereDeploymentZoneReconciler{controllerCtx}
err := reconciler.reconcileDelete(deploymentZoneCtx)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(vsphereDeploymentZone.Finalizers).To(HaveLen(0))
g.Expect(vsphereDeploymentZone.Finalizers).To(BeEmpty())
})
})

Expand All @@ -667,7 +667,7 @@ func TestVSphereDeploymentZoneReconciler_ReconcileDelete(t *testing.T) {
reconciler := vsphereDeploymentZoneReconciler{controllerCtx}
err := reconciler.reconcileDelete(deploymentZoneCtx)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(vsphereDeploymentZone.Finalizers).To(HaveLen(0))
g.Expect(vsphereDeploymentZone.Finalizers).To(BeEmpty())
})

t.Run("when no machines are present", func(t *testing.T) {
Expand All @@ -684,7 +684,7 @@ func TestVSphereDeploymentZoneReconciler_ReconcileDelete(t *testing.T) {
reconciler := vsphereDeploymentZoneReconciler{controllerCtx}
err := reconciler.reconcileDelete(deploymentZoneCtx)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(vsphereDeploymentZone.Finalizers).To(HaveLen(0))
g.Expect(vsphereDeploymentZone.Finalizers).To(BeEmpty())
})

t.Run("delete failure domain", func(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ password: '%s'

watch, err := InitializeWatch(fake.NewControllerManagerContext(), managerOptsTest)
// Match initial credentials
g.Expect(err).To(BeNil())
g.Expect(err).ToNot(HaveOccurred())
g.Expect(managerOptsTest.Username).To(Equal(username))
g.Expect(managerOptsTest.Password).To(Equal(password))

// Update the file and wait for watch to detect the change
content := fmt.Sprintf(contentFmt, updatedUsername, updatedPassword)
_, err = tmpFile.Write([]byte(content))
g.Expect(err).To(BeNil())
g.Expect(err).ToNot(HaveOccurred())

g.Eventually(func() bool {
return managerOptsTest.Username == updatedUsername && managerOptsTest.Password == updatedPassword
Expand All @@ -89,7 +89,7 @@ password: '%s'
}
watch, err := InitializeWatch(fake.NewControllerManagerContext(), managerOptsTest)
// Match initial credentials
g.Expect(err).To(BeNil())
g.Expect(err).ToNot(HaveOccurred())
g.Expect(managerOptsTest.Username).To(Equal(username))
g.Expect(managerOptsTest.Password).To(Equal(password))

Expand Down Expand Up @@ -124,6 +124,6 @@ password: '%s'
}
_, err = InitializeWatch(fake.NewControllerManagerContext(), managerOptsTest)
// Match initial credentials
g.Expect(err).NotTo(BeNil())
g.Expect(err).To(HaveOccurred())
})
}
10 changes: 5 additions & 5 deletions pkg/record/recorder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,21 @@ var _ = Describe("Event utils", func() {
It("should not publish an event", func() {
var err error
recorder.EmitEvent(nil, "Create", err, true)
Expect(len(fakeRecorder.Events)).Should(Equal(0))
Expect(fakeRecorder.Events).Should(BeEmpty())
})

It("should publish a success event", func() {
var err error
recorder.EmitEvent(nil, "Create", err, false)
Expect(len(fakeRecorder.Events)).Should(Equal(1))
Expect(fakeRecorder.Events).Should(HaveLen(1))
event := <-fakeRecorder.Events
Expect(event).Should(Equal("Normal CreateSuccess Create success"))
})

It("should publish a failure event", func() {
err := errors.New("something wrong")
recorder.EmitEvent(nil, "Create", err, false)
Expect(len(fakeRecorder.Events)).Should(Equal(1))
Expect(fakeRecorder.Events).Should(HaveLen(1))
event := <-fakeRecorder.Events
Expect(event).Should(Equal("Warning CreateFailure something wrong"))
})
Expand All @@ -60,13 +60,13 @@ var _ = Describe("Event utils", func() {

recorder.Eventf(nil, "Create", message, fmtArgs...)
recorder.Warnf(nil, "Create", message, fmtArgs...)
Expect(len(fakeRecorder.Events)).To(Equal(2))
Expect(fakeRecorder.Events).To(HaveLen(2))
eventFmt := <-fakeRecorder.Events
warnFmt := <-fakeRecorder.Events

recorder.Event(nil, "Create", message)
recorder.Warn(nil, "Create", message)
Expect(len(fakeRecorder.Events)).To(Equal(2))
Expect(fakeRecorder.Events).To(HaveLen(2))
eventNoFmt := <-fakeRecorder.Events
warnNoFmt := <-fakeRecorder.Events

Expand Down
2 changes: 1 addition & 1 deletion pkg/services/govmomi/cluster/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,5 @@ func TestListHostsFromGroup(t *testing.T) {

refs, err = ListHostsFromGroup(context.Background(), ccr, "blah")
g.Expect(err).NotTo(HaveOccurred())
g.Expect(refs).To(HaveLen(0))
g.Expect(refs).To(BeEmpty())
}
2 changes: 1 addition & 1 deletion pkg/services/govmomi/extra/extra_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func ConfigInitFnTester(method ConfigInitFn, methodName string, dataKey string,
method(&config, sampleData)

It("must set 2 keys in the config", func() {
Expect(len(config)).To(Equal(2))
Expect(config).To(HaveLen(2))
})

It(fmt.Sprintf("must set data as a base64 encoded string with the key %q", dataKey), func() {
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/govmomi/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func Test_ShouldRetryTask(t *testing.T) {
reconciled, err := checkAndRetryTask(vmCtx, &task)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(reconciled).To(BeTrue())
g.Expect(conditions.IsFalse(vmCtx.VSphereVM, infrav1.VMProvisionedCondition))
g.Expect(conditions.IsFalse(vmCtx.VSphereVM, infrav1.VMProvisionedCondition)).To(BeTrue())
g.Expect(vmCtx.VSphereVM.Status.TaskRef).To(BeEmpty())
g.Expect(vmCtx.VSphereVM.Status.RetryAfter.IsZero()).To(BeTrue())
})
Expand All @@ -123,7 +123,7 @@ func Test_ShouldRetryTask(t *testing.T) {
reconciled, err := checkAndRetryTask(vmCtx, &task)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(reconciled).To(BeTrue())
g.Expect(conditions.IsFalse(vmCtx.VSphereVM, infrav1.VMProvisionedCondition))
g.Expect(conditions.IsFalse(vmCtx.VSphereVM, infrav1.VMProvisionedCondition)).To(BeTrue())
g.Expect(vmCtx.VSphereVM.Status.RetryAfter.Unix()).To(BeNumerically("<=", metav1.Now().Add(1*time.Minute).Unix()))
})
}
Expand Down
38 changes: 19 additions & 19 deletions pkg/services/network/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ var _ = Describe("Network provider", func() {
np = DummyNetworkProvider()
})
It("should not add network interface", func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(vm.Spec.NetworkInterfaces).To(BeNil())
})
})
Expand Down Expand Up @@ -151,7 +151,7 @@ var _ = Describe("Network provider", func() {
})

AfterEach(func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(vm.Spec.NetworkInterfaces).To(HaveLen(1))
Expect(vm.Spec.NetworkInterfaces[0].NetworkType).To(Equal("vsphere-distributed"))
})
Expand Down Expand Up @@ -188,7 +188,7 @@ var _ = Describe("Network provider", func() {
err = np.ConfigureVirtualMachine(ctx, vm)
})
AfterEach(func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(vm.Spec.NetworkInterfaces[0].NetworkName).To(Equal(GetNSXTVirtualNetworkName(vSphereCluster.Name)))
Expect(vm.Spec.NetworkInterfaces[0].NetworkType).To(Equal("nsx-t"))
})
Expand Down Expand Up @@ -264,9 +264,9 @@ var _ = Describe("Network provider", func() {
err = np.ProvisionClusterNetwork(ctx)
})
It("should succeed", func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
vnet, localerr := np.GetClusterNetworkName(ctx)
Expect(localerr).To(BeNil())
Expect(localerr).ToNot(HaveOccurred())
Expect(vnet).To(BeEmpty())
})
})
Expand All @@ -283,7 +283,7 @@ var _ = Describe("Network provider", func() {
err = np.ProvisionClusterNetwork(ctx)
})
It("should succeed", func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(conditions.IsTrue(ctx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition)).To(BeTrue())
})
})
Expand All @@ -297,9 +297,9 @@ var _ = Describe("Network provider", func() {
})

It("should not update vnet with whitelist_source_ranges in spec", func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
vnet, localerr := np.GetClusterNetworkName(ctx)
Expect(localerr).To(BeNil())
Expect(localerr).ToNot(HaveOccurred())
Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(ctx.VSphereCluster.Name)))

createdVNET := &ncpv1.VirtualNetwork{}
Expand All @@ -308,7 +308,7 @@ var _ = Describe("Network provider", func() {
Namespace: dummyNs,
}, createdVNET)

Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(createdVNET.Spec.WhitelistSourceRanges).To(BeEmpty())
})

Expand All @@ -332,9 +332,9 @@ var _ = Describe("Network provider", func() {
})

It("should create vnet without whitelist_source_ranges in spec", func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
vnet, localerr := np.GetClusterNetworkName(ctx)
Expect(localerr).To(BeNil())
Expect(localerr).ToNot(HaveOccurred())
Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(ctx.VSphereCluster.Name)))

createdVNET := &ncpv1.VirtualNetwork{}
Expand All @@ -343,7 +343,7 @@ var _ = Describe("Network provider", func() {
Namespace: dummyNs,
}, createdVNET)

Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(createdVNET.Spec.WhitelistSourceRanges).To(BeEmpty())
Expect(conditions.IsTrue(ctx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition)).To(BeTrue())
})
Expand All @@ -358,9 +358,9 @@ var _ = Describe("Network provider", func() {
})

It("should update vnet with whitelist_source_ranges in spec", func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
vnet, localerr := np.GetClusterNetworkName(ctx)
Expect(localerr).To(BeNil())
Expect(localerr).ToNot(HaveOccurred())
Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(ctx.VSphereCluster.Name)))

// Verify WhitelistSourceRanges have been updated
Expand All @@ -370,7 +370,7 @@ var _ = Describe("Network provider", func() {
Namespace: dummyNs,
}, createdVNET)

Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
Expect(createdVNET.Spec.WhitelistSourceRanges).To(Equal(fakeSNATIP + "/32"))
Expect(conditions.IsTrue(ctx.VSphereCluster, vmwarev1.ClusterNetworkReadyCondition)).To(BeTrue())
})
Expand All @@ -386,9 +386,9 @@ var _ = Describe("Network provider", func() {
})

It("should create new vnet with whitelist_source_ranges in spec", func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
vnet, localerr := np.GetClusterNetworkName(ctx)
Expect(localerr).To(BeNil())
Expect(localerr).ToNot(HaveOccurred())
Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(ctx.VSphereCluster.Name)))

// Verify WhitelistSourceRanges have been updated
Expand Down Expand Up @@ -416,9 +416,9 @@ var _ = Describe("Network provider", func() {
})

It("should not update vnet with whitelist_source_ranges in spec", func() {
Expect(err).To(BeNil())
Expect(err).ToNot(HaveOccurred())
vnet, localerr := np.GetClusterNetworkName(ctx)
Expect(localerr).To(BeNil())
Expect(localerr).ToNot(HaveOccurred())
Expect(vnet).To(Equal(GetNSXTVirtualNetworkName(ctx.VSphereCluster.Name)))

// Verify WhitelistSourceRanges is not included
Expand Down
6 changes: 3 additions & 3 deletions pkg/services/vmoperator/control_plane_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func getVirtualMachineService(_ CPService, ctx *vmware.ClusterContext) *vmoprv1.
if apierrors.IsNotFound(err) {
return nil
}
Expect(err).Should(BeNil())
Expect(err).ShouldNot(HaveOccurred())
return vms
}

Expand Down Expand Up @@ -77,7 +77,7 @@ func updateVMServiceWithVIP(cpService CPService, ctx *vmware.ClusterContext, vip
vmService := getVirtualMachineService(cpService, ctx)
vmService.Status.LoadBalancer.Ingress = []vmoprv1.LoadBalancerIngress{{IP: vip}}
err := ctx.Client.Status().Update(ctx, vmService)
Expect(err).Should(BeNil())
Expect(err).ShouldNot(HaveOccurred())
}

var _ = Describe("ControlPlaneEndpoint Tests", func() {
Expand Down Expand Up @@ -140,7 +140,7 @@ var _ = Describe("ControlPlaneEndpoint Tests", func() {
for k, v := range expectedAnnotations {
Expect(vms.Annotations).To(HaveKeyWithValue(k, v))
}
Expect(len(vms.Spec.Ports)).To(Equal(1))
Expect(vms.Spec.Ports).To(HaveLen(1))
Expect(vms.Spec.Ports[0].Name).To(Equal(controlPlaneServiceAPIServerPortName))
Expect(vms.Spec.Ports[0].Protocol).To(Equal("TCP"))
Expect(vms.Spec.Ports[0].Port).To(Equal(int32(defaultAPIBindPort)))
Expand Down
Loading
Loading