[GitHub] [kvrocks] mapleFU commented on a diff in pull request #1680: Fix flaky test failure in TestSlotMigrateSync

["mapleFU (via GitHub)" <gi...@apache.org>]

mapleFU commented on code in PR #1680:
URL: https://github.com/apache/kvrocks/pull/1680#discussion_r1296187344


##########
tests/gocase/integration/slotmigrate/slotmigrate_test.go:
##########
@@ -473,9 +473,12 @@ func TestSlotMigrateSync(t *testing.T) {
 		validTimeouts := []int{0, 10, 20, 30}
 		for _, timeout := range validTimeouts {
 			slot++
-			result := rdb0.Do(ctx, "clusterx", "migrate", slot, id1, "sync", timeout)
-			require.NoError(t, result.Err(), "with timeout: %v", timeout)
-			require.Equal(t, "OK", result.Val(), "with timeout: %d", timeout)
+			err := rdb0.Do(ctx, "clusterx", "migrate", slot, id1, "sync", timeout).Err()
+			// go-redis will auto-retry if occurs timeout error, so it may return the already migrated slot error,
+			// so we just allow this error here to prevent the flaky test failure.
+			if err != nil && !strings.Contains(err.Error(), "ERR There is already a migrating slot") {
+				require.NoError(t, err)
+			}

Review Comment:
   ```suggestion
   			} else {
   			require.Equal(t, "OK", result.Val(), "with timeout: %d", timeout)
   			}
   ```
   
   Would this be better?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org