Skip to content

Commit 14170f9

Browse files
author
Ruben Bridgewater
committed
Improve tests a bit
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
1 parent 79c1767 commit 14170f9

21 files changed

+136
-86
lines changed

‎index.js

+2
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,8 @@ RedisClient.prototype.connection_gone = function (why, error) {
568568
error.code = 'UNCERTAIN_STATE';
569569
this.flush_and_error(error, ['command_queue']);
570570
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
571573
this.emit('error', error);
572574
}
573575

‎lib/individualCommands.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ RedisClient.prototype.batch = RedisClient.prototype.BATCH = function batch (args
2525
// Store db in this.select_db to restore it on reconnect
2626
RedisClient.prototype.select = RedisClient.prototype.SELECT = function select (db, callback) {
2727
var self = this;
28-
db = Array.isArray(db) && db.length === 1 ? db[0] : db;
2928
return this.internal_send_command('select', [db], function (err, res) {
3029
if (err === null) {
3130
self.selected_db = db;
@@ -149,7 +148,6 @@ RedisClient.prototype.auth = RedisClient.prototype.AUTH = function auth (pass, c
149148
debug('Sending auth to ' + self.address + ' id ' + self.connection_id);
150149

151150
// Stash auth for connect and reconnect.
152-
pass = Array.isArray(pass) && pass.length === 1 ? pass[0] : pass;
153151
this.auth_pass = pass;
154152
this.ready = this.offline_queue.length === 0; // keep the execution order intakt
155153
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
163161
debug('Redis still loading, trying to authenticate later');
164162
setTimeout(function () {
165163
self.auth(pass, callback);
166-
}, 200);
164+
}, 100);
167165
return;
168166
}
169167
}

‎test/auth.spec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ describe('client authentication', function () {
5454
assert.strictEqual('retry worked', res);
5555
var now = Date.now();
5656
// Hint: setTimeout sometimes triggers early and therefor the value can be like one or two ms to early
57-
assert(now - time >= 198, 'Time should be above 200 ms (the reconnect time) and is ' + (now - time));
58-
assert(now - time < 300, 'Time should be below 300 ms (the reconnect should only take a bit above 200 ms) and is ' + (now - time));
57+
assert(now - time >= 98, 'Time should be above 100 ms (the reconnect time) and is ' + (now - time));
58+
assert(now - time < 225, 'Time should be below 255 ms (the reconnect should only take a bit above 100 ms) and is ' + (now - time));
5959
done();
6060
});
6161
var tmp = client.command_queue.get(0).callback;

‎test/batch.spec.js

+2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ describe("The 'batch' method", function () {
1212
describe('using ' + parser + ' and ' + ip, function () {
1313

1414
describe('when not connected', function () {
15+
// TODO: This is somewhat broken and should be fixed in v.3
16+
// The commands should return an error instead of returning an empty result
1517
var client;
1618

1719
beforeEach(function (done) {

‎test/commands/expire.spec.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@ describe("The 'expire' method", function () {
2323
client.EXPIRE('expiry key', '1', helper.isNumber(1));
2424
setTimeout(function () {
2525
client.exists(['expiry key'], helper.isNumber(0, done));
26-
}, 1100);
26+
}, 1050);
2727
});
2828

2929
it('expires key after timeout with array syntax', function (done) {
3030
client.set(['expiry key', 'bar'], helper.isString('OK'));
3131
client.EXPIRE(['expiry key', '1'], helper.isNumber(1));
3232
setTimeout(function () {
3333
client.exists(['expiry key'], helper.isNumber(0, done));
34-
}, 1100);
34+
}, 1050);
3535
});
3636

3737
afterEach(function () {

‎test/commands/flushdb.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ describe("The 'flushdb' method", function () {
9090
it('results in a db size of zero without a callback', function (done) {
9191
client.flushdb();
9292
setTimeout(function (err, res) {
93-
client.dbsize([], function (err, res) {
93+
client.dbsize(function (err, res) {
9494
helper.isNotError()(err, res);
9595
helper.isType.number()(err, res);
9696
assert.strictEqual(0, res, 'Flushing db should result in db size 0');

‎test/commands/get.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ describe("The 'get' method", function () {
7777
client.on('error', function (err) {
7878
throw err;
7979
});
80-
setTimeout(done, 50);
80+
setTimeout(done, 25);
8181
});
8282
});
8383

‎test/commands/info.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ describe("The 'info' method", function () {
3434
setTimeout(function () {
3535
assert.strictEqual(typeof client.server_info.db2, 'object');
3636
done();
37-
}, 150);
37+
}, 30);
3838
});
3939

4040
it('works with optional section provided with and without callback', function (done) {

‎test/commands/select.spec.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ describe("The 'select' method", function () {
6262
client.select(1, function (err) {
6363
assert.equal(err, null);
6464
assert.equal(client.selected_db, 1, 'we should have selected the new valid DB');
65-
return done();
65+
done();
6666
});
6767
});
6868
});
@@ -73,7 +73,7 @@ describe("The 'select' method", function () {
7373
client.select(9999, function (err) {
7474
assert.equal(err.code, 'ERR');
7575
assert.equal(err.message, 'ERR invalid DB index');
76-
return done();
76+
done();
7777
});
7878
});
7979
});
@@ -86,8 +86,8 @@ describe("The 'select' method", function () {
8686
client.select(1);
8787
setTimeout(function () {
8888
assert.equal(client.selected_db, 1, 'we should have selected the new valid DB');
89-
return done();
90-
}, 100);
89+
done();
90+
}, 25);
9191
});
9292
});
9393

‎test/commands/set.spec.js

+30-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe("The 'set' method", function () {
4343
beforeEach(function (done) {
4444
client = redis.createClient.apply(null, args);
4545
client.once('ready', function () {
46-
done();
46+
client.flushdb(done);
4747
});
4848
});
4949

@@ -62,6 +62,35 @@ describe("The 'set' method", function () {
6262
});
6363
});
6464
});
65+
66+
it('set expire date in seconds', function (done) {
67+
client.set('foo', 'bar', 'ex', 10, helper.isString('OK'));
68+
client.pttl('foo', function (err, res) {
69+
assert(res >= 10000 - 50); // Max 50 ms should have passed
70+
assert(res <= 10000); // Max possible should be 10.000
71+
done(err);
72+
});
73+
});
74+
75+
it('set expire date in milliseconds', function (done) {
76+
client.set('foo', 'bar', 'px', 100, helper.isString('OK'));
77+
client.pttl('foo', function (err, res) {
78+
assert(res >= 50); // Max 50 ms should have passed
79+
assert(res <= 100); // Max possible should be 100
80+
done(err);
81+
});
82+
});
83+
84+
it('only set the key if (not) already set', function (done) {
85+
client.set('foo', 'bar', 'NX', helper.isString('OK'));
86+
client.set('foo', 'bar', 'nx', helper.isNull());
87+
client.set('foo', 'bar', 'EX', '10', 'XX', helper.isString('OK'));
88+
client.ttl('foo', function (err, res) {
89+
assert(res >= 9); // Min 9s should be left
90+
assert(res <= 10); // Max 10s should be left
91+
done(err);
92+
});
93+
});
6594
});
6695

6796
describe('reports an error with invalid parameters', function () {

‎test/commands/ttl.spec.js

+5-6
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,11 @@ describe("The 'ttl' method", function () {
2222
it('returns the current ttl on a key', function (done) {
2323
client.set(['ttl key', 'ttl val'], helper.isString('OK'));
2424
client.expire(['ttl key', '100'], helper.isNumber(1));
25-
setTimeout(function () {
26-
client.TTL(['ttl key'], function (err, ttl) {
27-
assert.ok(ttl > 50 && ttl <= 100);
28-
return done(err);
29-
});
30-
}, 200);
25+
client.TTL(['ttl key'], function (err, ttl) {
26+
assert(ttl >= 99);
27+
assert(ttl <= 100);
28+
done(err);
29+
});
3130
});
3231

3332
afterEach(function () {

‎test/conf/password.conf

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ requirepass porkchopsandwiches
22
port 6379
33
bind ::1 127.0.0.1
44
unixsocket /tmp/redis.sock
5-
unixsocketperm 755
5+
unixsocketperm 700

‎test/conf/redis.conf

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
port 6379
22
bind ::1 127.0.0.1
33
unixsocket /tmp/redis.sock
4-
unixsocketperm 755
4+
unixsocketperm 700

‎test/conf/rename.conf

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
port 6379
22
bind ::1 127.0.0.1
33
unixsocket /tmp/redis.sock
4-
unixsocketperm 755
4+
unixsocketperm 700
55
rename-command SET 807081f5afa96845a02816a28b7258c3
66
rename-command GET f397808a43ceca3963e22b4e13deb672
77
rename-command GETRANGE 9e3102b15cf231c4e9e940f284744fe0

‎test/conf/slave.conf

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
port 6381
22
bind ::1 127.0.0.1
33
unixsocket /tmp/redis6381.sock
4-
unixsocketperm 755
4+
unixsocketperm 700
55
slaveof localhost 6379
66
masterauth porkchopsandwiches

‎test/conf/stunnel.conf.template

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
pid = __dirname/stunnel.pid
2-
output = __dirname/stunnel.log
2+
; output = __dirname/stunnel.log
33
CAfile = __dirname/redis.js.org.cert
44
cert = __dirname/redis.js.org.cert
55
key = __dirname/redis.js.org.key

‎test/connection.spec.js

+34-13
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ describe('connection tests', function () {
8989
assert(called);
9090
done();
9191
});
92+
// TODO: In v.3 the quit command would be fired right away, so bool should be true
9293
assert.strictEqual(bool, false);
9394
});
9495

@@ -99,6 +100,18 @@ describe('connection tests', function () {
99100
assert.strictEqual(bool, false);
100101
});
101102

103+
it('quit "succeeds" even if the client connection is closed while doing so', function (done) {
104+
client = redis.createClient();
105+
client.set('foo', 'bar', function (err, res) {
106+
assert.strictEqual(res, 'OK');
107+
client.quit(function (err, res) {
108+
assert.strictEqual(res, 'OK');
109+
done(err);
110+
});
111+
client.end(true); // Flushing the quit command should result in a success
112+
});
113+
});
114+
102115
it('quit right away if connection drops while quit command is on the fly', function (done) {
103116
client = redis.createClient();
104117
client.once('ready', function () {
@@ -119,7 +132,7 @@ describe('connection tests', function () {
119132

120133
describe('on lost connection', function () {
121134
it('emit an error after max retry attempts and do not try to reconnect afterwards', function (done) {
122-
var max_attempts = 4;
135+
var max_attempts = 3;
123136
var options = {
124137
parser: parser,
125138
max_attempts: max_attempts
@@ -138,10 +151,14 @@ describe('connection tests', function () {
138151

139152
client.on('error', function (err) {
140153
if (/Redis connection in broken state: maximum connection attempts.*?exceeded./.test(err.message)) {
141-
setTimeout(function () {
154+
process.nextTick(function () { // End is called after the error got emitted
142155
assert.strictEqual(calls, max_attempts - 1);
156+
assert.strictEqual(client.emitted_end, true);
157+
assert.strictEqual(client.connected, false);
158+
assert.strictEqual(client.ready, false);
159+
assert.strictEqual(client.closing, true);
143160
done();
144-
}, 500);
161+
});
145162
}
146163
});
147164
});
@@ -167,11 +184,15 @@ describe('connection tests', function () {
167184

168185
client.on('error', function (err) {
169186
if (/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message)) {
170-
setTimeout(function () {
187+
process.nextTick(function () { // End is called after the error got emitted
188+
assert.strictEqual(client.emitted_end, true);
189+
assert.strictEqual(client.connected, false);
190+
assert.strictEqual(client.ready, false);
191+
assert.strictEqual(client.closing, true);
171192
assert.strictEqual(client.retry_totaltime, connect_timeout);
172193
assert.strictEqual(time, connect_timeout);
173194
done();
174-
}, 500);
195+
});
175196
}
176197
});
177198
});
@@ -190,7 +211,7 @@ describe('connection tests', function () {
190211
client.on('reconnecting', function (params) {
191212
client.end(true);
192213
assert.strictEqual(params.times_connected, 1);
193-
setTimeout(done, 100);
214+
setTimeout(done, 5);
194215
});
195216
});
196217

@@ -291,7 +312,6 @@ describe('connection tests', function () {
291312

292313
it('emit an error after the socket timeout exceeded the connect_timeout time', function (done) {
293314
var connect_timeout = 500; // in ms
294-
var time = Date.now();
295315
client = redis.createClient({
296316
parser: parser,
297317
// Auto detect ipv4 and use non routable ip to trigger the timeout
@@ -308,17 +328,18 @@ describe('connection tests', function () {
308328
throw new Error('No reconnect, since no connection was ever established');
309329
});
310330

331+
var time = Date.now();
311332
client.on('error', function (err) {
312333
if (err.code === 'ENETUNREACH') { // The test is run without a internet connection. Pretent it works
313334
return done();
314335
}
315336
assert(/Redis connection in broken state: connection timeout.*?exceeded./.test(err.message));
316337
// The code execution on windows is very slow at times
317-
var add = process.platform !== 'win32' ? 25 : 125;
338+
var add = process.platform !== 'win32' ? 15 : 200;
318339
var now = Date.now();
319340
assert(now - time < connect_timeout + add, 'The real timeout time should be below ' + (connect_timeout + add) + 'ms but is: ' + (now - time));
320341
// Timers sometimes trigger early (e.g. 1ms to early)
321-
assert(now - time >= connect_timeout - 3, 'The real timeout time should be above ' + connect_timeout + 'ms, but it is: ' + (now - time));
342+
assert(now - time >= connect_timeout - 5, 'The real timeout time should be above ' + connect_timeout + 'ms, but it is: ' + (now - time));
322343
done();
323344
});
324345
});
@@ -546,7 +567,7 @@ describe('connection tests', function () {
546567
});
547568
}
548569

549-
it('redis still loading <= 1000ms', function (done) {
570+
it('redis still loading <= 500', function (done) {
550571
client = redis.createClient.apply(null, args);
551572
var tmp = client.info.bind(client);
552573
var end = helper.callFuncAfter(done, 3);
@@ -568,9 +589,9 @@ describe('connection tests', function () {
568589
};
569590
client.on('ready', function () {
570591
var rest = Date.now() - time;
571-
assert(rest >= 498, 'Rest should be equal or above 500 ms but is: ' + rest); // setTimeout might trigger early
592+
assert(rest >= 495, 'Rest should be equal or above 500 ms but is: ' + rest); // setTimeout might trigger early
572593
// Be on the safe side and accept 200ms above the original value
573-
assert(rest - 200 < 500, 'Rest - 200 should be below 500 ms but is: ' + (rest - 200));
594+
assert(rest - 250 < 500, 'Rest - 250 should be below 500 ms but is: ' + (rest - 250));
574595
assert(delayed);
575596
end();
576597
});
@@ -601,7 +622,7 @@ describe('connection tests', function () {
601622
var rest = Date.now() - time;
602623
assert(rest >= 998, '`rest` should be equal or above 1000 ms but is: ' + rest); // setTimeout might trigger early
603624
// Be on the safe side and accept 200ms above the original value
604-
assert(rest - 200 < 1000, '`rest` - 200 should be below 1000 ms but is: ' + (rest - 200));
625+
assert(rest - 250 < 1000, '`rest` - 250 should be below 1000 ms but is: ' + (rest - 250));
605626
assert(delayed);
606627
end();
607628
});

‎test/multi.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ describe("The 'multi' method", function () {
7373
describe('pipeline limit', function () {
7474

7575
it('do not exceed maximum string size', function (done) {
76-
this.timeout(30000); // Windows tests are horribly slow
76+
this.timeout(process.platform !== 'win32' ? 10000 : 35000); // Windows tests are horribly slow
7777
// Triggers a RangeError: Invalid string length if not handled properly
7878
client = redis.createClient();
7979
var multi = client.multi();

0 commit comments

Comments
 (0)