Merge pull request #3020 from hashicorp/test-tweaks

Cleans up vet errors and tweaks unit tests that were failing in Travis CI.

api/catalog_test.go

@@ -17,7 +17,7 @@ func TestCatalog_Datacenters(t *testing.T) {
 	retry.Run(t, func(r *retry.R) {
 		datacenters, err := catalog.Datacenters()
 		if err != nil {
-			r.Fatalf("catalog.Datacenters: ", err)
+			r.Fatal(err)
 		}
 		if len(datacenters) < 1 {
 			r.Fatal("got 0 datacenters want at least one")
@@ -33,7 +33,7 @@ func TestCatalog_Nodes(t *testing.T) {
 	retry.RunWith(retry.ThreeTimes(), t, func(r *retry.R) {
 		nodes, meta, err := catalog.Nodes(nil)
 		if err != nil {
-			r.Fatalf("catalog.Nodes: ", err)
+			r.Fatal(err)
 		}
 		if meta.LastIndex == 0 {
 			r.Fatal("got last index 0 want > 0")

command/agent/dns_test.go

@@ -1306,10 +1306,15 @@ func TestDNS_ServiceLookup_WanAddress(t *testing.T) {
 		if got, want := len(srv1.agent.WANMembers()), 2; got < want {
 			r.Fatalf("got %d WAN members want at least %d", got, want)
 		}
+		if got, want := len(srv2.agent.WANMembers()), 2; got < want {
+			r.Fatalf("got %d WAN members want at least %d", got, want)
+		}
 	})
 
-	// Register a remote node with a service.
+	// Register a remote node with a service. This is in a retry since we
-	{
+	// need the datacenter to have a route which takes a little more time
+	// beyond the join, and we don't have direct access to the router here.
+	retry.Run(t, func(r *retry.R) {
 		args := &structs.RegisterRequest{
 			Datacenter: "dc2",
 			Node:       "foo",
@@ -1324,9 +1329,9 @@ func TestDNS_ServiceLookup_WanAddress(t *testing.T) {
 
 		var out struct{}
 		if err := srv2.agent.RPC("Catalog.Register", args, &out); err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
-	}
+	})
 
 	// Register an equivalent prepared query.
 	var id string
@@ -3381,10 +3386,15 @@ func TestDNS_PreparedQuery_Failover(t *testing.T) {
 		if got, want := len(srv1.agent.WANMembers()), 2; got < want {
 			r.Fatalf("got %d WAN members want at least %d", got, want)
 		}
+		if got, want := len(srv2.agent.WANMembers()), 2; got < want {
+			r.Fatalf("got %d WAN members want at least %d", got, want)
+		}
 	})
 
-	// Register a remote node with a service.
+	// Register a remote node with a service. This is in a retry since we
-	{
+	// need the datacenter to have a route which takes a little more time
+	// beyond the join, and we don't have direct access to the router here.
+	retry.Run(t, func(r *retry.R) {
 		args := &structs.RegisterRequest{
 			Datacenter: "dc2",
 			Node:       "foo",
@@ -3399,9 +3409,9 @@ func TestDNS_PreparedQuery_Failover(t *testing.T) {
 
 		var out struct{}
 		if err := srv2.agent.RPC("Catalog.Register", args, &out); err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
-	}
+	})
 
 	// Register a local prepared query.
 	{

consul/client_test.go

@@ -83,7 +83,7 @@ func TestClient_JoinLAN(t *testing.T) {
 	joinLAN(t, c1, s1)
 	retry.Run(t, func(r *retry.R) {
 		if got, want := c1.servers.NumServers(), 1; got != want {
-			r.Fatal("got %d servers want %d", got, want)
+			r.Fatalf("got %d servers want %d", got, want)
 		}
 		if got, want := len(s1.LANMembers()), 2; got != want {
 			r.Fatalf("got %d server LAN members want %d", got, want)
@@ -346,7 +346,7 @@ func TestClient_SnapshotRPC(t *testing.T) {
 	// Wait until we've got a healthy server.
 	retry.Run(t, func(r *retry.R) {
 		if got, want := c1.servers.NumServers(), 1; got != want {
-			r.Fatal("got %d servers want %d", got, want)
+			r.Fatalf("got %d servers want %d", got, want)
 		}
 	})
 
@@ -401,7 +401,7 @@ func TestClient_SnapshotRPC_TLS(t *testing.T) {
 	// Wait until we've got a healthy server.
 	retry.Run(t, func(r *retry.R) {
 		if got, want := c1.servers.NumServers(), 1; got != want {
-			r.Fatal("got %d servers want %d", got, want)
+			r.Fatalf("got %d servers want %d", got, want)
 		}
 	})
 

consul/leader_test.go

@@ -434,29 +434,29 @@ func TestLeader_LeftServer(t *testing.T) {
 	dir3, s3 := testServerDCBootstrap(t, "dc1", false)
 	defer os.RemoveAll(dir3)
 	defer s3.Shutdown()
-	servers := []*Server{s1, s2, s3}
+
+	// Put s1 last so we don't trigger a leader election.
+	servers := []*Server{s2, s3, s1}
 
 	// Try to join
 	joinLAN(t, s2, s1)
 	joinLAN(t, s3, s1)
-
 	for _, s := range servers {
 		retry.Run(t, func(r *retry.R) { r.Check(wantPeers(s, 3)) })
 	}
 
-	retry.Run(t, func(r *retry.R) {
+	// Kill any server
-		// Kill any server
+	servers[0].Shutdown()
-		servers[0].Shutdown()
 
-		// Force remove the non-leader (transition to left state)
+	// Force remove the non-leader (transition to left state)
-		if err := servers[1].RemoveFailedNode(servers[0].config.NodeName); err != nil {
+	if err := servers[1].RemoveFailedNode(servers[0].config.NodeName); err != nil {
-			r.Fatalf("err: %v", err)
+		t.Fatalf("err: %v", err)
-		}
+	}
 
-		for _, s := range servers[1:] {
+	// Wait until the remaining servers show only 2 peers.
-			retry.Run(t, func(r *retry.R) { r.Check(wantPeers(s, 2)) })
+	for _, s := range servers[1:] {
-		}
+		retry.Run(t, func(r *retry.R) { r.Check(wantPeers(s, 2)) })
-	})
+	}
 }
 
 func TestLeader_LeftLeader(t *testing.T) {

consul/server_test.go

@@ -184,12 +184,12 @@ func TestServer_JoinWAN(t *testing.T) {
 	})
 
 	// Check the router has both
-	if len(s1.router.GetDatacenters()) != 2 {
-		t.Fatalf("remote consul missing")
-	}
 	retry.Run(t, func(r *retry.R) {
+		if got, want := len(s1.router.GetDatacenters()), 2; got != want {
+			r.Fatalf("got %d routes want %d", got, want)
+		}
 		if got, want := len(s2.router.GetDatacenters()), 2; got != want {
-			r.Fatalf("got %d data centers want %d", got, want)
+			r.Fatalf("got %d datacenters want %d", got, want)
 		}
 	})
 }
@@ -273,17 +273,17 @@ func TestServer_JoinSeparateLanAndWanAddresses(t *testing.T) {
 	})
 
 	// Check the router has both
-	if len(s1.router.GetDatacenters()) != 2 {
+	retry.Run(t, func(r *retry.R) {
-		t.Fatalf("remote consul missing")
+		if len(s1.router.GetDatacenters()) != 2 {
-	}
+			r.Fatalf("remote consul missing")
-
+		}
-	if len(s2.router.GetDatacenters()) != 2 {
+		if len(s2.router.GetDatacenters()) != 2 {
-		t.Fatalf("remote consul missing")
+			r.Fatalf("remote consul missing")
-	}
+		}
-
+		if len(s2.localConsuls) != 2 {
-	if len(s2.localConsuls) != 2 {
+			r.Fatalf("local consul fellow s3 for s2 missing")
-		t.Fatalf("local consul fellow s3 for s2 missing")
+		}
-	}
+	})
 
 	// Get and check the wan address of s2 from s1
 	var s2WanAddr string

testutil/retry/retry.go

@@ -82,7 +82,7 @@ func decorate(s string) string {
 }
 
 func Run(t Failer, f func(r *R)) {
-	run(OneSec(), t, f)
+	run(TwoSeconds(), t, f)
 }
 
 func RunWith(r Retryer, t Failer, f func(r *R)) {
@@ -133,9 +133,9 @@ func run(r Retryer, t Failer, f func(r *R)) {
 	}
 }
 
-// OneSec repeats an operation for one second and waits 25ms in between.
+// TwoSeconds repeats an operation for two seconds and waits 25ms in between.
-func OneSec() *Timer {
+func TwoSeconds() *Timer {
-	return &Timer{Timeout: time.Second, Wait: 25 * time.Millisecond}
+	return &Timer{Timeout: 2 * time.Second, Wait: 25 * time.Millisecond}
 }
 
 // ThreeTimes repeats an operation three times and waits 25ms in between.

testutil/retry/retry_test.go

@@ -6,7 +6,7 @@ import (
 )
 
 // delta defines the time band a test run should complete in.
-var delta = 5 * time.Millisecond
+var delta = 10 * time.Millisecond
 
 func TestRetryer(t *testing.T) {
 	tests := []struct {