Enable ginkgolinter and fix finding (#1689)

Enable ginkgolinter in .golangci.yaml, and fix its findings.
nginxinc · Mar 15, 2024 · 907f74f · 907f74f
1 parent fe3bc2a
commit 907f74f
Show file tree

Hide file tree

Showing 17 changed files with 29 additions and 26 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,4 +1,6 @@
 linters-settings:
+  ginkgolinter:
+    forbid-focus-container: true
   goimports:
     local-prefixes: github.com/nginxinc/nginx-gateway-fabric
   misspell:
@@ -41,6 +43,7 @@ linters:
     - asciicheck
     - errcheck
     - errorlint
+    - ginkgolinter
     - gocyclo
     - gofmt
     - gofumpt

diff --git a/internal/framework/controller/register_test.go b/internal/framework/controller/register_test.go
@@ -150,7 +150,7 @@ func TestRegister(t *testing.T) {
 			} else {
 				err := register()
 				if test.expectedErr == nil {
-					g.Expect(err).To(BeNil())
+					g.Expect(err).ToNot(HaveOccurred())
 				} else {
 					g.Expect(err).To(MatchError(test.expectedErr))
 				}

diff --git a/internal/framework/events/events_test.go b/internal/framework/events/events_test.go
@@ -28,8 +28,8 @@ func TestEventLoop_SwapBatches(t *testing.T) {
 
 	eventLoop.swapBatches()
 
-	g.Expect(len(eventLoop.currentBatch)).To(Equal(len(nextBatch)))
+	g.Expect(eventLoop.currentBatch).To(HaveLen(len(nextBatch)))
 	g.Expect(eventLoop.currentBatch).To(Equal(nextBatch))
-	g.Expect(len(eventLoop.nextBatch)).To(Equal(0))
+	g.Expect(eventLoop.nextBatch).To(BeEmpty())
 	g.Expect(cap(eventLoop.nextBatch)).To(Equal(3))
 }
diff --git a/internal/framework/events/first_eventbatch_preparer_test.go b/internal/framework/events/first_eventbatch_preparer_test.go
@@ -58,7 +58,7 @@ var _ = Describe("FirstEventBatchPreparer", func() {
 			batch, err := preparer.Prepare(context.Background())
 
 			Expect(batch).Should(BeEmpty())
-			Expect(err).Should(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 		})
 
 		It("should prepare one event for each resource type", func() {
@@ -97,7 +97,7 @@ var _ = Describe("FirstEventBatchPreparer", func() {
 			batch, err := preparer.Prepare(context.Background())
 
 			Expect(batch).Should(Equal(expectedBatch))
-			Expect(err).Should(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 		})
 	})
 

diff --git a/internal/framework/events/loop_test.go b/internal/framework/events/loop_test.go
@@ -59,7 +59,7 @@ var _ = Describe("EventLoop", func() {
 
 			var err error
 			Eventually(errorCh).Should(Receive(&err))
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 		})
 
 		// Because BeforeEach() creates the first batch and waits for it to be handled, in the tests below
@@ -138,7 +138,7 @@ var _ = Describe("EventLoop", func() {
 			cancel()
 			err := eventLoop.Start(ctx)
 
-			Expect(err).Should(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 		})
 	})
 })
diff --git a/internal/framework/runnables/runnables_test.go b/internal/framework/runnables/runnables_test.go
@@ -32,7 +32,7 @@ func TestEnableAfterBecameLeader(t *testing.T) {
 	g.Expect(enabled).To(BeFalse())
 
 	err := enableAfterBecameLeader.Start(context.Background())
-	g.Expect(err).To(BeNil())
+	g.Expect(err).ToNot(HaveOccurred())
 
 	g.Expect(enabled).To(BeTrue())
 }
diff --git a/internal/mode/provisioner/handler_test.go b/internal/mode/provisioner/handler_test.go
@@ -342,7 +342,7 @@ var _ = Describe("handler", func() {
 				err := k8sclient.List(context.Background(), deps)
 
 				Expect(err).ToNot(HaveOccurred())
-				Expect(deps.Items).To(HaveLen(0))
+				Expect(deps.Items).To(BeEmpty())
 			})
 		})
 
