chore: fix some stuff, readme and test cases (#323)

Author: supercaracal

README.md

@@ -175,7 +175,7 @@ and conditional execution with `WATCH`. Redis does not support cross-node transa
 transaction must live in the same key slot. To use transactions, you can use `#multi` method same as the [redis-client](https://github.com/redis-rb/redis-client#usage):
 
 ```ruby
-conn.multi do |tx|
+cli.multi do |tx|
   tx.call('INCR', 'my_key')
   tx.call('INCR', 'my_key')
 end
@@ -192,7 +192,7 @@ it _is_ possible to perform a transaction on the keys `{tag}foo` and `{tag}bar`.
 To perform such transactions on this gem, use `hashtag:
 
 ```ruby
-conn.multi do |tx|
+cli.multi do |tx|
   tx.call('INCR', '{user123}coins_spent')
   tx.call('DECR', '{user123}coins_available')
 end
@@ -201,11 +201,11 @@ end
 ```ruby
 # Conditional execution with WATCH can be used to e.g. atomically swap two keys
 cli.call('MSET', '{myslot}1', 'v1', '{myslot}2', 'v2')
-conn.multi(watch: %w[{myslot}1 {myslot}2]) do |txn|
+cli.multi(watch: %w[{myslot}1 {myslot}2]) do |tx|
   old_key1 = cli.call('GET', '{myslot}1')
   old_key2 = cli.call('GET', '{myslot}2')
-  txn.call('SET', '{myslot}1', old_key2)
-  txn.call('SET', '{myslot}2', old_key1)
+  tx.call('SET', '{myslot}1', old_key2)
+  tx.call('SET', '{myslot}2', old_key1)
 end
 # This transaction will swap the values of {myslot}1 and {myslot}2 only if no concurrent connection modified
 # either of the values

lib/redis_client/cluster.rb

@@ -104,7 +104,6 @@ def pubsub
     # @see https://github.com/redis-rb/redis-cluster-client/issues/299
     def with(key: nil, hashtag: nil, write: true, _retry_count: 0, &_)
       key = process_with_arguments(key, hashtag)
-
       node_key = @router.find_node_key_by_key(key, primary: write)
       node = @router.find_node(node_key)
       yield ::RedisClient::Cluster::PinningNode.new(node)

test/redis_client/test_cluster.rb

@@ -308,6 +308,19 @@ def test_transaction_with_error
         assert_equal('x', @client.call('GET', 'key1'))
       end
 
+      def test_transaction_without_error_during_queueing
+        @client.call('SET', 'key1', 'x')
+
+        assert_raises(::RedisClient::CommandError) do
+          @client.multi do |tx|
+            tx.call('SET', 'key1', 'aaa')
+            tx.call('INCR', 'key1')
+          end
+        end
+
+        assert_equal('aaa', @client.call('GET', 'key1'))
+      end
+
       def test_transaction_with_block
         @client.call('MSET', '{key}1', 'a', '{key}2', 'b', '{key}3', 'c')
 
@@ -318,6 +331,14 @@ def test_transaction_with_block
         end
 
         assert_equal(%w[aaa bbb ccc], got)
+
+        got = @client.multi(watch: %w[{key}1 {key}2 {key}3]) do |tx|
+          tx.call('GET', '{key}1') { |x| "#{x}11" }
+          tx.call('GET', '{key}2') { |x| "#{x}22" }
+          tx.call('GET', '{key}3') { |x| "#{x}33" }
+        end
+
+        assert_equal(%w[a11 b22 c33], got)
       end
 
       def test_pubsub_without_subscription

test/test_against_cluster_state.rb

@@ -60,8 +60,11 @@ def test_the_state_of_cluster_resharding_with_pipelining
     end
 
     def test_the_state_of_cluster_resharding_with_transaction
+      call_cnt = 0
+
       do_resharding_test do |keys|
         @client.multi do |tx|
+          call_cnt += 1
           keys.each do |key|
             tx.call('SET', key, '0')
             tx.call('INCR', key)
@@ -74,6 +77,8 @@ def test_the_state_of_cluster_resharding_with_transaction
           assert_equal(want, got, "Case: GET: #{key}")
         end
       end
+
+      assert_equal(1, call_cnt)
     end
 
     def test_the_state_of_cluster_resharding_with_pipelining_on_new_connection
@@ -145,27 +150,16 @@ def new_test_client
   class ScaleReadRandom < TestingWrapper
     include Mixin
 
-    # FIXME: https://github.com/redis/redis/issues/11312
     def test_the_state_of_cluster_resharding
-      keys = nil
-      do_resharding_test { |ks| keys = ks }
-      keys.each { |key| assert_equal(key, @client.call('GET', key), "Case: GET: #{key}") }
+      skip('https://github.com/redis/redis/issues/11312')
     end
 
-    # FIXME: https://github.com/redis/redis/issues/11312
     def test_the_state_of_cluster_resharding_with_pipelining
-      keys = nil
-      do_resharding_test { |ks| keys = ks }
-      values = @client.pipelined { |pipeline| keys.each { |key| pipeline.call('GET', key) } }
-      keys.each_with_index { |key, i| assert_equal(key, values[i], "Case: GET: #{key}") }
+      skip('https://github.com/redis/redis/issues/11312')
     end
 
-    # FIXME: https://github.com/redis/redis/issues/11312
     def test_the_state_of_cluster_resharding_with_transaction
-      keys = nil
-      do_resharding_test { |ks| keys = ks }
-      @client.multi { |tx| keys.each { |key| tx.call('SET', key, key) } }
-      keys.each { |key| assert_equal(key, @client.call('GET', key), "Case: GET: #{key}") }
+      skip('https://github.com/redis/redis/issues/11312')
     end
 
     private
@@ -184,27 +178,16 @@ def new_test_client
   class ScaleReadRandomWithPrimary < TestingWrapper
     include Mixin
 
-    # FIXME: https://github.com/redis/redis/issues/11312
     def test_the_state_of_cluster_resharding
-      keys = nil
-      do_resharding_test { |ks| keys = ks }
-      keys.each { |key| assert_equal(key, @client.call('GET', key), "Case: GET: #{key}") }
+      skip('https://github.com/redis/redis/issues/11312')
     end
 
-    # FIXME: https://github.com/redis/redis/issues/11312
     def test_the_state_of_cluster_resharding_with_pipelining
-      keys = nil
-      do_resharding_test { |ks| keys = ks }
-      values = @client.pipelined { |pipeline| keys.each { |key| pipeline.call('GET', key) } }
-      keys.each_with_index { |key, i| assert_equal(key, values[i], "Case: GET: #{key}") }
+      skip('https://github.com/redis/redis/issues/11312')
     end
 
-    # FIXME: https://github.com/redis/redis/issues/11312
     def test_the_state_of_cluster_resharding_with_transaction
-      keys = nil
-      do_resharding_test { |ks| keys = ks }
-      @client.multi { |tx| keys.each { |key| tx.call('SET', key, key) } }
-      keys.each { |key| assert_equal(key, @client.call('GET', key), "Case: GET: #{key}") }
+      skip('https://github.com/redis/redis/issues/11312')
     end
 
     private
@@ -223,27 +206,16 @@ def new_test_client
   class ScaleReadLatency < TestingWrapper
     include Mixin
 
-    # FIXME: https://github.com/redis/redis/issues/11312
     def test_the_state_of_cluster_resharding
-      keys = nil
-      do_resharding_test { |ks| keys = ks }
-      keys.each { |key| assert_equal(key, @client.call('GET', key), "Case: GET: #{key}") }
+      skip('https://github.com/redis/redis/issues/11312')
     end
 
-    # FIXME: https://github.com/redis/redis/issues/11312
     def test_the_state_of_cluster_resharding_with_pipelining
-      keys = nil
-      do_resharding_test { |ks| keys = ks }
-      values = @client.pipelined { |pipeline| keys.each { |key| pipeline.call('GET', key) } }
-      keys.each_with_index { |key, i| assert_equal(key, values[i], "Case: GET: #{key}") }
+      skip('https://github.com/redis/redis/issues/11312')
     end
 
-    # FIXME: https://github.com/redis/redis/issues/11312
     def test_the_state_of_cluster_resharding_with_transaction
-      keys = nil
-      do_resharding_test { |ks| keys = ks }
-      @client.multi { |tx| keys.each { |key| tx.call('SET', key, key) } }
-      keys.each { |key| assert_equal(key, @client.call('GET', key), "Case: GET: #{key}") }
+      skip('https://github.com/redis/redis/issues/11312')
     end
 
     private