Add some test cases (#12)

Author: supercaracal

.github/workflows/test.yaml

@@ -48,11 +48,13 @@ jobs:
       # @see https://docs.docker.com/engine/reference/commandline/create/#options
       - name: Pull Docker images
         run: docker pull redis:$REDIS_VERSION
-      - name: Docker compose up
+      - name: Run containers
         run: docker compose up -d
       - name: Wait for Redis cluster to be ready
         run: bundle exec rake wait
       - name: Print containers
         run: docker compose ps
       - name: Run minitest
         run: bundle exec rake test
+      - name: Stop containers
+        run: docker compose down

lib/redis_client/cluster.rb

@@ -195,7 +195,7 @@ def send_script_command(method, *command, **kwargs, &block)
       end
     end
 
-    def send_pubsub_command(method, *command, **kwargs, &block) # rubocop:disable Metircs/AbcSize, Metrics/CyclomaticComplexity
+    def send_pubsub_command(method, *command, **kwargs, &block) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
       case command[1].to_s.downcase
       when 'channels' then @node.call_all(method, *command, **kwargs, &block).flatten.uniq.sort
       when 'numsub'

lib/redis_client/cluster/command.rb

@@ -22,8 +22,8 @@ def load(nodes)
         private
 
         def parse_command_details(rows)
-          rows.to_h do |row|
-            [row[0], { arity: row[1], flags: row[2], first: row[3], last: row[4], step: row[5] }]
+          rows&.reject { |row| row[0].nil? }.to_h do |row|
+            [row[0].downcase, { arity: row[1], flags: row[2], first: row[3], last: row[4], step: row[5] }]
           end
         end
       end
@@ -62,14 +62,14 @@ def pick_details(details)
       end
 
       def dig_details(command, key)
-        name = command.first.to_s
-        return unless @details.key?(name)
+        name = command&.flatten&.first.to_s.downcase
+        return if name.empty? || !@details.key?(name)
 
         @details.fetch(name).fetch(key)
       end
 
-      def determine_first_key_position(command)
-        case command.first.to_s.downcase
+      def determine_first_key_position(command) # rubocop:disable Metrics/CyclomaticComplexity
+        case command&.flatten&.first.to_s.downcase
         when 'eval', 'evalsha', 'migrate', 'zinterstore', 'zunionstore' then 3
         when 'object' then 2
         when 'memory'
@@ -81,13 +81,14 @@ def determine_first_key_position(command)
         end
       end
 
-      def determine_optional_key_position(command, option_name)
-        idx = command.map(&:to_s).map(&:downcase).index(option_name)
+      def determine_optional_key_position(command, option_name) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+        idx = command&.flatten&.map(&:to_s)&.map(&:downcase)&.index(option_name&.downcase)
         idx.nil? ? 0 : idx + 1
       end
 
       # @see https://redis.io/topics/cluster-spec#keys-hash-tags Keys hash tags
       def extract_hash_tag(key)
+        key = key.to_s
         s = key.index('{')
         e = key.index('}', s.to_i + 1)
 

test/.keep

@@ -7,11 +7,11 @@ class Cluster
     class Controller
       SLOT_SIZE = 16_384
 
-      def initialize(node_addrs, timeout: 30.0, reconnect_attempts: 10)
+      def initialize(node_addrs, timeout: 30.0, reconnect_attempts: 10, **kwargs)
         raise 'Redis Cluster requires at least 3 master nodes.' if node_addrs.size < 3
 
         @clients = node_addrs.map do |addr|
-          RedisClient.new(url: addr, timeout: timeout, reconnect_attempts: reconnect_attempts)
+          RedisClient.new(url: addr, timeout: timeout, reconnect_attempts: reconnect_attempts, **kwargs)
         end
 
         @timeout = timeout

test/redis_client/cluster/controller.rb

@@ -2,12 +2,16 @@
 
 require 'set'
 require 'testing_helper'
+require 'redis_client/cluster'
 require 'redis_client/cluster/command'
 
 class RedisClient
   class Cluster
     class TestCommand < Minitest::Test
+      include ::RedisClient::TestingHelper
+
       def test_parse_command_details
+        keys = %i[arity flags first last step].freeze
         [
           {
             rows: [
@@ -19,24 +23,30 @@ def test_parse_command_details
               'set' => { arity: -3, flags: Set['write', 'denyoom', 'movablekeys'], first: 1, last: 1, step: 1 }
             }
           },
+          {
+            rows: [
+              ['GET', 2, Set['readonly', 'fast'], 1, 1, 1, Set['@read', '@string', '@fast'], Set[], Set[], Set[]]
+            ],
+            want: {
+              'get' => { arity: 2, flags: Set['readonly', 'fast'], first: 1, last: 1, step: 1 }
+            }
+          },
+          { rows: [[]], want: {} },
           { rows: [], want: {} },
           { rows: nil, want: {} }
-        ].each do |c|
+        ].each_with_index do |c, idx|
+          msg = "Case: #{idx}"
           got = ::RedisClient::Cluster::Command.send(:parse_command_details, c[:rows])
