From 81b572cb31716816e4036e971aaea1379dc67266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 02:48:25 +0200 Subject: [PATCH 01/39] Proof of concept using `WaitForResult` in tests --- consul/catalog_endpoint_test.go | 11 ++++++----- testutil/wait.go | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 testutil/wait.go diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index 3a174ce45..a553538b8 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -2,6 +2,7 @@ package consul import ( "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "net/rpc" "os" @@ -35,12 +36,12 @@ func TestCatalogRegister(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for leader - time.Sleep(100 * time.Millisecond) - - if err := client.Call("Catalog.Register", &arg, &out); err != nil { + testutil.WaitForResult(func() (bool, error) { + err := client.Call("Catalog.Register", &arg, &out) + return err == nil, err + }, func(err error) { t.Fatalf("err: %v", err) - } + }) } func TestCatalogRegister_ForwardLeader(t *testing.T) { diff --git a/testutil/wait.go b/testutil/wait.go new file mode 100644 index 000000000..6071c94ce --- /dev/null +++ b/testutil/wait.go @@ -0,0 +1,26 @@ +package testutil + +import ( + "time" +) + +type testFn func() (bool, error) +type errorFn func(error) + +func WaitForResult(test testFn, error errorFn) { + retries := 500 // 5 seconds timeout + + for retries > 0 { + time.Sleep(10 * time.Millisecond) + retries-- + + success, err := test() + if success { + return + } + + if retries == 0 { + error(err) + } + } +} From 82ec4e5fbaa1664ac2f3704a43d8010828691e19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 11:43:42 +0200 Subject: [PATCH 02/39] Add `testutils.WaitForLeader` and truly wait --- command/agent/dns_test.go | 68 +++++++++++++++++++-------------------- testutil/wait.go | 18 +++++++++-- 2 files changed, 50 insertions(+), 36 deletions(-) diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index 52f9eae73..e874e3f64 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -2,12 +2,12 @@ package agent import ( "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "github.com/miekg/dns" "os" "strings" "testing" - "time" ) func makeDNSServer(t *testing.T) (string, *DNSServer) { @@ -65,15 +65,15 @@ func TestDNS_NodeLookup(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", Address: "127.0.0.1", } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -129,15 +129,15 @@ func TestDNS_NodeLookup_PeriodName(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register node with period in name args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo.bar", Address: "127.0.0.1", } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -171,15 +171,15 @@ func TestDNS_NodeLookup_AAAA(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "bar", Address: "::4242:4242", } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -213,15 +213,15 @@ func TestDNS_NodeLookup_CNAME(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "google", Address: "www.google.com", } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -256,9 +256,6 @@ func TestDNS_ServiceLookup(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -270,6 +267,9 @@ func TestDNS_ServiceLookup(t *testing.T) { Port: 12345, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -317,9 +317,6 @@ func TestDNS_ServiceLookup_TagPeriod(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -331,6 +328,9 @@ func TestDNS_ServiceLookup_TagPeriod(t *testing.T) { Port: 12345, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -378,9 +378,6 @@ func TestDNS_ServiceLookup_Dedup(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -392,6 +389,9 @@ func TestDNS_ServiceLookup_Dedup(t *testing.T) { Port: 12345, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -455,9 +455,6 @@ func TestDNS_ServiceLookup_Dedup_SRV(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -469,6 +466,9 @@ func TestDNS_ServiceLookup_Dedup_SRV(t *testing.T) { Port: 12345, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -584,9 +584,6 @@ func TestDNS_ServiceLookup_FilterCritical(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register nodes args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -603,6 +600,9 @@ func TestDNS_ServiceLookup_FilterCritical(t *testing.T) { Status: structs.HealthCritical, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -648,9 +648,6 @@ func TestDNS_ServiceLookup_Randomize(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register nodes for i := 0; i < 3*maxServiceResponses; i++ { args := &structs.RegisterRequest{ @@ -662,6 +659,9 @@ func TestDNS_ServiceLookup_Randomize(t *testing.T) { Port: 8000, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -712,9 +712,6 @@ func TestDNS_ServiceLookup_CNAME(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -725,6 +722,9 @@ func TestDNS_ServiceLookup_CNAME(t *testing.T) { Port: 80, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) diff --git a/testutil/wait.go b/testutil/wait.go index 6071c94ce..6cc2d9559 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -2,16 +2,18 @@ package testutil import ( "time" + "testing" + "github.com/hashicorp/consul/consul/structs" ) type testFn func() (bool, error) type errorFn func(error) func WaitForResult(test testFn, error errorFn) { - retries := 500 // 5 seconds timeout + retries := 100 // 5 seconds timeout for retries > 0 { - time.Sleep(10 * time.Millisecond) + time.Sleep(50 * time.Millisecond) retries-- success, err := test() @@ -24,3 +26,15 @@ func WaitForResult(test testFn, error errorFn) { } } } + +type clientRPC func(string, interface {}, interface {}) error + +func WaitForLeader(t *testing.T, rpc clientRPC, args interface{}) { + WaitForResult(func() (bool, error) { + var out structs.IndexedNodes + err := rpc("Catalog.ListNodes", args, &out) + return out.QueryMeta.KnownLeader, err + }, func(err error) { + t.Fatalf("failed to find leader: %v", err) + }) +} From 2019f454d0621869dd5b4033cdc48201cbe2f557 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Wed, 7 May 2014 13:53:29 +0200 Subject: [PATCH 03/39] Fix `TestCatalogListNodes_StaleRaad` with `WaitForLeader` --- consul/catalog_endpoint_test.go | 15 ++++++++------- testutil/wait.go | 7 ++++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index a553538b8..3f01b49db 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -2,8 +2,8 @@ package consul import ( "fmt" - "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" + "github.com/hashicorp/consul/testutil" "net/rpc" "os" "sort" @@ -254,8 +254,13 @@ func TestCatalogListNodes_StaleRaad(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := structs.DCSpecificRequest{ + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{AllowStale: true}, + } + + testutil.WaitForLeader(t, client1.Call, args) + testutil.WaitForLeader(t, client2.Call, args) // Use the follower as the client var client *rpc.Client @@ -271,10 +276,6 @@ func TestCatalogListNodes_StaleRaad(t *testing.T) { s2.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) } - args := structs.DCSpecificRequest{ - Datacenter: "dc1", - QueryOptions: structs.QueryOptions{AllowStale: true}, - } var out structs.IndexedNodes if err := client.Call("Catalog.ListNodes", &args, &out); err != nil { t.Fatalf("err: %v", err) diff --git a/testutil/wait.go b/testutil/wait.go index 6cc2d9559..78ef2d140 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -27,14 +27,15 @@ func WaitForResult(test testFn, error errorFn) { } } -type clientRPC func(string, interface {}, interface {}) error +type rpcFn func(string, interface {}, interface {}) error -func WaitForLeader(t *testing.T, rpc clientRPC, args interface{}) { +func WaitForLeader(t *testing.T, rpc rpcFn, args interface{}) structs.IndexedNodes { + var out structs.IndexedNodes WaitForResult(func() (bool, error) { - var out structs.IndexedNodes err := rpc("Catalog.ListNodes", args, &out) return out.QueryMeta.KnownLeader, err }, func(err error) { t.Fatalf("failed to find leader: %v", err) }) + return out } From e76f9f36c6be5c7a5d9547adf2992e148aa7ef60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 22:45:16 +0200 Subject: [PATCH 04/39] Remove all sleeps from `force_leave_test.go` --- command/force_leave_test.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/command/force_leave_test.go b/command/force_leave_test.go index 6b66b4fb1..40a2224f7 100644 --- a/command/force_leave_test.go +++ b/command/force_leave_test.go @@ -2,12 +2,12 @@ package command import ( "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/serf/serf" - "github.com/hashicorp/serf/testutil" "github.com/mitchellh/cli" "strings" "testing" - "time" + "errors" ) func TestForceLeaveCommand_implements(t *testing.T) { @@ -26,13 +26,9 @@ func TestForceLeaveCommandRun(t *testing.T) { t.Fatalf("err: %s", err) } - testutil.Yield() - // Forcibly shutdown a2 so that it appears "failed" in a1 a2.Shutdown() - time.Sleep(time.Second) - ui := new(cli.MockUi) c := &ForceLeaveCommand{Ui: ui} args := []string{ @@ -50,9 +46,14 @@ func TestForceLeaveCommandRun(t *testing.T) { t.Fatalf("should have 2 members: %#v", m) } - if m[1].Status != serf.StatusLeft { - t.Fatalf("should be left: %#v", m[1]) - } + testutil.WaitForResult(func() (bool, error) { + m = a1.agent.LANMembers() + success := m[1].Status == serf.StatusLeft + err := errors.New(m[1].Status.String()) + return success, err + }, func(err error) { + t.Fatalf("member status is %v, not left", err) + }) } func TestForceLeaveCommandRun_noAddrs(t *testing.T) { From e77c3180d8dd02ef4255393820435dfde5f80e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 22:50:18 +0200 Subject: [PATCH 05/39] Remove all sleeps from `agent_endpoint_test.go` --- command/agent/agent_endpoint_test.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 15182e495..78406de73 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -2,12 +2,14 @@ package agent import ( "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/serf/serf" "net/http" "os" "testing" "time" + "errors" ) func TestHTTPAgentServices(t *testing.T) { @@ -189,9 +191,7 @@ func TestHTTPAgentForceLeave(t *testing.T) { t.Fatalf("err: %v", err) } - // Shutdown, wait for detection a2.Shutdown() - time.Sleep(500 * time.Millisecond) // Force leave now req, err := http.NewRequest("GET", fmt.Sprintf("/v1/agent/force-leave/%s", a2.config.NodeName), nil) @@ -207,11 +207,13 @@ func TestHTTPAgentForceLeave(t *testing.T) { t.Fatalf("Err: %v", obj) } - // SHould be left - mem := srv.agent.LANMembers() - if mem[1].Status != serf.StatusLeft { - t.Fatalf("should have left: %v", mem) - } + testutil.WaitForResult(func() (bool, error) { + m := srv.agent.LANMembers() + success := m[1].Status == serf.StatusLeft + return success, errors.New(m[1].Status.String()) + }, func(err error) { + t.Fatalf("member status is %v, should be left", err) + }) } func TestHTTPAgentRegisterCheck(t *testing.T) { From 16a2f14ef1e3377244463c0baf38316d95c2d8e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 22:50:34 +0200 Subject: [PATCH 06/39] Fix formatting in `TestForceLeaveCommandRun` --- command/force_leave_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/command/force_leave_test.go b/command/force_leave_test.go index 40a2224f7..8189d41d2 100644 --- a/command/force_leave_test.go +++ b/command/force_leave_test.go @@ -49,10 +49,9 @@ func TestForceLeaveCommandRun(t *testing.T) { testutil.WaitForResult(func() (bool, error) { m = a1.agent.LANMembers() success := m[1].Status == serf.StatusLeft - err := errors.New(m[1].Status.String()) - return success, err + return success, errors.New(m[1].Status.String()) }, func(err error) { - t.Fatalf("member status is %v, not left", err) + t.Fatalf("member status is %v, should be left", err) }) } From 6e39de1d26033d8813b36e1a0ec4ee6e0185575b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 23:02:59 +0200 Subject: [PATCH 07/39] Remove all sleeps from `catalog_endpoint_test.go` --- command/agent/catalog_endpoint_test.go | 72 +++++++++++++------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/command/agent/catalog_endpoint_test.go b/command/agent/catalog_endpoint_test.go index 2d6dc39d4..fa553d112 100644 --- a/command/agent/catalog_endpoint_test.go +++ b/command/agent/catalog_endpoint_test.go @@ -2,6 +2,7 @@ package agent import ( "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "net/http" "net/http/httptest" @@ -16,18 +17,19 @@ func TestCatalogRegister(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + } + + testutil.WaitForLeader(t, srv.agent.RPC, args) // Register node req, err := http.NewRequest("GET", "/v1/catalog/register", nil) if err != nil { t.Fatalf("err: %v", err) } - args := &structs.RegisterRequest{ - Node: "foo", - Address: "127.0.0.1", - } req.Body = encodeReq(args) obj, err := srv.CatalogRegister(nil, req) @@ -47,17 +49,18 @@ func TestCatalogDeregister(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := &structs.DeregisterRequest{ + Datacenter: "dc1", + Node: "foo", + } + + testutil.WaitForLeader(t, srv.agent.RPC, args) // Register node req, err := http.NewRequest("GET", "/v1/catalog/deregister", nil) if err != nil { t.Fatalf("err: %v", err) } - args := &structs.DeregisterRequest{ - Node: "foo", - } req.Body = encodeReq(args) obj, err := srv.CatalogDeregister(nil, req) @@ -77,9 +80,6 @@ func TestCatalogDatacenters(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for initialization - time.Sleep(10 * time.Millisecond) - obj, err := srv.CatalogDatacenters(nil, nil) if err != nil { t.Fatalf("err: %v", err) @@ -97,15 +97,15 @@ func TestCatalogNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", Address: "127.0.0.1", } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + + // Register node var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -137,13 +137,13 @@ func TestCatalogNodes_Blocking(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - - // Register node args := &structs.DCSpecificRequest{ Datacenter: "dc1", } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + + // Register node var out structs.IndexedNodes if err := srv.agent.RPC("Catalog.ListNodes", *args, &out); err != nil { t.Fatalf("err: %v", err) @@ -152,7 +152,7 @@ func TestCatalogNodes_Blocking(t *testing.T) { // Do an update in a little while start := time.Now() go func() { - time.Sleep(100 * time.Millisecond) + time.Sleep(50 * time.Millisecond) args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", @@ -178,7 +178,7 @@ func TestCatalogNodes_Blocking(t *testing.T) { } // Should block for a while - if time.Now().Sub(start) < 100*time.Millisecond { + if time.Now().Sub(start) < 50 * time.Millisecond { t.Fatalf("too fast") } @@ -198,10 +198,6 @@ func TestCatalogServices(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", @@ -210,6 +206,10 @@ func TestCatalogServices(t *testing.T) { Service: "api", }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + + // Register node var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -240,10 +240,6 @@ func TestCatalogServiceNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", @@ -253,6 +249,10 @@ func TestCatalogServiceNodes(t *testing.T) { Tags: []string{"a"}, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + + // Register node var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -283,10 +283,6 @@ func TestCatalogNodeServices(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - - // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", @@ -296,6 +292,10 @@ func TestCatalogNodeServices(t *testing.T) { Tags: []string{"a"}, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + + // Register node var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) From 70444ea4b7f1cfe069c181427bf2aa8842cd6ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 23:12:53 +0200 Subject: [PATCH 08/39] Remove all sleeps from `health_endpoint_test.go` --- command/agent/health_endpoint_test.go | 39 ++++++++++++++++----------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/command/agent/health_endpoint_test.go b/command/agent/health_endpoint_test.go index e51f26084..2f669846c 100644 --- a/command/agent/health_endpoint_test.go +++ b/command/agent/health_endpoint_test.go @@ -2,12 +2,12 @@ package agent import ( "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "net/http" "net/http/httptest" "os" "testing" - "time" ) func TestHealthChecksInState(t *testing.T) { @@ -16,8 +16,11 @@ func TestHealthChecksInState(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := &structs.RegisterRequest{ + Datacenter: "dc1", + } + + testutil.WaitForLeader(t, srv.agent.RPC, args) req, err := http.NewRequest("GET", "/v1/health/state/passing?dc=dc1", nil) if err != nil { @@ -44,8 +47,11 @@ func TestHealthNodeChecks(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := &structs.RegisterRequest{ + Datacenter: "dc1", + } + + testutil.WaitForLeader(t, srv.agent.RPC, args) req, err := http.NewRequest("GET", fmt.Sprintf("/v1/health/node/%s?dc=dc1", srv.agent.config.NodeName), nil) @@ -73,10 +79,6 @@ func TestHealthServiceChecks(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - - // Create a service check args := &structs.RegisterRequest{ Datacenter: "dc1", Node: srv.agent.config.NodeName, @@ -87,6 +89,10 @@ func TestHealthServiceChecks(t *testing.T) { ServiceID: "consul", }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + + // Create a service check var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -117,8 +123,11 @@ func TestHealthServiceNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := &structs.RegisterRequest{ + Datacenter: "dc1", + } + + testutil.WaitForLeader(t, srv.agent.RPC, args) req, err := http.NewRequest("GET", "/v1/health/service/consul?dc=dc1", nil) if err != nil { @@ -145,10 +154,6 @@ func TestHealthServiceNodes_PassingFilter(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - - // Create a failing service check args := &structs.RegisterRequest{ Datacenter: "dc1", Node: srv.agent.config.NodeName, @@ -160,6 +165,10 @@ func TestHealthServiceNodes_PassingFilter(t *testing.T) { Status: structs.HealthCritical, }, } + + testutil.WaitForLeader(t, srv.agent.RPC, args) + + // Create a failing service check var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) From 58bb06ff54459e38462726167300290786ede7d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 23:17:00 +0200 Subject: [PATCH 09/39] Drop unnecessary sleep --- command/agent/http_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 8d4a63d00..60d28b6f4 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -99,9 +99,6 @@ func TestContentTypeIsJSON(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - resp := httptest.NewRecorder() handler := func(resp http.ResponseWriter, req *http.Request) (interface{}, error) { From 89ead7bc4ee9413cd79021277b12714b56c69bdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 23:17:10 +0200 Subject: [PATCH 10/39] Remove all sleeps from `kvs_endpoint_test.go` --- command/agent/kvs_endpoint_test.go | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/command/agent/kvs_endpoint_test.go b/command/agent/kvs_endpoint_test.go index 8d3679e92..97851573c 100644 --- a/command/agent/kvs_endpoint_test.go +++ b/command/agent/kvs_endpoint_test.go @@ -3,13 +3,13 @@ package agent import ( "bytes" "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "net/http" "net/http/httptest" "os" "reflect" "testing" - "time" ) func TestKVSEndpoint_PUT_GET_DELETE(t *testing.T) { @@ -18,8 +18,11 @@ func TestKVSEndpoint_PUT_GET_DELETE(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := &structs.RegisterRequest{ + Datacenter: "dc1", + } + + testutil.WaitForLeader(t, srv.agent.RPC, args) keys := []string{ "baz", @@ -94,8 +97,11 @@ func TestKVSEndpoint_Recurse(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := &structs.RegisterRequest{ + Datacenter: "dc1", + } + + testutil.WaitForLeader(t, srv.agent.RPC, args) keys := []string{ "bar", @@ -191,8 +197,11 @@ func TestKVSEndpoint_CAS(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := &structs.RegisterRequest{ + Datacenter: "dc1", + } + + testutil.WaitForLeader(t, srv.agent.RPC, args) { buf := bytes.NewBuffer([]byte("test")) @@ -289,8 +298,11 @@ func TestKVSEndpoint_ListKeys(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + args := &structs.RegisterRequest{ + Datacenter: "dc1", + } + + testutil.WaitForLeader(t, srv.agent.RPC, args) keys := []string{ "bar", From 795fab8a6134dea5e854f0a1fdd7383b22ee6464 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 23:27:32 +0200 Subject: [PATCH 11/39] Remove some sleeps from `local_test.go` --- command/agent/local_test.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/command/agent/local_test.go b/command/agent/local_test.go index a84c1a292..8c3f8bda9 100644 --- a/command/agent/local_test.go +++ b/command/agent/local_test.go @@ -1,6 +1,7 @@ package agent import ( + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "os" "reflect" @@ -14,18 +15,16 @@ func TestAgentAntiEntropy_Services(t *testing.T) { defer os.RemoveAll(dir) defer agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - - // Register info args := &structs.RegisterRequest{ Datacenter: "dc1", Node: agent.config.NodeName, Address: "127.0.0.1", } - var out struct{} - // Exists both, same (noop) + testutil.WaitForLeader(t, agent.RPC, args) + + // Register info. Exists both, same (noop) + var out struct{} srv1 := &structs.NodeService{ ID: "mysql", Service: "mysql", @@ -137,18 +136,16 @@ func TestAgentAntiEntropy_Checks(t *testing.T) { defer os.RemoveAll(dir) defer agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) - - // Register info args := &structs.RegisterRequest{ Datacenter: "dc1", Node: agent.config.NodeName, Address: "127.0.0.1", } - var out struct{} - // Exists both, same (noop) + testutil.WaitForLeader(t, agent.RPC, args) + + // Register info. Exists both, same (noop) + var out struct{} chk1 := &structs.HealthCheck{ Node: agent.config.NodeName, CheckID: "mysql", From bdbe97545ae4b35f7ed7440d2669f4390a0c1424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 23:41:14 +0200 Subject: [PATCH 12/39] Drop client arguments from `WaitForLeader` --- command/agent/catalog_endpoint_test.go | 42 +++++++++++++------------- command/agent/dns_test.go | 22 +++++++------- command/agent/health_endpoint_test.go | 22 +++----------- command/agent/kvs_endpoint_test.go | 24 +++------------ command/agent/local_test.go | 4 +-- consul/catalog_endpoint_test.go | 4 +-- consul/leader_test.go | 5 +-- testutil/wait.go | 5 ++- 8 files changed, 52 insertions(+), 76 deletions(-) diff --git a/command/agent/catalog_endpoint_test.go b/command/agent/catalog_endpoint_test.go index fa553d112..9ae11c4db 100644 --- a/command/agent/catalog_endpoint_test.go +++ b/command/agent/catalog_endpoint_test.go @@ -17,15 +17,15 @@ func TestCatalogRegister(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", Address: "127.0.0.1", } - testutil.WaitForLeader(t, srv.agent.RPC, args) - - // Register node req, err := http.NewRequest("GET", "/v1/catalog/register", nil) if err != nil { t.Fatalf("err: %v", err) @@ -49,14 +49,14 @@ func TestCatalogDeregister(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + + // Register node args := &structs.DeregisterRequest{ Datacenter: "dc1", Node: "foo", } - testutil.WaitForLeader(t, srv.agent.RPC, args) - - // Register node req, err := http.NewRequest("GET", "/v1/catalog/deregister", nil) if err != nil { t.Fatalf("err: %v", err) @@ -97,15 +97,15 @@ func TestCatalogNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", Address: "127.0.0.1", } - testutil.WaitForLeader(t, srv.agent.RPC, args) - - // Register node var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -137,13 +137,13 @@ func TestCatalogNodes_Blocking(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + + // Register node args := &structs.DCSpecificRequest{ Datacenter: "dc1", } - testutil.WaitForLeader(t, srv.agent.RPC, args) - - // Register node var out structs.IndexedNodes if err := srv.agent.RPC("Catalog.ListNodes", *args, &out); err != nil { t.Fatalf("err: %v", err) @@ -198,6 +198,9 @@ func TestCatalogServices(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", @@ -207,9 +210,6 @@ func TestCatalogServices(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) - - // Register node var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -240,6 +240,9 @@ func TestCatalogServiceNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", @@ -250,9 +253,6 @@ func TestCatalogServiceNodes(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) - - // Register node var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -283,6 +283,9 @@ func TestCatalogNodeServices(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", Node: "foo", @@ -293,9 +296,6 @@ func TestCatalogNodeServices(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) - - // Register node var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index e874e3f64..f1b0ba46a 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -72,7 +72,7 @@ func TestDNS_NodeLookup(t *testing.T) { Address: "127.0.0.1", } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -136,7 +136,7 @@ func TestDNS_NodeLookup_PeriodName(t *testing.T) { Address: "127.0.0.1", } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -178,7 +178,7 @@ func TestDNS_NodeLookup_AAAA(t *testing.T) { Address: "::4242:4242", } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -220,7 +220,7 @@ func TestDNS_NodeLookup_CNAME(t *testing.T) { Address: "www.google.com", } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -268,7 +268,7 @@ func TestDNS_ServiceLookup(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -329,7 +329,7 @@ func TestDNS_ServiceLookup_TagPeriod(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -390,7 +390,7 @@ func TestDNS_ServiceLookup_Dedup(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -467,7 +467,7 @@ func TestDNS_ServiceLookup_Dedup_SRV(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -601,7 +601,7 @@ func TestDNS_ServiceLookup_FilterCritical(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -660,7 +660,7 @@ func TestDNS_ServiceLookup_Randomize(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { @@ -723,7 +723,7 @@ func TestDNS_ServiceLookup_CNAME(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { diff --git a/command/agent/health_endpoint_test.go b/command/agent/health_endpoint_test.go index 2f669846c..10f84be9d 100644 --- a/command/agent/health_endpoint_test.go +++ b/command/agent/health_endpoint_test.go @@ -16,11 +16,7 @@ func TestHealthChecksInState(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - args := &structs.RegisterRequest{ - Datacenter: "dc1", - } - - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) req, err := http.NewRequest("GET", "/v1/health/state/passing?dc=dc1", nil) if err != nil { @@ -47,11 +43,7 @@ func TestHealthNodeChecks(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - args := &structs.RegisterRequest{ - Datacenter: "dc1", - } - - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) req, err := http.NewRequest("GET", fmt.Sprintf("/v1/health/node/%s?dc=dc1", srv.agent.config.NodeName), nil) @@ -90,7 +82,7 @@ func TestHealthServiceChecks(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) // Create a service check var out struct{} @@ -123,11 +115,7 @@ func TestHealthServiceNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - args := &structs.RegisterRequest{ - Datacenter: "dc1", - } - - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) req, err := http.NewRequest("GET", "/v1/health/service/consul?dc=dc1", nil) if err != nil { @@ -166,7 +154,7 @@ func TestHealthServiceNodes_PassingFilter(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) // Create a failing service check var out struct{} diff --git a/command/agent/kvs_endpoint_test.go b/command/agent/kvs_endpoint_test.go index 97851573c..3ccc12bdf 100644 --- a/command/agent/kvs_endpoint_test.go +++ b/command/agent/kvs_endpoint_test.go @@ -18,11 +18,7 @@ func TestKVSEndpoint_PUT_GET_DELETE(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - args := &structs.RegisterRequest{ - Datacenter: "dc1", - } - - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) keys := []string{ "baz", @@ -97,11 +93,7 @@ func TestKVSEndpoint_Recurse(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - args := &structs.RegisterRequest{ - Datacenter: "dc1", - } - - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) keys := []string{ "bar", @@ -197,11 +189,7 @@ func TestKVSEndpoint_CAS(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - args := &structs.RegisterRequest{ - Datacenter: "dc1", - } - - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) { buf := bytes.NewBuffer([]byte("test")) @@ -298,11 +286,7 @@ func TestKVSEndpoint_ListKeys(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - args := &structs.RegisterRequest{ - Datacenter: "dc1", - } - - testutil.WaitForLeader(t, srv.agent.RPC, args) + testutil.WaitForLeader(t, srv.agent.RPC) keys := []string{ "bar", diff --git a/command/agent/local_test.go b/command/agent/local_test.go index 8c3f8bda9..69a85a8e5 100644 --- a/command/agent/local_test.go +++ b/command/agent/local_test.go @@ -21,7 +21,7 @@ func TestAgentAntiEntropy_Services(t *testing.T) { Address: "127.0.0.1", } - testutil.WaitForLeader(t, agent.RPC, args) + testutil.WaitForLeader(t, agent.RPC) // Register info. Exists both, same (noop) var out struct{} @@ -142,7 +142,7 @@ func TestAgentAntiEntropy_Checks(t *testing.T) { Address: "127.0.0.1", } - testutil.WaitForLeader(t, agent.RPC, args) + testutil.WaitForLeader(t, agent.RPC) // Register info. Exists both, same (noop) var out struct{} diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index 3f01b49db..66059b5e7 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -259,8 +259,8 @@ func TestCatalogListNodes_StaleRaad(t *testing.T) { QueryOptions: structs.QueryOptions{AllowStale: true}, } - testutil.WaitForLeader(t, client1.Call, args) - testutil.WaitForLeader(t, client2.Call, args) + testutil.WaitForLeader(t, client1.Call) + testutil.WaitForLeader(t, client2.Call) // Use the follower as the client var client *rpc.Client diff --git a/consul/leader_test.go b/consul/leader_test.go index f1ea3b0db..09e1856e5 100644 --- a/consul/leader_test.go +++ b/consul/leader_test.go @@ -2,6 +2,7 @@ package consul import ( "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/serf/serf" "os" @@ -18,8 +19,8 @@ func TestLeader_RegisterMember(t *testing.T) { defer os.RemoveAll(dir2) defer c1.Shutdown() - // Wait until we have a leader - time.Sleep(100 * time.Millisecond) + client := rpcClient(t, s1) + testutil.WaitForLeader(t, client.Call) // Try to join addr := fmt.Sprintf("127.0.0.1:%d", diff --git a/testutil/wait.go b/testutil/wait.go index 78ef2d140..20ab8ea5e 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -29,9 +29,12 @@ func WaitForResult(test testFn, error errorFn) { type rpcFn func(string, interface {}, interface {}) error -func WaitForLeader(t *testing.T, rpc rpcFn, args interface{}) structs.IndexedNodes { +func WaitForLeader(t *testing.T, rpc rpcFn) structs.IndexedNodes { var out structs.IndexedNodes WaitForResult(func() (bool, error) { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + } err := rpc("Catalog.ListNodes", args, &out) return out.QueryMeta.KnownLeader, err }, func(err error) { From 20eb682ef683b6d9a1a4397aeaa2766c6907a5e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 23:42:15 +0200 Subject: [PATCH 13/39] Add test file for testutil --- testutil/wait_test.go | 1 + 1 file changed, 1 insertion(+) create mode 100644 testutil/wait_test.go diff --git a/testutil/wait_test.go b/testutil/wait_test.go new file mode 100644 index 000000000..110b2e6a7 --- /dev/null +++ b/testutil/wait_test.go @@ -0,0 +1 @@ +package testutil From c624161827f0e7e93fa30d14b35fc87b56e39dde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 23:47:16 +0200 Subject: [PATCH 14/39] Move `WaitForLeader` calls to top of test --- command/agent/dns_test.go | 44 +++++++++++++-------------- command/agent/health_endpoint_test.go | 12 ++++---- command/agent/local_test.go | 14 +++++---- consul/catalog_endpoint_test.go | 6 ++-- 4 files changed, 39 insertions(+), 37 deletions(-) diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index f1b0ba46a..1aec2f2a0 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -65,6 +65,8 @@ func TestDNS_NodeLookup(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -72,8 +74,6 @@ func TestDNS_NodeLookup(t *testing.T) { Address: "127.0.0.1", } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -129,6 +129,8 @@ func TestDNS_NodeLookup_PeriodName(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register node with period in name args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -136,8 +138,6 @@ func TestDNS_NodeLookup_PeriodName(t *testing.T) { Address: "127.0.0.1", } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -171,6 +171,8 @@ func TestDNS_NodeLookup_AAAA(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -178,8 +180,6 @@ func TestDNS_NodeLookup_AAAA(t *testing.T) { Address: "::4242:4242", } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -213,6 +213,8 @@ func TestDNS_NodeLookup_CNAME(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -220,8 +222,6 @@ func TestDNS_NodeLookup_CNAME(t *testing.T) { Address: "www.google.com", } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -256,6 +256,8 @@ func TestDNS_ServiceLookup(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -268,8 +270,6 @@ func TestDNS_ServiceLookup(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -317,6 +317,8 @@ func TestDNS_ServiceLookup_TagPeriod(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -329,8 +331,6 @@ func TestDNS_ServiceLookup_TagPeriod(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -378,6 +378,8 @@ func TestDNS_ServiceLookup_Dedup(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -390,8 +392,6 @@ func TestDNS_ServiceLookup_Dedup(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -455,6 +455,8 @@ func TestDNS_ServiceLookup_Dedup_SRV(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -467,8 +469,6 @@ func TestDNS_ServiceLookup_Dedup_SRV(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -584,6 +584,8 @@ func TestDNS_ServiceLookup_FilterCritical(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register nodes args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -601,8 +603,6 @@ func TestDNS_ServiceLookup_FilterCritical(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -648,6 +648,8 @@ func TestDNS_ServiceLookup_Randomize(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register nodes for i := 0; i < 3*maxServiceResponses; i++ { args := &structs.RegisterRequest{ @@ -660,8 +662,6 @@ func TestDNS_ServiceLookup_Randomize(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -712,6 +712,8 @@ func TestDNS_ServiceLookup_CNAME(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + // Register node args := &structs.RegisterRequest{ Datacenter: "dc1", @@ -723,8 +725,6 @@ func TestDNS_ServiceLookup_CNAME(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC) - var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) diff --git a/command/agent/health_endpoint_test.go b/command/agent/health_endpoint_test.go index 10f84be9d..4a097ae65 100644 --- a/command/agent/health_endpoint_test.go +++ b/command/agent/health_endpoint_test.go @@ -71,6 +71,9 @@ func TestHealthServiceChecks(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + + // Create a service check args := &structs.RegisterRequest{ Datacenter: "dc1", Node: srv.agent.config.NodeName, @@ -82,9 +85,6 @@ func TestHealthServiceChecks(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC) - - // Create a service check var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) @@ -142,6 +142,9 @@ func TestHealthServiceNodes_PassingFilter(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() + testutil.WaitForLeader(t, srv.agent.RPC) + + // Create a failing service check args := &structs.RegisterRequest{ Datacenter: "dc1", Node: srv.agent.config.NodeName, @@ -154,9 +157,6 @@ func TestHealthServiceNodes_PassingFilter(t *testing.T) { }, } - testutil.WaitForLeader(t, srv.agent.RPC) - - // Create a failing service check var out struct{} if err := srv.agent.RPC("Catalog.Register", args, &out); err != nil { t.Fatalf("err: %v", err) diff --git a/command/agent/local_test.go b/command/agent/local_test.go index 69a85a8e5..2eed35d68 100644 --- a/command/agent/local_test.go +++ b/command/agent/local_test.go @@ -15,15 +15,16 @@ func TestAgentAntiEntropy_Services(t *testing.T) { defer os.RemoveAll(dir) defer agent.Shutdown() + testutil.WaitForLeader(t, agent.RPC) + + // Register info args := &structs.RegisterRequest{ Datacenter: "dc1", Node: agent.config.NodeName, Address: "127.0.0.1", } - testutil.WaitForLeader(t, agent.RPC) - - // Register info. Exists both, same (noop) + // Exists both, same (noop) var out struct{} srv1 := &structs.NodeService{ ID: "mysql", @@ -136,15 +137,16 @@ func TestAgentAntiEntropy_Checks(t *testing.T) { defer os.RemoveAll(dir) defer agent.Shutdown() + testutil.WaitForLeader(t, agent.RPC) + + // Register info args := &structs.RegisterRequest{ Datacenter: "dc1", Node: agent.config.NodeName, Address: "127.0.0.1", } - testutil.WaitForLeader(t, agent.RPC) - - // Register info. Exists both, same (noop) + // Exists both, same (noop) var out struct{} chk1 := &structs.HealthCheck{ Node: agent.config.NodeName, diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index 66059b5e7..4c5a2c70b 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -254,14 +254,14 @@ func TestCatalogListNodes_StaleRaad(t *testing.T) { t.Fatalf("err: %v", err) } + testutil.WaitForLeader(t, client1.Call) + testutil.WaitForLeader(t, client2.Call) + args := structs.DCSpecificRequest{ Datacenter: "dc1", QueryOptions: structs.QueryOptions{AllowStale: true}, } - testutil.WaitForLeader(t, client1.Call) - testutil.WaitForLeader(t, client2.Call) - // Use the follower as the client var client *rpc.Client if !s1.IsLeader() { From 463f85c3c7f3597d09a7ab4ea6b985392b8f9afa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tis=C3=A4ter?= Date: Wed, 7 May 2014 23:51:15 +0200 Subject: [PATCH 15/39] Move argument structs back in original position --- command/agent/catalog_endpoint_test.go | 18 +++++++----------- consul/catalog_endpoint_test.go | 9 ++++----- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/command/agent/catalog_endpoint_test.go b/command/agent/catalog_endpoint_test.go index 9ae11c4db..c9e094dcb 100644 --- a/command/agent/catalog_endpoint_test.go +++ b/command/agent/catalog_endpoint_test.go @@ -20,16 +20,14 @@ func TestCatalogRegister(t *testing.T) { testutil.WaitForLeader(t, srv.agent.RPC) // Register node - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo", - Address: "127.0.0.1", - } - req, err := http.NewRequest("GET", "/v1/catalog/register", nil) if err != nil { t.Fatalf("err: %v", err) } + args := &structs.RegisterRequest{ + Node: "foo", + Address: "127.0.0.1", + } req.Body = encodeReq(args) obj, err := srv.CatalogRegister(nil, req) @@ -52,15 +50,13 @@ func TestCatalogDeregister(t *testing.T) { testutil.WaitForLeader(t, srv.agent.RPC) // Register node - args := &structs.DeregisterRequest{ - Datacenter: "dc1", - Node: "foo", - } - req, err := http.NewRequest("GET", "/v1/catalog/deregister", nil) if err != nil { t.Fatalf("err: %v", err) } + args := &structs.DeregisterRequest{ + Node: "foo", + } req.Body = encodeReq(args) obj, err := srv.CatalogDeregister(nil, req) diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index 4c5a2c70b..149651de6 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -257,11 +257,6 @@ func TestCatalogListNodes_StaleRaad(t *testing.T) { testutil.WaitForLeader(t, client1.Call) testutil.WaitForLeader(t, client2.Call) - args := structs.DCSpecificRequest{ - Datacenter: "dc1", - QueryOptions: structs.QueryOptions{AllowStale: true}, - } - // Use the follower as the client var client *rpc.Client if !s1.IsLeader() { @@ -276,6 +271,10 @@ func TestCatalogListNodes_StaleRaad(t *testing.T) { s2.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) } + args := structs.DCSpecificRequest{ + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{AllowStale: true}, + } var out structs.IndexedNodes if err := client.Call("Catalog.ListNodes", &args, &out); err != nil { t.Fatalf("err: %v", err) From 1ee709ba3899535c278247f7c03a3378186f82ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 01:17:35 +0200 Subject: [PATCH 16/39] Require datacenter argument for `WaitForLeader` --- command/agent/catalog_endpoint_test.go | 14 +++++++------- command/agent/dns_test.go | 22 +++++++++++----------- command/agent/health_endpoint_test.go | 10 +++++----- command/agent/kvs_endpoint_test.go | 8 ++++---- command/agent/local_test.go | 4 ++-- consul/leader_test.go | 2 +- testutil/wait.go | 4 ++-- 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/command/agent/catalog_endpoint_test.go b/command/agent/catalog_endpoint_test.go index c9e094dcb..d9454ab7e 100644 --- a/command/agent/catalog_endpoint_test.go +++ b/command/agent/catalog_endpoint_test.go @@ -17,7 +17,7 @@ func TestCatalogRegister(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node req, err := http.NewRequest("GET", "/v1/catalog/register", nil) @@ -47,7 +47,7 @@ func TestCatalogDeregister(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node req, err := http.NewRequest("GET", "/v1/catalog/deregister", nil) @@ -93,7 +93,7 @@ func TestCatalogNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -133,7 +133,7 @@ func TestCatalogNodes_Blocking(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.DCSpecificRequest{ @@ -194,7 +194,7 @@ func TestCatalogServices(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -236,7 +236,7 @@ func TestCatalogServiceNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -279,7 +279,7 @@ func TestCatalogNodeServices(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ diff --git a/command/agent/dns_test.go b/command/agent/dns_test.go index 1aec2f2a0..eeab068d9 100644 --- a/command/agent/dns_test.go +++ b/command/agent/dns_test.go @@ -65,7 +65,7 @@ func TestDNS_NodeLookup(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -129,7 +129,7 @@ func TestDNS_NodeLookup_PeriodName(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node with period in name args := &structs.RegisterRequest{ @@ -171,7 +171,7 @@ func TestDNS_NodeLookup_AAAA(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -213,7 +213,7 @@ func TestDNS_NodeLookup_CNAME(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -256,7 +256,7 @@ func TestDNS_ServiceLookup(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -317,7 +317,7 @@ func TestDNS_ServiceLookup_TagPeriod(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -378,7 +378,7 @@ func TestDNS_ServiceLookup_Dedup(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -455,7 +455,7 @@ func TestDNS_ServiceLookup_Dedup_SRV(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ @@ -584,7 +584,7 @@ func TestDNS_ServiceLookup_FilterCritical(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register nodes args := &structs.RegisterRequest{ @@ -648,7 +648,7 @@ func TestDNS_ServiceLookup_Randomize(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register nodes for i := 0; i < 3*maxServiceResponses; i++ { @@ -712,7 +712,7 @@ func TestDNS_ServiceLookup_CNAME(t *testing.T) { defer os.RemoveAll(dir) defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Register node args := &structs.RegisterRequest{ diff --git a/command/agent/health_endpoint_test.go b/command/agent/health_endpoint_test.go index 4a097ae65..910d37df8 100644 --- a/command/agent/health_endpoint_test.go +++ b/command/agent/health_endpoint_test.go @@ -16,7 +16,7 @@ func TestHealthChecksInState(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") req, err := http.NewRequest("GET", "/v1/health/state/passing?dc=dc1", nil) if err != nil { @@ -43,7 +43,7 @@ func TestHealthNodeChecks(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") req, err := http.NewRequest("GET", fmt.Sprintf("/v1/health/node/%s?dc=dc1", srv.agent.config.NodeName), nil) @@ -71,7 +71,7 @@ func TestHealthServiceChecks(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Create a service check args := &structs.RegisterRequest{ @@ -115,7 +115,7 @@ func TestHealthServiceNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") req, err := http.NewRequest("GET", "/v1/health/service/consul?dc=dc1", nil) if err != nil { @@ -142,7 +142,7 @@ func TestHealthServiceNodes_PassingFilter(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") // Create a failing service check args := &structs.RegisterRequest{ diff --git a/command/agent/kvs_endpoint_test.go b/command/agent/kvs_endpoint_test.go index 3ccc12bdf..61216faaf 100644 --- a/command/agent/kvs_endpoint_test.go +++ b/command/agent/kvs_endpoint_test.go @@ -18,7 +18,7 @@ func TestKVSEndpoint_PUT_GET_DELETE(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") keys := []string{ "baz", @@ -93,7 +93,7 @@ func TestKVSEndpoint_Recurse(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") keys := []string{ "bar", @@ -189,7 +189,7 @@ func TestKVSEndpoint_CAS(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") { buf := bytes.NewBuffer([]byte("test")) @@ -286,7 +286,7 @@ func TestKVSEndpoint_ListKeys(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - testutil.WaitForLeader(t, srv.agent.RPC) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") keys := []string{ "bar", diff --git a/command/agent/local_test.go b/command/agent/local_test.go index 2eed35d68..9ce16fed3 100644 --- a/command/agent/local_test.go +++ b/command/agent/local_test.go @@ -15,7 +15,7 @@ func TestAgentAntiEntropy_Services(t *testing.T) { defer os.RemoveAll(dir) defer agent.Shutdown() - testutil.WaitForLeader(t, agent.RPC) + testutil.WaitForLeader(t, agent.RPC, "dc1") // Register info args := &structs.RegisterRequest{ @@ -137,7 +137,7 @@ func TestAgentAntiEntropy_Checks(t *testing.T) { defer os.RemoveAll(dir) defer agent.Shutdown() - testutil.WaitForLeader(t, agent.RPC) + testutil.WaitForLeader(t, agent.RPC, "dc1") // Register info args := &structs.RegisterRequest{ diff --git a/consul/leader_test.go b/consul/leader_test.go index 09e1856e5..f5b56174e 100644 --- a/consul/leader_test.go +++ b/consul/leader_test.go @@ -20,7 +20,7 @@ func TestLeader_RegisterMember(t *testing.T) { defer c1.Shutdown() client := rpcClient(t, s1) - testutil.WaitForLeader(t, client.Call) + testutil.WaitForLeader(t, client.Call, "dc1") // Try to join addr := fmt.Sprintf("127.0.0.1:%d", diff --git a/testutil/wait.go b/testutil/wait.go index 20ab8ea5e..7eb49a8e4 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -29,11 +29,11 @@ func WaitForResult(test testFn, error errorFn) { type rpcFn func(string, interface {}, interface {}) error -func WaitForLeader(t *testing.T, rpc rpcFn) structs.IndexedNodes { +func WaitForLeader(t *testing.T, rpc rpcFn, dc string) structs.IndexedNodes { var out structs.IndexedNodes WaitForResult(func() (bool, error) { args := &structs.RegisterRequest{ - Datacenter: "dc1", + Datacenter: dc, } err := rpc("Catalog.ListNodes", args, &out) return out.QueryMeta.KnownLeader, err From c391c5bfcb1492a475d57991576ea4ffe12cfa39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 01:18:08 +0200 Subject: [PATCH 17/39] Remove all sleeps from `health_endpoint_test.go` --- consul/health_endpoint_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/consul/health_endpoint_test.go b/consul/health_endpoint_test.go index 465f35846..b3dc39f43 100644 --- a/consul/health_endpoint_test.go +++ b/consul/health_endpoint_test.go @@ -14,8 +14,7 @@ func TestHealth_ChecksInState(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") arg := structs.RegisterRequest{ Datacenter: "dc1", @@ -61,8 +60,7 @@ func TestHealth_NodeChecks(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") arg := structs.RegisterRequest{ Datacenter: "dc1", @@ -103,8 +101,7 @@ func TestHealth_ServiceChecks(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") arg := structs.RegisterRequest{ Datacenter: "dc1", @@ -150,8 +147,7 @@ func TestHealth_ServiceNodes(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") arg := structs.RegisterRequest{ Datacenter: "dc1", From 89c018b68128c0839a1470280cbd73f1b093f1d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 01:18:35 +0200 Subject: [PATCH 18/39] Remove all sleeps from `catalog_endpoint_test.go` --- consul/catalog_endpoint_test.go | 57 +++++++++++++-------------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index 149651de6..25a35dba0 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -64,8 +64,8 @@ func TestCatalogRegister_ForwardLeader(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for a leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client1.Call, "dc1") + testutil.WaitForLeader(t, client2.Call, "dc1") // Use the follower as the client var client *rpc.Client @@ -109,8 +109,7 @@ func TestCatalogRegister_ForwardDC(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for the leaders - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc2") arg := structs.RegisterRequest{ Datacenter: "dc2", // SHould forward through s1 @@ -146,8 +145,7 @@ func TestCatalogDeregister(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") if err := client.Call("Catalog.Deregister", &arg, &out); err != nil { t.Fatalf("err: %v", err) @@ -171,7 +169,8 @@ func TestCatalogListDatacenters(t *testing.T) { if _, err := s2.JoinWAN([]string{addr}); err != nil { t.Fatalf("err: %v", err) } - time.Sleep(10 * time.Millisecond) + + testutil.WaitForLeader(t, client.Call, "dc1") var out []string if err := client.Call("Catalog.ListDatacenters", struct{}{}, &out); err != nil { @@ -208,8 +207,7 @@ func TestCatalogListNodes(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") // Just add a node s1.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) @@ -254,8 +252,8 @@ func TestCatalogListNodes_StaleRaad(t *testing.T) { t.Fatalf("err: %v", err) } - testutil.WaitForLeader(t, client1.Call) - testutil.WaitForLeader(t, client2.Call) + testutil.WaitForLeader(t, client1.Call, "dc1") + testutil.WaitForLeader(t, client2.Call, "dc1") // Use the follower as the client var client *rpc.Client @@ -318,8 +316,8 @@ func TestCatalogListNodes_ConsistentRead_Fail(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for a leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client1.Call, "dc1") + testutil.WaitForLeader(t, client2.Call, "dc1") // Use the leader as the client, kill the follower var client *rpc.Client @@ -368,8 +366,8 @@ func TestCatalogListNodes_ConsistentRead(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for a leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client1.Call, "dc1") + testutil.WaitForLeader(t, client2.Call, "dc1") // Use the leader as the client, kill the follower var client *rpc.Client @@ -403,9 +401,6 @@ func BenchmarkCatalogListNodes(t *testing.B) { client := rpcClient(nil, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Just add a node s1.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) @@ -436,8 +431,7 @@ func TestCatalogListServices(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") // Just add a node s1.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) @@ -474,8 +468,7 @@ func TestCatalogListServices_Blocking(t *testing.T) { } var out structs.IndexedServices - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") // Run the query if err := client.Call("Catalog.ListServices", &args, &out); err != nil { @@ -489,7 +482,7 @@ func TestCatalogListServices_Blocking(t *testing.T) { // Async cause a change start := time.Now() go func() { - time.Sleep(100 * time.Millisecond) + time.Sleep(50 * time.Millisecond) s1.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) s1.fsm.State().EnsureService(2, "foo", &structs.NodeService{"db", "db", []string{"primary"}, 5000}) }() @@ -501,7 +494,7 @@ func TestCatalogListServices_Blocking(t *testing.T) { } // Should block at least 100ms - if time.Now().Sub(start) < 100*time.Millisecond { + if time.Now().Sub(start) < 50 * time.Millisecond { t.Fatalf("too fast") } @@ -528,8 +521,7 @@ func TestCatalogListServices_Timeout(t *testing.T) { } var out structs.IndexedServices - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") // Run the query if err := client.Call("Catalog.ListServices", &args, &out); err != nil { @@ -538,7 +530,7 @@ func TestCatalogListServices_Timeout(t *testing.T) { // Setup a blocking query args.MinQueryIndex = out.Index - args.MaxQueryTime = 100 * time.Millisecond + args.MaxQueryTime = 50 * time.Millisecond // Re-run the query start := time.Now() @@ -548,7 +540,7 @@ func TestCatalogListServices_Timeout(t *testing.T) { } // Should block at least 100ms - if time.Now().Sub(start) < 100*time.Millisecond { + if time.Now().Sub(start) < 50 * time.Millisecond { t.Fatalf("too fast") } @@ -610,8 +602,7 @@ func TestCatalogListServiceNodes(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") // Just add a node s1.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) @@ -654,8 +645,7 @@ func TestCatalogNodeServices(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") // Just add a node s1.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) @@ -706,8 +696,7 @@ func TestCatalogRegister_FailedCase1(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") if err := client.Call("Catalog.Register", &arg, &out); err != nil { t.Fatalf("err: %v", err) From bf540c72178e3135c951ee2515ceb7fb235b4a84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 01:19:05 +0200 Subject: [PATCH 19/39] Remove most sleeps from `rpc_client_test.go` --- command/agent/rpc_client_test.go | 74 +++++++++++++------------------- 1 file changed, 30 insertions(+), 44 deletions(-) diff --git a/command/agent/rpc_client_test.go b/command/agent/rpc_client_test.go index a31d6672a..95b7292d8 100644 --- a/command/agent/rpc_client_test.go +++ b/command/agent/rpc_client_test.go @@ -3,12 +3,13 @@ package agent import ( "fmt" "github.com/hashicorp/serf/serf" - "github.com/hashicorp/serf/testutil" + "github.com/hashicorp/consul/testutil" "io" "net" "os" "strings" "testing" + "errors" "time" ) @@ -59,35 +60,32 @@ func TestRPCClientForceLeave(t *testing.T) { p2 := testRPCClient(t) defer p1.Close() defer p2.Close() - testutil.Yield() s2Addr := fmt.Sprintf("127.0.0.1:%d", p2.agent.config.Ports.SerfLan) if _, err := p1.agent.JoinLAN([]string{s2Addr}); err != nil { t.Fatalf("err: %s", err) } - testutil.Yield() - if err := p2.agent.Shutdown(); err != nil { t.Fatalf("err: %s", err) } - time.Sleep(time.Second) - if err := p1.client.ForceLeave(p2.agent.config.NodeName); err != nil { t.Fatalf("err: %s", err) } - testutil.Yield() - m := p1.agent.LANMembers() if len(m) != 2 { t.Fatalf("should have 2 members: %#v", m) } - if m[1].Status != serf.StatusLeft { - t.Fatalf("should be left: %#v %v", m[1], m[1].Status == serf.StatusLeft) - } + testutil.WaitForResult(func() (bool, error) { + m := p1.agent.LANMembers() + success := m[1].Status == serf.StatusLeft + return success, errors.New(m[1].Status.String()) + }, func(err error) { + t.Fatalf("member status is %v, should be left", err) + }) } func TestRPCClientJoinLAN(t *testing.T) { @@ -95,7 +93,6 @@ func TestRPCClientJoinLAN(t *testing.T) { p2 := testRPCClient(t) defer p1.Close() defer p2.Close() - testutil.Yield() s2Addr := fmt.Sprintf("127.0.0.1:%d", p2.agent.config.Ports.SerfLan) n, err := p1.client.Join([]string{s2Addr}, false) @@ -113,7 +110,6 @@ func TestRPCClientJoinWAN(t *testing.T) { p2 := testRPCClient(t) defer p1.Close() defer p2.Close() - testutil.Yield() s2Addr := fmt.Sprintf("127.0.0.1:%d", p2.agent.config.Ports.SerfWan) n, err := p1.client.Join([]string{s2Addr}, true) @@ -131,7 +127,6 @@ func TestRPCClientLANMembers(t *testing.T) { p2 := testRPCClient(t) defer p1.Close() defer p2.Close() - testutil.Yield() mem, err := p1.client.LANMembers() if err != nil { @@ -148,8 +143,6 @@ func TestRPCClientLANMembers(t *testing.T) { t.Fatalf("err: %s", err) } - testutil.Yield() - mem, err = p1.client.LANMembers() if err != nil { t.Fatalf("err: %s", err) @@ -165,7 +158,6 @@ func TestRPCClientWANMembers(t *testing.T) { p2 := testRPCClient(t) defer p1.Close() defer p2.Close() - testutil.Yield() mem, err := p1.client.WANMembers() if err != nil { @@ -182,8 +174,6 @@ func TestRPCClientWANMembers(t *testing.T) { t.Fatalf("err: %s", err) } - testutil.Yield() - mem, err = p1.client.WANMembers() if err != nil { t.Fatalf("err: %s", err) @@ -194,16 +184,33 @@ func TestRPCClientWANMembers(t *testing.T) { } } +func TestRPCClientStats(t *testing.T) { + p1 := testRPCClient(t) + defer p1.Close() + + stats, err := p1.client.Stats() + if err != nil { + t.Fatalf("err: %s", err) + } + + if _, ok := stats["agent"]; !ok { + t.Fatalf("bad: %#v", stats) + } + + if _, ok := stats["consul"]; !ok { + t.Fatalf("bad: %#v", stats) + } +} + func TestRPCClientLeave(t *testing.T) { p1 := testRPCClient(t) defer p1.Close() - testutil.Yield() if err := p1.client.Leave(); err != nil { t.Fatalf("err: %s", err) } - testutil.Yield() + time.Sleep(1 * time.Second) select { case <-p1.agent.ShutdownCh(): @@ -215,7 +222,6 @@ func TestRPCClientLeave(t *testing.T) { func TestRPCClientMonitor(t *testing.T) { p1 := testRPCClient(t) defer p1.Close() - testutil.Yield() eventCh := make(chan string, 64) if handle, err := p1.client.Monitor("debug", eventCh); err != nil { @@ -224,8 +230,6 @@ func TestRPCClientMonitor(t *testing.T) { defer p1.client.Stop(handle) } - testutil.Yield() - found := false OUTER1: for { @@ -244,7 +248,8 @@ OUTER1: // Join a bad thing to generate more events p1.agent.JoinLAN(nil) - testutil.Yield() + + time.Sleep(1 * time.Second) found = false OUTER2: @@ -262,22 +267,3 @@ OUTER2: t.Fatalf("should log joining") } } - -func TestRPCClientStats(t *testing.T) { - p1 := testRPCClient(t) - defer p1.Close() - testutil.Yield() - - stats, err := p1.client.Stats() - if err != nil { - t.Fatalf("err: %s", err) - } - - if _, ok := stats["agent"]; !ok { - t.Fatalf("bad: %#v", stats) - } - - if _, ok := stats["consul"]; !ok { - t.Fatalf("bad: %#v", stats) - } -} From d0b38ad641e4a73e3194d46bcfb30f546bedb304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 01:21:13 +0200 Subject: [PATCH 20/39] Fix imports in `health_endpoint_test.go` --- consul/health_endpoint_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consul/health_endpoint_test.go b/consul/health_endpoint_test.go index b3dc39f43..b817f9db0 100644 --- a/consul/health_endpoint_test.go +++ b/consul/health_endpoint_test.go @@ -1,10 +1,10 @@ package consul import ( + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "os" "testing" - "time" ) func TestHealth_ChecksInState(t *testing.T) { From 5a56d8a2c6ab3748438bfdd0e70dce584b552d83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 01:23:54 +0200 Subject: [PATCH 21/39] Remove all sleeps from `status_endpoint_test.go` --- consul/status_endpoint_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/consul/status_endpoint_test.go b/consul/status_endpoint_test.go index 683b4bb74..75ff51947 100644 --- a/consul/status_endpoint_test.go +++ b/consul/status_endpoint_test.go @@ -1,6 +1,7 @@ package consul import ( + "github.com/hashicorp/consul/testutil" "github.com/ugorji/go/codec" "net" "net/rpc" @@ -38,7 +39,7 @@ func TestStatusLeader(t *testing.T) { t.Fatalf("unexpected leader: %v", leader) } - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") if err := client.Call("Status.Leader", arg, &leader); err != nil { t.Fatalf("err: %v", err) From b108af2dd3f81e2e466a7514d5ab66573f41724b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 01:38:58 +0200 Subject: [PATCH 22/39] Remove all sleeps from `server_test.go` --- consul/server_test.go | 53 +++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/consul/server_test.go b/consul/server_test.go index 45edac53d..1b03cf83d 100644 --- a/consul/server_test.go +++ b/consul/server_test.go @@ -2,11 +2,13 @@ package consul import ( "fmt" + "github.com/hashicorp/consul/testutil" "io/ioutil" "net" "os" "testing" "time" + "errors" ) var nextPort = 15000 @@ -168,16 +170,16 @@ func TestServer_JoinWAN(t *testing.T) { t.Fatalf("bad len") } - time.Sleep(10 * time.Millisecond) - // Check the remoteConsuls has both if len(s1.remoteConsuls) != 2 { t.Fatalf("remote consul missing") } - if len(s2.remoteConsuls) != 2 { + testutil.WaitForResult(func() (bool, error) { + return len(s2.remoteConsuls) == 2, nil + }, func(err error) { t.Fatalf("remote consul missing") - } + }) } func TestServer_Leave(t *testing.T) { @@ -197,17 +199,22 @@ func TestServer_Leave(t *testing.T) { t.Fatalf("err: %v", err) } - time.Sleep(time.Second) + var p1 []net.Addr + var p2 []net.Addr - p1, _ := s1.raftPeers.Peers() - if len(p1) != 2 { - t.Fatalf("should have 2 peers: %v", p1) - } + testutil.WaitForResult(func() (bool, error) { + p1, _ = s1.raftPeers.Peers() + return len(p1) == 2, errors.New(fmt.Sprintf("%v", p1)) + }, func(err error) { + t.Fatalf("should have 2 peers: %v", err) + }) - p2, _ := s2.raftPeers.Peers() - if len(p2) != 2 { - t.Fatalf("should have 2 peers: %v", p2) - } + testutil.WaitForResult(func() (bool, error) { + p2, _ = s2.raftPeers.Peers() + return len(p2) == 2, errors.New(fmt.Sprintf("%v", p1)) + }, func(err error) { + t.Fatalf("should have 2 peers: %v", err) + }) // Issue a leave if err := s2.Leave(); err != nil { @@ -272,14 +279,16 @@ func TestServer_JoinLAN_TLS(t *testing.T) { t.Fatalf("bad len") } - // Wait a while - time.Sleep(100 * time.Millisecond) - // Verify Raft has established a peer - if s1.Stats()["raft"]["num_peers"] != "1" { - t.Fatalf("bad: %v", s1.Stats()["raft"]) - } - if s2.Stats()["raft"]["num_peers"] != "1" { - t.Fatalf("bad: %v", s2.Stats()["raft"]) - } + testutil.WaitForResult(func() (bool, error) { + return s1.Stats()["raft"]["num_peers"] == "1", nil + }, func(err error) { + t.Fatalf("no peer established") + }) + + testutil.WaitForResult(func() (bool, error) { + return s2.Stats()["raft"]["num_peers"] == "1", nil + }, func(err error) { + t.Fatalf("no peer established") + }) } From 44d3985ea8cfdea184154405189cb428e5b1d298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:04:52 +0200 Subject: [PATCH 23/39] Remove all sleeps from `leader_test.go` --- consul/leader_test.go | 168 +++++++++++++++++------------------------- 1 file changed, 66 insertions(+), 102 deletions(-) diff --git a/consul/leader_test.go b/consul/leader_test.go index f5b56174e..955c00e22 100644 --- a/consul/leader_test.go +++ b/consul/leader_test.go @@ -7,7 +7,7 @@ import ( "github.com/hashicorp/serf/serf" "os" "testing" - "time" + "errors" ) func TestLeader_RegisterMember(t *testing.T) { @@ -19,9 +19,6 @@ func TestLeader_RegisterMember(t *testing.T) { defer os.RemoveAll(dir2) defer c1.Shutdown() - client := rpcClient(t, s1) - testutil.WaitForLeader(t, client.Call, "dc1") - // Try to join addr := fmt.Sprintf("127.0.0.1:%d", s1.config.SerfLANConfig.MemberlistConfig.BindPort) @@ -29,8 +26,8 @@ func TestLeader_RegisterMember(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for registration - time.Sleep(10 * time.Millisecond) + client := rpcClient(t, s1) + testutil.WaitForLeader(t, client.Call, "dc1") // Client should be registered state := s1.fsm.State() @@ -76,8 +73,8 @@ func TestLeader_FailedMember(t *testing.T) { defer os.RemoveAll(dir2) defer c1.Shutdown() - // Wait until we have a leader - time.Sleep(100 * time.Millisecond) + client := rpcClient(t, s1) + testutil.WaitForLeader(t, client.Call, "dc1") // Try to join addr := fmt.Sprintf("127.0.0.1:%d", @@ -89,9 +86,6 @@ func TestLeader_FailedMember(t *testing.T) { // Fail the member c1.Shutdown() - // Wait for failure detection - time.Sleep(500 * time.Millisecond) - // Should be registered state := s1.fsm.State() _, found, _ := state.GetNode(c1.config.NodeName) @@ -110,9 +104,13 @@ func TestLeader_FailedMember(t *testing.T) { if checks[0].Name != SerfCheckName { t.Fatalf("bad check: %v", checks[0]) } - if checks[0].Status != structs.HealthCritical { - t.Fatalf("bad check: %v", checks[0]) - } + + testutil.WaitForResult(func() (bool, error) { + _, checks = state.NodeChecks(c1.config.NodeName) + return checks[0].Status == structs.HealthCritical, errors.New(checks[0].Status) + }, func(err error) { + t.Fatalf("check status is %v, should be critical", err) + }) } func TestLeader_LeftMember(t *testing.T) { @@ -124,9 +122,6 @@ func TestLeader_LeftMember(t *testing.T) { defer os.RemoveAll(dir2) defer c1.Shutdown() - // Wait until we have a leader - time.Sleep(100 * time.Millisecond) - // Try to join addr := fmt.Sprintf("127.0.0.1:%d", s1.config.SerfLANConfig.MemberlistConfig.BindPort) @@ -134,28 +129,28 @@ func TestLeader_LeftMember(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for registration - time.Sleep(10 * time.Millisecond) + var found bool + state := s1.fsm.State() // Should be registered - state := s1.fsm.State() - _, found, _ := state.GetNode(c1.config.NodeName) - if !found { - t.Fatalf("client not registered") - } + testutil.WaitForResult(func() (bool, error) { + _, found, _ = state.GetNode(c1.config.NodeName) + return found == true, nil + }, func(err error) { + t.Fatalf("client should be registered") + }) // Node should leave c1.Leave() c1.Shutdown() - // Wait for failure detection - time.Sleep(500 * time.Millisecond) - // Should be deregistered - _, found, _ = state.GetNode(c1.config.NodeName) - if found { - t.Fatalf("client registered") - } + testutil.WaitForResult(func() (bool, error) { + _, found, _ = state.GetNode(c1.config.NodeName) + return found == false, nil + }, func(err error) { + t.Fatalf("client should not be registered") + }) } func TestLeader_ReapMember(t *testing.T) { @@ -167,9 +162,6 @@ func TestLeader_ReapMember(t *testing.T) { defer os.RemoveAll(dir2) defer c1.Shutdown() - // Wait until we have a leader - time.Sleep(100 * time.Millisecond) - // Try to join addr := fmt.Sprintf("127.0.0.1:%d", s1.config.SerfLANConfig.MemberlistConfig.BindPort) @@ -177,15 +169,16 @@ func TestLeader_ReapMember(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait for registration - time.Sleep(10 * time.Millisecond) + var found bool + state := s1.fsm.State() // Should be registered - state := s1.fsm.State() - _, found, _ := state.GetNode(c1.config.NodeName) - if !found { - t.Fatalf("client not registered") - } + testutil.WaitForResult(func() (bool, error) { + _, found, _ = state.GetNode(c1.config.NodeName) + return found == true, nil + }, func(err error) { + t.Fatalf("client should be registered") + }) // Simulate a node reaping mems := s1.LANMembers() @@ -199,14 +192,13 @@ func TestLeader_ReapMember(t *testing.T) { } s1.reconcileCh <- c1mem - // Wait to reconcile - time.Sleep(10 * time.Millisecond) - // Should be deregistered - _, found, _ = state.GetNode(c1.config.NodeName) - if found { - t.Fatalf("client registered") - } + testutil.WaitForResult(func() (bool, error) { + _, found, _ = state.GetNode(c1.config.NodeName) + return found == false, nil + }, func(err error) { + t.Fatalf("client should not be registered") + }) } func TestLeader_Reconcile_ReapMember(t *testing.T) { @@ -214,8 +206,8 @@ func TestLeader_Reconcile_ReapMember(t *testing.T) { defer os.RemoveAll(dir1) defer s1.Shutdown() - // Wait until we have a leader - time.Sleep(100 * time.Millisecond) + client := rpcClient(t, s1) + testutil.WaitForLeader(t, client.Call, "dc1") // Register a non-existing member dead := structs.RegisterRequest{ @@ -270,14 +262,13 @@ func TestLeader_Reconcile(t *testing.T) { t.Fatalf("client registered") } - // Wait for leader - time.Sleep(100 * time.Millisecond) - // Should be registered - _, found, _ = state.GetNode(c1.config.NodeName) - if !found { - t.Fatalf("client not registered") - } + testutil.WaitForResult(func() (bool, error) { + _, found, _ = state.GetNode(c1.config.NodeName) + return found == true, nil + }, func(err error) { + t.Fatalf("client should be registered") + }) } func TestLeader_LeftServer(t *testing.T) { @@ -304,48 +295,30 @@ func TestLeader_LeftServer(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait until we have 3 peers - start := time.Now() -CHECK1: for _, s := range servers { - peers, _ := s.raftPeers.Peers() - if len(peers) != 3 { - if time.Now().Sub(start) >= 2*time.Second { - t.Fatalf("should have 3 peers") - } else { - time.Sleep(100 * time.Millisecond) - goto CHECK1 - } - } + testutil.WaitForResult(func() (bool, error) { + peers, _ := s.raftPeers.Peers() + return len(peers) == 3, nil + }, func(err error) { + t.Fatalf("should have 3 peers") + }) } // Kill any server servers[0].Shutdown() - // Wait for failure detection - time.Sleep(500 * time.Millisecond) - // 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) } - // Wait for intent propagation - time.Sleep(500 * time.Millisecond) - - // Wait until we have 2 peers - start = time.Now() -CHECK2: for _, s := range servers[1:] { - peers, _ := s.raftPeers.Peers() - if len(peers) != 2 { - if time.Now().Sub(start) >= 2*time.Second { - t.Fatalf("should have 2 peers") - } else { - time.Sleep(100 * time.Millisecond) - goto CHECK2 - } - } + testutil.WaitForResult(func() (bool, error) { + peers, _ := s.raftPeers.Peers() + return len(peers) == 2, nil + }, func(err error) { + t.Fatalf("should have 2 peers") + }) } } @@ -367,24 +340,15 @@ func TestLeader_MultiBootstrap(t *testing.T) { t.Fatalf("err: %v", err) } - // Wait until we have 2 peers - start := time.Now() -CHECK1: for _, s := range servers { - peers := s.serfLAN.Members() - if len(peers) != 2 { - if time.Now().Sub(start) >= 2*time.Second { - t.Fatalf("should have 2 peers") - } else { - time.Sleep(100 * time.Millisecond) - goto CHECK1 - } - } + testutil.WaitForResult(func() (bool, error) { + peers := s.serfLAN.Members() + return len(peers) == 2, nil + }, func(err error) { + t.Fatalf("should have 2 peers") + }) } - // Wait to ensure no peer is added - time.Sleep(200 * time.Millisecond) - // Ensure we don't have multiple raft peers for _, s := range servers { peers, _ := s.raftPeers.Peers() From e2e5999fdb9385fd7e658d58719a5cf176e31d18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:06:19 +0200 Subject: [PATCH 24/39] Remove all sleeps from `status_endpoint_test.go --- command/agent/status_endpoint_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/command/agent/status_endpoint_test.go b/command/agent/status_endpoint_test.go index 835b80bd9..4dd1f4e20 100644 --- a/command/agent/status_endpoint_test.go +++ b/command/agent/status_endpoint_test.go @@ -3,7 +3,7 @@ package agent import ( "os" "testing" - "time" + "github.com/hashicorp/consul/testutil" ) func TestStatusLeader(t *testing.T) { @@ -12,8 +12,7 @@ func TestStatusLeader(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for a leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") obj, err := srv.StatusLeader(nil, nil) if err != nil { From 3a71349ee40e026930a659e916b9b4e857dab4fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:08:01 +0200 Subject: [PATCH 25/39] Remove all sleeps from `ui_endpoint_test.go` --- command/agent/ui_endpoint_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/command/agent/ui_endpoint_test.go b/command/agent/ui_endpoint_test.go index c056e433e..ab734f3cb 100644 --- a/command/agent/ui_endpoint_test.go +++ b/command/agent/ui_endpoint_test.go @@ -3,6 +3,7 @@ package agent import ( "bytes" "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "io" "io/ioutil" @@ -12,7 +13,6 @@ import ( "path/filepath" "reflect" "testing" - "time" ) func TestUiIndex(t *testing.T) { @@ -60,8 +60,7 @@ func TestUiNodes(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") req, err := http.NewRequest("GET", "/v1/internal/ui/nodes/dc1", nil) if err != nil { @@ -88,8 +87,7 @@ func TestUiNodeInfo(t *testing.T) { defer srv.Shutdown() defer srv.agent.Shutdown() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, srv.agent.RPC, "dc1") req, err := http.NewRequest("GET", fmt.Sprintf("/v1/internal/ui/node/%s", srv.agent.config.NodeName), nil) From e27f89fec1a494f3f96b4a8d597e477e8bf71929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:09:53 +0200 Subject: [PATCH 26/39] Remove all sleeps from `kvs_endpoint_test.go` --- consul/kvs_endpoint_test.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/consul/kvs_endpoint_test.go b/consul/kvs_endpoint_test.go index 5c3eef621..32ab830ea 100644 --- a/consul/kvs_endpoint_test.go +++ b/consul/kvs_endpoint_test.go @@ -1,10 +1,10 @@ package consul import ( + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "os" "testing" - "time" ) func TestKVS_Apply(t *testing.T) { @@ -14,8 +14,7 @@ func TestKVS_Apply(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") arg := structs.KVSRequest{ Datacenter: "dc1", @@ -71,8 +70,7 @@ func TestKVS_Get(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") arg := structs.KVSRequest{ Datacenter: "dc1", @@ -119,8 +117,7 @@ func TestKVSEndpoint_List(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") keys := []string{ "/test/key1", @@ -179,8 +176,7 @@ func TestKVSEndpoint_ListKeys(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") keys := []string{ "/test/key1", From d6f3dc510c08d2d177ed76a2fab07599946fa98d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:11:40 +0200 Subject: [PATCH 27/39] Remove all sleeps from `internal_endpoint_test.go` --- consul/internal_endpoint_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/consul/internal_endpoint_test.go b/consul/internal_endpoint_test.go index a5ff17373..9b08fe74d 100644 --- a/consul/internal_endpoint_test.go +++ b/consul/internal_endpoint_test.go @@ -1,10 +1,10 @@ package consul import ( + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "os" "testing" - "time" ) func TestInternal_NodeInfo(t *testing.T) { @@ -14,8 +14,7 @@ func TestInternal_NodeInfo(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") arg := structs.RegisterRequest{ Datacenter: "dc1", @@ -68,8 +67,7 @@ func TestInternal_NodeDump(t *testing.T) { client := rpcClient(t, s1) defer client.Close() - // Wait for leader - time.Sleep(100 * time.Millisecond) + testutil.WaitForLeader(t, client.Call, "dc1") arg := structs.RegisterRequest{ Datacenter: "dc1", From 9b6b75f77cdb7dd6df5f28fcc9b280cebc7ee12f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:14:39 +0200 Subject: [PATCH 28/39] Fix race condition in `TestLeader_FailedMember` --- consul/leader_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/consul/leader_test.go b/consul/leader_test.go index 955c00e22..39935c2ae 100644 --- a/consul/leader_test.go +++ b/consul/leader_test.go @@ -88,10 +88,12 @@ func TestLeader_FailedMember(t *testing.T) { // Should be registered state := s1.fsm.State() - _, found, _ := state.GetNode(c1.config.NodeName) - if !found { + testutil.WaitForResult(func() (bool, error) { + _, found, _ := state.GetNode(c1.config.NodeName) + return found == true, nil + }, func(err error) { t.Fatalf("client not registered") - } + }) // Should have a check _, checks := state.NodeChecks(c1.config.NodeName) From cb54c4cb4952694a5d2f5a85bbd03e34469b8c9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:29:51 +0200 Subject: [PATCH 29/39] Remove all sleeps from `client_test.go` --- consul/client_test.go | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/consul/client_test.go b/consul/client_test.go index 7aff15fce..b0b748eef 100644 --- a/consul/client_test.go +++ b/consul/client_test.go @@ -2,6 +2,7 @@ package consul import ( "fmt" + "github.com/hashicorp/consul/testutil" "github.com/hashicorp/consul/consul/structs" "net" "os" @@ -80,12 +81,12 @@ func TestClient_JoinLAN(t *testing.T) { t.Fatalf("bad len") } - time.Sleep(10 * time.Millisecond) - // Check we have a new consul - if len(c1.consuls) != 1 { + testutil.WaitForResult(func() (bool, error) { + return len(c1.consuls) == 1, nil + }, func(err error) { t.Fatalf("expected consul server") - } + }) } func TestClient_RPC(t *testing.T) { @@ -119,12 +120,13 @@ func TestClient_RPC(t *testing.T) { t.Fatalf("bad len") } - time.Sleep(10 * time.Millisecond) - - // RPC shoudl succeed - if err := c1.RPC("Status.Ping", struct{}{}, &out); err != nil { + // RPC should succeed + testutil.WaitForResult(func() (bool, error) { + err := c1.RPC("Status.Ping", struct{}{}, &out) + return err == nil, err + }, func(err error) { t.Fatalf("err: %v", err) - } + }) } func TestClient_RPC_TLS(t *testing.T) { @@ -171,10 +173,11 @@ func TestClient_RPC_TLS(t *testing.T) { t.Fatalf("bad len") } - time.Sleep(10 * time.Millisecond) - - // RPC shoudl succeed - if err := c1.RPC("Status.Ping", struct{}{}, &out); err != nil { + // RPC should succeed + testutil.WaitForResult(func() (bool, error) { + err := c1.RPC("Status.Ping", struct{}{}, &out) + return err == nil, err + }, func(err error) { t.Fatalf("err: %v", err) - } + }) } From f3354b028e46d2bea0438d0925bebf311516a6a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:30:05 +0200 Subject: [PATCH 30/39] Fix race condition in `TestHTTPAgentJoin_WAN` --- command/agent/agent_endpoint_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/command/agent/agent_endpoint_test.go b/command/agent/agent_endpoint_test.go index 78406de73..fee1b28a6 100644 --- a/command/agent/agent_endpoint_test.go +++ b/command/agent/agent_endpoint_test.go @@ -169,9 +169,11 @@ func TestHTTPAgentJoin_WAN(t *testing.T) { t.Fatalf("Err: %v", obj) } - if len(a2.WANMembers()) != 2 { + testutil.WaitForResult(func() (bool, error) { + return len(a2.WANMembers()) == 2, nil + }, func(err error) { t.Fatalf("should have 2 members") - } + }) } func TestHTTPAgentForceLeave(t *testing.T) { From 30f8573c7801bad32d3539955539328b5501b5b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:36:25 +0200 Subject: [PATCH 31/39] Change blocking call sleep time --- consul/catalog_endpoint_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index 25a35dba0..3db8be3f2 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -482,7 +482,7 @@ func TestCatalogListServices_Blocking(t *testing.T) { // Async cause a change start := time.Now() go func() { - time.Sleep(50 * time.Millisecond) + time.Sleep(100 * time.Millisecond) s1.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) s1.fsm.State().EnsureService(2, "foo", &structs.NodeService{"db", "db", []string{"primary"}, 5000}) }() @@ -494,7 +494,7 @@ func TestCatalogListServices_Blocking(t *testing.T) { } // Should block at least 100ms - if time.Now().Sub(start) < 50 * time.Millisecond { + if time.Now().Sub(start) < 100 * time.Millisecond { t.Fatalf("too fast") } @@ -530,7 +530,7 @@ func TestCatalogListServices_Timeout(t *testing.T) { // Setup a blocking query args.MinQueryIndex = out.Index - args.MaxQueryTime = 50 * time.Millisecond + args.MaxQueryTime = 100 * time.Millisecond // Re-run the query start := time.Now() @@ -540,7 +540,7 @@ func TestCatalogListServices_Timeout(t *testing.T) { } // Should block at least 100ms - if time.Now().Sub(start) < 50 * time.Millisecond { + if time.Now().Sub(start) < 100 * time.Millisecond { t.Fatalf("too fast") } From e1471576a4d5863c79d991f0c654dc0131c3daca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 02:43:48 +0200 Subject: [PATCH 32/39] Increase timeout for `WaitForResult` --- testutil/wait.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testutil/wait.go b/testutil/wait.go index 7eb49a8e4..dd51d8c26 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -10,10 +10,10 @@ type testFn func() (bool, error) type errorFn func(error) func WaitForResult(test testFn, error errorFn) { - retries := 100 // 5 seconds timeout + retries := 100 for retries > 0 { - time.Sleep(50 * time.Millisecond) + time.Sleep(100 * time.Millisecond) retries-- success, err := test() From f7a33c2731d726455f40d0f88e2b826dfbaf7507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 03:00:43 +0200 Subject: [PATCH 33/39] Speed up test runs in `WaitForResult` --- testutil/wait.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testutil/wait.go b/testutil/wait.go index dd51d8c26..e3e0e9149 100644 --- a/testutil/wait.go +++ b/testutil/wait.go @@ -10,10 +10,10 @@ type testFn func() (bool, error) type errorFn func(error) func WaitForResult(test testFn, error errorFn) { - retries := 100 + retries := 1000 for retries > 0 { - time.Sleep(100 * time.Millisecond) + time.Sleep(10 * time.Millisecond) retries-- success, err := test() From a211e1164c5068854c39e9431a888ca67a240689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 03:00:56 +0200 Subject: [PATCH 34/39] Fix race condition in `TestCatalogListNodes` --- consul/catalog_endpoint_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index 3db8be3f2..3945dd51d 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -212,13 +212,12 @@ func TestCatalogListNodes(t *testing.T) { // Just add a node s1.fsm.State().EnsureNode(1, structs.Node{"foo", "127.0.0.1"}) - if err := client.Call("Catalog.ListNodes", &args, &out); err != nil { + testutil.WaitForResult(func() (bool, error) { + client.Call("Catalog.ListNodes", &args, &out) + return len(out.Nodes) == 2, nil + }, func(err error) { t.Fatalf("err: %v", err) - } - - if len(out.Nodes) != 2 { - t.Fatalf("bad: %v", out) - } + }) // Server node is auto added from Serf if out.Nodes[0].Node != s1.config.NodeName { From 16bfc8fd1d1781886374cd15cef65081ce496cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 03:05:38 +0200 Subject: [PATCH 35/39] Fix race condition in `TestLeader_RegisterMember` --- consul/leader_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/consul/leader_test.go b/consul/leader_test.go index 39935c2ae..b99924eeb 100644 --- a/consul/leader_test.go +++ b/consul/leader_test.go @@ -31,10 +31,12 @@ func TestLeader_RegisterMember(t *testing.T) { // Client should be registered state := s1.fsm.State() - _, found, _ := state.GetNode(c1.config.NodeName) - if !found { + testutil.WaitForResult(func() (bool, error) { + _, found, _ := state.GetNode(c1.config.NodeName) + return found == true, nil + }, func(err error) { t.Fatalf("client not registered") - } + }) // Should have a check _, checks := state.NodeChecks(c1.config.NodeName) @@ -52,7 +54,7 @@ func TestLeader_RegisterMember(t *testing.T) { } // Server should be registered - _, found, _ = state.GetNode(s1.config.NodeName) + _, found, _ := state.GetNode(s1.config.NodeName) if !found { t.Fatalf("server not registered") } From 31c962f2a9543680010925f1e086edcc21a09374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 03:24:13 +0200 Subject: [PATCH 36/39] Sleep since no obvious way to wait for shutdown --- consul/leader_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/consul/leader_test.go b/consul/leader_test.go index b99924eeb..8e4c4a63b 100644 --- a/consul/leader_test.go +++ b/consul/leader_test.go @@ -8,6 +8,7 @@ import ( "os" "testing" "errors" + "time" ) func TestLeader_RegisterMember(t *testing.T) { @@ -310,6 +311,7 @@ func TestLeader_LeftServer(t *testing.T) { // Kill any server servers[0].Shutdown() + time.Sleep(100 * time.Millisecond) // Force remove the non-leader (transition to left state) if err := servers[1].RemoveFailedNode(servers[0].config.NodeName); err != nil { From d199dcde9bb8739554994cc6d1c449244e33d2fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 03:41:10 +0200 Subject: [PATCH 37/39] Run checks more often in `expectStatus` --- command/agent/check_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/agent/check_test.go b/command/agent/check_test.go index 54a9b15c9..07b4f598d 100644 --- a/command/agent/check_test.go +++ b/command/agent/check_test.go @@ -31,7 +31,7 @@ func expectStatus(t *testing.T, script, status string) { Notify: mock, CheckID: "foo", Script: script, - Interval: 25 * time.Millisecond, + Interval: 10 * time.Millisecond, Logger: log.New(os.Stderr, "", log.LstdFlags), } check.Start() From e749e2ebc97a5804579f2dc6912cda80690677da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 03:41:47 +0200 Subject: [PATCH 38/39] Wait wrap tests in `TestServer_JoinLAN` --- consul/server_test.go | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/consul/server_test.go b/consul/server_test.go index 1b03cf83d..64342f0e5 100644 --- a/consul/server_test.go +++ b/consul/server_test.go @@ -136,13 +136,17 @@ func TestServer_JoinLAN(t *testing.T) { } // Check the members - if len(s1.LANMembers()) != 2 { + testutil.WaitForResult(func() (bool, error) { + return len(s1.LANMembers()) == 2, nil + }, func(err error) { t.Fatalf("bad len") - } + }) - if len(s2.LANMembers()) != 2 { + testutil.WaitForResult(func() (bool, error) { + return len(s2.LANMembers()) == 2, nil + }, func(err error) { t.Fatalf("bad len") - } + }) } func TestServer_JoinWAN(t *testing.T) { @@ -162,13 +166,17 @@ func TestServer_JoinWAN(t *testing.T) { } // Check the members - if len(s1.WANMembers()) != 2 { + testutil.WaitForResult(func() (bool, error) { + return len(s1.WANMembers()) == 2, nil + }, func(err error) { t.Fatalf("bad len") - } + }) - if len(s2.WANMembers()) != 2 { + testutil.WaitForResult(func() (bool, error) { + return len(s2.WANMembers()) == 2, nil + }, func(err error) { t.Fatalf("bad len") - } + }) // Check the remoteConsuls has both if len(s1.remoteConsuls) != 2 { @@ -271,13 +279,17 @@ func TestServer_JoinLAN_TLS(t *testing.T) { } // Check the members - if len(s1.LANMembers()) != 2 { + testutil.WaitForResult(func() (bool, error) { + return len(s1.LANMembers()) == 2, nil + }, func(err error) { t.Fatalf("bad len") - } + }) - if len(s2.LANMembers()) != 2 { + testutil.WaitForResult(func() (bool, error) { + return len(s2.LANMembers()) == 2, nil + }, func(err error) { t.Fatalf("bad len") - } + }) // Verify Raft has established a peer testutil.WaitForResult(func() (bool, error) { From 2f23eb42ea347563e535eab908132e6c3889ef17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Tisa=CC=88ter?= Date: Fri, 9 May 2014 11:38:29 +0200 Subject: [PATCH 39/39] Add TODOs for failing tests --- command/agent/catalog_endpoint_test.go | 1 + command/agent/health_endpoint_test.go | 7 +++++++ command/agent/rpc_client_test.go | 1 + command/agent/ui_endpoint_test.go | 2 ++ consul/catalog_endpoint_test.go | 1 + 5 files changed, 12 insertions(+) diff --git a/command/agent/catalog_endpoint_test.go b/command/agent/catalog_endpoint_test.go index d9454ab7e..f8b2b6a2c 100644 --- a/command/agent/catalog_endpoint_test.go +++ b/command/agent/catalog_endpoint_test.go @@ -175,6 +175,7 @@ func TestCatalogNodes_Blocking(t *testing.T) { // Should block for a while if time.Now().Sub(start) < 50 * time.Millisecond { + // TODO: Failing t.Fatalf("too fast") } diff --git a/command/agent/health_endpoint_test.go b/command/agent/health_endpoint_test.go index 910d37df8..a4b4b9817 100644 --- a/command/agent/health_endpoint_test.go +++ b/command/agent/health_endpoint_test.go @@ -28,6 +28,8 @@ func TestHealthChecksInState(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } + + // TODO: Failing assertIndex(t, resp) // Should be 1 health check for the server @@ -105,6 +107,7 @@ func TestHealthServiceChecks(t *testing.T) { // Should be 1 health check for consul nodes := obj.(structs.HealthChecks) if len(nodes) != 1 { + // TODO: Failing t.Fatalf("bad: %v", obj) } } @@ -127,6 +130,8 @@ func TestHealthServiceNodes(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } + + // TODO: Failing assertIndex(t, resp) // Should be 1 health check for consul @@ -172,6 +177,8 @@ func TestHealthServiceNodes_PassingFilter(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } + + // TODO: Failing assertIndex(t, resp) // Should be 0 health check for consul diff --git a/command/agent/rpc_client_test.go b/command/agent/rpc_client_test.go index 95b7292d8..77f34632a 100644 --- a/command/agent/rpc_client_test.go +++ b/command/agent/rpc_client_test.go @@ -243,6 +243,7 @@ OUTER1: } } if !found { + // TODO: Failing t.Fatalf("should log client accept") } diff --git a/command/agent/ui_endpoint_test.go b/command/agent/ui_endpoint_test.go index ab734f3cb..510ff973a 100644 --- a/command/agent/ui_endpoint_test.go +++ b/command/agent/ui_endpoint_test.go @@ -100,6 +100,8 @@ func TestUiNodeInfo(t *testing.T) { if err != nil { t.Fatalf("err: %v", err) } + + // TODO: Failing assertIndex(t, resp) // Should be 1 node for the server diff --git a/consul/catalog_endpoint_test.go b/consul/catalog_endpoint_test.go index 3945dd51d..f4028b656 100644 --- a/consul/catalog_endpoint_test.go +++ b/consul/catalog_endpoint_test.go @@ -540,6 +540,7 @@ func TestCatalogListServices_Timeout(t *testing.T) { // Should block at least 100ms if time.Now().Sub(start) < 100 * time.Millisecond { + // TODO: Failing t.Fatalf("too fast") }