VAULT-13169 Require go docs for all new test functions (#18971)
* example for checking go doc tests * add analyzer test and action * get metadata step * install revgrep * fix for ci * add revgrep to go.mod * clarify how analysistest works
This commit is contained in:
parent
79f6ff191c
commit
7cd5685e2e
27
.github/workflows/godoc-test-checker.yml
vendored
Normal file
27
.github/workflows/godoc-test-checker.yml
vendored
Normal file
@ -0,0 +1,27 @@
|
||||
name: Check Go Docs for tests
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, synchronize]
|
||||
# Runs on PRs to main
|
||||
branches:
|
||||
- main
|
||||
|
||||
jobs:
|
||||
godoc-test-check:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v3
|
||||
with:
|
||||
ref: ${{ github.event.pull_request.head.sha }}
|
||||
fetch-depth: 0
|
||||
- name: get metadata
|
||||
id: get-metadata
|
||||
run: echo "go-version=$(cat ./.go-version)" >> $GITHUB_OUTPUT
|
||||
- name: Set Up Go
|
||||
uses: actions/setup-go@v3
|
||||
with:
|
||||
cache: true
|
||||
go-version: ${{ steps.get-metadata.outputs.go-version }}
|
||||
- name: Verify new tests have go docs
|
||||
run: make ci-vet-godoctests
|
2
.gitignore
vendored
2
.gitignore
vendored
@ -126,3 +126,5 @@ website/components/node_modules
|
||||
.buildcache/
|
||||
.releaser/
|
||||
*.log
|
||||
|
||||
tools/godoctests/.bin
|
20
Makefile
20
Makefile
@ -8,13 +8,13 @@ EXTENDED_TEST_TIMEOUT=60m
|
||||
INTEG_TEST_TIMEOUT=120m
|
||||
VETARGS?=-asmdecl -atomic -bool -buildtags -copylocks -methods -nilfunc -printf -rangeloops -shift -structtags -unsafeptr
|
||||
EXTERNAL_TOOLS_CI=\
|
||||
golang.org/x/tools/cmd/goimports
|
||||
golang.org/x/tools/cmd/goimports \
|
||||
github.com/golangci/revgrep/cmd/revgrep
|
||||
EXTERNAL_TOOLS=\
|
||||
github.com/client9/misspell/cmd/misspell
|
||||
GOFMT_FILES?=$$(find . -name '*.go' | grep -v pb.go | grep -v vendor)
|
||||
SED?=$(shell command -v gsed || command -v sed)
|
||||
|
||||
|
||||
GO_VERSION_MIN=$$(cat $(CURDIR)/.go-version)
|
||||
PROTOC_VERSION_MIN=3.21.12
|
||||
GO_CMD?=go
|
||||
@ -102,6 +102,20 @@ vet:
|
||||
echo "and fix them if necessary before submitting the code for reviewal."; \
|
||||
fi
|
||||
|
||||
# tools/godoctests/.bin/godoctests builds the custom analyzer to check for godocs for tests
|
||||
tools/godoctests/.bin/godoctests:
|
||||
@cd tools/godoctests && $(GO_CMD) build -o .bin/godoctests .
|
||||
|
||||
# vet-godoctests runs godoctests on the test functions. All output gets piped to revgrep
|
||||
# which will only return an error if a new function is missing a godoc
|
||||
vet-godoctests: bootstrap tools/godoctests/.bin/godoctests
|
||||
@$(GO_CMD) vet -vettool=./tools/godoctests/.bin/godoctests $(TEST) 2>&1 | revgrep
|
||||
|
||||
# ci-vet-godoctests runs godoctests on the test functions. All output gets piped to revgrep
|
||||
# which will only return an error if a new function that is not on main is missing a godoc
|
||||
ci-vet-godoctests: ci-bootstrap tools/godoctests/.bin/godoctests
|
||||
@$(GO_CMD) vet -vettool=./tools/godoctests/.bin/godoctests $(TEST) 2>&1 | revgrep origin/main
|
||||
|
||||
# lint runs vet plus a number of other checkers, it is more comprehensive, but louder
|
||||
lint:
|
||||
@$(GO_CMD) list -f '{{.Dir}}' ./... | grep -v /vendor/ \
|
||||
@ -250,7 +264,7 @@ ci-config:
|
||||
ci-verify:
|
||||
@$(MAKE) -C .circleci ci-verify
|
||||
|
||||
.PHONY: bin default prep test vet bootstrap ci-bootstrap fmt fmtcheck mysql-database-plugin mysql-legacy-database-plugin cassandra-database-plugin influxdb-database-plugin postgresql-database-plugin mssql-database-plugin hana-database-plugin mongodb-database-plugin ember-dist ember-dist-dev static-dist static-dist-dev assetcheck check-vault-in-path packages build build-ci semgrep semgrep-ci
|
||||
.PHONY: bin default prep test vet bootstrap ci-bootstrap fmt fmtcheck mysql-database-plugin mysql-legacy-database-plugin cassandra-database-plugin influxdb-database-plugin postgresql-database-plugin mssql-database-plugin hana-database-plugin mongodb-database-plugin ember-dist ember-dist-dev static-dist static-dist-dev assetcheck check-vault-in-path packages build build-ci semgrep semgrep-ci vet-godoctests ci-vet-godoctests
|
||||
|
||||
.NOTPARALLEL: ember-dist ember-dist-dev
|
||||
|
||||
|
1
go.mod
1
go.mod
@ -314,6 +314,7 @@ require (
|
||||
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
|
||||
github.com/golang/mock v1.6.0 // indirect
|
||||
github.com/golang/snappy v0.0.4 // indirect
|
||||
github.com/golangci/revgrep v0.0.0-20220804021717-745bb2f7c2e6 // indirect
|
||||
github.com/google/flatbuffers v2.0.0+incompatible // indirect
|
||||
github.com/google/gnostic v0.5.7-v3refs // indirect
|
||||
github.com/google/go-querystring v1.1.0 // indirect
|
||||
|
2
go.sum
2
go.sum
@ -844,6 +844,8 @@ github.com/golang/snappy v0.0.2/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEW
|
||||
github.com/golang/snappy v0.0.3/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
|
||||
github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM=
|
||||
github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
|
||||
github.com/golangci/revgrep v0.0.0-20220804021717-745bb2f7c2e6 h1:DIPQnGy2Gv2FSA4B/hh8Q7xx3B7AIDk3DAMeHclH1vQ=
|
||||
github.com/golangci/revgrep v0.0.0-20220804021717-745bb2f7c2e6/go.mod h1:0AKcRCkMoKvUvlf89F6O7H2LYdhr1zBh736mBItOdRs=
|
||||
github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
|
||||
github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
|
||||
github.com/google/btree v1.0.1 h1:gK4Kx5IaGY9CD5sPJ36FHiBJ6ZXl0kilRiiCj+jdYp4=
|
||||
|
10
tools/godoctests/main.go
Normal file
10
tools/godoctests/main.go
Normal file
@ -0,0 +1,10 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"github.com/hashicorp/vault/tools/godoctests/pkg/analyzer"
|
||||
"golang.org/x/tools/go/analysis/singlechecker"
|
||||
)
|
||||
|
||||
func main() {
|
||||
singlechecker.Main(analyzer.Analyzer)
|
||||
}
|
78
tools/godoctests/pkg/analyzer/analyzer.go
Normal file
78
tools/godoctests/pkg/analyzer/analyzer.go
Normal file
@ -0,0 +1,78 @@
|
||||
package analyzer
|
||||
|
||||
import (
|
||||
"go/ast"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
"golang.org/x/tools/go/analysis/passes/inspect"
|
||||
"golang.org/x/tools/go/ast/inspector"
|
||||
)
|
||||
|
||||
var Analyzer = &analysis.Analyzer{
|
||||
Name: "godoctests",
|
||||
Doc: "Verifies that every go test has a go doc",
|
||||
Run: run,
|
||||
Requires: []*analysis.Analyzer{inspect.Analyzer},
|
||||
}
|
||||
|
||||
func run(pass *analysis.Pass) (interface{}, error) {
|
||||
inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
|
||||
|
||||
nodeFilter := []ast.Node{
|
||||
(*ast.FuncDecl)(nil),
|
||||
}
|
||||
|
||||
inspector.Preorder(nodeFilter, func(node ast.Node) {
|
||||
funcDecl, ok := node.(*ast.FuncDecl)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
|
||||
// starts with 'Test'
|
||||
if !strings.HasPrefix(funcDecl.Name.Name, "Test") {
|
||||
return
|
||||
}
|
||||
|
||||
// has one parameter
|
||||
params := funcDecl.Type.Params.List
|
||||
if len(params) != 1 {
|
||||
return
|
||||
}
|
||||
|
||||
// parameter is a pointer
|
||||
firstParamType, ok := params[0].Type.(*ast.StarExpr)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
|
||||
selector, ok := firstParamType.X.(*ast.SelectorExpr)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
|
||||
// the pointer comes from package 'testing'
|
||||
selectorIdent, ok := selector.X.(*ast.Ident)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
if selectorIdent.Name != "testing" {
|
||||
return
|
||||
}
|
||||
|
||||
// the pointer has type 'T'
|
||||
if selector.Sel == nil || selector.Sel.Name != "T" {
|
||||
return
|
||||
}
|
||||
|
||||
// then there must be a godoc
|
||||
if funcDecl.Doc == nil {
|
||||
pass.Reportf(node.Pos(), "Test %s is missing a go doc",
|
||||
funcDecl.Name.Name)
|
||||
} else if !strings.HasPrefix(funcDecl.Doc.Text(), funcDecl.Name.Name) {
|
||||
pass.Reportf(node.Pos(), "Test %s must have a go doc beginning with the function name",
|
||||
funcDecl.Name.Name)
|
||||
}
|
||||
})
|
||||
return nil, nil
|
||||
}
|
20
tools/godoctests/pkg/analyzer/analyzer_test.go
Normal file
20
tools/godoctests/pkg/analyzer/analyzer_test.go
Normal file
@ -0,0 +1,20 @@
|
||||
package analyzer
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"golang.org/x/tools/go/analysis/analysistest"
|
||||
)
|
||||
|
||||
// TestAnalyzer runs the analyzer on the test functions in testdata/funcs.go. The report from the analyzer is compared against
|
||||
// the comments in funcs.go beginning with "want." If there is no comment beginning with "want", then the analyzer is expected
|
||||
// not to report anything.
|
||||
func TestAnalyzer(t *testing.T) {
|
||||
f, err := os.Getwd()
|
||||
if err != nil {
|
||||
t.Fatal("failed to get working directory", err)
|
||||
}
|
||||
analysistest.Run(t, filepath.Join(f, "testdata"), Analyzer, ".")
|
||||
}
|
17
tools/godoctests/pkg/analyzer/testdata/funcs.go
vendored
Normal file
17
tools/godoctests/pkg/analyzer/testdata/funcs.go
vendored
Normal file
@ -0,0 +1,17 @@
|
||||
package testdata
|
||||
|
||||
import "testing"
|
||||
|
||||
// Test_GoDocOK is a test that has a go doc
|
||||
func Test_GoDocOK(t *testing.T) {}
|
||||
|
||||
func Test_NoGoDocFails(t *testing.T) {} // want "Test Test_NoGoDocFails is missing a go doc"
|
||||
|
||||
// This test does not have a go doc beginning with the function name
|
||||
func Test_BadGoDocFails(t *testing.T) {} // want "Test Test_BadGoDocFails must have a go doc beginning with the function name"
|
||||
|
||||
func test_TestHelperNoGoDocOK(t *testing.T) {}
|
||||
|
||||
func Test_DifferentSignatureNoGoDocOK() {}
|
||||
|
||||
func Test_DifferentSignature2NoGoDocOK(t *testing.T, a int) {}
|
@ -16,6 +16,7 @@ package tools
|
||||
//go:generate go install google.golang.org/protobuf/cmd/protoc-gen-go
|
||||
//go:generate go install google.golang.org/grpc/cmd/protoc-gen-go-grpc
|
||||
//go:generate go install github.com/favadi/protoc-go-inject-tag
|
||||
//go:generate go install github.com/golangci/revgrep/cmd/revgrep
|
||||
import (
|
||||
_ "golang.org/x/tools/cmd/goimports"
|
||||
|
||||
@ -28,4 +29,6 @@ import (
|
||||
_ "google.golang.org/grpc/cmd/protoc-gen-go-grpc"
|
||||
|
||||
_ "github.com/favadi/protoc-go-inject-tag"
|
||||
|
||||
_ "github.com/golangci/revgrep/cmd/revgrep"
|
||||
)
|
||||
|
Loading…
Reference in New Issue
Block a user