-          assert_equal(c[:want].size, got.size)
-          assert_equal(c[:want].keys.sort, got.keys.sort)
-          c[:want].each do |k, v|
-            a = got[k]
-            assert_equal(v[:arity], a[:arity])
-            assert_equal(v[:flags], a[:flags])
-            assert_equal(v[:first], a[:first])
-            assert_equal(v[:last], a[:last])
-            assert_equal(v[:step], a[:step])
+          assert_equal(c[:want].size, got.size, msg)
+          assert_equal(c[:want].keys.sort, got.keys.sort, msg)
+          c[:want].each do |k1, v|
+            keys.each { |k2| assert_equal(v[k2], got[k1][k2], "#{msg}: #{k2}") }
           end
         end
       end
 
       def test_pick_details
+        keys = %i[first_key_position write readonly].freeze
         [
           {
             details: {
@@ -50,19 +60,109 @@ def test_pick_details
           },
           { details: {}, want: {} },
           { details: nil, want: {} }
-        ].each do |c|
+        ].each_with_index do |c, idx|
+          msg = "Case: #{idx}"
           cmd = ::RedisClient::Cluster::Command.new(c[:details])
           got = cmd.send(:pick_details, c[:details])
-          assert_equal(c[:want].size, got.size)
-          assert_equal(c[:want].keys.sort, got.keys.sort)
-          c[:want].each do |k, v|
-            a = got[k]
-            assert_equal(v[:first_key_position], a[:first_key_position])
-            assert_equal(v[:write], a[:write])
-            assert_equal(v[:readonly], a[:readonly])
+          assert_equal(c[:want].size, got.size, msg)
+          assert_equal(c[:want].keys.sort, got.keys.sort, msg)
+          c[:want].each do |k1, v|
+            keys.each { |k2| assert_equal(v[k2], got[k1][k2], "#{msg}: #{k2}") }
           end
         end
       end
