Improve tests a bit

Ruben Bridgewater · Ruben Bridgewater · commit 14170f9d029e · 2016-04-01T11:42:22.000+02:00
Reduce timeouts if possible
Extend timeouts if needed (windows tests need their time)
Don't expose the redis socket to others than the owner
Don't create the stunnel log
diff --git a/index.js b/index.js
@@ -568,6 +568,8 @@ RedisClient.prototype.connection_gone = function (why, error) {
         error.code = 'UNCERTAIN_STATE';
         this.flush_and_error(error, ['command_queue']);
         error.message = 'Redis connection lost and commands aborted in uncertain state. They might have been processed.';
+        // TODO: Reconsider emitting this always, as each running command is handled anyway
+        // This should likely be removed in v.3. This is different to the broken connection as we'll reconnect here
         this.emit('error', error);
     }
 
diff --git a/lib/individualCommands.js b/lib/individualCommands.js
@@ -25,7 +25,6 @@ RedisClient.prototype.batch = RedisClient.prototype.BATCH = function batch (args
 // Store db in this.select_db to restore it on reconnect
 RedisClient.prototype.select = RedisClient.prototype.SELECT = function select (db, callback) {
     var self = this;
-    db = Array.isArray(db) && db.length === 1 ? db[0] : db;
     return this.internal_send_command('select', [db], function (err, res) {
         if (err === null) {
             self.selected_db = db;
@@ -149,7 +148,6 @@ RedisClient.prototype.auth = RedisClient.prototype.AUTH = function auth (pass, c
     debug('Sending auth to ' + self.address + ' id ' + self.connection_id);
 
     // Stash auth for connect and reconnect.
-    pass = Array.isArray(pass) && pass.length === 1 ? pass[0] : pass;
     this.auth_pass = pass;
     this.ready = this.offline_queue.length === 0; // keep the execution order intakt
     var tmp = this.internal_send_command('auth', [pass], function (err, res) {
@@ -163,7 +161,7 @@ RedisClient.prototype.auth = RedisClient.prototype.AUTH = function auth (pass, c
                 debug('Redis still loading, trying to authenticate later');
                 setTimeout(function () {
                     self.auth(pass, callback);
-                }, 200);
+                }, 100);
                 return;
             }
         }
diff --git a/test/auth.spec.js b/test/auth.spec.js
@@ -54,8 +54,8 @@ describe('client authentication', function () {
                     assert.strictEqual('retry worked', res);
                     var now = Date.now();
                     // Hint: setTimeout sometimes triggers early and therefor the value can be like one or two ms to early
-                    assert(now - time >= 198, 'Time should be above 200 ms (the reconnect time) and is ' + (now - time));
-                    assert(now - time < 300, 'Time should be below 300 ms (the reconnect should only take a bit above 200 ms) and is ' + (now - time));
+                    assert(now - time >= 98, 'Time should be above 100 ms (the reconnect time) and is ' + (now - time));
+                    assert(now - time < 225, 'Time should be below 255 ms (the reconnect should only take a bit above 100 ms) and is ' + (now - time));
                     done();
                 });
                 var tmp = client.command_queue.get(0).callback;
diff --git a/test/batch.spec.js b/test/batch.spec.js
@@ -12,6 +12,8 @@ describe("The 'batch' method", function () {
         describe('using ' + parser + ' and ' + ip, function () {
 
             describe('when not connected', function () {
+                // TODO: This is somewhat broken and should be fixed in v.3
+                // The commands should return an error instead of returning an empty result
                 var client;
 
                 beforeEach(function (done) {
diff --git a/test/commands/expire.spec.js b/test/commands/expire.spec.js
@@ -23,15 +23,15 @@ describe("The 'expire' method", function () {
                 client.EXPIRE('expiry key', '1', helper.isNumber(1));
                 setTimeout(function () {
                     client.exists(['expiry key'], helper.isNumber(0, done));
-                }, 1100);
+                }, 1050);
             });
 
             it('expires key after timeout with array syntax', function (done) {
                 client.set(['expiry key', 'bar'], helper.isString('OK'));
                 client.EXPIRE(['expiry key', '1'], helper.isNumber(1));
                 setTimeout(function () {
                     client.exists(['expiry key'], helper.isNumber(0, done));
-                }, 1100);
+                }, 1050);
             });
 
             afterEach(function () {
diff --git a/test/commands/flushdb.spec.js b/test/commands/flushdb.spec.js
@@ -90,7 +90,7 @@ describe("The 'flushdb' method", function () {
                     it('results in a db size of zero without a callback', function (done) {
                         client.flushdb();
                         setTimeout(function (err, res) {
-                            client.dbsize([], function (err, res) {
+                            client.dbsize(function (err, res) {
                                 helper.isNotError()(err, res);
                                 helper.isType.number()(err, res);
                                 assert.strictEqual(0, res, 'Flushing db should result in db size 0');
diff --git a/test/commands/get.spec.js b/test/commands/get.spec.js
@@ -77,7 +77,7 @@ describe("The 'get' method", function () {
                         client.on('error', function (err) {
                             throw err;
                         });
-                        setTimeout(done, 50);
+                        setTimeout(done, 25);
                     });
                 });
 
diff --git a/test/commands/info.spec.js b/test/commands/info.spec.js
@@ -34,7 +34,7 @@ describe("The 'info' method", function () {
                 setTimeout(function () {
                     assert.strictEqual(typeof client.server_info.db2, 'object');
                     done();
-                }, 150);
+                }, 30);
             });
 
             it('works with optional section provided with and without callback', function (done) {
diff --git a/test/commands/select.spec.js b/test/commands/select.spec.js
@@ -62,7 +62,7 @@ describe("The 'select' method", function () {
                             client.select(1, function (err) {
                                 assert.equal(err, null);
                                 assert.equal(client.selected_db, 1, 'we should have selected the new valid DB');
-                                return done();
+                                done();
                             });
                         });
                     });
@@ -73,7 +73,7 @@ describe("The 'select' method", function () {
                             client.select(9999, function (err) {
                                 assert.equal(err.code, 'ERR');
                                 assert.equal(err.message, 'ERR invalid DB index');
-                                return done();
+                                done();
                             });
                         });
                     });
@@ -86,8 +86,8 @@ describe("The 'select' method", function () {
                             client.select(1);
                             setTimeout(function () {
                                 assert.equal(client.selected_db, 1, 'we should have selected the new valid DB');
-                                return done();
-                            }, 100);
+                                done();
+                            }, 25);
                         });
                     });
 
diff --git a/test/commands/set.spec.js b/test/commands/set.spec.js
@@ -43,7 +43,7 @@ describe("The 'set' method", function () {
                 beforeEach(function (done) {
                     client = redis.createClient.apply(null, args);
                     client.once('ready', function () {
-                        done();
+                        client.flushdb(done);
                     });
                 });
 
@@ -62,6 +62,35 @@ describe("The 'set' method", function () {
                                 });
                             });
                         });
+
+                        it('set expire date in seconds', function (done) {
+                            client.set('foo', 'bar', 'ex', 10, helper.isString('OK'));
+                            client.pttl('foo', function (err, res) {
+                                assert(res >= 10000 - 50); // Max 50 ms should have passed
+                                assert(res <= 10000); // Max possible should be 10.000
+                                done(err);
+                            });
+                        });
+
+                        it('set expire date in milliseconds', function (done) {
+                            client.set('foo', 'bar', 'px', 100, helper.isString('OK'));
+                            client.pttl('foo', function (err, res) {
+                                assert(res >= 50); // Max 50 ms should have passed
+                                assert(res <= 100); // Max possible should be 100
+                                done(err);
+                            });
+                        });
+
+                        it('only set the key if (not) already set', function (done) {
+                            client.set('foo', 'bar', 'NX', helper.isString('OK'));
+                            client.set('foo', 'bar', 'nx', helper.isNull());
+                            client.set('foo', 'bar', 'EX', '10', 'XX', helper.isString('OK'));
+                            client.ttl('foo', function (err, res) {
+                                assert(res >= 9); // Min 9s should be left
+                                assert(res <= 10); // Max 10s should be left
+                                done(err);
+                            });
+                        });
                     });
 
                     describe('reports an error with invalid parameters', function () {
diff --git a/test/commands/ttl.spec.js b/test/commands/ttl.spec.js
@@ -22,12 +22,11 @@ describe("The 'ttl' method", function () {
             it('returns the current ttl on a key', function (done) {
                 client.set(['ttl key', 'ttl val'], helper.isString('OK'));
                 client.expire(['ttl key', '100'], helper.isNumber(1));
-                setTimeout(function () {
-                    client.TTL(['ttl key'], function (err, ttl) {
-                        assert.ok(ttl > 50 && ttl <= 100);
-                        return done(err);
-                    });
-                }, 200);
+                client.TTL(['ttl key'], function (err, ttl) {
+                    assert(ttl >= 99);
+                    assert(ttl <= 100);
+                    done(err);
+                });
             });
 
             afterEach(function () {
diff --git a/test/conf/password.conf b/test/conf/password.conf
@@ -2,4 +2,4 @@ requirepass porkchopsandwiches
 port 6379
 bind ::1 127.0.0.1
 unixsocket /tmp/redis.sock
-unixsocketperm 755
+unixsocketperm 700
diff --git a/test/conf/redis.conf b/test/conf/redis.conf
@@ -1,4 +1,4 @@
 port 6379
 bind ::1 127.0.0.1
 unixsocket /tmp/redis.sock
-unixsocketperm 755
+unixsocketperm 700
diff --git a/test/conf/rename.conf b/test/conf/rename.conf
@@ -1,7 +1,7 @@
 port 6379
 bind ::1 127.0.0.1
 unixsocket /tmp/redis.sock
-unixsocketperm 755
+unixsocketperm 700
 rename-command SET      807081f5afa96845a02816a28b7258c3
 rename-command GET      f397808a43ceca3963e22b4e13deb672
 rename-command GETRANGE 9e3102b15cf231c4e9e940f284744fe0
diff --git a/test/conf/slave.conf b/test/conf/slave.conf
@@ -1,6 +1,6 @@
 port 6381
 bind ::1 127.0.0.1
 unixsocket /tmp/redis6381.sock
-unixsocketperm 755
+unixsocketperm 700
 slaveof localhost 6379
 masterauth porkchopsandwiches
diff --git a/test/conf/stunnel.conf.template b/test/conf/stunnel.conf.template
@@ -1,5 +1,5 @@
 pid = __dirname/stunnel.pid
-output = __dirname/stunnel.log
+; output = __dirname/stunnel.log
 CAfile = __dirname/redis.js.org.cert
 cert = __dirname/redis.js.org.cert
 key = __dirname/redis.js.org.key
diff --git a/test/connection.spec.js b/test/connection.spec.js
@@ -89,6 +89,7 @@ describe('connection tests', function () {
                 assert(called);
                 done();
             });
+            // TODO: In v.3 the quit command would be fired right away, so bool should be true
             assert.strictEqual(bool, false);
         });
 
@@ -99,6 +100,18 @@ describe('connection tests', function () {
             assert.strictEqual(bool, false);
         });
 
+        it('quit "succeeds" even if the client connection is closed while doing so', function (done) {
+            client = redis.createClient();
+            client.set('foo', 'bar', function (err, res) {
+                assert.strictEqual(res, 'OK');
+                client.quit(function (err, res) {
+                    assert.strictEqual(res, 'OK');
+                    done(err);
+                });
+                client.end(true); // Flushing the quit command should result in a success
+            });
+        });
+
         it('quit right away if connection drops while quit command is on the fly', function (done) {
             client = redis.createClient();
             client.once('ready', function () {
@@ -119,7 +132,7 @@ describe('connection tests', function () {
 
             describe('on lost connection', function () {
                 it('emit an error after max retry attempts and do not try to reconnect afterwards', function (done) {
-                    var max_attempts = 4;
+                    var max_attempts = 3;
                     var options = {
                         parser: parser,
                         max_attempts: max_attempts
@@ -138,10 +151,14 @@ describe('connection tests', function () {
 
                     client.on('error', function (err) {
                         if (/Redis connection in broken state: maximum connection attempts.*?exceeded./.test(err.message)) {
-                            setTimeout(function () {
+                            process.nextTick(function () { // End is called after the error got emitted
                                 assert.strictEqual(calls, max_attempts - 1);
+                                assert.strictEqual(client.emitted_end, true);
+                                assert.strictEqual(client.connected, false);
+                                assert.strictEqual(client.ready, false);
+                                assert.strictEqual(client.closing, true);
                                 done();
-                            }, 500);
+                            });
                         }
                     });
                 });
@@ -167,11 +184,15 @@ describe('connection tests', function () {
 
                     client.on('error', function (err) {
                         if (/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message)) {
-                            setTimeout(function () {
+                            process.nextTick(function () { // End is called after the error got emitted
+                                assert.strictEqual(client.emitted_end, true);
+                                assert.strictEqual(client.connected, false);
+                                assert.strictEqual(client.ready, false);
+                                assert.strictEqual(client.closing, true);
                                 assert.strictEqual(client.retry_totaltime, connect_timeout);
                                 assert.strictEqual(time, connect_timeout);
                                 done();
-                            }, 500);
+                            });
                         }
                     });
                 });
@@ -190,7 +211,7 @@ describe('connection tests', function () {
                     client.on('reconnecting', function (params) {
                         client.end(true);
                         assert.strictEqual(params.times_connected, 1);
-                        setTimeout(done, 100);
+                        setTimeout(done, 5);
                     });
                 });
 
@@ -291,7 +312,6 @@ describe('connection tests', function () {
 
                 it('emit an error after the socket timeout exceeded the connect_timeout time', function (done) {
                     var connect_timeout = 500; // in ms
-                    var time = Date.now();
                     client = redis.createClient({
                         parser: parser,
                         // Auto detect ipv4 and use non routable ip to trigger the timeout
@@ -308,17 +328,18 @@ describe('connection tests', function () {
                         throw new Error('No reconnect, since no connection was ever established');
                     });
 
+                    var time = Date.now();
                     client.on('error', function (err) {
                         if (err.code === 'ENETUNREACH') { // The test is run without a internet connection. Pretent it works
                             return done();
                         }
                         assert(/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message));
                         // The code execution on windows is very slow at times
-                        var add = process.platform !== 'win32' ? 25 : 125;
+                        var add = process.platform !== 'win32' ? 15 : 200;
                         var now = Date.now();
                         assert(now - time < connect_timeout + add, 'The real timeout time should be below ' + (connect_timeout + add) + 'ms but is: ' + (now - time));
                          // Timers sometimes trigger early (e.g. 1ms to early)
-                        assert(now - time >= connect_timeout - 3, 'The real timeout time should be above ' + connect_timeout + 'ms, but it is: ' + (now - time));
+                        assert(now - time >= connect_timeout - 5, 'The real timeout time should be above ' + connect_timeout + 'ms, but it is: ' + (now - time));
                         done();
                     });
                 });
@@ -546,7 +567,7 @@ describe('connection tests', function () {
                     });
                 }
 
-                it('redis still loading <= 1000ms', function (done) {
+                it('redis still loading <= 500', function (done) {
                     client = redis.createClient.apply(null, args);
                     var tmp = client.info.bind(client);
                     var end = helper.callFuncAfter(done, 3);
@@ -568,9 +589,9 @@ describe('connection tests', function () {
                     };
                     client.on('ready', function () {
                         var rest = Date.now() - time;
-                        assert(rest >= 498, 'Rest should be equal or above 500 ms but is: ' + rest); // setTimeout might trigger early
+                        assert(rest >= 495, 'Rest should be equal or above 500 ms but is: ' + rest); // setTimeout might trigger early
                         // Be on the safe side and accept 200ms above the original value
-                        assert(rest - 200 < 500, 'Rest - 200 should be below 500 ms but is: ' + (rest - 200));
+                        assert(rest - 250 < 500, 'Rest - 250 should be below 500 ms but is: ' + (rest - 250));
                         assert(delayed);
                         end();
                     });
@@ -601,7 +622,7 @@ describe('connection tests', function () {
                         var rest = Date.now() - time;
                         assert(rest >= 998, '`rest` should be equal or above 1000 ms but is: ' + rest); // setTimeout might trigger early
                         // Be on the safe side and accept 200ms above the original value
-                        assert(rest - 200 < 1000, '`rest` - 200 should be below 1000 ms but is: ' + (rest - 200));
+                        assert(rest - 250 < 1000, '`rest` - 250 should be below 1000 ms but is: ' + (rest - 250));
                         assert(delayed);
                         end();
                     });
diff --git a/test/multi.spec.js b/test/multi.spec.js
@@ -73,7 +73,7 @@ describe("The 'multi' method", function () {
     describe('pipeline limit', function () {
 
         it('do not exceed maximum string size', function (done) {
-            this.timeout(30000); // Windows tests are horribly slow
+            this.timeout(process.platform !== 'win32' ? 10000 : 35000); // Windows tests are horribly slow
             // Triggers a RangeError: Invalid string length if not handled properly
             client = redis.createClient();
             var multi = client.multi();
diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js
diff --git a/test/pubsub.spec.js b/test/pubsub.spec.js
diff --git a/test/tls.spec.js b/test/tls.spec.js

Original file line number	Diff line number	Diff line change
`@@ -568,6 +568,8 @@ RedisClient.prototype.connection_gone = function (why, error) {`
`568`	`568`	`error.code = 'UNCERTAIN_STATE';`
`569`	`569`	`this.flush_and_error(error, ['command_queue']);`
`570`	`570`	`error.message = 'Redis connection lost and commands aborted in uncertain state. They might have been processed.';`
	`571`	`+ // TODO: Reconsider emitting this always, as each running command is handled anyway`
	`572`	`+ // This should likely be removed in v.3. This is different to the broken connection as we'll reconnect here`
`571`	`573`	`this.emit('error', error);`
`572`	`574`	`}`
`573`	`575`