From 1de0d9395795753d303113abf1a3b8f3830a2279 Mon Sep 17 00:00:00 2001 From: Nahshon Unna-Tsameret Date: Thu, 14 Mar 2024 08:16:45 +0200 Subject: [PATCH 1/2] fix ginkgolinter findins --- internal/framework/controller/register_test.go | 2 +- internal/framework/events/events_test.go | 4 ++-- .../events/first_eventbatch_preparer_test.go | 4 ++-- internal/framework/events/loop_test.go | 4 ++-- internal/framework/runnables/runnables_test.go | 2 +- internal/mode/provisioner/handler_test.go | 4 ++-- internal/mode/static/handler_test.go | 4 ++-- internal/mode/static/sort/sort_test.go | 2 +- .../static/state/graph/backend_tls_policy_test.go | 2 +- internal/mode/static/state/graph/httproute_test.go | 2 +- internal/mode/static/state/graph/secret_test.go | 2 +- internal/mode/static/telemetry/collector_test.go | 12 ++++++------ internal/mode/static/telemetry/exporter_test.go | 2 +- tests/suite/longevity_test.go | 2 +- tests/suite/system_suite_test.go | 2 +- tests/suite/upgrade_test.go | 2 +- 16 files changed, 26 insertions(+), 26 deletions(-) diff --git a/internal/framework/controller/register_test.go b/internal/framework/controller/register_test.go index 03f5b7738..ecb433730 100644 --- 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 index 17f1383aa..7d2415c2d 100644 --- 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 index 8fd79b1b7..483434b93 100644 --- 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 index 589b1a32d..807e1b0d4 100644 --- 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 index f756146cd..28af82930 100644 --- 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 index 0faa894a9..d31e36a87 100644 --- 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 index 9b8a60fc5..b99ec2866 100644 --- 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 index 6379309f6..e7cac0655 100644 --- 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 index 0cedce222..386e76049 100644 --- 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 index 7f4b99d37..d7580a37e 100644 --- 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 index 81aa59503..54ebc3ca8 100644 --- 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 index cd5ae9cda..22bb40c39 100644 --- 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 index 1a51c6f31..6a86a0168 100644 --- 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 index 0f1382620..9ce38fe9a 100644 --- 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 index e1451fb97..a9b7fc85a 100644 --- 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 index 0e5983401..a64b6689a 100644 --- 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) From 80b2d3b42703d7cdabda88f0b1b33dc620c9f4bc Mon Sep 17 00:00:00 2001 From: Nahshon Unna-Tsameret Date: Thu, 14 Mar 2024 08:16:20 +0200 Subject: [PATCH 2/2] enable ginkgolinter --- .golangci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.golangci.yml b/.golangci.yml index 1afc5317b..88dfe6d3b 100644 --- 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