From 41db54a5573a04d035156d3a42e30ae5bbce3230 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Thu, 10 Dec 2015 12:58:28 +0100
Subject: [PATCH] Cluster: more reliable migration tests.

The old version was modeled with two failovers, however after the first
it is possible that another slave will migrate to the new master, since
for some time the new master is not backed by any slave. Probably there
should be some pause after a failover, before the migration. Anyway the
test is simpler in this way, and depends less on timing.
---
 tests/cluster/tests/07-replica-migration.tcl | 71 +++++++-------------
 1 file changed, 26 insertions(+), 45 deletions(-)

diff --git a/tests/cluster/tests/07-replica-migration.tcl b/tests/cluster/tests/07-replica-migration.tcl
index 1e3efc9f..68231cd2 100644
--- a/tests/cluster/tests/07-replica-migration.tcl
+++ b/tests/cluster/tests/07-replica-migration.tcl
@@ -61,62 +61,43 @@ test "Cluster is up" {
     assert_cluster_state ok
 }
 
-set failover_cycle 0
-set new_master 2
+test "Kill slave #7 of master #2. Only slave left is #12 now" {
+    kill_instance redis 7
+}
 
-while {$failover_cycle != 2} {
-    incr failover_cycle
-    set current_epoch [CI 1 cluster_current_epoch]
+set current_epoch [CI 1 cluster_current_epoch]
 
-    test "Wait for slave of #$new_master to sync" {
-        wait_for_condition 1000 50 {
-            [string match {*state=online*} [RI $new_master slave0]]
-        } else {
-            fail "Slave of node #$new_master is not ok"
-        }
-    }
+test "Killing master node #2, #12 should failover" {
+    kill_instance redis 2
+}
 
-    test "Killing master node #$new_master" {
-        kill_instance redis $new_master
-    }
-
-    test "Wait for failover" {
-        wait_for_condition 1000 50 {
-            [CI 1 cluster_current_epoch] > $current_epoch
-        } else {
-            fail "No failover detected"
-        }
-    }
-
-    test "Cluster should eventually be up again" {
-        assert_cluster_state ok
-    }
-
-    test "Cluster is writable" {
-        cluster_write_test 1
-    }
-
-    test "Instance #7 or #12 is now a master" {
-        assert {
-            (![instance_is_killed redis 7] && [RI 7 role] eq {master}) ||
-            (![instance_is_killed redis 12] && [RI 12 role] eq {master})
-        }
-    }
-
-    if {![instance_is_killed redis 7] && [RI 7 role] eq {master}} {
-        set new_master 7
+test "Wait for failover" {
+    wait_for_condition 1000 50 {
+        [CI 1 cluster_current_epoch] > $current_epoch
     } else {
-        set new_master 12
+        fail "No failover detected"
     }
 }
 
+test "Cluster should eventually be up again" {
+    assert_cluster_state ok
+}
+
+test "Cluster is writable" {
+    cluster_write_test 1
+}
+
+test "Instance 12 is now a master without slaves" {
+    assert {[RI 12 role] eq {master}}
+}
+
 # The remaining instance is now without slaves. Some other slave
 # should migrate to it.
 
-test "Master #$new_master should have at least one replica" {
+test "Master #12 should get at least one migrated replica" {
     wait_for_condition 1000 50 {
-        [llength [lindex [R $new_master role] 2]] >= 1
+        [llength [lindex [R 12 role] 2]] >= 1
     } else {
-        fail "Master #$new_master has no replicas"
+        fail "Master #12 has no replicas"
     }
 }