From b6c317dbb0625ba4fe8b3c39d26bdce80ec007bc Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 30 May 2017 06:45:28 +0200 Subject: [PATCH] chore: improve coverage --- index.js | 11 +++++++---- lib/createClient.js | 1 + lib/multi.js | 11 ++++------- lib/writeCommands.js | 2 +- test/commands/client.spec.js | 16 ++++++++++++++++ test/connection.spec.js | 14 ++++++++++++++ test/multi.spec.js | 3 +++ test/node_redis.spec.js | 36 ++++++++++++++++++++++++++++++++++++ 8 files changed, 82 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index 54df396863..356e9b5775 100644 --- a/index.js +++ b/index.js @@ -38,6 +38,7 @@ class RedisClient extends EventEmitter { // TODO: Add a more restrictive options validation const cnxOptions = {} for (const tlsOption in options.tls) { + /* istanbul ignore else */ if (options.tls.hasOwnProperty(tlsOption)) { cnxOptions[tlsOption] = options.tls[tlsOption] // Copy the tls options into the general options to make sure the address is set right @@ -56,7 +57,7 @@ class RedisClient extends EventEmitter { } else { cnxOptions.port = +options.port || 6379 cnxOptions.host = options.host || '127.0.0.1' - cnxOptions.family = (!options.family && net.isIP(cnxOptions.host)) || (options.family === 'IPv6' ? 6 : 4) + cnxOptions.family = (!options.family && net.isIP(cnxOptions.host)) || (options.host && options.family === 'IPv6' ? 6 : 4) this.address = `${cnxOptions.host}:${cnxOptions.port}` } // TODO: Properly fix typo @@ -64,6 +65,7 @@ class RedisClient extends EventEmitter { options.socketKeepalive = true } for (const command in options.renameCommands) { + /* istanbul ignore else */ if (options.renameCommands.hasOwnProperty(command)) { options.renameCommands[command.toLowerCase()] = options.renameCommands[command] } @@ -234,7 +236,7 @@ class RedisClient extends EventEmitter { this._stream.unref() } else { debug('Not connected yet, will unref later') - this.once('connect', () => this._stream.unref()) + this.once('connect', () => this.unref()) } } @@ -251,6 +253,7 @@ class RedisClient extends EventEmitter { const existingOptions = utils.clone(this._options) options = utils.clone(options) for (const elem in options) { + /* istanbul ignore else */ if (options.hasOwnProperty(elem)) { existingOptions[elem] = options[elem] } @@ -280,11 +283,11 @@ class RedisClient extends EventEmitter { // Note: this overrides a native function! multi (args) { - return new Multi(this, args, 'multi') + return new Multi(this, 'multi', args) } batch (args) { - return new Multi(this, args, 'batch') + return new Multi(this, 'batch', args) } } diff --git a/lib/createClient.js b/lib/createClient.js index 820dc93a7f..ce1e77b4e4 100644 --- a/lib/createClient.js +++ b/lib/createClient.js @@ -43,6 +43,7 @@ function unifyOptions (portArg, hostArg, options) { } if (parsed.search !== '') { for (var elem in parsed.query) { + /* istanbul ignore if */ if (!Object.prototype.hasOwnProperty.call(parsed.query, elem)) { continue } diff --git a/lib/multi.js b/lib/multi.js index 4d2c3ab714..f55986cd03 100644 --- a/lib/multi.js +++ b/lib/multi.js @@ -80,10 +80,7 @@ function execTransaction (multi) { err.command = 'EXEC' err.code = 'EXECABORT' return new Promise((resolve, reject) => { - utils.replyInOrder(client, (err, res) => { - if (err) return reject(err) - resolve(res) - }, null, []) + utils.replyInOrder(client, reject, err) }) } const len = queue.length @@ -145,14 +142,14 @@ class Multi { /** * Creates an instance of Multi. * @param {RedisClient} client - * @param {any[]} [args] * @param {string} [type] + * @param {any[]} [args] * * @memberof Multi */ - constructor (client, args, type) { + constructor (client, type, args) { this._client = client - this._type = typeof type === 'string' ? type : 'multi' + this._type = type this._queue = new Queue() // Either undefined or an array. Fail hard if it's not an array if (args) { diff --git a/lib/writeCommands.js b/lib/writeCommands.js index 6eab6c3c17..2999e77315 100644 --- a/lib/writeCommands.js +++ b/lib/writeCommands.js @@ -112,7 +112,7 @@ function toString (arg) { function returnErr (client, command) { const err = new TypeError('NodeRedis can not handle the provided arguments (see "error.issues" property).\n\nFurther information https://github.com/asd') - err.command = command.name.toUpperCase() + err.command = command.command.toUpperCase() err.args = command.args err.issues = errors errors = null diff --git a/test/commands/client.spec.js b/test/commands/client.spec.js index 5ad65c36c4..6f21e1869c 100644 --- a/test/commands/client.spec.js +++ b/test/commands/client.spec.js @@ -71,6 +71,14 @@ describe('The \'client\' method', () => { promises.push(client.get('foo').then(helper.isString('bar'))) return Promise.all(promises) }) + + it('weird', function () { + helper.serverVersionAtLeast.call(this, client, [3, 2, 0]) + assert.strictEqual(client._reply, 'ON') + const promise = client.client('REPLY', 'WEIRD').then(helper.fail, helper.isError()) + assert.strictEqual(client._reply, 'ON') + return promise + }) }) describe('in a batch context', () => { @@ -112,6 +120,14 @@ describe('The \'client\' method', () => { assert.deepStrictEqual(res, ['OK', undefined, undefined, 'bar']) }) }) + + it('weird', function () { + helper.serverVersionAtLeast.call(this, client, [3, 2, 0]) + assert.strictEqual(client._reply, 'ON') + const promise = client.batch().client('REPLY', 'WEIRD').exec().then(helper.fail, helper.isError()) + assert.strictEqual(client._reply, 'ON') + return promise + }) }) }) diff --git a/test/connection.spec.js b/test/connection.spec.js index d14788c5a5..6a5d3c297b 100644 --- a/test/connection.spec.js +++ b/test/connection.spec.js @@ -413,6 +413,20 @@ describe('connection tests', () => { }) }) + it('connects correctly even if the info command returns a empty string', (done) => { + client = Redis.createClient.apply(null, args) + const end = helper.callFuncAfter(done, 2) + client.info = function () { + // Mock the result + end() + return Promise.resolve('') + } + client.once('ready', () => { + assert.strictEqual(Object.keys(client.serverInfo).length, 0) + end() + }) + }) + if (ip === 'IPv4') { it('allows connecting with the redis url to the default host and port, select db 3 and warn about duplicate db option', (done) => { client = Redis.createClient('redis:///3?db=3') diff --git a/test/multi.spec.js b/test/multi.spec.js index b3a0422d1d..da55c61606 100644 --- a/test/multi.spec.js +++ b/test/multi.spec.js @@ -198,9 +198,12 @@ describe('The \'multi\' method', () => { client = redis.createClient({ host: 'somewhere', port: 6379, + family: 'IPv6', retryStrategy () {} }) + assert.strictEqual(client._connectionOptions.family, 6) + client.on('error', (err) => { assert.strictEqual(err.code, 'NR_CLOSED') done() diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index 6885383099..2910d37652 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -57,6 +57,26 @@ describe('The nodeRedis client', () => { client._stream.destroy() }) + describe('arguments validation', () => { + it('no boolean for enableOfflineQueue', () => { + try { + redis.createClient({ enableOfflineQueue: 'test' }) + throw new Error('failed') + } catch (err) { + assert.strictEqual(err.name, 'TypeError') + } + }) + + it('no boolean for enableOfflineQueue', () => { + try { + redis.createClient({ password: 'test', authPass: 'foobar' }) + throw new Error('failed') + } catch (err) { + assert.strictEqual(err.name, 'TypeError') + } + }) + }) + helper.allTests((ip, args) => { describe(`using ${ip}`, () => { afterEach(() => { @@ -69,6 +89,22 @@ describe('The nodeRedis client', () => { return client.flushdb() }) + describe('argument errors', () => { + it('should return an error in case of "null" argument', () => { + return client.set('foo', null).then(helper.fail, (err) => { + assert.deepStrictEqual(err.issues, [null]) + }) + }) + + it('should return an error in case of unknown types', () => { + class Foo {} + const tmp = new Foo() + return client.set(tmp, undefined).then(helper.fail, (err) => { + assert.deepStrictEqual(err.issues, [tmp, undefined]) + }) + }) + }) + describe('duplicate', () => { it('check if all options got copied properly', (done) => { client.selectedDb = 2