From 36cceff0a3c050cac15d36a04f5e91dc8d127713 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Wed, 8 Nov 2023 18:00:08 +0100
Subject: [PATCH] e2e: forbid usage of gomega.BeTrue/False

`BeTrue` without explanation in `Should` just prints "false is not true", which
isn't a useful failure message. Even with an explanation that message is still
printed, which is just noise. The new BeTrue/FalseBecause avoid that by
requiring that an explanation is provided and using that instead of "false is
not true".

In Kubernetes, we can enforce the usage of those better alternatives or the
if+Fail combination via forbidigo. Because we have existing code which still
needs to be updated, this can only be a hint for now.

Some examples found with this:

ERROR: test/e2e/apimachinery/protocol.go:75:20: use of `o.BeTrue` forbidden because "it does not produce a good failure message, use BeTrueBecause with an explicit printf-style failure message instead or plain Go: if ... { ginkgo.Fail(...) }" (forbidigo)
ERROR: 			o.Expect(ok).To(o.BeTrue())
ERROR: 			                ^

ERROR: test/e2e_node/unknown_pods_test.go:163:52: use of `gomega.BeTrue` forbidden because "it does not produce a good failure message, use BeTrueBecause with an explicit printf-style failure message instead or plain Go: if ... { ginkgo.Fail(...) }" (forbidigo)
ERROR: 			}, f.Timeouts.PodStart, f.Timeouts.Poll).Should(gomega.BeTrue())
ERROR: 			                                                ^

The solution might also be to write a custom matcher, but that is a bit hard to
explain in the forbidigo message.
---
 hack/golangci-hints.yaml | 6 ++++++
 hack/golangci.yaml.in    | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/hack/golangci-hints.yaml b/hack/golangci-hints.yaml
index 28bf2dff9a0..fed4241f4ff 100644
--- a/hack/golangci-hints.yaml
+++ b/hack/golangci-hints.yaml
@@ -150,6 +150,12 @@ linters-settings: # please keep this alphabetized
     - p: \.Extract
       pkg: ^k8s\.io/client-go/applyconfigurations/
       msg: should not be used because managedFields was removed
+    - p: ^gomega\.BeTrue$
+      pkg: ^github.com/onsi/gomega$
+      msg: "it does not produce a good failure message - use BeTrueBecause with an explicit printf-style failure message instead, or plain Go: if ... { ginkgo.Fail(...) }"
+    - p: ^gomega\.BeFalse$
+      pkg: ^github.com/onsi/gomega$
+      msg: "it does not produce a good failure message - use BeFalseBecause with an explicit printf-style failure message instead, or plain Go: if ... { ginkgo.Fail(...) }"
   revive:
     # Only these rules are enabled.
     rules:
diff --git a/hack/golangci.yaml.in b/hack/golangci.yaml.in
index c2284551c75..0b556cf5cf4 100644
--- a/hack/golangci.yaml.in
+++ b/hack/golangci.yaml.in
@@ -171,6 +171,14 @@ linters-settings: # please keep this alphabetized
     - p: \.Extract
       pkg: ^k8s\.io/client-go/applyconfigurations/
       msg: should not be used because managedFields was removed
+    {{- if .Hints}}
+    - p: ^gomega\.BeTrue$
+      pkg: ^github.com/onsi/gomega$
+      msg: "it does not produce a good failure message - use BeTrueBecause with an explicit printf-style failure message instead, or plain Go: if ... { ginkgo.Fail(...) }"
+    - p: ^gomega\.BeFalse$
+      pkg: ^github.com/onsi/gomega$
+      msg: "it does not produce a good failure message - use BeFalseBecause with an explicit printf-style failure message instead, or plain Go: if ... { ginkgo.Fail(...) }"
+    {{- end}}
   {{- if .Base }}
   gocritic:
     enabled-checks: