Merge pull request #2828 from hashicorp/pr/2827

Updates testing harness.

.travis.yml

@@ -8,6 +8,6 @@ branches:
     - master
 
 script:
-  - make ci
+  - make test vet
 
 sudo: false

GNUmakefile

@@ -4,59 +4,51 @@ GOTOOLS = \
 	github.com/mitchellh/gox \
 	golang.org/x/tools/cmd/cover \
 	golang.org/x/tools/cmd/stringer
-PACKAGES=$(shell go list ./... | grep -v '/vendor/')
-VETARGS?=-asmdecl -atomic -bool -buildtags -copylocks -methods \
-         -nilfunc -printf -rangeloops -shift -structtags -unsafeptr
-BUILD_TAGS?=consul
+TEST ?= ./...
+GOTAGS ?= consul
+GOFILES ?= $(shell go list $(TEST) | grep -v /vendor/)
 
 # all builds binaries for all targets
 all: bin
 
-ci:
-	if [ "${TRAVIS_PULL_REQUEST}" = "false" ]; then \
-		$(MAKE) bin ;\
-	fi
-	@$(MAKE) test
-
 bin: tools
 	@mkdir -p bin/
-	@BUILD_TAGS='$(BUILD_TAGS)' sh -c "'$(CURDIR)/scripts/build.sh'"
+	@GOTAGS='$(GOTAGS)' sh -c "'$(CURDIR)/scripts/build.sh'"
 
 # dev creates binaries for testing locally - these are put into ./bin and $GOPATH
 dev: format
-	@CONSUL_DEV=1 BUILD_TAGS='$(BUILD_TAGS)' sh -c "'$(CURDIR)/scripts/build.sh'"
+	@CONSUL_DEV=1 GOTAGS='$(GOTAGS)' sh -c "'$(CURDIR)/scripts/build.sh'"
 
 # dist builds binaries for all platforms and packages them for distribution
 dist:
-	@BUILD_TAGS='$(BUILD_TAGS)' sh -c "'$(CURDIR)/scripts/dist.sh'"
+	@GOTAGS='$(GOTAGS)' sh -c "'$(CURDIR)/scripts/dist.sh'"
 
 cov:
-	gocov test ./... | gocov-html > /tmp/coverage.html
+	gocov test ${GOFILES} | gocov-html > /tmp/coverage.html
 	open /tmp/coverage.html
 
-test: format
-	@$(MAKE) vet
+test:
 	@./scripts/verify_no_uuid.sh
-	@BUILD_TAGS='$(BUILD_TAGS)' sh -c "'$(CURDIR)/scripts/test.sh'"
+	@env \
+		GOTAGS="${GOTAGS}" \
+		GOFILES="${GOFILES}" \
+		TESTARGS="${TESTARGS}" \
+		sh -c "'$(CURDIR)/scripts/test.sh'"
 
 cover:
-	go list ./... | xargs -n1 go test --cover
+	go test ${GOFILES} --cover
 
 format:
 	@echo "--> Running go fmt"
-	@go fmt $(PACKAGES)
+	@go fmt ${GOFILES}
 
 vet:
-	@echo "--> Running go tool vet $(VETARGS) ."
-	@go list ./... \
-		| grep -v /vendor/ \
-		| cut -d '/' -f 4- \
-		| xargs -n1 \
-			go tool vet $(VETARGS) ;\
-	if [ $$? -ne 0 ]; then \
+	@echo "--> Running go vet"
+	@go vet ${GOFILES}; if [ $$? -eq 1 ]; then \
 		echo ""; \
 		echo "Vet found suspicious constructs. Please check the reported constructs"; \
-		echo "and fix them if necessary before submitting the code for reviewal."; \
+		echo "and fix them if necessary before submitting the code for review."; \
+		exit 1; \
 	fi
 
 # build the static web ui and build static assets inside a Docker container, the

consul/flood.go

@@ -46,7 +46,6 @@ func (s *Server) Flood(portFn servers.FloodPortFn, global *serf.Serf) {
 	}()
 
 	for {
-	WAIT:
 		select {
 		case <-s.serfLAN.ShutdownCh():
 			return
@@ -60,7 +59,6 @@ func (s *Server) Flood(portFn servers.FloodPortFn, global *serf.Serf) {
 		case <-floodCh:
 			goto FLOOD
 		}
-		goto WAIT
 
 	FLOOD:
 		servers.FloodJoins(s.logger, portFn, s.config.Datacenter, s.serfLAN, global)

consul/fsm.go

@@ -309,7 +309,10 @@ func (c *consulFSM) applyTxn(buf []byte, index uint64) interface{} {
 	}
 	defer metrics.MeasureSince([]string{"consul", "fsm", "txn"}, time.Now())
 	results, errors := c.state.TxnRW(index, req.Ops)
-	return structs.TxnResponse{results, errors}
+	return structs.TxnResponse{
+		Results: results,
+		Errors:  errors,
+	}
 }
 
 func (c *consulFSM) applyAutopilotUpdate(buf []byte, index uint64) interface{} {

consul/leader_test.go

@@ -374,7 +374,7 @@ func TestLeader_Reconcile_Races(t *testing.T) {
 	// Wait for the server to reconcile the client and register it.
 	state := s1.fsm.State()
 	var nodeAddr string
-	testutil.WaitForResult(func() (bool, error) {
+	if err := testutil.WaitForResult(func() (bool, error) {
 		_, node, err := state.GetNode(c1.config.NodeName)
 		if err != nil {
 			t.Fatalf("err: %v", err)
@@ -385,9 +385,9 @@ func TestLeader_Reconcile_Races(t *testing.T) {
 		} else {
 			return false, nil
 		}
-	}, func(err error) {
-		t.Fatalf("client should be registered")
-	})
+	}); err != nil {
+		t.Fatalf("client should be registered: %v", err)
+	}
 
 	// Add in some metadata via the catalog (as if the agent synced it
 	// there). We also set the serfHealth check to failing so the reconile
@@ -428,15 +428,15 @@ func TestLeader_Reconcile_Races(t *testing.T) {
 
 	// Fail the member and wait for the health to go critical.
 	c1.Shutdown()
-	testutil.WaitForResult(func() (bool, error) {
+	if err := testutil.WaitForResult(func() (bool, error) {
 		_, checks, err := state.NodeChecks(nil, c1.config.NodeName)
 		if err != nil {
 			t.Fatalf("err: %v", err)
 		}
 		return checks[0].Status == structs.HealthCritical, errors.New(checks[0].Status)
-	}, func(err error) {
-		t.Fatalf("check status is %v, should be critical", err)
-	})
+	}); err != nil {
+		t.Fatalf("check status should be critical: %v", err)
+	}
 
 	// Make sure the metadata didn't get clobbered.
 	_, node, err = state.GetNode(c1.config.NodeName)

consul/servers/router_test.go

@@ -405,7 +405,10 @@ func TestRouter_GetDatacenterMaps(t *testing.T) {
 				Datacenter: "dc0",
 				AreaID:     types.AreaWAN,
 				Coordinates: structs.Coordinates{
-					&structs.Coordinate{"node0.dc0", lib.GenerateCoordinate(10 * time.Millisecond)},
+					&structs.Coordinate{
+						Node:  "node0.dc0",
+						Coord: lib.GenerateCoordinate(10 * time.Millisecond),
+					},
 				},
 			}) {
 				t.Fatalf("bad: %#v", entry)
@@ -415,9 +418,18 @@ func TestRouter_GetDatacenterMaps(t *testing.T) {
 				Datacenter: "dc1",
 				AreaID:     types.AreaWAN,
 				Coordinates: structs.Coordinates{
-					&structs.Coordinate{"node1.dc1", lib.GenerateCoordinate(3 * time.Millisecond)},
-					&structs.Coordinate{"node2.dc1", lib.GenerateCoordinate(2 * time.Millisecond)},
-					&structs.Coordinate{"node3.dc1", lib.GenerateCoordinate(5 * time.Millisecond)},
+					&structs.Coordinate{
+						Node:  "node1.dc1",
+						Coord: lib.GenerateCoordinate(3 * time.Millisecond),
+					},
+					&structs.Coordinate{
+						Node:  "node2.dc1",
+						Coord: lib.GenerateCoordinate(2 * time.Millisecond),
+					},
+					&structs.Coordinate{
+						Node:  "node3.dc1",
+						Coord: lib.GenerateCoordinate(5 * time.Millisecond),
+					},
 				},
 			}) {
 				t.Fatalf("bad: %#v", entry)
@@ -427,7 +439,10 @@ func TestRouter_GetDatacenterMaps(t *testing.T) {
 				Datacenter: "dc2",
 				AreaID:     types.AreaWAN,
 				Coordinates: structs.Coordinates{
-					&structs.Coordinate{"node1.dc2", lib.GenerateCoordinate(8 * time.Millisecond)},
+					&structs.Coordinate{
+						Node:  "node1.dc2",
+						Coord: lib.GenerateCoordinate(8 * time.Millisecond),
+					},
 				},
 			}) {
 				t.Fatalf("bad: %#v", entry)

consul/state/txn.go

@@ -124,7 +124,10 @@ func (s *StateStore) txnDispatch(tx *memdb.Txn, idx uint64, ops structs.TxnOps)
 		// Capture any error along with the index of the operation that
 		// failed.
 		if err != nil {
-			errors = append(errors, &structs.TxnError{i, err.Error()})
+			errors = append(errors, &structs.TxnError{
+				OpIndex: i,
+				What:    err.Error(),
+			})
 		}
 	}
 

consul/txn_endpoint.go

@@ -24,10 +24,16 @@ func (t *Txn) preCheck(acl acl.ACL, ops structs.TxnOps) structs.TxnErrors {
 		if op.KV != nil {
 			ok, err := kvsPreApply(t.srv, acl, op.KV.Verb, &op.KV.DirEnt)
 			if err != nil {
-				errors = append(errors, &structs.TxnError{i, err.Error()})
+				errors = append(errors, &structs.TxnError{
+					OpIndex: i,
+					What:    err.Error(),
+				})
 			} else if !ok {
 				err = fmt.Errorf("failed to lock key %q due to lock delay", op.KV.DirEnt.Key)
-				errors = append(errors, &structs.TxnError{i, err.Error()})
+				errors = append(errors, &structs.TxnError{
+					OpIndex: i,
+					What:    err.Error(),
+				})
 			}
 		}
 	}

consul/txn_endpoint_test.go

@@ -251,7 +251,10 @@ func TestTxn_Apply_ACLDeny(t *testing.T) {
 			// These get filtered but won't result in an error.
 
 		default:
-			expected.Errors = append(expected.Errors, &structs.TxnError{i, permissionDeniedErr.Error()})
+			expected.Errors = append(expected.Errors, &structs.TxnError{
+				OpIndex: i,
+				What:    permissionDeniedErr.Error(),
+			})
 		}
 	}
 	if !reflect.DeepEqual(out, expected) {
@@ -509,7 +512,10 @@ func TestTxn_Read_ACLDeny(t *testing.T) {
 			// These get filtered but won't result in an error.
 
 		default:
-			expected.Errors = append(expected.Errors, &structs.TxnError{i, permissionDeniedErr.Error()})
+			expected.Errors = append(expected.Errors, &structs.TxnError{
+				OpIndex: i,
+				What:    permissionDeniedErr.Error(),
+			})
 		}
 	}
 	if !reflect.DeepEqual(out, expected) {

logger/logger.go

@@ -39,7 +39,7 @@ type Config struct {
 func Setup(config *Config, ui cli.Ui) (*logutils.LevelFilter, *GatedWriter, *LogWriter, io.Writer, bool) {
 	// The gated writer buffers logs at startup and holds until it's flushed.
 	logGate := &GatedWriter{
-		Writer: &cli.UiWriter{ui},
+		Writer: &cli.UiWriter{Ui: ui},
 	}
 
 	// Set up the level filter.

scripts/test.sh

@@ -1,14 +1,24 @@
 #!/usr/bin/env bash
 set -e
 
-# Create a temp dir and clean it up on exit
-TEMPDIR=`mktemp -d -t consul-test.XXX`
-trap "rm -rf $TEMPDIR" EXIT HUP INT QUIT TERM
+if [ -n "$TRAVIS" ]; then
+  # Install all packages - this will make running the suite faster
+  echo "--> Installing packages for faster tests"
+  go install -tags="${GOTAGS}" -a ./...
+fi
 
-# Build the Consul binary for the API tests
-echo "--> Building consul"
-go build -tags="${BUILD_TAGS}" -o $TEMPDIR/consul || exit 1
+# If we are testing the API, build and install consul
+if grep -q "/consul/api" <<< "${GOFILES}"; then
+  # Create a temp dir and clean it up on exit
+  TEMPDIR=`mktemp -d -t consul-test.XXX`
+  trap "rm -rf ${TEMPDIR}" EXIT HUP INT QUIT TERM
+
+  # Build the Consul binary for the API tests
+  echo "--> Building consul"
+  go build -tags="${GOTAGS}" -o $TEMPDIR/consul
+  PATH="${TEMPDIR}:${PATH}"
+fi
 
 # Run the tests
 echo "--> Running tests"
-go list ./... | grep -v '/vendor/' | PATH=$TEMPDIR:$PATH xargs -n1 go test -tags="${BUILD_TAGS}" ${GOTEST_FLAGS:--cover -timeout=360s}
+go test -timeout=360s -parallel=20 -tags="${GOTAGS}" ${GOFILES} ${TESTARGS}