@@ -370,7 +370,7 @@ var _ = Describe("handler", func() {
 				err := k8sclient.List(context.Background(), deps)
 
 				Expect(err).ToNot(HaveOccurred())
-				Expect(deps.Items).To(HaveLen(0))
+				Expect(deps.Items).To(BeEmpty())
 			})
 		})
 

diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go
@@ -229,7 +229,7 @@ var _ = Describe("eventHandler", func() {
 				"Failed to update control plane configuration: logging.level: Unsupported value: " +
 					"\"invalid\": supported values: \"info\", \"debug\", \"error\"")
 			Expect(statuses).To(Equal(expStatuses(cond)))
-			Expect(len(fakeEventRecorder.Events)).To(Equal(1))
+			Expect(fakeEventRecorder.Events).To(HaveLen(1))
 			event := <-fakeEventRecorder.Events
 			Expect(event).To(Equal(
 				"Warning UpdateFailed Failed to update control plane configuration: logging.level: Unsupported value: " +
@@ -252,7 +252,7 @@ var _ = Describe("eventHandler", func() {
 
 			Expect(handler.GetLatestConfiguration()).To(BeNil())
 
-			Expect(len(fakeEventRecorder.Events)).To(Equal(1))
+			Expect(fakeEventRecorder.Events).To(HaveLen(1))
 			event := <-fakeEventRecorder.Events
 			Expect(event).To(Equal("Warning ResourceDeleted NginxGateway configuration was deleted; using defaults"))
 			Expect(zapLogLevelSetter.Enabled(zap.InfoLevel)).To(BeTrue())

diff --git a/internal/mode/static/sort/sort_test.go b/internal/mode/static/sort/sort_test.go
@@ -85,7 +85,7 @@ func TestLessObjectMeta(t *testing.T) {
 			invertedResult := LessObjectMeta(test.meta2, test.meta1)
 
 			g.Expect(result).To(Equal(test.expected))
-			g.Expect(invertedResult).To(Equal(false))
+			g.Expect(invertedResult).To(BeFalse())
 		})
 	}
 }
diff --git a/internal/mode/static/state/graph/backend_tls_policy_test.go b/internal/mode/static/state/graph/backend_tls_policy_test.go
@@ -405,7 +405,7 @@ func TestValidateBackendTLSPolicy(t *testing.T) {
 			if !test.isValid && !test.ignored {
 				g.Expect(conds).To(HaveLen(1))
 			} else {
-				g.Expect(conds).To(HaveLen(0))
+				g.Expect(conds).To(BeEmpty())
 			}
 		})
 	}

diff --git a/internal/mode/static/state/graph/httproute_test.go b/internal/mode/static/state/graph/httproute_test.go
@@ -250,7 +250,7 @@ func TestBuildSectionNameRefs(t *testing.T) {
 			if test.expectedError != nil {
 				g.Expect(err).To(Equal(test.expectedError))
 			} else {
-				g.Expect(err).To(BeNil())
+				g.Expect(err).ToNot(HaveOccurred())
 			}
 		})
 	}

diff --git a/internal/mode/static/state/graph/secret_test.go b/internal/mode/static/state/graph/secret_test.go
@@ -191,7 +191,7 @@ func TestSecretResolver(t *testing.T) {
 	for _, test := range tests {
 		err := resolver.resolve(test.nsname)
 		if test.expectedErrMsg == "" {
-			g.Expect(err).To(BeNil(), fmt.Sprintf("case %q", test.name))
+			g.Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("case %q", test.name))
 		} else {
 			g.Expect(err).To(MatchError(test.expectedErrMsg), fmt.Sprintf("case %q", test.name))
 		}

diff --git a/internal/mode/static/telemetry/collector_test.go b/internal/mode/static/telemetry/collector_test.go
@@ -347,7 +347,7 @@ var _ = Describe("Collector", Ordered, func() {
 
 				data, err := dataCollector.Collect(ctx)
 
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 				Expect(expData).To(Equal(data))
 			})
 		})
