test: move a test case to the right place (#394)

Author: supercaracal

Gemfile

@@ -6,6 +6,7 @@ gemspec name: 'redis-cluster-client'
 gem 'async-redis', platform: :mri
 gem 'benchmark-ips'
 gem 'hiredis-client', '~> 0.6'
+gem 'logger'
 gem 'memory_profiler'
 gem 'minitest'
 gem 'rake'

bin/pubsub

@@ -49,7 +49,7 @@ module PubSubDebug
         handle_errors('Subscriber') do
           e = ps.next_event(0.01)
           log "#{role}: recv: #{e.nil? ? 'nil' : e}"
-          ps.call('ssubscribe', c) if e.first == 'sunsubscribe'
+          ps.call('ssubscribe', c) if !e.nil? && e.first == 'sunsubscribe'
         end
       ensure
         sleep 1.0

test/cluster_controller.rb

@@ -398,9 +398,11 @@ def wait_failover(clients, primary_node_key:, replica_node_key:, max_attempts:)
 
   def wait_replication_delay(clients, replica_size:, timeout:)
     timeout_msec = timeout.to_i * 1000
+    server_side_timeout = timeout_msec > 100 ? timeout_msec - 100 : 10
+
     wait_for_state(clients, max_attempts: clients.size + 1) do |client|
       swap_timeout(client, timeout: 0.1) do |cli|
-        cli.blocking_call(timeout, 'WAIT', replica_size, timeout_msec - 100) if primary_client?(cli)
+        cli.blocking_call(timeout, 'WAIT', replica_size, server_side_timeout) if primary_client?(cli)
       end
       true
     rescue ::RedisClient::ConnectionError

test/redis_client/test_cluster.rb

@@ -580,6 +580,7 @@ def test_pubsub_with_wrong_command
         assert_nil(pubsub.call_v(%w[SUBSCRIBE]))
         assert_raises(::RedisClient::CommandError, 'unknown command') { pubsub.next_event }
         assert_raises(::RedisClient::CommandError, 'wrong number of arguments') { pubsub.next_event }
+        assert_nil(pubsub.next_event(0.01))
         pubsub.close
       end
 
@@ -891,6 +892,8 @@ def wait_for_replication
         server_side_timeout = (TEST_TIMEOUT_SEC * 1000).to_i
         swap_timeout(@client, timeout: 0.1) do |client|
           client&.blocking_call(client_side_timeout, 'WAIT', TEST_REPLICA_SIZE, server_side_timeout)
+        rescue RedisClient::ConnectionError
+          # ignore
         end
       end
 

test/test_against_cluster_broken.rb

@@ -21,39 +21,37 @@ def setup
     print "\n"
     @logger.info('setup: test')
     prepare_test_data
-    @clients = Array.new(3) do
-      build_client.tap { |c| c.call('echo', 'init') }
-    end
+    @clients = Array.new(3) { build_client.tap { |c| c.call('echo', 'init') } }
   end
 
   def teardown
     @logger.info('teardown: test')
     revive_dead_nodes
     @clients.each(&:close)
     @controller&.close
-    refute(@captured_commands.count('cluster', 'nodes').zero?, @captured_commands.to_a.map(&:command))
-    print "#{@redirect_count.get}, "\
-      "ClusterNodesCall: #{@captured_commands.count('cluster', 'nodes')}, "\
-      "ClusterDownError: #{@cluster_down_error_count} = "
   end
 
   def test_client_patience
+    do_manual_failover
+    wait_for_cluster_to_be_ready
+    do_assertions(offset: 0)
+
     # a replica
     sacrifice_replica = @controller.select_sacrifice_of_replica
     kill_a_node(sacrifice_replica)
     wait_for_cluster_to_be_ready(ignore: [sacrifice_replica])
-    do_assertions(offset: 0)
+    do_assertions(offset: 1)
 
     # a primary
     sacrifice_primary = @controller.select_sacrifice_of_primary
     kill_a_node(sacrifice_primary)
     wait_for_cluster_to_be_ready(ignore: [sacrifice_replica, sacrifice_primary])
-    do_assertions(offset: 1)
+    do_assertions(offset: 2)
 
     # recovery
     revive_dead_nodes
     wait_for_cluster_to_be_ready
-    do_assertions(offset: 2)
+    do_assertions(offset: 3)
   end
 
   private
@@ -77,6 +75,10 @@ def prepare_test_data
   end
 
   def do_assertions(offset:)
+    @captured_commands.clear
+    @redirect_count.clear
+    @cluster_down_error_count = 0
+
     log_info('assertions') do
       log_info('assertions: single') do
         NUMBER_OF_KEYS.times do |i|
@@ -122,6 +124,8 @@ def do_assertions(offset:)
           assert_equal(want, got, 'Case: Transaction: SET')
         end
       end
+
+      log_metrics
     end
   end
 
@@ -139,6 +143,12 @@ def wait_for_cluster_to_be_ready(ignore: [])
     end
   end
 
+  def do_manual_failover
+    log_info('failover') do
+      @controller.failover
+    end
+  end
+
   def kill_a_node(sacrifice)
     log_info("kill #{sacrifice.config.host}:#{sacrifice.config.port}") do
       refute_nil(sacrifice, "#{sacrifice.config.host}:#{sacrifice.config.port}")
@@ -171,6 +181,12 @@ def log_info(message)
     @logger.info(" done: #{message}")
   end
 
+  def log_metrics
+    print "#{@redirect_count.get}, "\
+      "ClusterNodesCall: #{@captured_commands.count('cluster', 'nodes')}, "\
+      "ClusterDownError: #{@cluster_down_error_count}\n"
+  end
+
   def retryable(attempts: MAX_ATTEMPTS, wait_sec: WAIT_SEC)
     loop do
       raise MaxRetryExceeded if attempts <= 0

test/test_against_cluster_state.rb

@@ -29,14 +29,6 @@ def teardown
         "ClusterNodesCall: #{@captured_commands.count('cluster', 'nodes')} = "
     end
 
-    def test_the_state_of_cluster_failover
-      @controller.failover
-      1000.times { |i| assert_equal('OK', @client.call('SET', "key#{i}", i)) }
-      wait_for_replication
-      1000.times { |i| assert_equal(i.to_s, @client.call('GET', "key#{i}")) }
-      refute(@redirect_count.zero?, @redirect_count.get)
-    end
-
     def test_the_state_of_cluster_resharding
       resharded_keys = nil
       do_resharding_test do |keys|