From 33a7b8e4019193460579f268d4455476c6c5d888 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 8 May 2017 21:57:06 -0700 Subject: [PATCH 1/2] Tweaks some tests that were having a hard time in Travis CI and bumps up the default retry time. --- command/agent/dns_test.go | 26 ++++++++++++++++++-------- consul/leader_test.go | 26 +++++++++++++------------- consul/server_test.go | 30 +++++++++++++++--------------- testutil/retry/retry.go | 8 ++++---- testutil/retry/retry_test.go | 2 +- 5 files changed, 51 insertions(+), 41 deletions(-) diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index dbcec63a3d..9e23586c85 100644 --- a/command/agent/dns_test.go +++ b/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. { diff --git a/consul/leader_test.go b/consul/leader_test.go index 29a72c46b0..d804bc070e 100644 --- a/consul/leader_test.go +++ b/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 - servers[0].Shutdown() + // Kill any server + servers[0].Shutdown() - // Force remove the non-leader (transition to left state) - if err := servers[1].RemoveFailedNode(servers[0].config.NodeName); err != nil { - r.Fatalf("err: %v", err) - } + // Force remove the non-leader (transition to left state) + if err := servers[1].RemoveFailedNode(servers[0].config.NodeName); err != nil { + t.Fatalf("err: %v", err) + } - for _, s := range servers[1:] { - retry.Run(t, func(r *retry.R) { r.Check(wantPeers(s, 2)) }) - } - }) + // Wait until the remaining servers show only 2 peers. + for _, s := range servers[1:] { + retry.Run(t, func(r *retry.R) { r.Check(wantPeers(s, 2)) }) + } } func TestLeader_LeftLeader(t *testing.T) { diff --git a/consul/server_test.go b/consul/server_test.go index ade7be77b5..3377a06380 100644 --- a/consul/server_test.go +++ b/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 { - t.Fatalf("remote consul missing") - } - - if len(s2.router.GetDatacenters()) != 2 { - t.Fatalf("remote consul missing") - } - - if len(s2.localConsuls) != 2 { - t.Fatalf("local consul fellow s3 for s2 missing") - } + retry.Run(t, func(r *retry.R) { + if len(s1.router.GetDatacenters()) != 2 { + r.Fatalf("remote consul missing") + } + if len(s2.router.GetDatacenters()) != 2 { + r.Fatalf("remote consul missing") + } + if len(s2.localConsuls) != 2 { + r.Fatalf("local consul fellow s3 for s2 missing") + } + }) // Get and check the wan address of s2 from s1 var s2WanAddr string diff --git a/testutil/retry/retry.go b/testutil/retry/retry.go index d1faa75474..cfbdde3c9d 100644 --- a/testutil/retry/retry.go +++ b/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. -func OneSec() *Timer { - return &Timer{Timeout: time.Second, Wait: 25 * time.Millisecond} +// TwoSeconds repeats an operation for two seconds and waits 25ms in between. +func TwoSeconds() *Timer { + return &Timer{Timeout: 2 * time.Second, Wait: 25 * time.Millisecond} } // ThreeTimes repeats an operation three times and waits 25ms in between. diff --git a/testutil/retry/retry_test.go b/testutil/retry/retry_test.go index 2caa302266..d22f154438 100644 --- a/testutil/retry/retry_test.go +++ b/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 { From c1d196152f6b4ace7b5e707e48edcec6dab3d298 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Mon, 8 May 2017 22:31:41 -0700 Subject: [PATCH 2/2] Fixes vet errors. --- api/catalog_test.go | 4 ++-- consul/client_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/catalog_test.go b/api/catalog_test.go index d2f29ab504..29a3578802 100644 --- a/api/catalog_test.go +++ b/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") diff --git a/consul/client_test.go b/consul/client_test.go index b9ce3ccd6a..730721346d 100644 --- a/consul/client_test.go +++ b/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) } })