Flakes: remove non-determinism from vtctldclient MoveTables unit test (…

…vitessio#13765)
slackhq · Aug 10, 2023 · 4e03d2f · 4e03d2f
1 parent a30b19e
commit 4e03d2f
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 29 deletions.
diff --git a/go/vt/binlog/binlogplayer/mock_dbclient.go b/go/vt/binlog/binlogplayer/mock_dbclient.go
@@ -39,7 +39,6 @@ type MockDBClient struct {
 	currentResult int
 	done          chan struct{}
 	invariants    map[string]*sqltypes.Result
-	ignored       map[string]struct{}
 }
 
 type mockExpect struct {
@@ -59,8 +58,29 @@ func NewMockDBClient(t *testing.T) *MockDBClient {
 			"CREATE TABLE IF NOT EXISTS _vt.vreplication_log":           {},
 			"select id, type, state, message from _vt.vreplication_log": {},
 			"insert into _vt.vreplication_log":                          {},
+			// The following statements don't have a deterministic order as they are
+			// executed in the normal program flow, but ALSO done in a defer as a protective
+			// measure as they are resetting the values back to the original one. This also
+			// means that the values they set are based on the session defaults, which can
+			// change. So we make these invariants for unit test stability.
+			"select @@foreign_key_checks": sqltypes.MakeTestResult(
+				sqltypes.MakeTestFields(
+					"@@foreign_key_checks",
+					"int64",
+				),
+				"1",
+			),
+			"set @@session.foreign_key_checks": {},
+			"set foreign_key_checks":           {},
+			"select @@session.sql_mode": sqltypes.MakeTestResult(
+				sqltypes.MakeTestFields(
+					"sql_mode", "varchar",
+				),
+				"ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION",
+			),
+			"set @@session.sql_mode": {},
+			"set sql_mode":           {},
 		},
-		ignored: map[string]struct{}{},
 	}
 }
 
@@ -159,12 +179,8 @@ func (dc *MockDBClient) ExecuteFetch(query string, maxrows int) (qr *sqltypes.Re
 	dc.t.Helper()
 	dc.t.Logf("DBClient query: %v", query)
 
-	if _, ok := dc.ignored[query]; ok {
-		return qr, nil
-	}
-
 	for q, result := range dc.invariants {
-		if strings.Contains(query, q) {
+		if strings.Contains(strings.ToLower(query), strings.ToLower(q)) {
 			return result, nil
 		}
 	}

diff --git a/go/vt/vttablet/tabletmanager/rpc_vreplication_test.go b/go/vt/vttablet/tabletmanager/rpc_vreplication_test.go
@@ -57,13 +57,6 @@ const (
 	getAutoIncrementStep     = "select @@session.auto_increment_increment"
 	setSessionTZ             = "set @@session.time_zone = '+00:00'"
 	setNames                 = "set names 'binary'"
-	setSQLMode               = "set @@session.sql_mode = CONCAT(@@session.sql_mode, ',NO_AUTO_VALUE_ON_ZERO')"
-	setPermissiveSQLMode     = "SET @@session.sql_mode='NO_AUTO_VALUE_ON_ZERO'"
-	setStrictSQLMode         = "SET @@session.sql_mode='ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'"
-	getSQLMode               = "SELECT @@session.sql_mode AS sql_mode"
-	getFKChecks              = "select @@foreign_key_checks"
-	enableFKChecks           = "set foreign_key_checks=1"
-	sqlMode                  = "ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION"
 	getBinlogRowImage        = "select @@binlog_row_image"
 	insertStreamsCreatedLog  = "insert into _vt.vreplication_log(vrepl_id, type, state, message) values(1, 'Stream Created', '', '%s'"
 	getVReplicationRecord    = "select * from _vt.vreplication where id = 1"
@@ -323,11 +316,6 @@ func TestMoveTables(t *testing.T) {
 		ftc.vrdbClient.ExpectRequest(`update _vt.vreplication set message='Picked source tablet: cell:\"zone1\" uid:200' where id=1`, &sqltypes.Result{}, nil)
 		ftc.vrdbClient.ExpectRequest(setSessionTZ, &sqltypes.Result{}, nil)
 		ftc.vrdbClient.ExpectRequest(setNames, &sqltypes.Result{}, nil)
-		ftc.vrdbClient.ExpectRequest(setSQLMode, &sqltypes.Result{}, nil)
-		ftc.vrdbClient.ExpectRequest(getSQLMode, sqltypes.MakeTestResult(
-			sqltypes.MakeTestFields("sql_mode", "varchar"),
-			sqlMode,
-		), nil)
 		ftc.vrdbClient.ExpectRequest(getWorkflowState, sqltypes.MakeTestResult(
 			sqltypes.MakeTestFields(
 				"pos|stop_pos|max_tps|max_replication_lag|state|workflow_type|workflow|workflow_sub_type|defer_secondary_keys",
@@ -342,14 +330,6 @@ func TestMoveTables(t *testing.T) {
 			),
 			"1",
 		), nil)
-		ftc.vrdbClient.ExpectRequest(setPermissiveSQLMode, &sqltypes.Result{}, nil)
-		ftc.vrdbClient.ExpectRequest(getFKChecks, sqltypes.MakeTestResult(
-			sqltypes.MakeTestFields(
-				"@@foreign_key_checks",
-				"int64",
-			),
-			"1",
-		), nil)
 		ftc.vrdbClient.ExpectRequest(getWorkflowState, sqltypes.MakeTestResult(
 			sqltypes.MakeTestFields(
 				"pos|stop_pos|max_tps|max_replication_lag|state|workflow_type|workflow|workflow_sub_type|defer_secondary_keys",
@@ -371,8 +351,6 @@ func TestMoveTables(t *testing.T) {
 			),
 			"FULL",
 		), nil)
-		ftc.vrdbClient.ExpectRequest(enableFKChecks, &sqltypes.Result{}, nil)
-		ftc.vrdbClient.ExpectRequest(setStrictSQLMode, &sqltypes.Result{}, nil)
 
 		ftc.vrdbClient.ExpectRequest(fmt.Sprintf(insertStreamsCreatedLog, bls), &sqltypes.Result{}, nil)
 		tenv.tmc.setVReplicationExecResults(ftc.tablet, fmt.Sprintf(getWorkflow, targetKs, wf),