Skip to content

Commit

Permalink
disable the focused container rule by default
Browse files Browse the repository at this point in the history
Enabling this rule by default may case issues in projects with a complex build
systems.

This PR change this rule to be opt-in.

The `--suppress-focus-container` flag is now depracated and ignored.
Instead, this PR introduces the `--forbid-focus-container` flag, with
default value of `false`.
  • Loading branch information
nunnatsa committed Jul 24, 2023
1 parent 993cf24 commit 5412fd3
Show file tree
Hide file tree
Showing 11 changed files with 65 additions and 60 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ This rule finds ginkgo focus containers in the code.
ginkgo supports the `FDescribe`, `FContext`, `FWhen` and `FIt` containers to allow the developer to focus
on a specific test or set of tests during test development or debug.

***This rule is disabled by default***. Use the `--forbid-focus-container=true` command line flag to enable it.

For example:
```go
var _ = Describe("checking something", func() {
Expand Down Expand Up @@ -304,7 +306,6 @@ Expect(c1 == x1).Should(BeTrue()) // ==> Expect(x1).Should(Equal(c1))
* Use the `--suppress-err-assertion=true` flag to suppress the wrong error assertion warning
* Use the `--suppress-compare-assertion=true` flag to suppress the wrong comparison assertion warning
* Use the `--suppress-async-assertion=true` flag to suppress the function call in async assertion warning
* Use the `--suppress-focus-container=true` flag to suppress the focus container warning
* Use the `--allow-havelen-0=true` flag to avoid warnings about `HaveLen(0)`; Note: this parameter is only supported from
command line, and not from a comment.

Expand Down
6 changes: 5 additions & 1 deletion ginkgo_linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func NewAnalyzer() *analysis.Analyzer {
SuppressNil: false,
SuppressErr: false,
SuppressCompare: false,
SuppressFocus: true,
AllowHaveLen0: false,
},
}
Expand All @@ -90,15 +91,18 @@ func NewAnalyzer() *analysis.Analyzer {
Run: linter.run,
}

var ignored bool
a.Flags.Init("ginkgolinter", flag.ExitOnError)
a.Flags.Var(&linter.config.SuppressLen, "suppress-len-assertion", "Suppress warning for wrong length assertions")
a.Flags.Var(&linter.config.SuppressNil, "suppress-nil-assertion", "Suppress warning for wrong nil assertions")
a.Flags.Var(&linter.config.SuppressErr, "suppress-err-assertion", "Suppress warning for wrong error assertions")
a.Flags.Var(&linter.config.SuppressCompare, "suppress-compare-assertion", "Suppress warning for wrong comparison assertions")
a.Flags.Var(&linter.config.SuppressAsync, "suppress-async-assertion", "Suppress warning for function call in async assertion, like Eventually")
a.Flags.Var(&linter.config.SuppressFocus, "suppress-focus-container", "Suppress warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt")
a.Flags.Var(&linter.config.AllowHaveLen0, "allow-havelen-0", "Do not warn for HaveLen(0); default = false")

a.Flags.BoolVar(&ignored, "suppress-focus-container", true, "Suppress warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt. Deprecated and ignored: use --forbid-focus-container instead")
a.Flags.Var(&linter.config.SuppressFocus, "forbid-focus-container", "trigger a warning for ginkgo focus containers like FDescribe, FContext, FWhen or FIt; default = false.")

return a
}

Expand Down
24 changes: 12 additions & 12 deletions ginkgo_linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,48 +88,48 @@ func TestFlags(t *testing.T) {
for _, tc := range []struct {
testName string
testData []string
flags []string
flags map[string]string
}{
{
testName: "test the suppress-len-assertion flag",
testData: []string{"a/configlen"},
flags: []string{"suppress-len-assertion"},
flags: map[string]string{"suppress-len-assertion": "true"},
},
{
testName: "test the suppress-nil-assertion flag",
testData: []string{"a/confignil"},
flags: []string{"suppress-nil-assertion"},
flags: map[string]string{"suppress-nil-assertion": "true"},
},
{
testName: "test the suppress-err-assertion flag",
testData: []string{"a/configerr"},
flags: []string{"suppress-err-assertion"},
flags: map[string]string{"suppress-err-assertion": "true"},
},
{
testName: "test the suppress-compare-assertion flag",
testData: []string{"a/configcompare"},
flags: []string{"suppress-compare-assertion"},
flags: map[string]string{"suppress-compare-assertion": "true"},
},
{
testName: "test the allow-havelen-0 flag",
testData: []string{"a/havelen0config"},
flags: []string{"allow-havelen-0"},
flags: map[string]string{"allow-havelen-0": "true"},
},
{
testName: "test the suppress-async-assertion flag",
testData: []string{"a/asyncconfig"},
flags: []string{"suppress-async-assertion"},
flags: map[string]string{"suppress-async-assertion": "true"},
},
{
testName: "test the suppress-focus-container flag",
testData: []string{"a/focusconfig"},
flags: []string{"suppress-focus-container"},
testName: "test the forbid-focus-container flag",
testData: []string{"a/focus"},
flags: map[string]string{"forbid-focus-container": "true"},
},
} {
t.Run(tc.testName, func(tt *testing.T) {
analyzer := ginkgolinter.NewAnalyzer()
for _, flag := range tc.flags {
err := analyzer.Flags.Set(flag, "true")
for flag, value := range tc.flags {
err := analyzer.Flags.Set(flag, value)
if err != nil {
tt.Errorf(`failed to set the "%s" flag; %v`, flag, err)
return
Expand Down
8 changes: 4 additions & 4 deletions testdata/src/a/focus/focus_name_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
. "github.com/onsi/gomega"
)

var _ = tester.FDescribe("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Describe"`
var _ = tester.FDescribe("should warn", func() {
tester.When("should ignore", func() {
tester.It("should ignore", func() {
Expect(len("abcd")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("abcd"\)\.Should\(HaveLen\(4\)\). instead`
Expand All @@ -23,7 +23,7 @@ var _ = tester.FDescribe("should warn", func() { // want `ginkgo-linter: Focus c
})

var _ = tester.Describe("should ignore", func() {
tester.FWhen("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "When"`
tester.FWhen("should warn", func() {
tester.Context("should ignore", func() {
tester.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -35,7 +35,7 @@ var _ = tester.Describe("should ignore", func() {
})
})

tester.FContext("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Context"`
tester.FContext("should warn", func() {
tester.Context("should ignore", func() {
tester.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -48,7 +48,7 @@ var _ = tester.Describe("should ignore", func() {
})

tester.Context("ignore", func() {
tester.FIt("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "It"`
tester.FIt("should warn", func() {
Expect("abcd").Should(HaveLen(4))
})
})
Expand Down
8 changes: 4 additions & 4 deletions testdata/src/a/focus/focus_noname_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
. "github.com/onsi/gomega"
)

var _ = ginkgo.FDescribe("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Describe"`
var _ = ginkgo.FDescribe("should warn", func() {
ginkgo.When("should ignore", func() {
ginkgo.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -23,7 +23,7 @@ var _ = ginkgo.FDescribe("should warn", func() { // want `ginkgo-linter: Focus c
})

var _ = ginkgo.Describe("should ignore", func() {
ginkgo.FWhen("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "When"`
ginkgo.FWhen("should warn", func() {
ginkgo.Context("should ignore", func() {
ginkgo.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -35,7 +35,7 @@ var _ = ginkgo.Describe("should ignore", func() {
})
})

ginkgo.FContext("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Context"`
ginkgo.FContext("should warn", func() {
ginkgo.Context("should ignore", func() {
ginkgo.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -48,7 +48,7 @@ var _ = ginkgo.Describe("should ignore", func() {
})

ginkgo.Context("ignore", func() {
ginkgo.FIt("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "It"`
ginkgo.FIt("should warn", func() {
Expect("abcd").Should(HaveLen(4))
})
})
Expand Down
21 changes: 4 additions & 17 deletions testdata/src/a/focus/focus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
. "github.com/onsi/gomega"
)

var _ = FDescribe("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Describe"`
var _ = FDescribe("should warn", func() {
When("should ignore", func() {
It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -23,7 +23,7 @@ var _ = FDescribe("should warn", func() { // want `ginkgo-linter: Focus containe
})

var _ = Describe("should ignore", func() {
FWhen("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "When"`
FWhen("should warn", func() {
Context("should ignore", func() {
It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -35,7 +35,7 @@ var _ = Describe("should ignore", func() {
})
})

FContext("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Context"`
FContext("should warn", func() {
Context("should ignore", func() {
It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -48,21 +48,8 @@ var _ = Describe("should ignore", func() {
})

Context("ignore", func() {
FIt("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "It"`
FIt("should warn", func() {
Expect("abcd").Should(HaveLen(4))
})
})
})

var _ = Describe("suppress", func() {
// ginkgo-linter:ignore-focus-container-warning
FContext("supress", func() {
// ginkgo-linter:ignore-focus-container-warning
FWhen("suppress", func() {
// ginkgo-linter:ignore-focus-container-warning
FIt("suppress", func() {
Expect(len("abcd")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("abcd"\)\.Should\(HaveLen\(4\)\). instead`
})
})
})
})
8 changes: 4 additions & 4 deletions testdata/src/a/focusconfig/focus_name_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
. "github.com/onsi/gomega"
)

var _ = tester.FDescribe("should warn", func() {
var _ = tester.FDescribe("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Describe"`
tester.When("should ignore", func() {
tester.It("should ignore", func() {
Expect(len("abcd")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("abcd"\)\.Should\(HaveLen\(4\)\). instead`
Expand All @@ -23,7 +23,7 @@ var _ = tester.FDescribe("should warn", func() {
})

var _ = tester.Describe("should ignore", func() {
tester.FWhen("should warn", func() {
tester.FWhen("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "When"`
tester.Context("should ignore", func() {
tester.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -35,7 +35,7 @@ var _ = tester.Describe("should ignore", func() {
})
})

tester.FContext("should warn", func() {
tester.FContext("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Context"`
tester.Context("should ignore", func() {
tester.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -48,7 +48,7 @@ var _ = tester.Describe("should ignore", func() {
})

tester.Context("ignore", func() {
tester.FIt("should warn", func() {
tester.FIt("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "It"`
Expect("abcd").Should(HaveLen(4))
})
})
Expand Down
8 changes: 4 additions & 4 deletions testdata/src/a/focusconfig/focus_noname_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
. "github.com/onsi/gomega"
)

var _ = ginkgo.FDescribe("should warn", func() {
var _ = ginkgo.FDescribe("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Describe"`
ginkgo.When("should ignore", func() {
ginkgo.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -23,7 +23,7 @@ var _ = ginkgo.FDescribe("should warn", func() {
})

var _ = ginkgo.Describe("should ignore", func() {
ginkgo.FWhen("should warn", func() {
ginkgo.FWhen("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "When"`
ginkgo.Context("should ignore", func() {
ginkgo.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -35,7 +35,7 @@ var _ = ginkgo.Describe("should ignore", func() {
})
})

ginkgo.FContext("should warn", func() {
ginkgo.FContext("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Context"`
ginkgo.Context("should ignore", func() {
ginkgo.It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -48,7 +48,7 @@ var _ = ginkgo.Describe("should ignore", func() {
})

ginkgo.Context("ignore", func() {
ginkgo.FIt("should warn", func() {
ginkgo.FIt("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "It"`
Expect("abcd").Should(HaveLen(4))
})
})
Expand Down
21 changes: 17 additions & 4 deletions testdata/src/a/focusconfig/focus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
. "github.com/onsi/gomega"
)

var _ = FDescribe("should warn", func() {
var _ = FDescribe("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Describe"`
When("should ignore", func() {
It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -23,7 +23,7 @@ var _ = FDescribe("should warn", func() {
})

var _ = Describe("should ignore", func() {
FWhen("should warn", func() {
FWhen("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "When"`
Context("should ignore", func() {
It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -35,7 +35,7 @@ var _ = Describe("should ignore", func() {
})
})

FContext("should warn", func() {
FContext("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "Context"`
Context("should ignore", func() {
It("should ignore", func() {
Expect("abcd").Should(HaveLen(4))
Expand All @@ -48,8 +48,21 @@ var _ = Describe("should ignore", func() {
})

Context("ignore", func() {
FIt("should warn", func() {
FIt("should warn", func() { // want `ginkgo-linter: Focus container found. This is used only for local debug and should not be part of the actual source code, consider to replace with "It"`
Expect("abcd").Should(HaveLen(4))
})
})
})

var _ = Describe("suppress", func() {
// ginkgo-linter:ignore-focus-container-warning
FContext("supress", func() {
// ginkgo-linter:ignore-focus-container-warning
FWhen("suppress", func() {
// ginkgo-linter:ignore-focus-container-warning
FIt("suppress", func() {
Expect(len("abcd")).Should(Equal(4)) // want `ginkgo-linter: wrong length assertion; consider using .Expect\("abcd"\)\.Should\(HaveLen\(4\)\). instead`
})
})
})
})
File renamed without changes.
18 changes: 9 additions & 9 deletions tests/e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@ cp ginkgolinter testdata/src/a
cd testdata/src/a

# no suppress
[[ $(./ginkgolinter a/... 2>&1 | wc -l) == 2398 ]]
[[ $(./ginkgolinter a/... 2>&1 | wc -l) == 2374 ]]
# suppress all but nil
[[ $(./ginkgolinter --suppress-len-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 1452 ]]
[[ $(./ginkgolinter --suppress-len-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 1452 ]]
# suppress all but len
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 744 ]]
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 744 ]]
# suppress all but err
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 212 ]]
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 212 ]]
# suppress all but compare
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-async-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 222 ]]
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 222 ]]
# suppress all but async
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 119 ]]
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true a/... 2>&1 | wc -l) == 119 ]]
# suppress all but focus
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true a/... 2>&1 | wc -l) == 112 ]]
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-err-assertion=true --suppress-len-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --forbid-focus-container=false a/... 2>&1 | wc -l) == 112 ]]
# allow HaveLen(0)
[[ $(./ginkgolinter --allow-havelen-0=true a/... 2>&1 | wc -l) == 2385 ]]
[[ $(./ginkgolinter --allow-havelen-0=true a/... 2>&1 | wc -l) == 2361 ]]
# suppress all - should only return the few non-suppressble
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-len-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --suppress-focus-container=true a/... 2>&1 | wc -l) == 88 ]]
[[ $(./ginkgolinter --suppress-nil-assertion=true --suppress-len-assertion=true --suppress-err-assertion=true --suppress-compare-assertion=true --suppress-async-assertion=true --forbid-focus-container=false a/... 2>&1 | wc -l) == 112 ]]

0 comments on commit 5412fd3

Please sign in to comment.