Set up golangci config and fix linter messages.

Simplify test code with helpers where useful.

.golangci.yml

@@ -2,21 +2,47 @@ service:
   prepare:
     - make install_deps
 
+run:
+  tests: true
+  build-tags:
+    - integration
+  skip-files:
+    - internal/vm/parser/parser.go
+
 linters-settings:
   govet:
     check-shadowing: true
 
+linters:
+  enable-all: true
+  disable:
+    - maligned
+    - megacheck
+    - lll
+    - gocyclo
+    - unparam
+    # Not sure what this is telling me yet.
+    - scopelint
+    # How dare you tell me not to use inits.
+    - gochecknoinits
+    # Flags are fine, as are test tables.
+    - gochecknoglobals
+
 issues:
   max-per-linter: 0
   max-same: 0
-  exclude-use-default: false
+  exclude-use-default: true
   exclude:
-    # Captured by errcheck.
-    - '^(G104|G204):'
-    # Very commonly not checked.
-    - 'Error return value of .(.*\.Help|.*\.MarkFlagRequired|(os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked'
-    #- 'exported method (.*\.MarshalJSON|.*\.UnmarshalJSON|.*\.EntityURN|.*\.GoString|.*\.Pos) should have comment or be unexported'
-    - 'composite literal uses unkeyed fields'
-    - 'declaration of "err" shadows declaration'
-    #- 'bad syntax for struct tag key'
-    #- 'bad syntax for struct tag pair'
+  #   # Captured by errcheck.
+  #   - '^(G104|G204):'
+  #   # Very commonly not checked.
+  #   - 'Error return value of .(.*\.Help|.*\.MarkFlagRequired|(os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked'
+  #   #- 'exported method (.*\.MarshalJSON|.*\.UnmarshalJSON|.*\.EntityURN|.*\.GoString|.*\.Pos) should have comment or be unexported'
+     # If you liked it you shoulda put a gofix on it.
+     - 'composite literal uses unkeyed fields'
+     # I like shadowing err
+     - 'declaration of "err" shadows declaration'
+  #   #- 'bad syntax for struct tag key'
+  #   #- 'bad syntax for struct tag pair'
+     # goyacc generated error in three locations
+     - 'this value of `mtailDollar.* is never used'

Makefile

@@ -93,6 +93,10 @@ covclean:
 crossclean:
 	rm -rf build
 
+.PHONY: lint
+lint:
+	golangci-lint run ./...
+
 version := $(shell git describe --tags --always --dirty)
 revision := $(shell git rev-parse HEAD)
 release := $(shell git describe --tags | cut -d"-" -f 1,2)

internal/exporter/export_test.go

