diff --git a/go/vt/vtadmin/api_test.go b/go/vt/vtadmin/api_test.go index cf003954c07..b0680bb1421 100644 --- a/go/vt/vtadmin/api_test.go +++ b/go/vt/vtadmin/api_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "vitess.io/vitess/go/vt/grpccommon" "vitess.io/vitess/go/vt/topo" "vitess.io/vitess/go/vt/topo/memorytopo" "vitess.io/vitess/go/vt/topo/topoproto" @@ -528,6 +529,8 @@ func TestFindSchema(t *testing.T) { } func TestGetClusters(t *testing.T) { + t.Parallel() + tests := []struct { name string clusters []*cluster.Cluster @@ -565,10 +568,15 @@ func TestGetClusters(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + api := NewAPI(tt.clusters, grpcserver.Options{}, http.Options{}) - ctx := context.Background() resp, err := api.GetClusters(ctx, &vtadminpb.GetClustersRequest{}) assert.NoError(t, err) @@ -578,6 +586,8 @@ func TestGetClusters(t *testing.T) { } func TestGetGates(t *testing.T) { + t.Parallel() + fakedisco1 := fakediscovery.New() cluster1 := &cluster.Cluster{ ID: "c1", @@ -663,6 +673,8 @@ func TestGetGates(t *testing.T) { } func TestGetKeyspaces(t *testing.T) { + t.Parallel() + tests := []struct { name string clusterKeyspaces [][]*vtctldatapb.Keyspace @@ -849,53 +861,63 @@ func TestGetKeyspaces(t *testing.T) { ctx := context.Background() for _, tt := range tests { - // Note that these test cases were written prior to the existence of - // WithTestServers, so they are all written with the assumption that - // there are exactly 2 clusters. - topos := []*topo.Server{ - memorytopo.NewServer("c0_cell1"), - memorytopo.NewServer("c1_cell1"), - } - - for cdx, cks := range tt.clusterKeyspaces { - for _, ks := range cks { - testutil.AddKeyspace(ctx, t, topos[cdx], ks) + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Note that these test cases were written prior to the existence of + // WithTestServers, so they are all written with the assumption that + // there are exactly 2 clusters. + topos := []*topo.Server{ + memorytopo.NewServer("c0_cell1"), + memorytopo.NewServer("c1_cell1"), } - } - for cdx, css := range tt.clusterShards { - testutil.AddShards(ctx, t, topos[cdx], css...) - } + for cdx, cks := range tt.clusterKeyspaces { + for _, ks := range cks { + testutil.AddKeyspace(ctx, t, topos[cdx], ks) + } + } - servers := []vtctlservicepb.VtctldServer{ - grpcvtctldserver.NewVtctldServer(topos[0]), - grpcvtctldserver.NewVtctldServer(topos[1]), - } + for cdx, css := range tt.clusterShards { + testutil.AddShards(ctx, t, topos[cdx], css...) + } - testutil.WithTestServers(t, func(t *testing.T, clients ...vtctldclient.VtctldClient) { - clusters := []*cluster.Cluster{ - vtadmintestutil.BuildCluster(vtadmintestutil.TestClusterConfig{ - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", - }, - VtctldClient: clients[0], + servers := []vtctlservicepb.VtctldServer{ + testutil.NewVtctldServerWithTabletManagerClient(t, topos[0], nil, func(ts *topo.Server) vtctlservicepb.VtctldServer { + return grpcvtctldserver.NewVtctldServer(ts) }), - vtadmintestutil.BuildCluster(vtadmintestutil.TestClusterConfig{ - Cluster: &vtadminpb.Cluster{ - Id: "c1", - Name: "cluster1", - }, - VtctldClient: clients[1], + testutil.NewVtctldServerWithTabletManagerClient(t, topos[1], nil, func(ts *topo.Server) vtctlservicepb.VtctldServer { + return grpcvtctldserver.NewVtctldServer(ts) }), } - api := NewAPI(clusters, grpcserver.Options{}, http.Options{}) - resp, err := api.GetKeyspaces(ctx, tt.req) - require.NoError(t, err) + testutil.WithTestServers(t, func(t *testing.T, clients ...vtctldclient.VtctldClient) { + clusters := []*cluster.Cluster{ + vtadmintestutil.BuildCluster(vtadmintestutil.TestClusterConfig{ + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", + }, + VtctldClient: clients[0], + }), + vtadmintestutil.BuildCluster(vtadmintestutil.TestClusterConfig{ + Cluster: &vtadminpb.Cluster{ + Id: "c1", + Name: "cluster1", + }, + VtctldClient: clients[1], + }), + } - vtadmintestutil.AssertKeyspaceSlicesEqual(t, tt.expected.Keyspaces, resp.Keyspaces) - }, servers...) + api := NewAPI(clusters, grpcserver.Options{}, http.Options{}) + resp, err := api.GetKeyspaces(ctx, tt.req) + require.NoError(t, err) + + vtadmintestutil.AssertKeyspaceSlicesEqual(t, tt.expected.Keyspaces, resp.Keyspaces) + }, servers...) + }) } } @@ -1089,13 +1111,14 @@ func TestGetSchema(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - ctx := context.Background() vtctld := testutil.NewVtctldServerWithTabletManagerClient(t, tt.ts, tt.tmc, func(ts *topo.Server) vtctlservicepb.VtctldServer { return grpcvtctldserver.NewVtctldServer(ts) }) @@ -1634,13 +1657,15 @@ func TestGetSchemas(t *testing.T) { } } -func TestGetTablets(t *testing.T) { +func TestGetTablet(t *testing.T) { + t.Parallel() + tests := []struct { name string clusterTablets [][]*vtadminpb.Tablet dbconfigs map[string]vtadmintestutil.Dbcfg - req *vtadminpb.GetTabletsRequest - expected []*vtadminpb.Tablet + req *vtadminpb.GetTabletRequest + expected *vtadminpb.Tablet shouldErr bool }{ { @@ -1664,24 +1689,24 @@ func TestGetTablets(t *testing.T) { }, }, dbconfigs: map[string]vtadmintestutil.Dbcfg{}, - req: &vtadminpb.GetTabletsRequest{}, - expected: []*vtadminpb.Tablet{ - { - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", - }, - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Uid: 100, - Cell: "zone1", - }, - Hostname: "ks1-00-00-zone1-a", - Keyspace: "ks1", - Shard: "-", - Type: topodatapb.TabletType_MASTER, + req: &vtadminpb.GetTabletRequest{ + Hostname: "ks1-00-00-zone1-a", + }, + expected: &vtadminpb.Tablet{ + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", + }, + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Uid: 100, + Cell: "zone1", }, + Hostname: "ks1-00-00-zone1-a", + Keyspace: "ks1", + Shard: "-", + Type: topodatapb.TabletType_MASTER, }, }, shouldErr: false, @@ -1725,12 +1750,14 @@ func TestGetTablets(t *testing.T) { dbconfigs: map[string]vtadmintestutil.Dbcfg{ "c1": {ShouldErr: true}, }, - req: &vtadminpb.GetTabletsRequest{}, + req: &vtadminpb.GetTabletRequest{ + Hostname: "doesn't matter", + }, expected: nil, shouldErr: true, }, { - name: "multi cluster, selecting one", + name: "multi cluster, selecting one with tablet", clusterTablets: [][]*vtadminpb.Tablet{ /* cluster 0 */ { @@ -1766,32 +1793,95 @@ func TestGetTablets(t *testing.T) { }, }, dbconfigs: map[string]vtadmintestutil.Dbcfg{}, - req: &vtadminpb.GetTabletsRequest{ClusterIds: []string{"c0"}}, - expected: []*vtadminpb.Tablet{ + req: &vtadminpb.GetTabletRequest{ + Hostname: "ks1-00-00-zone1-a", + ClusterIds: []string{"c0"}, + }, + expected: &vtadminpb.Tablet{ + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", + }, + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Uid: 100, + Cell: "zone1", + }, + Hostname: "ks1-00-00-zone1-a", + Keyspace: "ks1", + Shard: "-", + Type: topodatapb.TabletType_MASTER, + }, + }, + shouldErr: false, + }, + { + name: "multi cluster, multiple results", + clusterTablets: [][]*vtadminpb.Tablet{ + /* cluster 0 */ { - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", + { + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Uid: 100, + Cell: "zone1", + }, + Hostname: "ks1-00-00-zone1-a", + Keyspace: "ks1", + Shard: "-", + Type: topodatapb.TabletType_MASTER, + }, }, - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Uid: 100, - Cell: "zone1", + }, + /* cluster 1 */ + { + { + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Uid: 200, + Cell: "zone1", + }, + Hostname: "ks1-00-00-zone1-a", + Keyspace: "ks1", + Shard: "-", + Type: topodatapb.TabletType_MASTER, }, - Hostname: "ks1-00-00-zone1-a", - Keyspace: "ks1", - Shard: "-", - Type: topodatapb.TabletType_MASTER, }, }, }, - shouldErr: false, + dbconfigs: map[string]vtadmintestutil.Dbcfg{}, + req: &vtadminpb.GetTabletRequest{ + Hostname: "ks1-00-00-zone1-a", + }, + expected: nil, + shouldErr: true, + }, + { + name: "no results", + clusterTablets: [][]*vtadminpb.Tablet{ + /* cluster 0 */ + {}, + }, + dbconfigs: map[string]vtadmintestutil.Dbcfg{}, + req: &vtadminpb.GetTabletRequest{ + Hostname: "ks1-00-00-zone1-a", + }, + expected: nil, + shouldErr: true, }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + clusters := make([]*cluster.Cluster, len(tt.clusterTablets)) for i, tablets := range tt.clusterTablets { @@ -1809,25 +1899,27 @@ func TestGetTablets(t *testing.T) { } api := NewAPI(clusters, grpcserver.Options{}, http.Options{}) - resp, err := api.GetTablets(context.Background(), tt.req) + resp, err := api.GetTablet(ctx, tt.req) if tt.shouldErr { assert.Error(t, err) return } assert.NoError(t, err) - assert.ElementsMatch(t, tt.expected, resp.Tablets) + assert.Equal(t, tt.expected, resp) }) } } -func TestGetTablet(t *testing.T) { +func TestGetTablets(t *testing.T) { + t.Parallel() + tests := []struct { name string clusterTablets [][]*vtadminpb.Tablet dbconfigs map[string]vtadmintestutil.Dbcfg - req *vtadminpb.GetTabletRequest - expected *vtadminpb.Tablet + req *vtadminpb.GetTabletsRequest + expected []*vtadminpb.Tablet shouldErr bool }{ { @@ -1851,24 +1943,24 @@ func TestGetTablet(t *testing.T) { }, }, dbconfigs: map[string]vtadmintestutil.Dbcfg{}, - req: &vtadminpb.GetTabletRequest{ - Hostname: "ks1-00-00-zone1-a", - }, - expected: &vtadminpb.Tablet{ - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", - }, - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Uid: 100, - Cell: "zone1", + req: &vtadminpb.GetTabletsRequest{}, + expected: []*vtadminpb.Tablet{ + { + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", + }, + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Uid: 100, + Cell: "zone1", + }, + Hostname: "ks1-00-00-zone1-a", + Keyspace: "ks1", + Shard: "-", + Type: topodatapb.TabletType_MASTER, }, - Hostname: "ks1-00-00-zone1-a", - Keyspace: "ks1", - Shard: "-", - Type: topodatapb.TabletType_MASTER, }, }, shouldErr: false, @@ -1912,14 +2004,12 @@ func TestGetTablet(t *testing.T) { dbconfigs: map[string]vtadmintestutil.Dbcfg{ "c1": {ShouldErr: true}, }, - req: &vtadminpb.GetTabletRequest{ - Hostname: "doesn't matter", - }, + req: &vtadminpb.GetTabletsRequest{}, expected: nil, shouldErr: true, }, { - name: "multi cluster, selecting one with tablet", + name: "multi cluster, selecting one", clusterTablets: [][]*vtadminpb.Tablet{ /* cluster 0 */ { @@ -1955,89 +2045,38 @@ func TestGetTablet(t *testing.T) { }, }, dbconfigs: map[string]vtadmintestutil.Dbcfg{}, - req: &vtadminpb.GetTabletRequest{ - Hostname: "ks1-00-00-zone1-a", - ClusterIds: []string{"c0"}, - }, - expected: &vtadminpb.Tablet{ - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", - }, - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Uid: 100, - Cell: "zone1", - }, - Hostname: "ks1-00-00-zone1-a", - Keyspace: "ks1", - Shard: "-", - Type: topodatapb.TabletType_MASTER, - }, - }, - shouldErr: false, - }, - { - name: "multi cluster, multiple results", - clusterTablets: [][]*vtadminpb.Tablet{ - /* cluster 0 */ + req: &vtadminpb.GetTabletsRequest{ClusterIds: []string{"c0"}}, + expected: []*vtadminpb.Tablet{ { - { - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Uid: 100, - Cell: "zone1", - }, - Hostname: "ks1-00-00-zone1-a", - Keyspace: "ks1", - Shard: "-", - Type: topodatapb.TabletType_MASTER, - }, + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", }, - }, - /* cluster 1 */ - { - { - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Uid: 200, - Cell: "zone1", - }, - Hostname: "ks1-00-00-zone1-a", - Keyspace: "ks1", - Shard: "-", - Type: topodatapb.TabletType_MASTER, + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Uid: 100, + Cell: "zone1", }, + Hostname: "ks1-00-00-zone1-a", + Keyspace: "ks1", + Shard: "-", + Type: topodatapb.TabletType_MASTER, }, }, }, - dbconfigs: map[string]vtadmintestutil.Dbcfg{}, - req: &vtadminpb.GetTabletRequest{ - Hostname: "ks1-00-00-zone1-a", - }, - expected: nil, - shouldErr: true, - }, - { - name: "no results", - clusterTablets: [][]*vtadminpb.Tablet{ - /* cluster 0 */ - {}, - }, - dbconfigs: map[string]vtadmintestutil.Dbcfg{}, - req: &vtadminpb.GetTabletRequest{ - Hostname: "ks1-00-00-zone1-a", - }, - expected: nil, - shouldErr: true, + shouldErr: false, }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + clusters := make([]*cluster.Cluster, len(tt.clusterTablets)) for i, tablets := range tt.clusterTablets { @@ -2055,14 +2094,14 @@ func TestGetTablet(t *testing.T) { } api := NewAPI(clusters, grpcserver.Options{}, http.Options{}) - resp, err := api.GetTablet(context.Background(), tt.req) + resp, err := api.GetTablets(ctx, tt.req) if tt.shouldErr { assert.Error(t, err) return } assert.NoError(t, err) - assert.Equal(t, tt.expected, resp) + assert.ElementsMatch(t, tt.expected, resp.Tablets) }) } } @@ -2531,6 +2570,8 @@ func TestGetVSchemas(t *testing.T) { } func TestVTExplain(t *testing.T) { + t.Parallel() + tests := []struct { name string keyspaces []*vtctldatapb.Keyspace @@ -2748,8 +2789,14 @@ func TestVTExplain(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + toposerver := memorytopo.NewServer("c0_cell1") tmc := testutil.TabletManagerClient{ @@ -2765,14 +2812,14 @@ func TestVTExplain(t *testing.T) { testutil.WithTestServer(t, vtctldserver, func(t *testing.T, vtctldClient vtctldclient.VtctldClient) { if tt.srvVSchema != nil { - err := toposerver.UpdateSrvVSchema(context.Background(), "c0_cell1", tt.srvVSchema) + err := toposerver.UpdateSrvVSchema(ctx, "c0_cell1", tt.srvVSchema) require.NoError(t, err) } - testutil.AddKeyspaces(context.Background(), t, toposerver, tt.keyspaces...) - testutil.AddShards(context.Background(), t, toposerver, tt.shards...) + testutil.AddKeyspaces(ctx, t, toposerver, tt.keyspaces...) + testutil.AddShards(ctx, t, toposerver, tt.shards...) for _, tablet := range tt.tablets { - testutil.AddTablet(context.Background(), t, toposerver, tablet.Tablet, nil) + testutil.AddTablet(ctx, t, toposerver, tablet.Tablet, nil) // Adds each SchemaDefinition to the fake TabletManagerClient, or nil // if there are no schemas for that tablet. (All tablet aliases must @@ -2800,7 +2847,7 @@ func TestVTExplain(t *testing.T) { } api := NewAPI(clusters, grpcserver.Options{}, http.Options{}) - resp, err := api.VTExplain(context.Background(), tt.req) + resp, err := api.VTExplain(ctx, tt.req) if tt.expectedError != nil { assert.True(t, errors.Is(err, tt.expectedError), "expected error type %w does not match actual error type %w", err, tt.expectedError) @@ -2828,4 +2875,15 @@ func init() { tmclient.RegisterTabletManagerClientFactory("vtadmin.test", func() tmclient.TabletManagerClient { return nil }) + + // This prevents data-race failures in tests involving grpc client or server + // creation. For example, vtctldclient.New() eventually ends up calling + // grpccommon.EnableTracingOpt() which does a synchronized, one-time + // mutation of the global grpc.EnableTracing. This variable is also read, + // unguarded, by grpc.NewServer(), which is a function call that appears in + // most, if not all, vtadmin.API tests. + // + // Calling this here ensures that one-time write happens before any test + // attempts to read that value by way of grpc.NewServer(). + grpccommon.EnableTracingOpt() } diff --git a/go/vt/vtadmin/cluster/cluster_test.go b/go/vt/vtadmin/cluster/cluster_test.go index 6c2dccf9779..74f4f59a955 100644 --- a/go/vt/vtadmin/cluster/cluster_test.go +++ b/go/vt/vtadmin/cluster/cluster_test.go @@ -41,282 +41,97 @@ import ( vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata" ) -// This test only validates the error handling on dialing database connections. -// Other cases are covered by one or both of TestFindTablets and TestFindTablet. -func TestGetTablets(t *testing.T) { - disco := fakediscovery.New() - disco.AddTaggedGates(nil, &vtadminpb.VTGate{Hostname: "gate"}) - - db := vtsql.New(&vtsql.Config{ - Cluster: &vtadminpb.Cluster{ - Id: "c1", - Name: "one", - }, - Discovery: disco, - }) - db.DialFunc = func(cfg vitessdriver.Configuration) (*sql.DB, error) { - return nil, assert.AnError - } - - c := &cluster.Cluster{DB: db} - _, err := c.GetTablets(context.Background()) - assert.Error(t, err) -} - -func TestGetSchema(t *testing.T) { +func TestFindTablet(t *testing.T) { t.Parallel() tests := []struct { - name string - vtctld vtctldclient.VtctldClient - req *vtctldatapb.GetSchemaRequest - tablet *vtadminpb.Tablet - expected *vtadminpb.Schema - shouldErr bool + name string + tablets []*vtadminpb.Tablet + filter func(*vtadminpb.Tablet) bool + expected *vtadminpb.Tablet + expectedError error }{ { - name: "success", - vtctld: &testutil.VtctldClient{ - GetSchemaResults: map[string]struct { - Response *vtctldatapb.GetSchemaResponse - Error error - }{ - "zone1-0000000100": { - Response: &vtctldatapb.GetSchemaResponse{ - Schema: &tabletmanagerdatapb.SchemaDefinition{ - TableDefinitions: []*tabletmanagerdatapb.TableDefinition{ - { - Name: "some_table", - }, - }, - }, + name: "returns the first matching tablet", + tablets: []*vtadminpb.Tablet{ + { + State: vtadminpb.Tablet_NOT_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "c0_cell1", + Uid: 100, }, + Keyspace: "commerce", }, }, - }, - req: &vtctldatapb.GetSchemaRequest{}, - tablet: &vtadminpb.Tablet{ - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "zone1", - Uid: 100, - }, - Keyspace: "testkeyspace", - }, - }, - expected: &vtadminpb.Schema{ - Cluster: &vtadminpb.Cluster{ - Name: "cluster0", - Id: "c0", - }, - Keyspace: "testkeyspace", - TableDefinitions: []*tabletmanagerdatapb.TableDefinition{ - { - Name: "some_table", - }, - }, - }, - shouldErr: false, - }, - { - name: "error getting schema", - vtctld: &testutil.VtctldClient{ - GetSchemaResults: map[string]struct { - Response *vtctldatapb.GetSchemaResponse - Error error - }{ - "zone1-0000000100": { - Error: assert.AnError, - }, - }, - }, - req: &vtctldatapb.GetSchemaRequest{}, - tablet: &vtadminpb.Tablet{ - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "zone1", - Uid: 100, - }, - Keyspace: "testkeyspace", - }, - }, - expected: nil, - shouldErr: true, - }, - { - name: "underlying schema is nil", - vtctld: &testutil.VtctldClient{ - GetSchemaResults: map[string]struct { - Response *vtctldatapb.GetSchemaResponse - Error error - }{ - "zone1-0000000100": { - Response: &vtctldatapb.GetSchemaResponse{ - Schema: nil, + { + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "c0_cell1", + Uid: 101, }, - Error: nil, + Keyspace: "commerce", }, }, - }, - req: &vtctldatapb.GetSchemaRequest{}, - tablet: &vtadminpb.Tablet{ - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "zone1", - Uid: 100, + { + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "c0_cell1", + Uid: 102, + }, + Keyspace: "commerce", }, - Keyspace: "testkeyspace", - }, - }, - expected: nil, - shouldErr: false, - }, - } - - for i, tt := range tests { - i := i - tt := tt - - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - cluster := testutil.BuildCluster(testutil.TestClusterConfig{ - Cluster: &vtadminpb.Cluster{ - Id: fmt.Sprintf("c%d", i), - Name: fmt.Sprintf("cluster%d", i), - }, - VtctldClient: tt.vtctld, - Tablets: nil, - DBConfig: testutil.Dbcfg{}, - }) - - ctx := context.Background() - err := cluster.Vtctld.Dial(ctx) - require.NoError(t, err, "could not dial test vtctld") - - schema, err := cluster.GetSchema(ctx, tt.req, tt.tablet) - if tt.shouldErr { - assert.Error(t, err) - - return - } - - assert.NoError(t, err) - assert.Equal(t, tt.expected, schema) - }) - } - - t.Run("does not modify passed-in request", func(t *testing.T) { - t.Parallel() - - vtctld := &testutil.VtctldClient{ - GetSchemaResults: map[string]struct { - Response *vtctldatapb.GetSchemaResponse - Error error - }{ - "zone1-0000000100": { - Response: &vtctldatapb.GetSchemaResponse{}, - }, - }, - } - - ctx := context.Background() - req := &vtctldatapb.GetSchemaRequest{ - TabletAlias: &topodatapb.TabletAlias{ - Cell: "otherzone", - Uid: 500, - }, - } - tablet := &vtadminpb.Tablet{ - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "zone1", - Uid: 100, }, }, - } - cluster := testutil.BuildCluster(testutil.TestClusterConfig{ - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", + filter: func(t *vtadminpb.Tablet) bool { + return t.State == vtadminpb.Tablet_SERVING }, - VtctldClient: vtctld, - }) - - err := cluster.Vtctld.Dial(ctx) - require.NoError(t, err, "could not dial test vtctld") - - cluster.GetSchema(ctx, req, tablet) - - assert.NotEqual(t, req.TabletAlias, tablet.Tablet.Alias, "expected GetSchema to not modify original request object") - }) -} - -func TestGetVSchema(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - cfg testutil.TestClusterConfig - keyspace string - expected *vtadminpb.VSchema - shouldErr bool - }{ - { - name: "success", - cfg: testutil.TestClusterConfig{ + expected: &vtadminpb.Tablet{ Cluster: &vtadminpb.Cluster{ Id: "c0", Name: "cluster0", }, - VtctldClient: &testutil.VtctldClient{ - GetVSchemaResults: map[string]struct { - Response *vtctldatapb.GetVSchemaResponse - Error error - }{ - "testkeyspace": { - Response: &vtctldatapb.GetVSchemaResponse{ - VSchema: &vschemapb.Keyspace{Sharded: true}, - }, - }, + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "c0_cell1", + Uid: 101, }, + Keyspace: "commerce", }, }, - keyspace: "testkeyspace", - expected: &vtadminpb.VSchema{ - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", - }, - Name: "testkeyspace", - VSchema: &vschemapb.Keyspace{Sharded: true}, - }, - shouldErr: false, }, { - name: "error", - cfg: testutil.TestClusterConfig{ - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", + name: "returns an error if no match found", + tablets: []*vtadminpb.Tablet{ + { + State: vtadminpb.Tablet_NOT_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "c0_cell1", + Uid: 100, + }, + Keyspace: "commerce", + }, }, - VtctldClient: &testutil.VtctldClient{ - GetVSchemaResults: map[string]struct { - Response *vtctldatapb.GetVSchemaResponse - Error error - }{ - "testkeyspace": { - Response: &vtctldatapb.GetVSchemaResponse{ - VSchema: &vschemapb.Keyspace{Sharded: true}, - }, + { + State: vtadminpb.Tablet_NOT_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "c0_cell1", + Uid: 101, }, + Keyspace: "commerce", }, }, }, - keyspace: "notfound", - expected: nil, - shouldErr: true, + filter: func(t *vtadminpb.Tablet) bool { + return t.State == vtadminpb.Tablet_SERVING + }, + expectedError: vtadminerrors.ErrNoTablet, }, } @@ -328,24 +143,28 @@ func TestGetVSchema(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - cluster := testutil.BuildCluster(tt.cfg) - err := cluster.Vtctld.Dial(ctx) - require.NoError(t, err, "could not dial test vtctld") - - vschema, err := cluster.GetVSchema(ctx, tt.keyspace) - if tt.shouldErr { - assert.Error(t, err) + cluster := testutil.BuildCluster(testutil.TestClusterConfig{ + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", + }, + Tablets: tt.tablets, + }) + tablet, err := cluster.FindTablet(ctx, tt.filter) - return + if tt.expectedError != nil { + assert.True(t, errors.Is(err, tt.expectedError), "expected error type %w does not match actual error type %w", err, tt.expectedError) + } else { + assert.NoError(t, err) + testutil.AssertTabletsEqual(t, tt.expected, tablet) } - - assert.NoError(t, err) - assert.Equal(t, tt.expected, vschema) }) } } func TestFindTablets(t *testing.T) { + t.Parallel() + tests := []struct { name string tablets []*vtadminpb.Tablet @@ -509,134 +328,325 @@ func TestFindTablets(t *testing.T) { Keyspace: "commerce", }, }, - { - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", - }, - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "c0_cell1", - Uid: 103, - }, - Keyspace: "commerce", + { + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", + }, + State: vtadminpb.Tablet_SERVING, + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "c0_cell1", + Uid: 103, + }, + Keyspace: "commerce", + }, + }, + }, + }, + } + + ctx := context.Background() + + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cluster := testutil.BuildCluster(testutil.TestClusterConfig{ + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", + }, + Tablets: tt.tablets, + }) + tablets, err := cluster.FindTablets(ctx, tt.filter, tt.n) + + assert.NoError(t, err) + testutil.AssertTabletSlicesEqual(t, tt.expected, tablets) + }) + } +} + +func TestGetSchema(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + vtctld vtctldclient.VtctldClient + req *vtctldatapb.GetSchemaRequest + tablet *vtadminpb.Tablet + expected *vtadminpb.Schema + shouldErr bool + }{ + { + name: "success", + vtctld: &testutil.VtctldClient{ + GetSchemaResults: map[string]struct { + Response *vtctldatapb.GetSchemaResponse + Error error + }{ + "zone1-0000000100": { + Response: &vtctldatapb.GetSchemaResponse{ + Schema: &tabletmanagerdatapb.SchemaDefinition{ + TableDefinitions: []*tabletmanagerdatapb.TableDefinition{ + { + Name: "some_table", + }, + }, + }, + }, + }, + }, + }, + req: &vtctldatapb.GetSchemaRequest{}, + tablet: &vtadminpb.Tablet{ + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 100, + }, + Keyspace: "testkeyspace", + }, + }, + expected: &vtadminpb.Schema{ + Cluster: &vtadminpb.Cluster{ + Name: "cluster0", + Id: "c0", + }, + Keyspace: "testkeyspace", + TableDefinitions: []*tabletmanagerdatapb.TableDefinition{ + { + Name: "some_table", + }, + }, + }, + shouldErr: false, + }, + { + name: "error getting schema", + vtctld: &testutil.VtctldClient{ + GetSchemaResults: map[string]struct { + Response *vtctldatapb.GetSchemaResponse + Error error + }{ + "zone1-0000000100": { + Error: assert.AnError, + }, + }, + }, + req: &vtctldatapb.GetSchemaRequest{}, + tablet: &vtadminpb.Tablet{ + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 100, + }, + Keyspace: "testkeyspace", + }, + }, + expected: nil, + shouldErr: true, + }, + { + name: "underlying schema is nil", + vtctld: &testutil.VtctldClient{ + GetSchemaResults: map[string]struct { + Response *vtctldatapb.GetSchemaResponse + Error error + }{ + "zone1-0000000100": { + Response: &vtctldatapb.GetSchemaResponse{ + Schema: nil, + }, + Error: nil, + }, + }, + }, + req: &vtctldatapb.GetSchemaRequest{}, + tablet: &vtadminpb.Tablet{ + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 100, }, + Keyspace: "testkeyspace", }, }, + expected: nil, + shouldErr: false, }, } ctx := context.Background() - for _, tt := range tests { + for i, tt := range tests { + i := i tt := tt t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cluster := testutil.BuildCluster(testutil.TestClusterConfig{ Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", + Id: fmt.Sprintf("c%d", i), + Name: fmt.Sprintf("cluster%d", i), }, - Tablets: tt.tablets, + VtctldClient: tt.vtctld, + Tablets: nil, + DBConfig: testutil.Dbcfg{}, }) - tablets, err := cluster.FindTablets(ctx, tt.filter, tt.n) + + err := cluster.Vtctld.Dial(ctx) + require.NoError(t, err, "could not dial test vtctld") + + schema, err := cluster.GetSchema(ctx, tt.req, tt.tablet) + if tt.shouldErr { + assert.Error(t, err) + + return + } assert.NoError(t, err) - testutil.AssertTabletSlicesEqual(t, tt.expected, tablets) + assert.Equal(t, tt.expected, schema) + }) + } + + t.Run("does not modify passed-in request", func(t *testing.T) { + t.Parallel() + + vtctld := &testutil.VtctldClient{ + GetSchemaResults: map[string]struct { + Response *vtctldatapb.GetSchemaResponse + Error error + }{ + "zone1-0000000100": { + Response: &vtctldatapb.GetSchemaResponse{}, + }, + }, + } + + req := &vtctldatapb.GetSchemaRequest{ + TabletAlias: &topodatapb.TabletAlias{ + Cell: "otherzone", + Uid: 500, + }, + } + tablet := &vtadminpb.Tablet{ + Tablet: &topodatapb.Tablet{ + Alias: &topodatapb.TabletAlias{ + Cell: "zone1", + Uid: 100, + }, + }, + } + + cluster := testutil.BuildCluster(testutil.TestClusterConfig{ + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", + }, + VtctldClient: vtctld, }) + + err := cluster.Vtctld.Dial(ctx) + require.NoError(t, err, "could not dial test vtctld") + + cluster.GetSchema(ctx, req, tablet) + + assert.NotEqual(t, req.TabletAlias, tablet.Tablet.Alias, "expected GetSchema to not modify original request object") + }) +} + +// This test only validates the error handling on dialing database connections. +// Other cases are covered by one or both of TestFindTablets and TestFindTablet. +func TestGetTablets(t *testing.T) { + t.Parallel() + + disco := fakediscovery.New() + disco.AddTaggedGates(nil, &vtadminpb.VTGate{Hostname: "gate"}) + + db := vtsql.New(&vtsql.Config{ + Cluster: &vtadminpb.Cluster{ + Id: "c1", + Name: "one", + }, + Discovery: disco, + }) + db.DialFunc = func(cfg vitessdriver.Configuration) (*sql.DB, error) { + return nil, assert.AnError } + + c := &cluster.Cluster{DB: db} + _, err := c.GetTablets(context.Background()) + assert.Error(t, err) } -func TestFindTablet(t *testing.T) { +func TestGetVSchema(t *testing.T) { + t.Parallel() + tests := []struct { - name string - tablets []*vtadminpb.Tablet - filter func(*vtadminpb.Tablet) bool - expected *vtadminpb.Tablet - expectedError error + name string + cfg testutil.TestClusterConfig + keyspace string + expected *vtadminpb.VSchema + shouldErr bool }{ { - name: "returns the first matching tablet", - tablets: []*vtadminpb.Tablet{ - { - State: vtadminpb.Tablet_NOT_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "c0_cell1", - Uid: 100, - }, - Keyspace: "commerce", - }, - }, - { - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "c0_cell1", - Uid: 101, - }, - Keyspace: "commerce", - }, + name: "success", + cfg: testutil.TestClusterConfig{ + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", }, - { - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "c0_cell1", - Uid: 102, + VtctldClient: &testutil.VtctldClient{ + GetVSchemaResults: map[string]struct { + Response *vtctldatapb.GetVSchemaResponse + Error error + }{ + "testkeyspace": { + Response: &vtctldatapb.GetVSchemaResponse{ + VSchema: &vschemapb.Keyspace{Sharded: true}, + }, }, - Keyspace: "commerce", }, }, }, - - filter: func(t *vtadminpb.Tablet) bool { - return t.State == vtadminpb.Tablet_SERVING - }, - expected: &vtadminpb.Tablet{ + keyspace: "testkeyspace", + expected: &vtadminpb.VSchema{ Cluster: &vtadminpb.Cluster{ Id: "c0", Name: "cluster0", }, - State: vtadminpb.Tablet_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "c0_cell1", - Uid: 101, - }, - Keyspace: "commerce", - }, + Name: "testkeyspace", + VSchema: &vschemapb.Keyspace{Sharded: true}, }, + shouldErr: false, }, { - name: "returns an error if no match found", - tablets: []*vtadminpb.Tablet{ - { - State: vtadminpb.Tablet_NOT_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "c0_cell1", - Uid: 100, - }, - Keyspace: "commerce", - }, + name: "error", + cfg: testutil.TestClusterConfig{ + Cluster: &vtadminpb.Cluster{ + Id: "c0", + Name: "cluster0", }, - { - State: vtadminpb.Tablet_NOT_SERVING, - Tablet: &topodatapb.Tablet{ - Alias: &topodatapb.TabletAlias{ - Cell: "c0_cell1", - Uid: 101, + VtctldClient: &testutil.VtctldClient{ + GetVSchemaResults: map[string]struct { + Response *vtctldatapb.GetVSchemaResponse + Error error + }{ + "testkeyspace": { + Response: &vtctldatapb.GetVSchemaResponse{ + VSchema: &vschemapb.Keyspace{Sharded: true}, + }, }, - Keyspace: "commerce", }, }, }, - filter: func(t *vtadminpb.Tablet) bool { - return t.State == vtadminpb.Tablet_SERVING - }, - expectedError: vtadminerrors.ErrNoTablet, + keyspace: "notfound", + expected: nil, + shouldErr: true, }, } @@ -646,21 +656,21 @@ func TestFindTablet(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { - cluster := testutil.BuildCluster(testutil.TestClusterConfig{ - Cluster: &vtadminpb.Cluster{ - Id: "c0", - Name: "cluster0", - }, - Tablets: tt.tablets, - }) - tablet, err := cluster.FindTablet(ctx, tt.filter) + t.Parallel() - if tt.expectedError != nil { - assert.True(t, errors.Is(err, tt.expectedError), "expected error type %w does not match actual error type %w", err, tt.expectedError) - } else { - assert.NoError(t, err) - testutil.AssertTabletsEqual(t, tt.expected, tablet) + cluster := testutil.BuildCluster(tt.cfg) + err := cluster.Vtctld.Dial(ctx) + require.NoError(t, err, "could not dial test vtctld") + + vschema, err := cluster.GetVSchema(ctx, tt.keyspace) + if tt.shouldErr { + assert.Error(t, err) + + return } + + assert.NoError(t, err) + assert.Equal(t, tt.expected, vschema) }) } } diff --git a/go/vt/vtadmin/cluster/config_test.go b/go/vt/vtadmin/cluster/config_test.go index e087fa1a300..5bdfaf49b65 100644 --- a/go/vt/vtadmin/cluster/config_test.go +++ b/go/vt/vtadmin/cluster/config_test.go @@ -25,6 +25,8 @@ import ( ) func TestMergeConfig(t *testing.T) { + t.Parallel() + tests := []struct { name string base Config @@ -131,7 +133,11 @@ func TestMergeConfig(t *testing.T) { } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + actual := tt.base.Merge(tt.override) assert.Equal(t, tt.expected, actual) }) @@ -139,6 +145,8 @@ func TestMergeConfig(t *testing.T) { } func TestConfigUnmarshalYAML(t *testing.T) { + t.Parallel() + tests := []struct { name string yaml string @@ -187,7 +195,11 @@ discovery-zk-whatever: 5 } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cfg := Config{ DiscoveryFlagsByImpl: map[string]map[string]string{}, } diff --git a/go/vt/vtadmin/cluster/discovery/discovery_consul_test.go b/go/vt/vtadmin/cluster/discovery/discovery_consul_test.go index 8d8e67722c3..a298e5906c9 100644 --- a/go/vt/vtadmin/cluster/discovery/discovery_consul_test.go +++ b/go/vt/vtadmin/cluster/discovery/discovery_consul_test.go @@ -87,6 +87,8 @@ func consulServiceEntry(name string, tags []string, meta map[string]string) *con } func TestConsulDiscoverVTGates(t *testing.T) { + t.Parallel() + tests := []struct { name string disco *ConsulDiscovery @@ -228,15 +230,21 @@ func TestConsulDiscoverVTGates(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + tt.disco.client = &fakeConsulClient{ health: &fakeConsulHealth{ entries: tt.entries, }, } - gates, err := tt.disco.DiscoverVTGates(context.Background(), tt.tags) + gates, err := tt.disco.DiscoverVTGates(ctx, tt.tags) if tt.shouldErr { assert.Error(t, err, assert.AnError) return @@ -249,6 +257,8 @@ func TestConsulDiscoverVTGates(t *testing.T) { } func TestConsulDiscoverVTGate(t *testing.T) { + t.Parallel() + tests := []struct { name string disco *ConsulDiscovery @@ -331,15 +341,21 @@ func TestConsulDiscoverVTGate(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + tt.disco.client = &fakeConsulClient{ health: &fakeConsulHealth{ entries: tt.entries, }, } - gate, err := tt.disco.DiscoverVTGate(context.Background(), tt.tags) + gate, err := tt.disco.DiscoverVTGate(ctx, tt.tags) if tt.shouldErr { assert.Error(t, err, assert.AnError) return @@ -352,6 +368,8 @@ func TestConsulDiscoverVTGate(t *testing.T) { } func TestConsulDiscoverVTGateAddr(t *testing.T) { + t.Parallel() + tests := []struct { name string disco *ConsulDiscovery @@ -421,15 +439,21 @@ func TestConsulDiscoverVTGateAddr(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + tt.disco.client = &fakeConsulClient{ health: &fakeConsulHealth{ entries: tt.entries, }, } - addr, err := tt.disco.DiscoverVTGateAddr(context.Background(), tt.tags) + addr, err := tt.disco.DiscoverVTGateAddr(ctx, tt.tags) if tt.shouldErr { assert.Error(t, err, assert.AnError) return diff --git a/go/vt/vtadmin/cluster/discovery/discovery_static_file_test.go b/go/vt/vtadmin/cluster/discovery/discovery_static_file_test.go index 52cf970b3f3..8fa049f8540 100644 --- a/go/vt/vtadmin/cluster/discovery/discovery_static_file_test.go +++ b/go/vt/vtadmin/cluster/discovery/discovery_static_file_test.go @@ -28,6 +28,8 @@ import ( ) func TestDiscoverVTGate(t *testing.T) { + t.Parallel() + tests := []struct { name string contents []byte @@ -89,13 +91,19 @@ func TestDiscoverVTGate(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + disco := &StaticFileDiscovery{} err := disco.parseConfig(tt.contents) require.NoError(t, err) - gate, err := disco.DiscoverVTGate(context.Background(), tt.tags) + gate, err := disco.DiscoverVTGate(ctx, tt.tags) if tt.shouldErr { assert.Error(t, err) return @@ -108,6 +116,8 @@ func TestDiscoverVTGate(t *testing.T) { } func TestDiscoverVTGates(t *testing.T) { + t.Parallel() + tests := []struct { name string contents []byte @@ -226,8 +236,14 @@ func TestDiscoverVTGates(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + disco := &StaticFileDiscovery{} err := disco.parseConfig(tt.contents) @@ -237,7 +253,7 @@ func TestDiscoverVTGates(t *testing.T) { require.NoError(t, err) } - gates, err := disco.DiscoverVTGates(context.Background(), tt.tags) + gates, err := disco.DiscoverVTGates(ctx, tt.tags) if tt.shouldErr { assert.Error(t, err) return @@ -250,6 +266,8 @@ func TestDiscoverVTGates(t *testing.T) { } func TestDiscoverVtctld(t *testing.T) { + t.Parallel() + tests := []struct { name string contents []byte @@ -311,13 +329,19 @@ func TestDiscoverVtctld(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + disco := &StaticFileDiscovery{} err := disco.parseConfig(tt.contents) require.NoError(t, err) - vtctld, err := disco.DiscoverVtctld(context.Background(), tt.tags) + vtctld, err := disco.DiscoverVtctld(ctx, tt.tags) if tt.shouldErr { assert.Error(t, err) return @@ -330,6 +354,8 @@ func TestDiscoverVtctld(t *testing.T) { } func TestDiscoverVtctlds(t *testing.T) { + t.Parallel() + tests := []struct { name string contents []byte @@ -448,8 +474,14 @@ func TestDiscoverVtctlds(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + disco := &StaticFileDiscovery{} err := disco.parseConfig(tt.contents) @@ -459,7 +491,7 @@ func TestDiscoverVtctlds(t *testing.T) { require.NoError(t, err) } - vtctlds, err := disco.DiscoverVtctlds(context.Background(), tt.tags) + vtctlds, err := disco.DiscoverVtctlds(ctx, tt.tags) if tt.shouldErr { assert.Error(t, err) return diff --git a/go/vt/vtadmin/cluster/discovery/discovery_test.go b/go/vt/vtadmin/cluster/discovery/discovery_test.go index b00071288c7..3ee67cbc9df 100644 --- a/go/vt/vtadmin/cluster/discovery/discovery_test.go +++ b/go/vt/vtadmin/cluster/discovery/discovery_test.go @@ -17,7 +17,9 @@ limitations under the License. package discovery import ( + "fmt" "testing" + "time" "github.com/stretchr/testify/assert" @@ -25,6 +27,8 @@ import ( ) func TestNew(t *testing.T) { + t.Parallel() + tests := []struct { name string impl string @@ -46,7 +50,11 @@ func TestNew(t *testing.T) { } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + disco, err := New(tt.impl, &vtadminpb.Cluster{Id: "testid", Name: "testcluster"}, []string{}) if tt.err != nil { assert.Error(t, err, tt.err.Error()) @@ -60,7 +68,13 @@ func TestNew(t *testing.T) { } func TestRegister(t *testing.T) { - Register("testfactory", nil) + t.Parallel() + + // Use a timestamp to allow running tests with `-count=N`. + ts := time.Now().UnixNano() + factoryName := fmt.Sprintf("testfactory-%d", ts) + + Register(factoryName, nil) defer func() { err := recover() @@ -70,6 +84,6 @@ func TestRegister(t *testing.T) { }() // this one panics - Register("testfactory", nil) + Register(factoryName, nil) assert.Equal(t, 1, 2, "double register should have panicked") } diff --git a/go/vt/vtadmin/cluster/discovery/fakediscovery/discovery_test.go b/go/vt/vtadmin/cluster/discovery/fakediscovery/discovery_test.go index 25bdd0f57ae..841f73e70de 100644 --- a/go/vt/vtadmin/cluster/discovery/fakediscovery/discovery_test.go +++ b/go/vt/vtadmin/cluster/discovery/fakediscovery/discovery_test.go @@ -26,6 +26,8 @@ import ( ) func TestDiscoverVTGates(t *testing.T) { + t.Parallel() + fake := New() gates := []*vtadminpb.VTGate{ { @@ -39,33 +41,35 @@ func TestDiscoverVTGates(t *testing.T) { }, } + ctx := context.Background() + fake.AddTaggedGates(nil, gates...) fake.AddTaggedGates([]string{"tag1:val1"}, gates[0], gates[1]) fake.AddTaggedGates([]string{"tag2:val2"}, gates[0], gates[2]) - actual, err := fake.DiscoverVTGates(context.Background(), nil) + actual, err := fake.DiscoverVTGates(ctx, nil) assert.NoError(t, err) assert.ElementsMatch(t, gates, actual) - actual, err = fake.DiscoverVTGates(context.Background(), []string{"tag1:val1"}) + actual, err = fake.DiscoverVTGates(ctx, []string{"tag1:val1"}) assert.NoError(t, err) assert.ElementsMatch(t, []*vtadminpb.VTGate{gates[0], gates[1]}, actual) - actual, err = fake.DiscoverVTGates(context.Background(), []string{"tag2:val2"}) + actual, err = fake.DiscoverVTGates(ctx, []string{"tag2:val2"}) assert.NoError(t, err) assert.ElementsMatch(t, []*vtadminpb.VTGate{gates[0], gates[2]}, actual) - actual, err = fake.DiscoverVTGates(context.Background(), []string{"tag1:val1", "tag2:val2"}) + actual, err = fake.DiscoverVTGates(ctx, []string{"tag1:val1", "tag2:val2"}) assert.NoError(t, err) assert.ElementsMatch(t, []*vtadminpb.VTGate{gates[0]}, actual) - actual, err = fake.DiscoverVTGates(context.Background(), []string{"differentTag:val"}) + actual, err = fake.DiscoverVTGates(ctx, []string{"differentTag:val"}) assert.NoError(t, err) assert.Equal(t, []*vtadminpb.VTGate{}, actual) fake.SetGatesError(true) - actual, err = fake.DiscoverVTGates(context.Background(), nil) + actual, err = fake.DiscoverVTGates(ctx, nil) assert.Error(t, err) assert.Nil(t, actual) } diff --git a/go/vt/vtadmin/cluster/file_config_test.go b/go/vt/vtadmin/cluster/file_config_test.go index 68d5092df8f..07bf6dbddbd 100644 --- a/go/vt/vtadmin/cluster/file_config_test.go +++ b/go/vt/vtadmin/cluster/file_config_test.go @@ -25,6 +25,8 @@ import ( ) func TestFileConfigUnmarshalYAML(t *testing.T) { + t.Parallel() + tests := []struct { name string yaml string @@ -92,7 +94,11 @@ clusters: } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cfg := FileConfig{ Defaults: Config{ DiscoveryFlagsByImpl: map[string]map[string]string{}, @@ -113,6 +119,8 @@ clusters: } func TestCombine(t *testing.T) { + t.Parallel() + tests := []struct { name string fc FileConfig @@ -254,7 +262,11 @@ func TestCombine(t *testing.T) { } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + actual := tt.fc.Combine(tt.defaults, tt.configs) assert.ElementsMatch(t, tt.expected, actual) }) diff --git a/go/vt/vtadmin/cluster/flags_test.go b/go/vt/vtadmin/cluster/flags_test.go index 48867ac9441..676c0243029 100644 --- a/go/vt/vtadmin/cluster/flags_test.go +++ b/go/vt/vtadmin/cluster/flags_test.go @@ -23,6 +23,8 @@ import ( ) func TestMergeFlagsByImpl(t *testing.T) { + t.Parallel() + var NilMap map[string]map[string]string tests := []struct { @@ -95,11 +97,15 @@ func TestMergeFlagsByImpl(t *testing.T) { }, } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - flags := FlagsByImpl(test.base) - flags.Merge(test.in) - assert.Equal(t, FlagsByImpl(test.expected), flags) + for _, tt := range tests { + tt := tt + + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + flags := FlagsByImpl(tt.base) + flags.Merge(tt.in) + assert.Equal(t, FlagsByImpl(tt.expected), flags) }) } } diff --git a/go/vt/vtadmin/credentials/credentials_test.go b/go/vt/vtadmin/credentials/credentials_test.go index 530f73eddca..eacea70408c 100644 --- a/go/vt/vtadmin/credentials/credentials_test.go +++ b/go/vt/vtadmin/credentials/credentials_test.go @@ -28,6 +28,8 @@ import ( ) func Test_loadCredentials(t *testing.T) { + t.Parallel() + tests := []struct { name string contents []byte @@ -55,7 +57,11 @@ func Test_loadCredentials(t *testing.T) { } for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + path := "" if len(tt.contents) > 0 { diff --git a/go/vt/vtadmin/vtctldclient/config_test.go b/go/vt/vtadmin/vtctldclient/config_test.go index a9979420d4e..69d09c7bbba 100644 --- a/go/vt/vtadmin/vtctldclient/config_test.go +++ b/go/vt/vtadmin/vtctldclient/config_test.go @@ -42,7 +42,11 @@ func withTempFile(t *testing.T, tmpdir string, name string, f func(*testing.T, * } func TestParse(t *testing.T) { + t.Parallel() + t.Run("no credentials provided", func(t *testing.T) { + t.Parallel() + cfg, err := Parse(nil, nil, []string{}) require.NoError(t, err) @@ -56,6 +60,8 @@ func TestParse(t *testing.T) { }) t.Run("credential loading", func(t *testing.T) { + t.Parallel() + withTempFile(t, "", "vtctldclient.config_test.testcluster.*", func(t *testing.T, credsfile *os.File) { creds := &grpcclient.StaticAuthClientCreds{ Username: "admin", diff --git a/go/vt/vtadmin/vtsql/config_test.go b/go/vt/vtadmin/vtsql/config_test.go index e2ba9acdc4d..e32b6855a4f 100644 --- a/go/vt/vtadmin/vtsql/config_test.go +++ b/go/vt/vtadmin/vtsql/config_test.go @@ -33,6 +33,8 @@ import ( ) func TestConfigParse(t *testing.T) { + t.Parallel() + cfg := Config{} // This asserts we do not attempt to load a credentialsFlag via its Set func @@ -41,6 +43,8 @@ func TestConfigParse(t *testing.T) { assert.NoError(t, err) t.Run("", func(t *testing.T) { + t.Parallel() + f, err := ioutil.TempFile("", "vtsql-config-test-testcluster-*") // testcluster is going to appear in the template require.NoError(t, err) @@ -93,6 +97,8 @@ func TestConfigParse(t *testing.T) { }) t.Run("", func(t *testing.T) { + t.Parallel() + f, err := ioutil.TempFile("", "vtsql-config-test-testcluster-*") // testcluster is going to appear in the template require.NoError(t, err) diff --git a/go/vt/vtadmin/vtsql/vtsql_test.go b/go/vt/vtadmin/vtsql/vtsql_test.go index 4746a3fb420..17c1b503398 100644 --- a/go/vt/vtadmin/vtsql/vtsql_test.go +++ b/go/vt/vtadmin/vtsql/vtsql_test.go @@ -36,11 +36,15 @@ import ( ) func assertImmediateCaller(t *testing.T, im *querypb.VTGateCallerID, expected string) { + t.Helper() + require.NotNil(t, im, "immediate caller cannot be nil") assert.Equal(t, im.Username, expected, "immediate caller username mismatch") } func assertEffectiveCaller(t *testing.T, ef *vtrpcpb.CallerID, principal string, component string, subcomponent string) { + t.Helper() + require.NotNil(t, ef, "effective caller cannot be nil") assert.Equal(t, ef.Principal, principal, "effective caller principal mismatch") assert.Equal(t, ef.Component, component, "effective caller component mismatch") @@ -48,6 +52,8 @@ func assertEffectiveCaller(t *testing.T, ef *vtrpcpb.CallerID, principal string, } func Test_getQueryContext(t *testing.T) { + t.Parallel() + ctx := context.Background() creds := &StaticAuthCredentials{ @@ -81,6 +87,8 @@ func Test_getQueryContext(t *testing.T) { } func TestDial(t *testing.T) { + t.Helper() + tests := []struct { name string disco *fakediscovery.Fake @@ -144,8 +152,14 @@ func TestDial(t *testing.T) { }, } + ctx := context.Background() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if tt.disco != nil { if len(tt.gates) > 0 { tt.disco.AddTaggedGates(nil, tt.gates...) @@ -154,7 +168,7 @@ func TestDial(t *testing.T) { tt.proxy.discovery = tt.disco } - err := tt.proxy.Dial(context.Background(), "") + err := tt.proxy.Dial(ctx, "") if tt.shouldErr { assert.Error(t, err) return