Enable ginkgolinter and fix findings

nunnatsa · nunnatsa · commit 932f65fb7fb6 · 2023-08-26T20:03:32.000+03:00
Signed-off-by: Nahshon Unna-Tsameret &lt;nunnatsa@redhat.com&gt;
diff --git a/.golangci.yml b/.golangci.yml
@@ -21,7 +21,7 @@ linters:
   - errchkjson
   - exportloopref
   - gci
-  # - ginkgolinter
+  - ginkgolinter
   - goconst
   - gocritic
   - godot
@@ -172,6 +172,9 @@ linters-settings:
       - name: bool-literal-in-expr
       - name: constant-logical-expr
 
+  ginkgolinter:
+    forbid-focus-container: true
+
 issues:
   max-same-issues: 0
   max-issues-per-linter: 0
diff --git a/controllers/clustermodule_reconciler_test.go b/controllers/clustermodule_reconciler_test.go
@@ -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"))
@@ -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())
 			},
 		},
 		{
@@ -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())
 			},
 		},
 	}
diff --git a/controllers/serviceaccount_controller_suite_test.go b/controllers/serviceaccount_controller_suite_test.go
@@ -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{
 		{
@@ -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,
diff --git a/controllers/vmware/vspherecluster_reconciler_test.go b/controllers/vmware/vspherecluster_reconciler_test.go
@@ -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() {
@@ -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))
 		})
diff --git a/controllers/vspheredeploymentzone_controller_test.go b/controllers/vspheredeploymentzone_controller_test.go
@@ -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())
 		})
 	})
 
@@ -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) {
@@ -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) {
diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go
@@ -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
@@ -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))
 
@@ -124,6 +124,6 @@ password: '%s'
 		}
 		_, err = InitializeWatch(fake.NewControllerManagerContext(), managerOptsTest)
 		// Match initial credentials
-		g.Expect(err).NotTo(BeNil())
+		g.Expect(err).To(HaveOccurred())
 	})
 }
diff --git a/pkg/record/recorder_test.go b/pkg/record/recorder_test.go
@@ -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"))
 		})
@@ -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
 
diff --git a/pkg/services/govmomi/cluster/service_test.go b/pkg/services/govmomi/cluster/service_test.go
@@ -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())
 }
diff --git a/pkg/services/govmomi/extra/extra_suite_test.go b/pkg/services/govmomi/extra/extra_suite_test.go
@@ -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() {
diff --git a/pkg/services/govmomi/util_test.go b/pkg/services/govmomi/util_test.go
@@ -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())
 		})
@@ -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()))
 	})
 }
diff --git a/pkg/services/network/network_test.go b/pkg/services/network/network_test.go
@@ -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())
 			})
 		})
@@ -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"))
 					})
@@ -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"))
 			})
@@ -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())
 			})
 		})
@@ -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())
 			})
 		})
@@ -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{}
@@ -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())
 			})
 
@@ -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{}
@@ -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())
 			})
@@ -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
@@ -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())
 			})
@@ -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
@@ -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
diff --git a/pkg/services/vmoperator/control_plane_endpoint_test.go b/pkg/services/vmoperator/control_plane_endpoint_test.go
@@ -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
 }
 
@@ -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() {
@@ -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)))
diff --git a/pkg/services/vmoperator/vmopmachine_test.go b/pkg/services/vmoperator/vmopmachine_test.go
diff --git a/pkg/util/supervisor_test.go b/pkg/util/supervisor_test.go
diff --git a/test/e2e/anti_affinity_test.go b/test/e2e/anti_affinity_test.go
diff --git a/test/e2e/multivc_test.go b/test/e2e/multivc_test.go
diff --git a/test/e2e/storage_policy_test.go b/test/e2e/storage_policy_test.go

Original file line number	Diff line number	Diff line change
`@@ -129,7 +129,7 @@ func assertRoleWithGetPVC(ctx *helpers.UnitTestContextForController, ctrlClient`
`129`	`129`	`}`
`130`	`130`	`err := ctrlClient.List(ctx, &roleList, opts)`
`131`	`131`	`Expect(err).ShouldNot(HaveOccurred())`
`132`		`- Expect(len(roleList.Items)).To(Equal(1))`
	`132`	`+ Expect(roleList.Items).To(HaveLen(1))`
`133`	`133`	`Expect(roleList.Items[0].Name).To(Equal(name))`
`134`	`134`	`Expect(roleList.Items[0].Rules).To(Equal([]rbacv1.PolicyRule{`
`135`	`135`	`{`
`@@ -147,7 +147,7 @@ func assertRoleBinding(_ *helpers.UnitTestContextForController, ctrlClient clien`
`147`	`147`	`}`
`148`	`148`	`err := ctrlClient.List(goctx.TODO(), &roleBindingList, opts)`
`149`	`149`	`Expect(err).ShouldNot(HaveOccurred())`
`150`		`- Expect(len(roleBindingList.Items)).To(Equal(1))`
	`150`	`+ Expect(roleBindingList.Items).To(HaveLen(1))`
`151`	`151`	`Expect(roleBindingList.Items[0].Name).To(Equal(name))`
`152`	`152`	`Expect(roleBindingList.Items[0].RoleRef).To(Equal(rbacv1.RoleRef{`
`153`	`153`	`Name: name,`
Original file line number	Diff line number	Diff line change
`@@ -57,5 +57,5 @@ func TestListHostsFromGroup(t *testing.T) {`
`57`	`57`
`58`	`58`	`refs, err = ListHostsFromGroup(context.Background(), ccr, "blah")`
`59`	`59`	`g.Expect(err).NotTo(HaveOccurred())`
`60`		`- g.Expect(refs).To(HaveLen(0))`
	`60`	`+ g.Expect(refs).To(BeEmpty())`
`61`	`61`	`}`