Increase the coverage by adding tests and fix a failing ready check

index.js

@@ -337,6 +337,12 @@ RedisClient.prototype.on_ready = function () {
 };
 
 RedisClient.prototype.on_info_cmd = function (err, res) {
+    if (err) {
+        err.message = "Ready check failed: " + err.message;
+        this.emit("error", err);
+        return;
+    }
+
     var self = this;
     var obj = {};
     var lines = res.toString().split("\r\n");
@@ -344,11 +350,6 @@ RedisClient.prototype.on_info_cmd = function (err, res) {
     var key = 'db' + i;
     var line, retry_time, parts, sub_parts;
 
-    if (err) {
-        err.message = "Ready check failed: " + err.message;
-        return self.emit("error", err);
-    }
-
     for (i = 0; i < lines.length; i++) {
         parts = lines[i].split(':');
         if (parts[1]) {
@@ -369,9 +370,7 @@ RedisClient.prototype.on_info_cmd = function (err, res) {
         obj[key] = {};
         while (line = parts.pop()) {
             sub_parts = line.split('=');
-            if (sub_parts[1]) {
+            obj[key][sub_parts[0]] = +sub_parts[1];
-                obj[key][sub_parts[0]] = +sub_parts[1];
-            }
         }
         i++;
         key = 'db' + i;
@@ -471,7 +470,7 @@ RedisClient.prototype.connection_gone = function (why) {
 
     // If this is a requested shutdown, then don't retry
     if (this.closing) {
-        debug("connection ended from quit command, not retrying.");
+        debug("Connection ended from quit command, not retrying.");
         this.flush_and_error(new Error("Redis connection gone from " + why + " event."));
         return;
     }
@@ -656,7 +655,7 @@ RedisClient.prototype.return_reply = function (reply) {
         this.emit("monitor", timestamp, args);
     } else {
         var err = new Error("node_redis command queue state error. If you can reproduce this, please report it.");
-        err.command = command_obj.command.toUpperCase();
+        err.command_obj = command_obj;
         this.emit("error", err);
     }
 };
@@ -695,19 +694,8 @@ RedisClient.prototype.send_command = function (command, args, callback) {
         callback = process.domain.bind(callback);
     }
 
-    // if the last argument is an array and command is sadd or srem, expand it out:
-    //     client.sadd(arg1, [arg2, arg3, arg4], cb);
-    //  converts to:
-    //     client.sadd(arg1, arg2, arg3, arg4, cb);
-    if ((command === 'sadd' || command === 'srem') && args.length > 0 && Array.isArray(args[args.length - 1])) {
-        args = args.slice(0, -1).concat(args[args.length - 1]);
-    }
-
-    // if the value is undefined or null and command is set or setx, need not to send message to redis
     if (command === 'set' || command === 'setex') {
-        if (args.length === 0) {
+        // if the value is undefined or null and command is set or setx, need not to send message to redis
-            return;
-        }
         if (args[args.length - 1] === undefined || args[args.length - 1] === null) {
             command = command.toUpperCase();
             err = new Error('send_command: ' + command + ' value must not be undefined or null');

test/auth.spec.js

@@ -159,6 +159,34 @@ describe("client authentication", function () {
                     client.auth(auth, helper.isString('OK', done));
                 });
             });
+
+            it('does not allow any commands to be processed if not authenticated using no_ready_check true', function (done) {
+                if (helper.redisProcess().spawnFailed()) this.skip();
+
+                var args = config.configureClient(parser, ip, {
+                    no_ready_check: true
+                });
+                client = redis.createClient.apply(redis.createClient, args);
+                client.on("ready", function () {
+                    client.set('foo', 'bar', function (err, res) {
+                        assert.equal(err.message, 'NOAUTH Authentication required.');
+                        assert.equal(err.code, 'NOAUTH');
+                        assert.equal(err.command, 'SET');
+                        done();
+                    });
+                });
+            });
+
+            it('does not allow auth to be provided post-hoc with auth method if not authenticated before', function (done) {
+                if (helper.redisProcess().spawnFailed()) this.skip();
+                client = redis.createClient.apply(redis.createClient, args);
+                client.on("error", function (err) {
+                    assert.equal(err.code, 'NOAUTH');
+                    assert.equal(err.message, 'Ready check failed: NOAUTH Authentication required.');
+                    assert.equal(err.command, 'INFO');
+                    done();
+                });
+            });
         });
     });
 

test/commands/set.spec.js

@@ -70,6 +70,7 @@ describe("The 'set' method", function () {
                         it("reports an error", function (done) {
                             client.set(undefined, function (err, res) {
                                 helper.isError()(err, null);
+                                assert.equal(err.command, 'SET');
                                 done();
                             });
                         });
@@ -90,29 +91,36 @@ describe("The 'set' method", function () {
                     });
 
                     describe("with undefined 'key' and missing 'value' parameter", function () {
-                        it("does not emit an error", function (done) {
+                        it("emits an error without callback", function (done) {
-                            this.timeout(200);
-
-                            client.once("error", function (err) {
-                                done(err);
-                            });
-
-                            client.set();
-
-                            setTimeout(function () {
-                                done();
-                            }, 100);
-                        });
-
-                        it("does emit an error", function (done) {
                             client.on('error', function (err) {
-                                assert.equal(err.message, "ERR wrong number of arguments for 'set' command");
+                                assert.equal(err.message, 'send_command: SET value must not be undefined or null');
+                                assert.equal(err.command, 'SET');
                                 done();
                             });
-
+                            client.set(undefined);
-                            client.set('foo');
                         });
                     });
+
+                    it("emit an error with only the key set", function (done) {
+                        client.on('error', function (err) {
+                            assert.equal(err.message, "ERR wrong number of arguments for 'set' command");
+                            done();
+                        });
+
+                        client.set('foo');
+                    });
+
+                    it("emit an error without any parameters", function (done) {
+                        client.once("error", function (err) {
+                            assert.equal(err.message, 'send_command: SET value must not be undefined or null');
+                            assert.equal(err.command, 'SET');
+                            done();
+                        });
+
+                        // This was not supported not to throw earlier and was added by the test refactoring
+                        // https://github.com/NodeRedis/node_redis/commit/eaca486ab1aecd1329f7452ad2f2255b1263606f
+                        client.set();
+                    });
                 });
             });
         });

