Add some test cases (#14)

Author: supercaracal

lib/redis_client/cluster.rb

@@ -179,7 +179,7 @@ def send_cluster_command(method, *command, **kwargs, &block)
       case subcommand
       when 'addslots', 'delslots', 'failover', 'forget', 'meet', 'replicate',
            'reset', 'set-config-epoch', 'setslot'
-        raise ::RedisClient::Cluster::OrchestrationCommandNotSupported, 'cluster', subcommand
+        raise ::RedisClient::Cluster::OrchestrationCommandNotSupported, ['cluster', subcommand]
       when 'saveconfig' then @node.call_all(method, *command, **kwargs, &block).first
       else assign_node(*command).send(method, *command, **kwargs, &block)
       end

lib/redis_client/cluster/errors.rb

@@ -4,22 +4,24 @@
 
 class RedisClient
   class Cluster
+    ERR_ARG_NORMALIZATION = ->(arg) { Array[arg].flatten.reject { |e| e.nil? || (e.respond_to?(:empty?) && e.empty?) } }
+
     # Raised when client connected to redis as cluster mode
     # and failed to fetch cluster state information by commands.
     class InitialSetupError < ::RedisClient::Error
-      # @param errors [Array<Redis::BaseError>]
       def initialize(errors)
-        super("Redis client could not fetch cluster information: #{errors&.map(&:message)&.uniq&.join(',')}")
+        msg = ERR_ARG_NORMALIZATION.call(errors).map(&:message).uniq.join(',')
+        super("Redis client could not fetch cluster information: #{msg}")
       end
     end
 
     # Raised when client connected to redis as cluster mode
     # and some cluster subcommands were called.
     class OrchestrationCommandNotSupported < ::RedisClient::Error
-      def initialize(command, subcommand = '')
-        str = [command, subcommand].map(&:to_s).reject(&:empty?).join(' ').upcase
+      def initialize(command)
+        str = ERR_ARG_NORMALIZATION.call(command).map(&:to_s).join(' ').upcase
         msg = "#{str} command should be used with care "\
-              'only by applications orchestrating Redis Cluster, like redis-trib, '\
+              'only by applications orchestrating Redis Cluster, like redis-cli, '\
               'and the command if used out of the right context can leave the cluster '\
               'in a wrong state or cause data loss.'
         super(msg)
@@ -30,11 +32,9 @@ def initialize(command, subcommand = '')
     class CommandErrorCollection < ::RedisClient::Error
       attr_reader :errors
 
-      # @param errors [Hash{String => Redis::CommandError}]
-      # @param error_message [String]
-      def initialize(errors, error_message = 'Command errors were replied on any node')
-        @errors = errors
-        super(error_message)
+      def initialize(errors)
+        @errors = ERR_ARG_NORMALIZATION.call(errors)
+        super("Command errors were replied on any node: #{@errors.map(&:message).uniq.join(',')}")
       end
     end
 
@@ -49,7 +49,7 @@ def initialize(command)
     class CrossSlotPipeliningError < ::RedisClient::Error
       def initialize(keys)
         super("Cluster client couldn't send pipelining to single node. "\
-              "The commands include cross slot keys. #{keys}")
+              "The commands include cross slot keys: #{ERR_ARG_NORMALIZATION.call(keys).join(',')}")
       end
     end
   end

test/redis_client/cluster/test_errors.rb

@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+require 'testing_helper'
+require 'redis_client/cluster/errors'
+
+class RedisClient
+  class Cluster
+    class TestErrors < Minitest::Test
+      DummyError = Struct.new('DummyError', :message)
+
+      def test_initial_setup_error
+        [
+          {
+            errors: [DummyError.new('foo')],
+            want: 'Redis client could not fetch cluster information: foo'
+          },
+          {
+            errors: [DummyError.new('foo'), DummyError.new('bar')],
+            want: 'Redis client could not fetch cluster information: foo,bar'
+          },
+          { errors: [], want: 'Redis client could not fetch cluster information: ' },
+          { errors: '', want: 'Redis client could not fetch cluster information: ' },
+          { errors: nil, want: 'Redis client could not fetch cluster information: ' }
+        ].each_with_index do |c, idx|
+          raise ::RedisClient::Cluster::InitialSetupError, c[:errors]
+        rescue StandardError => e
+          assert_equal(c[:want], e.message, "Case: #{idx}")
+        end
+      end
+
+      def test_orchestration_command_not_supported_error
+        [
+          { command: %w[CLUSTER FORGET], want: 'CLUSTER FORGET command should be' },
+          { command: [], want: ' command should be' },
+          { command: '', want: ' command should be' },
+          { command: nil, want: ' command should be' }
+        ].each_with_index do |c, idx|
+          raise ::RedisClient::Cluster::OrchestrationCommandNotSupported, c[:command]
+        rescue StandardError => e
+          assert(e.message.start_with?(c[:want]), "Case: #{idx}")
+        end
+      end
+
+      def test_command_error_collection_error
+        [
+          {
+            errors: [DummyError.new('foo')],
+            want: { msg: 'Command errors were replied on any node: foo', size: 1 }
+          },
+          {
+            errors: [DummyError.new('foo'), DummyError.new('bar')],
+            want: { msg: 'Command errors were replied on any node: foo,bar', size: 2 }
+          },
+          { errors: [], want: { msg: 'Command errors were replied on any node: ', size: 0 } },
+          { errors: '', want: { msg: 'Command errors were replied on any node: ', size: 0 } },
+          { errors: nil, want: { msg: 'Command errors were replied on any node: ', size: 0 } }
+        ].each_with_index do |c, idx|
+          raise ::RedisClient::Cluster::CommandErrorCollection, c[:errors]
+        rescue StandardError => e
+          assert_equal(c[:want][:msg], e.message, "Case: #{idx}")
+          assert_equal(c[:want][:size], e.errors.size, "Case: #{idx}")
+        end
+      end
+
+      def test_ambiguous_node_error
+        [
+          { command: 'MULTI', want: "Cluster client doesn't know which node the MULTI command should be sent to." },
+          { command: nil, want: "Cluster client doesn't know which node the  command should be sent to." }
+        ].each_with_index do |c, idx|
+          raise ::RedisClient::Cluster::AmbiguousNodeError, c[:command]
+        rescue StandardError => e
+          assert_equal(e.message, c[:want], "Case: #{idx}")
+        end
+      end
+
+      def test_cross_slot_pipelining_error
+        [
+          { keys: %w[foo bar baz], want: 'keys: foo,bar,baz' },
+          { keys: '', want: 'keys: ' },
+          { keys: nil, want: 'keys: ' }
+        ].each_with_index do |c, idx|
+          raise ::RedisClient::Cluster::CrossSlotPipeliningError, c[:keys]
+        rescue StandardError => e
+          assert(e.message.end_with?(c[:want]), "Case: #{idx}")
+        end
+      end
+    end
+  end
+end