@@ -35,7 +35,8 @@ func TestCreateExporter(t *testing.T) {
 }
 
 func FakeSocketWrite(f formatter, m *metrics.Metric) []string {
-	var ret []string
+	// TODO(jaq): urgh looking inside m to find preallocation size
+	ret := make([]string, 0, len(m.LabelValues))
 	lc := make(chan *metrics.LabelSet)
 	go m.EmitLabelSets(lc)
 	for l := range lc {
@@ -55,7 +56,7 @@ func TestMetricToCollectd(t *testing.T) {
 	scalarMetric := metrics.NewMetric("foo", "prog", metrics.Counter, metrics.Int)
 	d, _ := scalarMetric.GetDatum()
 	datum.SetInt(d, 37, ts)
-	ms.Add(scalarMetric)
+	testutil.FatalIfErr(t, ms.Add(scalarMetric))
 
 	r := FakeSocketWrite(metricToCollectd, scalarMetric)
 	expected := []string{"PUTVAL \"gunstar/mtail-prog/counter-foo\" interval=60 1343124840:37\n"}
@@ -70,7 +71,7 @@ func TestMetricToCollectd(t *testing.T) {
 	d, _ = dimensionedMetric.GetDatum("snuh")
 	datum.SetInt(d, 37, ts)
 	ms.ClearMetrics()
-	ms.Add(dimensionedMetric)
+	testutil.FatalIfErr(t, ms.Add(dimensionedMetric))
 
 	r = FakeSocketWrite(metricToCollectd, dimensionedMetric)
 	expected = []string{
@@ -84,7 +85,7 @@ func TestMetricToCollectd(t *testing.T) {
 	timingMetric := metrics.NewMetric("foo", "prog", metrics.Timer, metrics.Int)
 	d, _ = timingMetric.GetDatum()
 	datum.SetInt(d, 123, ts)
-	ms.Add(timingMetric)
+	testutil.FatalIfErr(t, ms.Add(timingMetric))
 
 	r = FakeSocketWrite(metricToCollectd, timingMetric)
 	expected = []string{"PUTVAL \"gunstar/mtail-prog/gauge-foo\" interval=60 1343124840:123\n"}

internal/exporter/json_test.go

@@ -95,7 +95,7 @@ func TestHandleJSON(t *testing.T) {
 			t.Parallel()
 			ms := metrics.NewStore()
 			for _, metric := range tc.metrics {
-				ms.Add(metric)
+				testutil.FatalIfErr(t, ms.Add(metric))
 			}
 			e, err := New(ms, Hostname("gunstar"))
 			if err != nil {

internal/exporter/prometheus.go

@@ -70,7 +70,7 @@ func (e *Exporter) HandlePrometheusMetrics(w http.ResponseWriter, r *http.Reques
 }
 
 func metricToPrometheus(m *metrics.Metric, l *metrics.LabelSet, omitProgLabel bool) string {
-	var s []string
+	s := make([]string, 0, len(l.Labels)+1)
 	for k, v := range l.Labels {
 		// Prometheus quotes the value of each label=value pair.
 		s = append(s, fmt.Sprintf("%s=%q", k, v))

internal/exporter/prometheus_test.go

@@ -194,7 +194,7 @@ func TestHandlePrometheus(t *testing.T) {
 			t.Parallel()
 			ms := metrics.NewStore()
 			for _, metric := range tc.metrics {
-				ms.Add(metric)
+				testutil.FatalIfErr(t, ms.Add(metric))
 			}
 			opts := []func(*Exporter) error{
 				Hostname("gunstar"),

internal/exporter/varz.go

@@ -42,7 +42,7 @@ func (e *Exporter) HandleVarz(w http.ResponseWriter, r *http.Request) {
 }
 
 func metricToVarz(m *metrics.Metric, l *metrics.LabelSet, omitProgLabel bool, hostname string) string {
-	var s []string
+	s := make([]string, 0, len(l.Labels)+2)
 	for k, v := range l.Labels {
 		s = append(s, fmt.Sprintf("%s=%s", k, v))
 	}

internal/exporter/varz_test.go

@@ -70,7 +70,7 @@ func TestHandleVarz(t *testing.T) {
 			t.Parallel()
 			ms := metrics.NewStore()
 			for _, metric := range tc.metrics {
-				ms.Add(metric)
+				testutil.FatalIfErr(t, ms.Add(metric))
 			}
 			e, err := New(ms, Hostname("gunstar"))
 			if err != nil {

internal/metrics/store.go

@@ -108,7 +108,7 @@ func (s *Store) Gc() error {
 	now := time.Now()
 	for _, ml := range s.Metrics {
 		for _, m := range ml {
-			for _, lv := range m.LabelValues[:] {
+			for _, lv := range m.LabelValues {
 				if lv.Expiry <= 0 {
 					continue
 				}

internal/metrics/store_test.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/golang/glog"
 	"github.com/google/mtail/internal/metrics/datum"
+	"github.com/google/mtail/internal/testutil"
 )
 
 func TestMatchingKind(t *testing.T) {
@@ -98,7 +99,7 @@ func TestAddMetricDifferentType(t *testing.T) {
 func TestExpireMetric(t *testing.T) {
 	s := NewStore()
 	m := NewMetric("foo", "prog", Counter, Int, "a", "b", "c")
-	s.Add(m)
+	testutil.FatalIfErr(t, s.Add(m))
 	d, err := m.GetDatum("1", "2", "3")
 	if err != nil {
 		t.Error(err)
@@ -119,7 +120,7 @@ func TestExpireMetric(t *testing.T) {
 		t.Errorf("couldn't find lv")
 	}
 
-	s.Gc()
+	testutil.FatalIfErr(t, s.Gc())
 	lv = m.FindLabelValueOrNil([]string{"1", "2", "3"})
 	if lv != nil {
 		t.Errorf("lv not expired: %#v", lv)

internal/mtail/benchmarks_test.go

@@ -7,7 +7,6 @@
 package mtail_test
 
 import (
-	"flag"
 	"fmt"
 	"io"
 	"os"
@@ -20,10 +19,6 @@ import (
 	"github.com/google/mtail/internal/watcher"
 )
 
-var (
-	recordBenchmark = flag.Bool("record_benchmark", false, "Record the benchmark results to 'benchmark_results.csv'.")
-)
-
 func BenchmarkProgram(b *testing.B) {
 	// exampleProgramTests live in ex_test.go
 	for _, bm := range exampleProgramTests {

internal/mtail/log_rotation_integration_test.go

@@ -10,7 +10,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/golang/glog"
 	"github.com/google/mtail/internal/mtail"
 	"github.com/google/mtail/internal/testutil"
 )
@@ -38,22 +37,14 @@ func TestLogRotation(t *testing.T) {
 	defer stopM()
 
 	{
-		n, err := f.WriteString("line 1\n")
-		if err != nil {
-			t.Fatal(err)
-		}
-		glog.Infof("Wrote %d bytes", n)
+		testutil.WriteString(t, f, "line 1\n")
 		time.Sleep(time.Second)
 	}
 	startLogLinesTotal := mtail.TestGetMetric(t, m.Addr(), "log_lines_total").(map[string]interface{})[logFile]
 
 	{
 
-		n, err := f.WriteString("line 2\n")
-		if err != nil {
-			t.Fatal(err)
-		}
-		glog.Infof("Wrote %d bytes", n)
+		testutil.WriteString(t, f, "line 2\n")
 		time.Sleep(time.Second)
 
 		logLinesTotal := mtail.TestGetMetric(t, m.Addr(), "log_lines_total").(map[string]interface{})[logFile]
@@ -69,11 +60,7 @@ func TestLogRotation(t *testing.T) {
 	f = testutil.TestOpenFile(t, logFile)
 
 	{
-		n, err := f.WriteString("line 1\n")
-		if err != nil {
-			t.Fatal(err)
-		}
-		glog.Infof("Wrote %d bytes", n)
+		testutil.WriteString(t, f, "line 1\n")
 		time.Sleep(time.Second)
 
 		logLinesTotal := mtail.TestGetMetric(t, m.Addr(), "log_lines_total").(map[string]interface{})[logFile]

internal/mtail/mtail_test.go

@@ -132,7 +132,7 @@ func TestHandleLogUpdates(t *testing.T) {
 	inputLines := []string{"hi", "hi2", "hi3"}
 	for i, x := range inputLines {
 		// write to log file
-		logFile.WriteString(x + "\n")
+		testutil.WriteString(t, logFile, x+"\n")
 		// check log line count increase
 		expected := fmt.Sprintf("%d", i+1)
 		check := func() (bool, error) {
@@ -207,9 +207,7 @@ Loop:
 			if i >= 10 {
 				break Loop
 			}
-			if _, werr := logFile.WriteString(fmt.Sprintf("%d\n", i)); werr != nil {
-				t.Fatal(werr)
-			}
+			testutil.WriteString(t, logFile, fmt.Sprintf("%d\n", i))
 			i++
 		}
 	}
@@ -254,9 +252,7 @@ func TestHandleNewLogAfterStart(t *testing.T) {
 		t.Errorf("could not touch log file: %s", err)
 	}
 	defer logFile.Close()
-	if _, werr := logFile.WriteString("a\n"); werr != nil {
-		t.Error(werr)
-	}
+	testutil.WriteString(t, logFile, "a\n")
 
 	expectedLogCount := "1"
 	expectedLineCount := "1"
@@ -326,8 +322,8 @@ func TestHandleSoftLinkChange(t *testing.T) {
 	}
 	inputLines := []string{"hi1", "hi2", "hi3"}
 	for _, x := range inputLines {
-		trueLog1.WriteString(x + "\n")
-		trueLog1.Sync()
+		testutil.WriteString(t, trueLog1, x+"\n")
+		testutil.FatalIfErr(t, trueLog1.Sync())
 	}
 	check3 := func() (bool, error) {
 		if expvar.Get("log_count").String() != "1" {
@@ -360,8 +356,8 @@ func TestHandleSoftLinkChange(t *testing.T) {
 		t.Errorf("could not create symlink: %s", err)
 	}
 	for _, x := range inputLines {
-		trueLog2.WriteString(x + "\n")
-		trueLog2.Sync()
+		testutil.WriteString(t, trueLog2, x+"\n")
+		testutil.FatalIfErr(t, trueLog2.Sync())
 	}
 	check6 := func() (bool, error) {
 		if expvar.Get("log_count").String() != "1" {
@@ -433,8 +429,9 @@ func TestGlob(t *testing.T) {
 		if tt.expected {
 			count++
 		}
-		log.WriteString("\n")
-		log.Sync()
+		testutil.WriteString(t, log, "\n")
+		testutil.FatalIfErr(t, err)
+		testutil.FatalIfErr(t, log.Sync())
 	}
 	m := startMtailServer(t, LogPathPatterns(path.Join(workdir, "log*")))
 	defer m.Close()
@@ -494,8 +491,8 @@ func TestGlobAfterStart(t *testing.T) {
 		if tt.expected {
 			count++
 		}
-		log.WriteString("\n")
-		log.Sync()
+		testutil.WriteString(t, log, "\n")
+		testutil.FatalIfErr(t, log.Sync())
 	}
 	glog.Infof("count is %d", count)
 	check := func() (bool, error) {
@@ -574,7 +571,7 @@ func TestHandleLogTruncate(t *testing.T) {
 		}
 	}()
 
-	logFile.WriteString("x\n")
+	testutil.WriteString(t, logFile, "x\n")
 	glog.Info("Write")
 	check := func() (bool, error) {
 		if expvar.Get("line_count").String() != "1" {
@@ -589,9 +586,9 @@ func TestHandleLogTruncate(t *testing.T) {
 	if !ok {
 		t.Errorf("log line count received %s, expected 1", expvar.Get("log_count").String())
 	}
-	logFile.Truncate(0)
+	testutil.FatalIfErr(t, logFile.Truncate(0))
 	glog.Infof("Truncate")
-	logFile.WriteString("x\n")
+	testutil.WriteString(t, logFile, "x\n")
 	glog.Info("Write")
 	check2 := func() (bool, error) {
 		if expvar.Get("line_count").String() != "2" {
@@ -643,7 +640,7 @@ func TestHandleRelativeLogAppend(t *testing.T) {
 	inputLines := []string{"hi", "hi2", "hi3"}
 	for i, x := range inputLines {
 		// write to log file
-		logFile.WriteString(x + "\n")
+		testutil.WriteString(t, logFile, x+"\n")
 		// check log line count increase
 		expected := fmt.Sprintf("%d", i+1)
 		check := func() (bool, error) {
@@ -704,8 +701,8 @@ func TestProgramReloadNoDuplicateMetrics(t *testing.T) {
 	if err != nil {
 		t.Fatalf("couldn't open program file: %s", err)
 	}
-	p.WriteString("counter foo\n/^foo$/ {\n foo++\n }\n")
-	p.Close()
+	testutil.WriteString(t, p, "counter foo\n/^foo$/ {\n foo++\n }\n")
+	testutil.FatalIfErr(t, p.Close())
 
 	check := func() (bool, error) {
 		v := expvar.Get("prog_loads_total").(*expvar.Map).Get("program.mtail")
@@ -766,8 +763,8 @@ func TestProgramReloadNoDuplicateMetrics(t *testing.T) {
 	if err != nil {
 		t.Fatalf("couldn't open program file: %s", err)
 	}
-	p.WriteString("counter foo\n/^foo$/ {\n foo++\n }\n")
-	p.Close()
+	testutil.WriteString(t, p, "counter foo\n/^foo$/ {\n foo++\n }\n")
+	testutil.FatalIfErr(t, p.Close())
 
 	check2 := func() (bool, error) {
 		v := expvar.Get("prog_loads_total")