Skip to content

Commit

Permalink
lints: improve template_test.go (#367)
Browse files Browse the repository at this point in the history
Rather than hardcode a mapping of `LintSource` to package directory that
needs to be maintained the `template_test.go` logic should just walk the
filesystem under the `lints/` directory and check all `.go` files.

This makes a smaller `lint` API, removes two places that need to be kept
up to date with new `LintSource`s and results in a test that is robust
against further subdirectory modifications (e.g. a structure deeper than
1 package below `lints`).
  • Loading branch information
Daniel McCarney authored and zakird committed Jan 21, 2020
1 parent dbb54ce commit c0407b6
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 67 deletions.
37 changes: 0 additions & 37 deletions lint/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ type LintInterface interface {
// An Enum to programmatically represent the source of a lint
type LintSource int

// NOTE(@cpu): If you are adding a new LintSource make sure you have considered
// updating the Directory() function.
const (
UnknownLintSource LintSource = iota
CABFBaselineRequirements
Expand All @@ -62,41 +60,6 @@ const (
MozillaRootStorePolicy // https://github.com/mozilla/pkipolicy
)

// LintSources contains a list of the valid lint sources we expect to be used
// by ZLint lints.
var LintSources = []LintSource{
CABFBaselineRequirements,
CABFEVGuidelines,
RFC5280,
RFC5480,
RFC5891,
AppleCTPolicy,
EtsiEsi,
ZLint,
AWSLabs,
MozillaRootStorePolicy,
}

// Directory returns the directory name in `lints/` for the LintSource.
func (l LintSource) Directory() string {
switch l {
case CABFBaselineRequirements:
return "cabf_br"
case CABFEVGuidelines:
return "cabf_ev"
case RFC5280, RFC5480, RFC5891:
return "rfc"
case AppleCTPolicy:
return "apple"
case EtsiEsi:
return "etsi"
case MozillaRootStorePolicy:
return "mozilla"
default:
return "community"
}
}

// A Lint struct represents a single lint, e.g.
// "e_basic_constraints_not_critical". It contains an implementation of LintInterface.
type Lint struct {
Expand Down
93 changes: 63 additions & 30 deletions lints/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,25 @@ package lints

import (
"bytes"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"strings"
"testing"
)

"github.com/zmap/zlint/lint"
var (
// filesChecked is a global counter of the number of files tested by
// checkForLeftovers.
filesChecked int
)

// TestLeftoverTemplates tests that no .go files for each of the
// lint.LintSources contain leftovers from the new lint template that are
// intended to be replaced by the programmer.
func TestLeftoverTemplates(t *testing.T) {
// checkForLeftovers checks the given filename (assumed to be a .go src file)
// contains none of the template leftovers. An error is returned if there is
// a problem opening or reading the file, or if any template leftovers are
// found.
func checkForLeftovers(filename string) error {
// See the `template` file in the root directory of ZLint.
// None of these strings should appear outside of the template. They indicate
// the programmer forgot to replace template text.
Expand All @@ -25,32 +32,58 @@ func TestLeftoverTemplates(t *testing.T) {
"Change this to match source TEXT",
}

for _, lintSrc := range lint.LintSources {
files, err := ioutil.ReadDir(lintSrc.Directory())
if err != nil {
t.Fatalf("Failed to read directory %q", lintSrc.Directory())
}
src, err := ioutil.ReadFile(filename)
if err != nil {
return err
}

for _, f := range files {
// Skip non-Go files
if !strings.HasSuffix(f.Name(), ".go") {
continue
}

srcPath := filepath.Join(lintSrc.Directory(), f.Name())
src, err := ioutil.ReadFile(srcPath)
if err != nil {
t.Errorf("Failed to read src file %q: %v",
f.Name(), err)
continue
}

for _, leftover := range leftovers {
if bytes.Contains(src, []byte(leftover)) {
t.Errorf("Lint %q contains template leftover %q",
srcPath, leftover)
}
}
filesChecked++
for _, leftover := range leftovers {
if bytes.Contains(src, []byte(leftover)) {
return fmt.Errorf(
"file %q contains template leftover %q",
filename, leftover)
}
}

return nil
}

// checkFile is a filepath.WalkFunc handler that checks .go files for leftovers.
func checkFile(path string, info os.FileInfo, err error) error {
// Abort on any incoming errs from filepath.Walk
if err != nil {
return err
}
// Don't check directories
if info.IsDir() {
return nil
}
// Only check .go files
if !strings.HasSuffix(path, ".go") {
return nil
}
// Don't check the template test file, it has the strings we're checking for
// by design!
if strings.HasSuffix(path, "template_test.go") {
return nil
}

// Check the path for leftovers
return checkForLeftovers(path)
}

// TestLeftoverTemplates tests that no .go files under the current directory
// contain leftovers from the new lint template that are intended to be replaced
// by the programmer.
func TestLeftoverTemplates(t *testing.T) {
if err := filepath.Walk("./", checkFile); err != nil {
t.Errorf("%v", err)
}

// If no files were checked that means something fishy happened. Perhaps the
// test was moved to a different directory?
if filesChecked == 0 {
t.Fatalf("failed to find any files to check while traversing ./")
}
}

0 comments on commit c0407b6

Please sign in to comment.