Remove all shell presubmits (#3631)

This ports the shell presubmit to Go and makes them better; esp the
Errorf/Logf/Fatalf one because it actually parses the code and works of
the AST.

These error will now show up in the travis/circle CI runs where they
belong.

Signed-off-by: Miek Gieben <miek@miek.nl>
This commit is contained in:
Miek Gieben
2020-01-28 19:07:11 +00:00
committed by GitHub
parent 40d0fd8598
commit f66c2bac25
4 changed files with 134 additions and 35 deletions

View File

@@ -1,9 +0,0 @@
#!/usr/bin/env bash
echo "** presubmit/$(basename $0)"
# Check to make sure "testing" is not imported
if [[ $(go list -f '{{ join .Deps " "}}') == *" testing "* ]]; then
echo "** presubmit/$(basename $0): please remove any 'import \"testing\"'"
exit 1
fi

View File

@@ -1,19 +0,0 @@
#!/usr/bin/env bash
echo "** presubmit/$(basename $0)"
# Get the tests that call t.* without capitalizing the first char - seems we standardized on that.
if egrep -r '\bt\.Fatal.?\("[a-z]' "$@"; then
echo "** presubmit/$(basename $0): please start with an upper case letter when using t.Fatal*()"
exit 1
fi
if egrep -r '\bt\.Error.?\("[a-z]' "$@"; then
echo "** presubmit/$(basename $0): please start with an upper case letter when using t.Error*()"
exit 1
fi
if egrep -r '\bt\.Log.?\("[a-z]' "$@"; then
echo "** presubmit/$(basename $0): please start with an upper case letter when using t.Log*()"
exit 1
fi

View File

@@ -5,7 +5,6 @@ SYSTEM:=
CHECKS:=check
BUILDOPTS:=-v
GOPATH?=$(HOME)/go
PRESUBMIT:=core coremain plugin test request
MAKEPWD:=$(dir $(realpath $(firstword $(MAKEFILE_LIST))))
CGO_ENABLED:=0
@@ -17,7 +16,7 @@ coredns: $(CHECKS)
CGO_ENABLED=$(CGO_ENABLED) $(SYSTEM) go build $(BUILDOPTS) -ldflags="-s -w -X github.com/coredns/coredns/coremain.GitCommit=$(GITCOMMIT)" -o $(BINARY)
.PHONY: check
check: presubmit core/plugin/zplugin.go core/dnsserver/zdirectives.go
check: core/plugin/zplugin.go core/dnsserver/zdirectives.go
.PHONY: travis
travis:
@@ -69,11 +68,6 @@ gen:
pb:
$(MAKE) -C pb
# Presubmit runs all scripts in .presubmit; any non 0 exit code will fail the build.
.PHONY: presubmit
presubmit:
@for pre in $(MAKEPWD)/.presubmit/* ; do "$$pre" $(PRESUBMIT) || exit 1 ; done
.PHONY: clean
clean:
go clean

View File

@@ -5,6 +5,9 @@ package test
import (
"bufio"
"fmt"
"go/ast"
"go/parser"
"go/token"
"os"
"path/filepath"
"strings"
@@ -75,3 +78,133 @@ func hasHyphen(path string, info os.FileInfo, _ error) error {
return nil
}
// Test if error messages start with an upper case.
func TestLowercaseLog(t *testing.T) {
err := filepath.Walk("..", hasLowercase)
if err != nil {
t.Fatal(err)
}
}
func hasLowercase(path string, info os.FileInfo, _ error) error {
// only for regular files, not starting with a . and those that are go files.
if !info.Mode().IsRegular() {
return nil
}
if strings.HasPrefix(path, "../.") {
return nil
}
if !strings.HasSuffix(path, "_test.go") {
return nil
}
fs := token.NewFileSet()
f, err := parser.ParseFile(fs, path, nil, parser.AllErrors)
if err != nil {
return err
}
l := &logfmt{}
ast.Walk(l, f)
if l.err != nil {
return l.err
}
return nil
}
type logfmt struct {
err error
}
func (l logfmt) Visit(n ast.Node) ast.Visitor {
if n == nil {
return nil
}
ce, ok := n.(*ast.CallExpr)
if !ok {
return l
}
se, ok := ce.Fun.(*ast.SelectorExpr)
if !ok {
return l
}
id, ok := se.X.(*ast.Ident)
if !ok {
return l
}
if id.Name != "t" { //t *testing.T
return l
}
switch se.Sel.Name {
case "Errorf":
case "Logf":
case "Log":
case "Fatalf":
case "Fatal":
default:
return l
}
// Check first arg, that should have basic lit with capital
if len(ce.Args) < 1 {
return l
}
bl, ok := ce.Args[0].(*ast.BasicLit)
if !ok {
return l
}
if bl.Kind != token.STRING {
return l
}
if strings.HasPrefix(bl.Value, "\"%s") || strings.HasPrefix(bl.Value, "\"%d") {
return l
}
if strings.HasPrefix(bl.Value, "\"%v") || strings.HasPrefix(bl.Value, "\"%+v") {
return l
}
for i, u := range bl.Value {
// disregard "
if i == 1 && !unicode.IsUpper(u) {
l.err = fmt.Errorf("test error message %s doesn't start with an uppercase", bl.Value)
return nil
}
if i == 1 {
break
}
}
return l
}
func TestImportTesting(t *testing.T) {
err := filepath.Walk("..", hasImportTesting)
if err != nil {
t.Fatal(err)
}
}
func hasImportTesting(path string, info os.FileInfo, _ error) error {
// only for regular files, not starting with a . and those that are go files.
if !info.Mode().IsRegular() {
return nil
}
if strings.HasPrefix(path, "../.") {
return nil
}
if strings.HasSuffix(path, "_test.go") {
return nil
}
if strings.HasSuffix(path, ".go") {
fs := token.NewFileSet()
f, err := parser.ParseFile(fs, path, nil, parser.AllErrors)
if err != nil {
return err
}
for _, im := range f.Imports {
if im.Path.Value == `"testing"` {
return fmt.Errorf("file %q is importing %q", path, "testing")
}
}
}
return nil
}