+
+      def test_dig_details
+        cmd = ::RedisClient::Cluster::Command.new(
+          {
+            'get' => { arity: 2, flags: Set['readonly', 'fast'], first: 1, last: 1, step: 1 },
+            'set' => { arity: -3, flags: Set['write', 'denyoom', 'movablekeys'], first: 1, last: 1, step: 1 }
+          }
+        )
+        [
+          { params: { command: %w[SET foo 1], key: :first_key_position }, want: 1 },
+          { params: { command: %w[SET foo 1], key: :write }, want: true },
+          { params: { command: %w[set foo 1], key: :write }, want: true },
+          { params: { command: %w[SET foo 1], key: :readonly }, want: false },
+          { params: { command: %w[GET foo], key: :first_key_position }, want: 1 },
+          { params: { command: %w[GET foo], key: :write }, want: false },
+          { params: { command: %w[GET foo], key: :readonly }, want: true },
+          { params: { command: %w[get foo], key: :readonly }, want: true },
+          { params: { command: %w[UNKNOWN foo], key: :readonly }, want: nil },
+          { params: { command: [['SET'], 'foo', 1], key: :write }, want: true },
+          { params: { command: [], key: :readonly }, want: nil },
+          { params: { command: nil, key: :readonly }, want: nil }
+        ].each_with_index do |c, idx|
+          msg = "Case: #{idx}"
+          got = cmd.send(:dig_details, c[:params][:command], c[:params][:key])
+          c[:want].nil? ? assert_nil(got, msg) : assert_equal(c[:want], got, msg)
+        end
+      end
+
+      def test_determine_first_key_position
+        cmd = ::RedisClient::Cluster::Command.load(@clients)
+        [
+          { command: %w[EVAL "return ARGV[1]" 0 hello], want: 3 },
+          { command: [['EVAL'], '"return ARGV[1]"', 0, 'hello'], want: 3 },
+          { command: %w[EVALSHA sha1 2 foo bar baz zap], want: 3 },
+          { command: %w[MIGRATE host port key 0 5 COPY], want: 3 },
+          { command: %w[ZINTERSTORE out 2 zset1 zset2 WEIGHTS 2 3], want: 3 },
+          { command: %w[ZUNIONSTORE out 2 zset1 zset2 WEIGHTS 2 3], want: 3 },
+          { command: %w[OBJECT HELP], want: 2 },
+          { command: %w[MEMORY HELP], want: 0 },
+          { command: %w[MEMORY USAGE key], want: 2 },
+          { command: %w[XREAD COUNT 2 STREAMS mystream writers 0-0 0-0], want: 4 },
+          { command: %w[XREADGROUP GROUP group consumer STREAMS key id], want: 5 },
+          { command: %w[SET foo 1], want: 1 },
+          { command: %w[set foo 1], want: 1 },
+          { command: [['SET'], 'foo', 1], want: 1 },
+          { command: %w[GET foo], want: 1 }
+        ].each_with_index do |c, idx|
+          msg = "Case: #{idx}"
+          got = cmd.send(:determine_first_key_position, c[:command])
+          assert_equal(c[:want], got, msg)
+        end
+      end
+
+      def test_determine_optional_key_position
+        cmd = ::RedisClient::Cluster::Command.load(@clients)
+        [
+          { params: { command: %w[XREAD COUNT 2 STREAMS mystream writers 0-0 0-0], option_name: 'streams' }, want: 4 },
+          { params: { command: %w[XREADGROUP GROUP group consumer STREAMS key id], option_name: 'streams' }, want: 5 },
+          { params: { command: %w[GET foo], option_name: 'bar' }, want: 0 },
+          { params: { command: ['FOO', ['BAR'], 'BAZ'], option_name: 'bar' }, want: 2 },
+          { params: { command: %w[FOO BAR BAZ], option_name: 'BAR' }, want: 2 },
+          { params: { command: [], option_name: nil }, want: 0 },
+          { params: { command: [], option_name: '' }, want: 0 },
+          { params: { command: nil, option_name: nil }, want: 0 }
+        ].each_with_index do |c, idx|
+          msg = "Case: #{idx}"
+          got = cmd.send(:determine_optional_key_position, c[:params][:command], c[:params][:option_name])
+          assert_equal(c[:want], got, msg)
+        end
+      end
+
+      def test_extract_hash_tag
+        cmd = ::RedisClient::Cluster::Command.load(@clients)
+        [
+          { key: 'foo', want: '' },
+          { key: 'foo{bar}baz', want: 'bar' },
+          { key: 'foo{bar}baz{qux}quuc', want: 'bar' },
+          { key: 'foo}bar{baz', want: '' },
+          { key: 'foo{bar', want: '' },
+          { key: 'foo}bar', want: '' },
+          { key: 'foo{}bar', want: '' },
+          { key: '{}foo', want: '' },
+          { key: 'foo{}', want: '' },
+          { key: '{}', want: '' },
+          { key: '', want: '' },
+          { key: nil, want: '' }
+        ].each_with_index do |c, idx|
+          msg = "Case: #{idx}"
+          got = cmd.send(:extract_hash_tag, c[:key])
+          assert_equal(c[:want], got, msg)
+        end
+      end
     end
   end
 end

test/redis_client/cluster/test_command.rb

@@ -1,3 +1,19 @@
 # frozen_string_literal: true
 
 require 'minitest/autorun'
+require 'redis_client'
+
+class RedisClient
+  module TestingHelper
+    REDIS_SCHEME = ENV.fetch('REDIS_SCHEME', 'redis')
+    NODE_ADDRS = (7000..7005).map { |port| "#{REDIS_SCHEME}://127.0.0.1:#{port}" }.freeze
+
+    def setup
+      @clients = NODE_ADDRS.map { |addr| ::RedisClient.config(url: addr).new_client }
+    end
+
+    def teardown
+      @clients&.each(&:close)
+    end
+  end
+end