@@ -414,7 +414,7 @@ var _ = Describe("Collector", Ordered, func() {
 
 					data, err := dataCollector.Collect(ctx)
 
-					Expect(err).To(BeNil())
+					Expect(err).ToNot(HaveOccurred())
 					Expect(expData).To(Equal(data))
 				})
 			})
@@ -430,7 +430,7 @@ var _ = Describe("Collector", Ordered, func() {
 
 				data, err := dataCollector.Collect(ctx)
 
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 				Expect(expData).To(Equal(data))
 			})
 
@@ -539,7 +539,7 @@ var _ = Describe("Collector", Ordered, func() {
 
 				data, err := dataCollector.Collect(ctx)
 
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 				Expect(expData).To(Equal(data))
 			})
 
@@ -558,7 +558,7 @@ var _ = Describe("Collector", Ordered, func() {
 
 				data, err := dataCollector.Collect(ctx)
 
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 				Expect(expData).To(Equal(data))
 			})
 
@@ -576,7 +576,7 @@ var _ = Describe("Collector", Ordered, func() {
 
 				data, err := dataCollector.Collect(ctx)
 
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 				Expect(expData).To(Equal(data))
 			})
 

diff --git a/internal/mode/static/telemetry/exporter_test.go b/internal/mode/static/telemetry/exporter_test.go
@@ -18,7 +18,7 @@ func TestLoggingExporter(t *testing.T) {
 
 	err := exporter.Export(context.Background(), &Data{})
 
-	g.Expect(err).To(BeNil())
+	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(buffer.String()).To(ContainSubstring(`"level":"info"`))
 	g.Expect(buffer.String()).To(ContainSubstring(`"msg":"Exporting telemetry"`))
 }
diff --git a/tests/suite/longevity_test.go b/tests/suite/longevity_test.go
@@ -76,7 +76,7 @@ var _ = Describe("Longevity", Label("longevity-setup", "longevity-teardown"), fu
 		homeDir, err := os.UserHomeDir()
 		Expect(err).ToNot(HaveOccurred())
 
-		Expect(framework.WriteContent(resultsFile, "\n## Traffic\n"))
+		Expect(framework.WriteContent(resultsFile, "\n## Traffic\n")).To(Succeed())
 		Expect(writeTrafficResults(resultsFile, homeDir, "coffee.txt", "HTTP")).To(Succeed())
 		Expect(writeTrafficResults(resultsFile, homeDir, "tea.txt", "HTTPS")).To(Succeed())
 

diff --git a/tests/suite/system_suite_test.go b/tests/suite/system_suite_test.go
@@ -170,7 +170,7 @@ func setup(cfg setupConfig, extraInstallArgs ...string) {
 		timeoutConfig.CreateTimeout,
 	)
 	Expect(err).ToNot(HaveOccurred())
-	Expect(podNames).ToNot(HaveLen(0))
+	Expect(podNames).ToNot(BeEmpty())
 
 	if *serviceType != "LoadBalancer" {
 		portFwdPort, err = framework.PortForward(k8sConfig, installCfg.Namespace, podNames[0], portForwardStopCh)

diff --git a/tests/suite/upgrade_test.go b/tests/suite/upgrade_test.go
@@ -185,7 +185,7 @@ var _ = Describe("Upgrade testing", Label("nfr", "upgrade"), func() {
 
 		podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout)
 		Expect(err).ToNot(HaveOccurred())
-		Expect(podNames).ToNot(HaveLen(0))
+		Expect(podNames).ToNot(BeEmpty())
 
 		// ensure that the leader election lease has been updated to the new pods
 		leaseCtx, leaseCancel := context.WithTimeout(context.Background(), 1*time.Minute)