Skip to content

Unit test for scatter_conn using new HealthCheck#6458

Merged
harshit-gangal merged 1 commit intovitessio:masterfrom
planetscale:ds-sconn-newhc-test
Jul 22, 2020
Merged

Unit test for scatter_conn using new HealthCheck#6458
harshit-gangal merged 1 commit intovitessio:masterfrom
planetscale:ds-sconn-newhc-test

Conversation

@deepthi
Copy link
Copy Markdown
Collaborator

@deepthi deepthi commented Jul 20, 2020

Refactored interfaces and function signatures to allow unit testing.
Created enough of FakeHealthCheck for testing ScatterConn.

Signed-off-by: deepthi deepthi@planetscale.com

…nit testing of ScatterConn

Signed-off-by: deepthi <deepthi@planetscale.com>
Comment on lines -108 to -124
func TestScatterConnExecuteMulti(t *testing.T) {
testScatterConnGeneric(t, "TestScatterConnExecuteMultiShard", func(sc *ScatterConn, shards []string) (*sqltypes.Result, error) {
res := srvtopo.NewResolver(&sandboxTopo{}, sc.gateway, "aa")
rss, err := res.ResolveDestination(ctx, "TestScatterConnExecuteMultiShard", topodatapb.TabletType_REPLICA, key.DestinationShards(shards))
if err != nil {
return nil, err
}

queries := make([]*querypb.BoundQuery, len(rss))
for i := range rss {
queries[i] = &querypb.BoundQuery{
Sql: "query",
BindVariables: nil,
}
}

qr, errs := sc.ExecuteMultiShard(ctx, rss, queries, NewSafeSession(nil), false /*autocommit*/)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you removed others test. Needs to bring them back

Comment on lines +736 to +744
func newTestScatterConn(hc discovery.HealthCheck, serv srvtopo.Server, cell string) *ScatterConn {
// The topo.Server is used to start watching the cells described
// in '-cells_to_watch' command line parameter, which is
// empty by default. So it's unused in this test, set to nil.
gw := NewTabletGateway(ctx, hc, serv, cell)
tc := NewTxConn(gw, vtgatepb.TransactionMode_TWOPC)
return NewScatterConn("", tc, gw)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you moved it to separate file. Then this does not belong here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. we can fix it by moving it to the other test file.

Copy link
Copy Markdown
Member

@harshit-gangal harshit-gangal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will merge this PR

@harshit-gangal harshit-gangal merged commit d903d2d into vitessio:master Jul 22, 2020
@deepthi deepthi added this to the v7.0 milestone Jul 27, 2020
@deepthi deepthi deleted the ds-sconn-newhc-test branch July 31, 2020 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants