From 60270949d49737b9733782b3a2df0f36cfd4c86d Mon Sep 17 00:00:00 2001 From: Matt Braymer-Hayes Date: Mon, 13 Nov 2023 11:41:13 -0500 Subject: [PATCH] Fix tests --- bench/testserver_test.go | 17 +---------------- client_health_test.go | 1 - client_test.go | 30 +++--------------------------- context_test.go | 5 +---- mailbox_test.go | 10 +--------- query_test.go | 9 ++------- registry/registry_test.go | 18 +----------------- server_test.go | 18 +++++------------- testetcd/etcdserver.go | 31 +++++++++---------------------- 9 files changed, 23 insertions(+), 116 deletions(-) diff --git a/bench/testserver_test.go b/bench/testserver_test.go index 47809e4..d0c6136 100644 --- a/bench/testserver_test.go +++ b/bench/testserver_test.go @@ -13,7 +13,6 @@ import ( "github.com/lytics/grid/v3" "github.com/lytics/grid/v3/testetcd" - clientv3 "go.etcd.io/etcd/client/v3" ) const mailboxName = "pingpong-leader" @@ -73,21 +72,7 @@ func runPingPongGrid(t testing.TB) (*grid.Server, *grid.Client) { namespace := fmt.Sprintf("bench-pingpong-namespace-%d", rand.Int63()) logger := log.New(os.Stderr, namespace+": ", log.LstdFlags|log.Lshortfile) - embed := testetcd.NewEmbedded(t) - - cfg := clientv3.Config{ - Endpoints: embed.Endpoints(), - DialTimeout: time.Second, - } - etcd, err := clientv3.New(cfg) - if err != nil { - t.Fatalf("creating etcd client: %v", err) - } - t.Cleanup(func() { - if err := etcd.Close(); err != nil { - t.Fatalf("closing etcd client: %v", err) - } - }) + etcd := testetcd.StartAndConnect(t) server, err := grid.NewServer(etcd, grid.ServerCfg{Namespace: namespace}) if err != nil { diff --git a/client_health_test.go b/client_health_test.go index 9b42ec2..6d7aa1f 100644 --- a/client_health_test.go +++ b/client_health_test.go @@ -10,7 +10,6 @@ import ( ) func TestClientCheck(t *testing.T) { - t.Parallel() _, server, client := bootstrapClientTest(t) peer := server.registry.Registry() diff --git a/client_test.go b/client_test.go index 07a5bb9..321422c 100644 --- a/client_test.go +++ b/client_test.go @@ -84,10 +84,7 @@ func (a *echoActor) Act(c context.Context) { } } func TestNewClient(t *testing.T) { - t.Parallel() - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) - + etcd := testetcd.StartAndConnect(t) client, err := NewClient(etcd, ClientCfg{Namespace: newNamespace(t)}) if err != nil { t.Fatal(err) @@ -99,7 +96,6 @@ func TestNewClient(t *testing.T) { } func TestNewClientWithNilEtcd(t *testing.T) { - t.Parallel() _, err := NewClient(nil, ClientCfg{Namespace: newNamespace(t)}) if err == nil { t.Fatal("expected error") @@ -107,12 +103,7 @@ func TestNewClientWithNilEtcd(t *testing.T) { } func TestClientClose(t *testing.T) { - t.Parallel() - // Start etcd. - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) - - // Create client. + etcd := testetcd.StartAndConnect(t) client, err := NewClient(etcd, ClientCfg{Namespace: newNamespace(t)}) if err != nil { t.Fatal(err) @@ -129,7 +120,6 @@ func TestClientClose(t *testing.T) { } func TestClientRequestWithUnregisteredMailbox(t *testing.T) { - t.Parallel() // Bootstrap. _, _, client := bootstrapClientTest(t) @@ -155,7 +145,6 @@ func TestClientRequestWithUnregisteredMailbox(t *testing.T) { } func TestClientRequestWithUnknownMailbox(t *testing.T) { - t.Parallel() const timeout = 3 * time.Second // Bootstrap. @@ -194,7 +183,6 @@ func TestClientRequestWithUnknownMailbox(t *testing.T) { } func TestClientBroadcast(t *testing.T) { - t.Parallel() ctx := context.Background() const timeout = 3 * time.Second _, server, client := bootstrapClientTest(t) @@ -325,7 +313,6 @@ func TestClientBroadcast(t *testing.T) { } func TestClientWithRunningReceiver(t *testing.T) { - t.Parallel() const timeout = 3 * time.Second expected := &EchoMsg{Msg: "testing 1, 2, 3"} @@ -396,7 +383,6 @@ func TestClientWithRunningReceiver(t *testing.T) { } func TestClientWithErrConnectionIsUnregistered(t *testing.T) { - t.Parallel() const timeout = 3 * time.Second expected := &EchoMsg{Msg: "testing 1, 2, 3"} @@ -477,7 +463,6 @@ func TestClientWithErrConnectionIsUnregistered(t *testing.T) { } func TestClientWithBusyReceiver(t *testing.T) { - t.Parallel() const timeout = 3 * time.Second expected := &EchoMsg{Msg: "testing 1, 2, 3"} @@ -536,7 +521,6 @@ func TestClientWithBusyReceiver(t *testing.T) { } func TestClientStats(t *testing.T) { - t.Parallel() cs := newClientStats() cs.Inc(numGetWireClient) cs.Inc(numDeleteAddress) @@ -555,7 +539,6 @@ func TestClientStats(t *testing.T) { } func TestNilClientStats(t *testing.T) { - t.Parallel() defer func() { if r := recover(); r != nil { t.Fatal("expected no panic") @@ -567,17 +550,10 @@ func TestNilClientStats(t *testing.T) { func bootstrapClientTest(t testing.TB) (*clientv3.Client, *Server, *Client) { t.Helper() - // Namespace for test. namespace := newNamespace(t) - - // Start etcd. - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) - - // Logger for actors. + etcd := testetcd.StartAndConnect(t) logger := log.New(os.Stderr, namespace+": ", log.LstdFlags) - // Create the server. server, err := NewServer(etcd, ServerCfg{Namespace: namespace, Logger: logger}) require.NoError(t, err) diff --git a/context_test.go b/context_test.go index a674642..a3a860f 100644 --- a/context_test.go +++ b/context_test.go @@ -30,7 +30,6 @@ func (a *contextActor) Context() context.Context { } func TestContextError(t *testing.T) { - t.Parallel() // Create a context that is not valid to use // with the grid context methods. The context // is not valid because it does not contain @@ -63,11 +62,9 @@ func TestContextError(t *testing.T) { } func TestValidContext(t *testing.T) { - t.Parallel() const timeout = 2 * time.Second - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) + etcd := testetcd.StartAndConnect(t) a := &contextActor{started: make(chan bool)} diff --git a/mailbox_test.go b/mailbox_test.go index 3ed047e..c698b9e 100644 --- a/mailbox_test.go +++ b/mailbox_test.go @@ -14,14 +14,12 @@ import ( ) func TestNewMailboxRegistry(t *testing.T) { - t.Parallel() m := newMailboxRegistry() require.NotZero(t, m) assert.NotZero(t, m.r) } func TestMailboxRegistryGetSetDeleteSize(t *testing.T) { - t.Parallel() const n1 = "n1" const n2 = "n2" @@ -98,12 +96,10 @@ func TestMailboxRegistryGetSetDeleteSize(t *testing.T) { } func TestMailboxRegistryR(t *testing.T) { - t.Parallel() for _, n := range []int{0, 1, 2} { n := n t.Run(strconv.Itoa(n), func(t *testing.T) { - t.Parallel() r := newMailboxRegistry() ms := r.R() @@ -133,7 +129,6 @@ func TestMailboxRegistryR(t *testing.T) { // is safe to use concurrently. It relies on the -race test flag // to detect races: the test itself has no assertions. func TestMailboxRegistryConcurrent(t *testing.T) { - t.Parallel() // NOTE (2022-01) (mh): Only doing one combo for sanity. // Could test performance/correctness at higher contention. @@ -149,7 +144,6 @@ func TestMailboxRegistryConcurrent(t *testing.T) { for _, numKeys := range numKeysSet { numKeys := numKeys t.Run(fmt.Sprintf("%v-%v", numWorkers, numKeys), func(t *testing.T) { - t.Parallel() r := newMailboxRegistry() @@ -194,9 +188,7 @@ func TestMailboxRegistryConcurrent(t *testing.T) { } func TestMailboxClose(t *testing.T) { - t.Parallel() - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) + etcd := testetcd.StartAndConnect(t) s, err := NewServer(etcd, ServerCfg{Namespace: newNamespace(t)}) require.NoError(t, err) diff --git a/query_test.go b/query_test.go index 989a552..88f1cbb 100644 --- a/query_test.go +++ b/query_test.go @@ -11,7 +11,6 @@ import ( ) func TestQuery(t *testing.T) { - t.Parallel() const ( nrPeers = 2 backoff = 10 * time.Second @@ -19,9 +18,7 @@ func TestQuery(t *testing.T) { ) namespace := newNamespace(t) - - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) + etcd := testetcd.StartAndConnect(t) client, err := NewClient(etcd, ClientCfg{Namespace: namespace}) if err != nil { @@ -73,7 +70,6 @@ func TestQuery(t *testing.T) { } func TestQueryWatch(t *testing.T) { - t.Parallel() const ( nrPeers = 2 backoff = 10 * time.Second @@ -81,8 +77,7 @@ func TestQueryWatch(t *testing.T) { ) namespace := newNamespace(t) - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) + etcd := testetcd.StartAndConnect(t) client, err := NewClient(etcd, ClientCfg{Namespace: namespace}) if err != nil { diff --git a/registry/registry_test.go b/registry/registry_test.go index 7b4cc32..f720c32 100644 --- a/registry/registry_test.go +++ b/registry/registry_test.go @@ -21,7 +21,6 @@ const ( ) func TestInitialLeaseID(t *testing.T) { - t.Parallel() _, r, _ := bootstrap(t, dontStart) if r.leaseID != -1 { @@ -30,7 +29,6 @@ func TestInitialLeaseID(t *testing.T) { } func TestStartStop(t *testing.T) { - t.Parallel() _, r, _ := bootstrap(t, start) err := r.Stop() @@ -45,7 +43,6 @@ func TestStartStop(t *testing.T) { } func TestStartStopWaitForLeaseToExpireBetween(t *testing.T) { - t.Parallel() client, r, addr := bootstrap(t, start) // this should remove the lease which should clean up the registry lock on the address @@ -87,7 +84,6 @@ func TestStartStopWaitForLeaseToExpireBetween(t *testing.T) { } func TestWaitForLeaseThatNeverExpires(t *testing.T) { - t.Parallel() client, _, addr := bootstrap(t, dontStart) @@ -133,7 +129,6 @@ func TestWaitForLeaseThatNeverExpires(t *testing.T) { } func TestWaitForLeaseThatDoesExpires(t *testing.T) { - t.Parallel() start := false client, _, addr := bootstrap(t, start) @@ -186,7 +181,6 @@ func TestWaitForLeaseThatDoesExpires(t *testing.T) { } func TestRegister(t *testing.T) { - t.Parallel() client, r, _ := bootstrap(t, start) ctx, cancel := timeoutContext() @@ -216,7 +210,6 @@ func TestRegister(t *testing.T) { } func TestDeregistration(t *testing.T) { - t.Parallel() client, r, _ := bootstrap(t, start) ctx, cancel := timeoutContext() @@ -265,7 +258,6 @@ func TestDeregistration(t *testing.T) { } func TestRegisterDeregisterWhileNotStarted(t *testing.T) { - t.Parallel() _, r, _ := bootstrap(t, dontStart) ctx, cancel := timeoutContext() @@ -284,7 +276,6 @@ func TestRegisterDeregisterWhileNotStarted(t *testing.T) { } func TestRegisterTwiceNotAllowed(t *testing.T) { - t.Parallel() _, r, _ := bootstrap(t, start) for i := 0; i < 2; i++ { @@ -298,7 +289,6 @@ func TestRegisterTwiceNotAllowed(t *testing.T) { } func TestStop(t *testing.T) { - t.Parallel() client, r, _ := bootstrap(t, start) ctx, cancel := timeoutContext() @@ -321,7 +311,6 @@ func TestStop(t *testing.T) { } func TestFindRegistration(t *testing.T) { - t.Parallel() _, r, _ := bootstrap(t, start) ctx, cancel := timeoutContext() @@ -350,7 +339,6 @@ func TestFindRegistration(t *testing.T) { } func TestFindRegistrations(t *testing.T) { - t.Parallel() _, r, _ := bootstrap(t, start) ctx, cancel := timeoutContext() @@ -392,7 +380,6 @@ func TestFindRegistrations(t *testing.T) { } func TestKeepAlive(t *testing.T) { - t.Parallel() _, r, addr := bootstrap(t, dontStart) // Change the minimum for sake of testing quickly. @@ -418,7 +405,6 @@ func TestKeepAlive(t *testing.T) { } func TestWatch(t *testing.T) { - t.Parallel() _, r, addr := bootstrap(t, dontStart) // Change the minimum for sake of testing quickly. @@ -530,7 +516,6 @@ func TestWatch(t *testing.T) { } func TestWatchEventString(t *testing.T) { - t.Parallel() we := &WatchEvent{ Key: "foo", Type: Create, @@ -565,8 +550,7 @@ func TestWatchEventString(t *testing.T) { func bootstrap(t testing.TB, shouldStart bool) (*etcdv3.Client, *Registry, *net.TCPAddr) { t.Helper() - embed := testetcd.NewEmbedded(t) - client := testetcd.StartAndConnect(t, embed.Endpoints()) + client := testetcd.StartAndConnect(t) addr := &net.TCPAddr{ IP: []byte("localhost"), diff --git a/server_test.go b/server_test.go index 00ac067..d5872dd 100644 --- a/server_test.go +++ b/server_test.go @@ -23,13 +23,11 @@ func (a *startStopActor) Act(c context.Context) { } func TestServerStartStop(t *testing.T) { - t.Parallel() const ( timeout = 20 * time.Second ) - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) + etcd := testetcd.StartAndConnect(t) a := &startStopActor{ started: make(chan bool), @@ -86,13 +84,11 @@ func TestServerStartStop(t *testing.T) { } func TestServerWithFatalError(t *testing.T) { - t.Parallel() const ( timeout = 20 * time.Second ) - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) + etcd := testetcd.StartAndConnect(t) a := &startStopActor{ started: make(chan bool), @@ -141,11 +137,9 @@ func TestServerWithFatalError(t *testing.T) { } func TestServerStartNoEtcdRunning(t *testing.T) { - t.Parallel() // Start etcd, but shut it down right away. - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) - etcd.Close() + etcd := testetcd.StartAndConnect(t) + _ = etcd.Close() server, err := NewServer(etcd, ServerCfg{Namespace: newNamespace(t)}) if err != nil { @@ -164,7 +158,6 @@ func TestServerStartNoEtcdRunning(t *testing.T) { } func TestServerStartThenEtcdStop(t *testing.T) { - t.Parallel() t.Skip() a := &startStopActor{ @@ -172,8 +165,7 @@ func TestServerStartThenEtcdStop(t *testing.T) { stopped: make(chan bool), } - embed := testetcd.NewEmbedded(t) - etcd := testetcd.StartAndConnect(t, embed.Endpoints()) + etcd := testetcd.StartAndConnect(t) server, err := NewServer(etcd, ServerCfg{Namespace: newNamespace(t)}) if err != nil { diff --git a/testetcd/etcdserver.go b/testetcd/etcdserver.go index 6665f79..3482026 100644 --- a/testetcd/etcdserver.go +++ b/testetcd/etcdserver.go @@ -12,36 +12,23 @@ import ( clientv3 "go.etcd.io/etcd/client/v3" ) -type Embedded struct { - endpoints []string -} +func StartAndConnect(t testing.TB) *clientv3.Client { + t.Helper() -func NewEmbedded(t testing.TB) *Embedded { - endpointsStr := os.Getenv("GRID_ETCD_ENDPOINTS") - if endpointsStr == "" { + endpoints := os.Getenv("GRID_ETCD_ENDPOINTS") + if endpoints == "" { t.Log("GRID_ETCD_ENDPOINTS is not set") - endpointsStr = "http://127.0.0.1:2379" - } - endpoints := strings.Split(endpointsStr, ",") - return &Embedded{ - endpoints: endpoints, + endpoints = "http://127.0.0.1:2379" } -} -func (e *Embedded) Endpoints() []string { - return e.endpoints -} - -func StartAndConnect(t testing.TB, endpoints []string) *clientv3.Client { - cfg := clientv3.Config{ - Endpoints: endpoints, + etcd, err := clientv3.New(clientv3.Config{ + Endpoints: strings.Split(endpoints, ","), DialTimeout: time.Second, - } - etcd, err := clientv3.New(cfg) + }) require.NoError(t, err) t.Cleanup(func() { if err := etcd.Close(); err != nil && !errors.Is(err, context.Canceled) { - t.Fatal(err) + t.Error(err) } }) return etcd