Skip to content

Commit

Permalink
Update how comments in test files are handled (#13)
Browse files Browse the repository at this point in the history
Tackle some UX issues for comments in test files and fix a bug in the
logic for excluding checks on test files

UX: Change the flag that is used to lint test files. It's now
`--comment-tests` which is more inline with other flags and also easier
to specify on CLI (specifying a falsy value for a bool flag is not great
on CLI)

Bug: Only exclude test files when the flag is not set instead of all the
time. Add some additional testing to ensure this doesn't regress in the
future
  • Loading branch information
ashmrtn authored Jun 19, 2023
2 parents ec3c340 + e727f32 commit 56ffa8c
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 9 deletions.
14 changes: 7 additions & 7 deletions pkg/commentmimic/commentmimic.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func (m mimic) checkFuncDecl(pass *analysis.Pass, fun *ast.FuncDecl) {
commentExported := m.commentExportedFuncs
commentAllExported := m.commentAllExportedFuncs

if isTestFunc(pass.Fset, fun.Pos(), fun.Name.Name) {
if !m.commentTests && isTestFunc(pass.Fset, fun.Pos(), fun.Name.Name) {
commentExported = false
commentAllExported = false
}
Expand Down Expand Up @@ -365,7 +365,7 @@ const (
CommentExportedFuncsFlag = "comment-exported"
CommentAllExportedFuncsFlag = "comment-all-exported"
CommentInterfacesFlag = "comment-interfaces"
NoTestCommentsFlag = "no-test-comments"
CommentTestsFlag = "comment-tests"
CommentStructsFlag = "comment-structs"
)

Expand All @@ -374,7 +374,7 @@ type mimic struct {
commentAllExportedFuncs bool
commentInterfaces bool
commentStructs bool
noTestComments bool
commentTests bool
}

func New() *analysis.Analyzer {
Expand Down Expand Up @@ -403,10 +403,10 @@ func New() *analysis.Analyzer {
)

fs.BoolVar(
&m.noTestComments,
NoTestCommentsFlag,
true,
"don't require comments on tests, benchmarks, examples, and fuzz tests",
&m.commentTests,
CommentTestsFlag,
false,
"require comments on tests, benchmarks, examples, and fuzz tests",
)

fs.BoolVar(
Expand Down
58 changes: 57 additions & 1 deletion pkg/commentmimic/commentmimic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ func (s *CommentMimicSuite) TestSkipTestComments() {
commentmimic.CommentAllExportedFuncsFlag: true,
commentmimic.CommentInterfacesFlag: true,
commentmimic.CommentStructsFlag: true,
commentmimic.NoTestCommentsFlag: true,
commentmimic.CommentTestsFlag: false,
}

fileMap := map[string]string{
Expand All @@ -773,6 +773,62 @@ func (s *CommentMimicSuite) TestSkipTestComments() {
analysistest.Run(t, dir, mimic, "./...")
}

func (s *CommentMimicSuite) TestTestComments() {
table := []struct {
name string
input string
flags map[string]bool
}{
{
name: "DontRequireComments",
input: testdata.SkipTestComments,
flags: map[string]bool{
commentmimic.CommentExportedFuncsFlag: true,
commentmimic.CommentAllExportedFuncsFlag: true,
commentmimic.CommentInterfacesFlag: true,
commentmimic.CommentStructsFlag: true,
commentmimic.CommentTestsFlag: false,
},
},
{
name: "RequireComments",
input: testdata.TestComments,
flags: map[string]bool{
commentmimic.CommentExportedFuncsFlag: true,
commentmimic.CommentAllExportedFuncsFlag: true,
commentmimic.CommentInterfacesFlag: true,
commentmimic.CommentStructsFlag: true,
commentmimic.CommentTestsFlag: true,
},
},
}

for _, test := range table {
test := test

s.T().Run(test.name, func(t *testing.T) {
t.Parallel()

fileMap := map[string]string{
"a/a_test.go": test.input,
}

dir, cleanup, err := analysistest.WriteFiles(fileMap)
require.NoError(t, err)

defer cleanup()

mimic := commentmimic.New()

for flag, value := range test.flags {
require.NoError(t, mimic.Flags.Set(flag, strconv.FormatBool(value)))
}

analysistest.Run(t, dir, mimic, "./...")
})
}
}

func (s *CommentMimicSuite) TestFuncCommentErrors() {
element := "element"
base := generateCommentMimicCases(element)
Expand Down
27 changes: 27 additions & 0 deletions pkg/commentmimic/testdata/out_of_scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,4 +197,31 @@ type testIface interface {
func TestDoesntNeedComment(t *testing.T) {}
`

TestComments = `package a_test
import (
"testing"
)
type BenchmarkInterface interface {} // want "exported element 'BenchmarkInterface' should be commented"
type ExampleInterface interface {} // want "exported element 'ExampleInterface' should be commented"
type FuzzInterface interface {} // want "exported element 'FuzzInterface' should be commented"
type TestInterface interface {} // want "exported element 'TestInterface' should be commented"
func BenchmarkNeedsComment(b *testing.B) {} // want "exported element 'BenchmarkNeedsComment' should be commented"
func ExportedElement() bool { // want "exported element 'ExportedElement' should be commented"
return false
}
func ExampleNeedsComment() {} // want "exported element 'ExampleNeedsComment' should be commented"
func FuzzNeedsComment(f *testing.F) {} // want "exported element 'FuzzNeedsComment' should be commented"
func TestNeedsComment(t *testing.T) {} // want "exported element 'TestNeedsComment' should be commented"
`
)
2 changes: 1 addition & 1 deletion testdata/script/no_test_comments.txtar
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
! exec commentmimic --comment-all-exported --no-test-comments not_commented_errors.go not_commented_errors_test.go
! exec commentmimic --comment-all-exported not_commented_errors.go not_commented_errors_test.go
stderr -count=4 'exported element ''Func[A-D]'' should be commented'

-- not_commented_errors.go --
Expand Down

0 comments on commit 56ffa8c

Please sign in to comment.