nunnatsa / ginkgolinter

golang linter for ginkgo and gomega
MIT License
24 stars 6 forks source link

[BUG] golangci-lint v1.51.2: ginkgolinter not reporting anything #62

Closed pohly closed 1 year ago

pohly commented 1 year ago

Describe the bug

I tried ginkgolinter through golangci-lint v1.51.2 on the Kubernetes E2E test suite. It didn't report anything, not even after intentionally introducing an issue.

To Reproduce

  1. check out https://github.com/pohly/kubernetes/tree/lint-gomega
  2. patch test/e2e/storage/testsuites/subpath.go:
patch -p1 <<EOF
diff --git a/test/e2e/storage/testsuites/subpath.go b/test/e2e/storage/testsuites/subpath.go
index c8c2b787d81..e56948ce7d7 100644
--- a/test/e2e/storage/testsuites/subpath.go
+++ b/test/e2e/storage/testsuites/subpath.go
@@ -1007,7 +1007,7 @@ func testSubpathReconstruction(ctx context.Context, f *framework.Framework, host
                mountPointsAfter := storageutils.FindVolumeGlobalMountPoints(ctx, hostExec, podNode)
                s1 := mountPointsAfter.Difference(mountPoints)
                s2 := mountPoints.Difference(mountPointsAfter)
-               gomega.Expect(s1).To(gomega.BeEmpty(), "global mount points leaked: %v", s1)
+               gomega.Expect(len(s1)).To(gomega.Equal(0), "global mount points leaked: %v", s1)
                gomega.Expect(s2).To(gomega.BeEmpty(), "global mount points not found: %v", s2)
        }
 }

EOF
  1. hack/verify-golangci-lint.sh ./test/e2e/storage/testsuites

Once hack/verify-golangci-lint.sh has been used once, it is also possible to invoke golangci-lint manually:

$ _output/local/bin/golangci-lint run -v ./test/e2e/storage/testsuites/
INFO [config_reader] Config search paths: [./ /nvme/gopath/src/k8s.io/kubernetes/test/e2e/storage/testsuites /nvme/gopath/src/k8s.io/kubernetes/test/e2e/storage /nvme/gopath/src/k8s.io/kubernetes/test/e2e /nvme/gopath/src/k8s.io/kubernetes/test /nvme/gopath/src/k8s.io/kubernetes /nvme/gopath/src/k8s.io /nvme/gopath/src /nvme/gopath /nvme / /home/pohly] 
INFO [config_reader] Used config file .golangci.yaml 
INFO Loaded _output/local/bin/logcheck.so: logcheck 
INFO [lintersdb] Active 7 linters: [ginkgolinter gocritic ineffassign logcheck staticcheck stylecheck unused] 
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|files|name|types_sizes|exports_file|imports) took 727.946215ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 20.336856ms 
INFO [linters_context/goanalysis] analyzers took 0s with no stages 
INFO [runner] processing took 2.983µs with stages: skip_files: 473ns, max_same_issues: 334ns, filename_unadjuster: 311ns, skip_dirs: 300ns, nolint: 216ns, max_from_linter: 161ns, exclude: 156ns, cgo: 145ns, exclude-rules: 137ns, identifier_marker: 129ns, autogenerated_exclude: 128ns, path_prettifier: 124ns, source_code: 121ns, diff: 39ns, sort_results: 38ns, path_shortener: 36ns, max_per_file_from_linter: 35ns, path_prefixer: 35ns, severity-rules: 33ns, uniq_by_line: 32ns 
INFO [runner] linters took 969.562901ms with stages: goanalysis_metalinter: 969.513992ms 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 19 samples, avg is 55.1MB, max is 81.9MB 
INFO Execution took 1.727699192s                  

Expected behavior

It should report the issue

Environment:

Additional context Add any other context about the problem here.

pohly commented 1 year ago

User error...

pohly commented 1 year ago

I misinterpreted https://golangci-lint.run/usage/linters/#ginkgolinter: my reading was that suppress-len-xyz: true would suppress the issue in the code base, i.e. enable that check. It's the opposite, the check is on by default and can be disabled.

I would have preferred a configuration as in https://golangci-lint.run/usage/linters/#errorlint where "true" means "enable the check", but that could just be me making lame excuses for not reading properly :sweat_smile:

nunnatsa commented 1 year ago

(-: Thanks for trying it out!