test/commands/srem.spec.js

@@ -39,6 +39,16 @@ describe("The 'srem' method", function () {
                 });
             });
 
+            it('allows multiple values to be removed with send_command', function (done) {
+                client.send_command('sadd', ['set0', 'member0', 'member1', 'member2'], helper.isNumber(3));
+                client.send_command('srem', ["set0", "member1", "member2"], helper.isNumber(2));
+                client.smembers("set0", function (err, res) {
+                    assert.strictEqual(res.length, 1);
+                    assert.ok(~res.indexOf("member0"));
+                    return done(err);
+                });
+            });
+
             it('handles a value missing from the set of values being removed', function (done) {
                 client.sadd(["set0", "member0", "member1", "member2"], helper.isNumber(3));
                 client.SREM(["set0", "member3", "member4"], helper.isNumber(0));

test/node_redis.spec.js

@@ -829,17 +829,18 @@ describe("The node_redis client", function () {
                             var cb = function(err, reply) {
                                 assert.equal(err.code, 'CONNECTION_BROKEN');
                             };
-                            for (var i = 0; i < 10; i += 2) {
+                            for (var i = 0; i < 12; i += 3) {
-                                multi.set("foo" + i, "bar" + i);
+                                client.set("foo" + i, "bar" + i);
                                 multi.set("foo" + (i + 1), "bar" + (i + 1), cb);
+                                multi.set("foo" + (i + 2), "bar" + (i + 2));
                             }
                             multi.exec();
-                            assert.equal(client.command_queue.length, 13);
+                            assert.equal(client.command_queue.length, 15);
                             helper.killConnection(client);
                         });
 
                         client.on("reconnecting", function (params) {
-                            assert.equal(client.command_queue.length, 13);
+                            assert.equal(client.command_queue.length, 15);
                         });
 
                         client.on('error', function(err) {