review suggestions: more unit tests

Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>

go/test/endtoend/reparent/newfeaturetest/reparent_in_tx_test.go

@@ -27,7 +27,6 @@ import (
 	"vitess.io/vitess/go/test/endtoend/cluster"
 	"vitess.io/vitess/go/test/endtoend/reparent/utils"
 	"vitess.io/vitess/go/vt/vtctl/reparentutil/policy"
-	"vitess.io/vitess/go/vt/vterrors"
 )
 
 var primary int
@@ -43,7 +42,7 @@ func testCommitError(t *testing.T, conn *mysql.Conn, clusterInstance *cluster.Lo
 	go func() {
 		<-tabletStopped
 		_, err := conn.ExecuteFetch("commit", 0, false)
-		require.ErrorContains(t, err, vterrors.VT15001(0).ID)
+		require.ErrorContains(t, err, "VT15001")
 		commitDone <- true
 	}()
 
@@ -63,11 +62,11 @@ func testExecuteError(t *testing.T, conn *mysql.Conn, clusterInstance *cluster.L
 		idx += 5
 		<-tabletStopped
 		_, err := conn.ExecuteFetch(utils.GetInsertMultipleValuesQuery(idx, idx+1, idx+2, idx+3), 0, false)
-		require.ErrorContains(t, err, vterrors.VT15001(0).ID)
+		require.ErrorContains(t, err, "VT15001")
 
 		// Subsequent queries after a VT15001 should start returning a VT15002 error until we issue a ROLLBACK
 		_, err = conn.ExecuteFetch("select * from vt_insert_test", 1, false)
-		require.ErrorContains(t, err, vterrors.VT15002().ID)
+		require.ErrorContains(t, err, "VT15002")
 
 		_, err = conn.ExecuteFetch("rollback", 0, false)
 		require.NoError(t, err)
@@ -123,9 +122,9 @@ func reparent(t *testing.T, clusterInstance *cluster.LocalProcessCluster, tablet
 }
 
 func TestErrorsInTransaction(t *testing.T) {
-	clusterInstance := utils.SetupShardedReparentCluster(t, policy.DurabilitySemiSync, map[string]string{
-		"--queryserver-config-transaction-timeout": "5m",
-		"--queryserver-config-query-timeout":       "5m",
+	clusterInstance := utils.SetupShardedReparentCluster(t, policy.DurabilitySemiSync, []string{
+		"--queryserver-config-transaction-timeout", "5m",
+		"--queryserver-config-query-timeout", "5m",
 	})
 
 	defer utils.TeardownCluster(clusterInstance)

go/test/endtoend/reparent/utils/utils.go

@@ -77,7 +77,7 @@ func SetupRangeBasedCluster(ctx context.Context, t *testing.T) *cluster.LocalPro
 }
 
 // SetupShardedReparentCluster is used to setup a sharded cluster for testing
-func SetupShardedReparentCluster(t *testing.T, durability string, extraVttabletFlags map[string]string) *cluster.LocalProcessCluster {
+func SetupShardedReparentCluster(t *testing.T, durability string, extraVttabletFlags []string) *cluster.LocalProcessCluster {
 	clusterInstance := cluster.NewCluster(cell1, Hostname)
 	// Start topo server
 	err := clusterInstance.StartTopo()
@@ -89,8 +89,9 @@ func SetupShardedReparentCluster(t *testing.T, durability string, extraVttabletF
 		"--health_check_interval", "1s",
 		"--track_schema_versions=true",
 		"--queryserver_enable_online_ddl=false")
-	for flag, val := range extraVttabletFlags {
-		clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs, flag, val)
+
+	if len(extraVttabletFlags) > 0 {
+		clusterInstance.VtTabletExtraArgs = append(clusterInstance.VtTabletExtraArgs, extraVttabletFlags...)
 	}
 
 	clusterInstance.VtGateExtraArgs = append(clusterInstance.VtGateExtraArgs,

go/test/endtoend/tabletgateway/vtgate_test.go

@@ -271,6 +271,7 @@ func TestReplicaTransactions(t *testing.T) {
 	require.NoError(t, err)
 	serving := replicaTablet.VttabletProcess.WaitForStatus("SERVING", 60*time.Second)
 	assert.Equal(t, serving, true, "Tablet did not become ready within a reasonable time")
+	utils.Exec(t, readConn, fetchAllCustomers)
 
 	// create a new connection, should be able to query again
 	readConn, err = mysql.Connect(ctx, &vtParams)

go/vt/discovery/healthcheck.go

@@ -871,7 +871,6 @@ func (hc *HealthCheckImpl) TabletConnection(ctx context.Context, alias *topodata
 	thc := hc.healthByAlias[tabletAliasString(topoproto.TabletAliasString(alias))]
 	hc.mu.Unlock()
 	if thc == nil || thc.Conn == nil {
-		// TODO: test that throws this error
 		return nil, vterrors.Errorf(vtrpc.Code_NOT_FOUND, "tablet: %v is either down or nonexistent", alias)
 	}
 	return thc.Connection(ctx), nil

go/vt/vttablet/tabletconn/grpc_error_test.go

@@ -19,6 +19,10 @@ package tabletconn
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+
 	vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
 	"vitess.io/vitess/go/vt/vterrors"
 )
@@ -48,8 +52,86 @@ func TestTabletErrorFromRPCError(t *testing.T) {
 	}}
 	for _, tcase := range testcases {
 		got := vterrors.Code(ErrorFromVTRPC(tcase.in))
-		if got != tcase.want {
-			t.Errorf("FromVtRPCError(%v):\n%v, want\n%v", tcase.in, got, tcase.want)
+		require.Equal(t, tcase.want, got)
+	}
+}
+
+func TestGRPCErrorToVT15001(t *testing.T) {
+	msg := "connection error: desc = \"transport: Error while dialing: dial tcp 127.0.0.1:7108: connect: connection refused\""
+	testcases := []struct {
+		in   error
+		inTx bool
+		wrap bool
+	}{{
+		in:   status.Error(codes.Unavailable, msg),
+		inTx: true,
+		wrap: true,
+	}, {
+		in:   status.Error(codes.Unavailable, msg),
+		inTx: false,
+		wrap: false,
+	}, {
+		in:   status.Error(codes.Unavailable, "unavailable"),
+		inTx: true,
+		wrap: false,
+	}, {
+		in:   status.Error(codes.FailedPrecondition, msg),
+		inTx: true,
+		wrap: false,
+	}}
+	for _, tcase := range testcases {
+		got := ErrorsFromGRPCWrapTransientTxError(tcase.in, tcase.inTx)
+		require.Error(t, got)
+		if tcase.wrap {
+			require.ErrorContains(t, got, "VT15001")
+		} else {
+			require.NotContains(t, got.Error(), "VT15001")
+		}
+	}
+}
+
+func TestVTRPCErrorToVT15001(t *testing.T) {
+	msg := "connection error: desc = \"transport: Error while dialing: dial tcp 127.0.0.1:7108: connect: connection refused\""
+	testcases := []struct {
+		in   *vtrpcpb.RPCError
+		inTx bool
+		wrap bool
+	}{{
+		in: &vtrpcpb.RPCError{
+			Code:    vtrpcpb.Code_UNAVAILABLE,
+			Message: msg,
+		},
+		inTx: true,
+		wrap: true,
+	}, {
+		in: &vtrpcpb.RPCError{
+			Code:    vtrpcpb.Code_UNAVAILABLE,
+			Message: msg,
+		},
+		inTx: false,
+		wrap: false,
+	}, {
+		in: &vtrpcpb.RPCError{
+			Code:    vtrpcpb.Code_UNAVAILABLE,
+			Message: "unavailable",
+		},
+		inTx: true,
+		wrap: false,
+	}, {
+		in: &vtrpcpb.RPCError{
+			Code:    vtrpcpb.Code_FAILED_PRECONDITION,
+			Message: msg,
+		},
+		inTx: true,
+		wrap: false,
+	}}
+	for _, tcase := range testcases {
+		got := ErrorsFromVTRPCWrapTransientTxError(tcase.in, tcase.inTx)
+		require.Error(t, got)
+		if tcase.wrap {
+			require.ErrorContains(t, got, "VT15001")
+		} else {
+			require.NotContains(t, got.Error(), "VT15001")
 		}
 	}
 }