Skip to content

Commit e02adaf

Browse files
mattlordmattlord
committed
Fix unit tests
Signed-off-by: Matt Lord <mattalord@gmail.com>
1 parent d3ebbe7 commit e02adaf

File tree

21 files changed

+712
-382
lines changed

21 files changed

+712
-382
lines changed

‎go/test/utils/diff.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func MustMatchFn(ignoredFields ...string) func(t *testing.T, want, got any, errM
6868
t.Helper()
6969
diff := cmp.Diff(want, got, diffOpts...)
7070
if diff != "" {
71-
t.Fatalf("%v: (-want +got)\n%v", errMsg, diff)
71+
require.FailNow(t, "%v: (-want +got)\n%v", errMsg, diff)
7272
}
7373
}
7474
}

‎go/vt/topo/test/vschema.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,12 @@ func checkVSchema(t *testing.T, ctx context.Context, ts *topo.Server) {
4444
t.Error(err)
4545
}
4646

47-
err = ts.SaveVSchema(ctx, "test_keyspace", &vschemapb.Keyspace{
48-
Tables: map[string]*vschemapb.Table{
49-
"unsharded": {},
47+
err = ts.SaveVSchema(ctx, &topo.KeyspaceVSchemaInfo{
48+
Name: "test_keyspace",
49+
Keyspace: &vschemapb.Keyspace{
50+
Tables: map[string]*vschemapb.Table{
51+
"unsharded": {},
52+
},
5053
},
5154
})
5255
if err != nil {
@@ -64,8 +67,11 @@ func checkVSchema(t *testing.T, ctx context.Context, ts *topo.Server) {
6467
t.Errorf("GetVSchema: %s, want %s", got, want)
6568
}
6669

67-
err = ts.SaveVSchema(ctx, "test_keyspace", &vschemapb.Keyspace{
68-
Sharded: true,
70+
err = ts.SaveVSchema(ctx, &topo.KeyspaceVSchemaInfo{
71+
Name: "test_keyspace",
72+
Keyspace: &vschemapb.Keyspace{
73+
Sharded: true,
74+
},
6975
})
7076
require.NoError(t, err)
7177

‎go/vt/topo/topotests/srv_vschema_test.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
"google.golang.org/protobuf/proto"
2323

24+
"vitess.io/vitess/go/vt/topo"
2425
"vitess.io/vitess/go/vt/topo/memorytopo"
2526

2627
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
@@ -76,7 +77,10 @@ func TestRebuildVSchema(t *testing.T) {
7677
keyspace1 := &vschemapb.Keyspace{
7778
Sharded: true,
7879
}
79-
if err := ts.SaveVSchema(ctx, "ks1", keyspace1); err != nil {
80+
if err := ts.SaveVSchema(ctx, &topo.KeyspaceVSchemaInfo{
81+
Name: "ks1",
82+
Keyspace: keyspace1,
83+
}); err != nil {
8084
t.Fatalf("SaveVSchema(ks1) failed: %v", err)
8185
}
8286
if err := ts.RebuildSrvVSchema(ctx, cells); err != nil {
@@ -118,7 +122,10 @@ func TestRebuildVSchema(t *testing.T) {
118122
},
119123
},
120124
}
121-
if err := ts.SaveVSchema(ctx, "ks2", keyspace2); err != nil {
125+
if err := ts.SaveVSchema(ctx, &topo.KeyspaceVSchemaInfo{
126+
Name: "ks2",
127+
Keyspace: keyspace2,
128+
}); err != nil {
122129
t.Fatalf("SaveVSchema(ks1) failed: %v", err)
123130
}
124131
if err := ts.RebuildSrvVSchema(ctx, []string{"cell1"}); err != nil {
@@ -182,7 +189,10 @@ func TestRebuildVSchema(t *testing.T) {
182189
wanted4.RoutingRules = rr
183190

184191
// Delete a keyspace, checks vschema entry in map goes away.
185-
if err := ts.SaveVSchema(ctx, "ks2", &vschemapb.Keyspace{}); err != nil {
192+
if err := ts.SaveVSchema(ctx, &topo.KeyspaceVSchemaInfo{
193+
Name: "ks2",
194+
Keyspace: &vschemapb.Keyspace{},
195+
}); err != nil {
186196
t.Fatalf("SaveVSchema(ks1) failed: %v", err)
187197
}
188198
if err := ts.DeleteKeyspace(ctx, "ks2"); err != nil {

‎go/vt/topo/vschema.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import (
2020
"context"
2121
"path"
2222

23-
"google.golang.org/protobuf/proto"
24-
2523
"vitess.io/vitess/go/vt/log"
2624
"vitess.io/vitess/go/vt/vterrors"
2725

@@ -39,11 +37,17 @@ type KeyspaceVSchemaInfo struct {
3937
}
4038

4139
func (k *KeyspaceVSchemaInfo) CloneVT() *KeyspaceVSchemaInfo {
42-
return &KeyspaceVSchemaInfo{
43-
Name: k.Name,
44-
Keyspace: k.Keyspace.CloneVT(),
45-
version: Version(k.version),
40+
if k == nil {
41+
return (*KeyspaceVSchemaInfo)(nil)
42+
}
43+
kc := &KeyspaceVSchemaInfo{
44+
Name: k.Name,
45+
version: Version(k.version),
4646
}
47+
if k.Keyspace != nil {
48+
kc.Keyspace = k.Keyspace.CloneVT()
49+
}
50+
return kc
4751
}
4852

4953
// SaveVSchema saves a Vschema. A valid Vschema should be passed in. It does not verify its correctness.
@@ -91,14 +95,15 @@ func (ts *Server) GetVSchema(ctx context.Context, keyspace string) (*KeyspaceVSc
9195
if err != nil {
9296
return nil, err
9397
}
94-
var vs vschemapb.Keyspace
95-
err = proto.Unmarshal(data, &vs)
98+
99+
vs := &vschemapb.Keyspace{}
100+
err = vs.UnmarshalVT(data)
96101
if err != nil {
97102
return nil, vterrors.Wrapf(err, "bad vschema data: %q", data)
98103
}
99104
return &KeyspaceVSchemaInfo{
100105
Name: keyspace,
101-
Keyspace: &vs,
106+
Keyspace: vs,
102107
version: version,
103108
}, nil
104109
}

‎go/vt/vtctl/grpcvtctldserver/server.go

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -937,34 +937,37 @@ func (s *VtctldServer) CreateKeyspace(ctx context.Context, req *vtctldatapb.Crea
937937
}
938938

939939
if req.Type == topodatapb.KeyspaceType_SNAPSHOT {
940-
// Copy vschema from the base keyspace.
941940
bksvs, err := s.ts.GetVSchema(ctx, req.BaseKeyspace)
942-
ksvs := &topo.KeyspaceVSchemaInfo{
943-
Name: req.Name,
944-
}
945941
if err != nil {
946942
log.Infof("error from GetVSchema(%v) = %v", req.BaseKeyspace, err)
947943
if topo.IsErrType(err, topo.NoNode) {
948944
log.Infof("base keyspace %v does not exist; continuing with bare, unsharded vschema", req.BaseKeyspace)
949-
// Create an empty vschema for the keyspace.
950-
ksvs.Keyspace = &vschemapb.Keyspace{
951-
Sharded: false,
952-
Tables: map[string]*vschemapb.Table{},
953-
Vindexes: map[string]*vschemapb.Vindex{},
945+
bksvs = &topo.KeyspaceVSchemaInfo{
946+
Name: req.Name,
947+
Keyspace: &vschemapb.Keyspace{
948+
Sharded: false,
949+
Tables: map[string]*vschemapb.Table{},
950+
Vindexes: map[string]*vschemapb.Vindex{},
951+
},
954952
}
955953
} else {
956954
return nil, err
957955
}
958956
}
959957

960-
// Copy the vschema from the base keyspace to the new one.
961-
ksvs.Keyspace = bksvs.Keyspace.CloneVT()
958+
// We don't want to clone the base keyspace's key version
959+
// so we do NOT call bksvs.CloneVT() here. We instead only
960+
// clone the vschemapb.Keyspace field for the new snapshot
961+
// keyspace.
962+
sksvs := &topo.KeyspaceVSchemaInfo{
963+
Name: req.Name,
964+
Keyspace: bksvs.Keyspace.CloneVT(),
965+
}
962966
// SNAPSHOT keyspaces are excluded from global routing.
963-
ksvs.RequireExplicitRouting = true
967+
sksvs.RequireExplicitRouting = true
964968

965-
if err = s.ts.SaveVSchema(ctx, ksvs); err != nil {
966-
err = fmt.Errorf("SaveVSchema(%v) = %w", ksvs, err)
967-
return nil, err
969+
if err = s.ts.SaveVSchema(ctx, sksvs); err != nil {
970+
return nil, fmt.Errorf("SaveVSchema(%v) = %w", sksvs, err)
968971
}
969972
}
970973

‎go/vt/vtctl/grpcvtctldserver/server_test.go

Lines changed: 63 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ import (
2828
"testing"
2929
"time"
3030

31+
"google.golang.org/protobuf/proto"
32+
3133
_flag "vitess.io/vitess/go/internal/flag"
3234
"vitess.io/vitess/go/vt/vtctl/reparentutil"
3335
"vitess.io/vitess/go/vt/vtctl/reparentutil/policy"
@@ -608,15 +610,18 @@ func TestApplyVSchema(t *testing.T) {
608610
},
609611
})
610612

611-
origVSchema := &vschemapb.Keyspace{
612-
Sharded: true,
613-
Vindexes: map[string]*vschemapb.Vindex{
614-
"v1": {
615-
Type: "hash",
613+
origVSchema := &topo.KeyspaceVSchemaInfo{
614+
Name: tt.req.Keyspace,
615+
Keyspace: &vschemapb.Keyspace{
616+
Sharded: true,
617+
Vindexes: map[string]*vschemapb.Vindex{
618+
"v1": {
619+
Type: "hash",
620+
},
616621
},
617622
},
618623
}
619-
err := ts.SaveVSchema(ctx, tt.req.Keyspace, origVSchema)
624+
err := ts.SaveVSchema(ctx, origVSchema)
620625
require.NoError(t, err)
621626

622627
origSrvVSchema := &vschemapb.SrvVSchema{
@@ -2577,12 +2582,12 @@ func TestCreateKeyspace(t *testing.T) {
25772582
tests := []struct {
25782583
name string
25792584
topo map[string]*topodatapb.Keyspace
2580-
vschemas map[string]*vschemapb.Keyspace
2585+
vschemas map[string]*topo.KeyspaceVSchemaInfo
25812586
req *vtctldatapb.CreateKeyspaceRequest
25822587
expected *vtctldatapb.CreateKeyspaceResponse
25832588
shouldErr bool
25842589
vschemaShouldExist bool
2585-
expectedVSchema *vschemapb.Keyspace
2590+
expectedVSchema *topo.KeyspaceVSchemaInfo
25862591
}{
25872592
{
25882593
name: "normal keyspace",
@@ -2600,8 +2605,11 @@ func TestCreateKeyspace(t *testing.T) {
26002605
},
26012606
},
26022607
vschemaShouldExist: true,
2603-
expectedVSchema: &vschemapb.Keyspace{
2604-
Sharded: false,
2608+
expectedVSchema: &topo.KeyspaceVSchemaInfo{
2609+
Name: "testkeyspace",
2610+
Keyspace: &vschemapb.Keyspace{
2611+
Sharded: false,
2612+
},
26052613
},
26062614
shouldErr: false,
26072615
},
@@ -2612,12 +2620,15 @@ func TestCreateKeyspace(t *testing.T) {
26122620
KeyspaceType: topodatapb.KeyspaceType_NORMAL,
26132621
},
26142622
},
2615-
vschemas: map[string]*vschemapb.Keyspace{
2623+
vschemas: map[string]*topo.KeyspaceVSchemaInfo{
26162624
"testkeyspace": {
2617-
Sharded: true,
2618-
Vindexes: map[string]*vschemapb.Vindex{
2619-
"h1": {
2620-
Type: "hash",
2625+
Name: "testkeyspace",
2626+
Keyspace: &vschemapb.Keyspace{
2627+
Sharded: true,
2628+
Vindexes: map[string]*vschemapb.Vindex{
2629+
"h1": {
2630+
Type: "hash",
2631+
},
26212632
},
26222633
},
26232634
},
@@ -2643,14 +2654,17 @@ func TestCreateKeyspace(t *testing.T) {
26432654
},
26442655
},
26452656
vschemaShouldExist: true,
2646-
expectedVSchema: &vschemapb.Keyspace{
2647-
Sharded: true,
2648-
Vindexes: map[string]*vschemapb.Vindex{
2649-
"h1": {
2650-
Type: "hash",
2657+
expectedVSchema: &topo.KeyspaceVSchemaInfo{
2658+
Name: "testkeyspace",
2659+
Keyspace: &vschemapb.Keyspace{
2660+
Sharded: true,
2661+
Vindexes: map[string]*vschemapb.Vindex{
2662+
"h1": {
2663+
Type: "hash",
2664+
},
26512665
},
2666+
RequireExplicitRouting: true,
26522667
},
2653-
RequireExplicitRouting: true,
26542668
},
26552669
shouldErr: false,
26562670
},
@@ -2696,9 +2710,12 @@ func TestCreateKeyspace(t *testing.T) {
26962710
},
26972711
},
26982712
vschemaShouldExist: true,
2699-
expectedVSchema: &vschemapb.Keyspace{
2700-
Sharded: false,
2701-
RequireExplicitRouting: true,
2713+
expectedVSchema: &topo.KeyspaceVSchemaInfo{
2714+
Name: "testsnapshot",
2715+
Keyspace: &vschemapb.Keyspace{
2716+
Sharded: false,
2717+
RequireExplicitRouting: true,
2718+
},
27022719
},
27032720
shouldErr: false,
27042721
},
@@ -2748,8 +2765,11 @@ func TestCreateKeyspace(t *testing.T) {
27482765
},
27492766
},
27502767
vschemaShouldExist: true,
2751-
expectedVSchema: &vschemapb.Keyspace{
2752-
Sharded: false,
2768+
expectedVSchema: &topo.KeyspaceVSchemaInfo{
2769+
Name: "testkeyspace",
2770+
Keyspace: &vschemapb.Keyspace{
2771+
Sharded: false,
2772+
},
27532773
},
27542774
shouldErr: false,
27552775
},
@@ -2772,7 +2792,8 @@ func TestCreateKeyspace(t *testing.T) {
27722792
vschemaShouldExist: false,
27732793
expectedVSchema: nil,
27742794
shouldErr: false,
2775-
}, {
2795+
},
2796+
{
27762797
name: "keyspace with durability policy specified",
27772798
topo: nil,
27782799
req: &vtctldatapb.CreateKeyspaceRequest{
@@ -2790,8 +2811,11 @@ func TestCreateKeyspace(t *testing.T) {
27902811
},
27912812
},
27922813
vschemaShouldExist: true,
2793-
expectedVSchema: &vschemapb.Keyspace{
2794-
Sharded: false,
2814+
expectedVSchema: &topo.KeyspaceVSchemaInfo{
2815+
Name: "testkeyspace",
2816+
Keyspace: &vschemapb.Keyspace{
2817+
Sharded: false,
2818+
},
27952819
},
27962820
shouldErr: false,
27972821
},
@@ -2820,7 +2844,7 @@ func TestCreateKeyspace(t *testing.T) {
28202844
}
28212845

28222846
for name, vs := range tt.vschemas {
2823-
require.NoError(t, ts.SaveVSchema(ctx, name, vs), "error in SaveVSchema(%v, %+v)", name, vs)
2847+
require.NoError(t, ts.SaveVSchema(ctx, vs), "error in SaveVSchema(%v, %+v)", name, vs)
28242848
}
28252849

28262850
// Create the keyspace and make some assertions
@@ -2829,7 +2853,6 @@ func TestCreateKeyspace(t *testing.T) {
28292853
assert.Error(t, err)
28302854
return
28312855
}
2832-
28332856
assert.NoError(t, err)
28342857
testutil.AssertKeyspacesEqual(t, tt.expected.Keyspace, resp.Keyspace, "%+v\n%+v\n", tt.expected.Keyspace, resp.Keyspace)
28352858

@@ -2858,7 +2881,7 @@ func TestCreateKeyspace(t *testing.T) {
28582881
return
28592882
}
28602883
assert.NoError(t, err)
2861-
utils.MustMatch(t, tt.expectedVSchema, vs)
2884+
require.True(t, proto.Equal(tt.expectedVSchema, vs), "expected vschema for %s: %+v, got: %+v", tt.req.Name, tt.expectedVSchema, vs)
28622885
})
28632886
}
28642887
}
@@ -8382,11 +8405,14 @@ func TestGetVSchema(t *testing.T) {
83828405
})
83838406

83848407
t.Run("found", func(t *testing.T) {
8385-
err := ts.SaveVSchema(ctx, "testkeyspace", &vschemapb.Keyspace{
8386-
Sharded: true,
8387-
Vindexes: map[string]*vschemapb.Vindex{
8388-
"v1": {
8389-
Type: "hash",
8408+
err := ts.SaveVSchema(ctx, &topo.KeyspaceVSchemaInfo{
8409+
Name: "testkeyspace",
8410+
Keyspace: &vschemapb.Keyspace{
8411+
Sharded: true,
8412+
Vindexes: map[string]*vschemapb.Vindex{
8413+
"v1": {
8414+
Type: "hash",
8415+
},
83908416
},
83918417
},
83928418
})

0 commit comments

Comments
 (0)