From e8aec5822210403de0a3d189649213da4bb84285 Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Fri, 8 Aug 2014 18:15:00 -0700 Subject: [PATCH 1/5] test: add test for instanceof ctor guard --- test/ctor.test.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/ctor.test.js b/test/ctor.test.js index a09749d..ce6f36d 100644 --- a/test/ctor.test.js +++ b/test/ctor.test.js @@ -53,6 +53,14 @@ test('ensure Logger creation options', function (t) { }); +test('ensure Logger constructor is safe without new', function (t) { + t.doesNotThrow(function () { Logger({name: 'foo'}); }, + 'constructor should call self with new if necessary'); + + t.end(); +}); + + test('ensure Logger creation options (createLogger)', function (t) { t.throws(function () { bunyan.createLogger(); }, 'options (object) is required', From 58df808325289fc931c39b3d9d5130690884bd26 Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Fri, 8 Aug 2014 18:15:55 -0700 Subject: [PATCH 2/5] Fix ctor instanceof guard The ! operator is higher priority than the instanceof operator, so the expression (! this instanceof X) is the same as ((!this) instanceof X), which will always evaluate to false. --- lib/bunyan.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bunyan.js b/lib/bunyan.js index cb7c298..9e93d76 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -233,7 +233,7 @@ function resolveLevel(nameOrNum) { */ function Logger(options, _childOptions, _childSimple) { xxx('Logger start:', options) - if (! this instanceof Logger) { + if (!(this instanceof Logger)) { return new Logger(options, _childOptions); } From e4ea3fb05f1fa0e962dc62582ae29d898d16cbc7 Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Fri, 8 Aug 2014 18:17:14 -0700 Subject: [PATCH 3/5] test: ensure parent type guard is working --- test/ctor.test.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/ctor.test.js b/test/ctor.test.js index ce6f36d..2435497 100644 --- a/test/ctor.test.js +++ b/test/ctor.test.js @@ -132,3 +132,17 @@ test('ensure Logger child() options', function (t) { t.end(); }); + + +test('ensure Logger() rejects non-Logger parents', function (t) { + var dad = new Logger({name: 'dad', streams: []}); + + t.throws(function () { new Logger({}, {}); }, + /invalid Logger creation: do not pass a second arg/, + 'Logger arguments must be valid'); + + t.doesNotThrow(function () { new Logger(dad, {}); }, + 'Logger allows Logger instance as parent'); + + t.end(); +}); From c3b8dd26a7fc8b32e6fb4ff42970933bb2c749ed Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Fri, 8 Aug 2014 18:17:37 -0700 Subject: [PATCH 4/5] Fix parent instanceof guard The ! operator is higher priority than the instanceof operator, so the expression (! this instanceof X) is the same as ((!this) instanceof X), which will always evaluate to false. --- lib/bunyan.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bunyan.js b/lib/bunyan.js index 9e93d76..ea4d02f 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -242,7 +242,7 @@ function Logger(options, _childOptions, _childSimple) { if (_childOptions !== undefined) { parent = options; options = _childOptions; - if (! parent instanceof Logger) { + if (!(parent instanceof Logger)) { throw new TypeError( 'invalid Logger creation: do not pass a second arg'); } From 6b44120d5380d74da4f72309f3fa24503f97be48 Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Fri, 8 Aug 2014 18:25:19 -0700 Subject: [PATCH 5/5] test: make throw tests validate was is thrown When the second argument to nodeunit's assert.throws() is a string it is used as the failure message, not for exception message matching. To assert the contents of the exception's message a RegExp must be used. --- test/ctor.test.js | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/test/ctor.test.js b/test/ctor.test.js index 2435497..57920e6 100644 --- a/test/ctor.test.js +++ b/test/ctor.test.js @@ -19,11 +19,11 @@ var test = tap4nodeunit.test; test('ensure Logger creation options', function (t) { t.throws(function () { new Logger(); }, - 'options (object) is required', + /options \(object\) is required/, 'no options should throw'); t.throws(function () { new Logger({}); }, - 'options.name (string) is required', + /options\.name \(string\) is required/, 'no options.name should throw'); t.doesNotThrow(function () { new Logger({name: 'foo'}); }, @@ -31,22 +31,23 @@ test('ensure Logger creation options', function (t) { var options = {name: 'foo', stream: process.stdout, streams: []}; t.throws(function () { new Logger(options); }, + /cannot mix "streams" and "stream" options/, 'cannot use "stream" and "streams"'); // https://github.com/trentm/node-bunyan/issues/3 options = {name: 'foo', streams: {}}; t.throws(function () { new Logger(options); }, - 'invalid options.streams: must be an array', + /invalid options.streams: must be an array/, '"streams" must be an array'); options = {name: 'foo', serializers: 'a string'}; t.throws(function () { new Logger(options); }, - 'invalid options.serializers: must be an object', + /invalid options.serializers: must be an object/, '"serializers" cannot be a string'); options = {name: 'foo', serializers: [1, 2, 3]}; t.throws(function () { new Logger(options); }, - 'invalid options.serializers: must be an object', + /invalid options.serializers: must be an object/, '"serializers" cannot be an array'); t.end(); @@ -63,11 +64,11 @@ test('ensure Logger constructor is safe without new', function (t) { test('ensure Logger creation options (createLogger)', function (t) { t.throws(function () { bunyan.createLogger(); }, - 'options (object) is required', + /options \(object\) is required/, 'no options should throw'); t.throws(function () { bunyan.createLogger({}); }, - 'options.name (string) is required', + /options\.name \(string\) is required/, 'no options.name should throw'); t.doesNotThrow(function () { bunyan.createLogger({name: 'foo'}); }, @@ -75,22 +76,23 @@ test('ensure Logger creation options (createLogger)', function (t) { var options = {name: 'foo', stream: process.stdout, streams: []}; t.throws(function () { bunyan.createLogger(options); }, + /cannot mix "streams" and "stream" options/, 'cannot use "stream" and "streams"'); // https://github.com/trentm/node-bunyan/issues/3 options = {name: 'foo', streams: {}}; t.throws(function () { bunyan.createLogger(options); }, - 'invalid options.streams: must be an array', + /invalid options.streams: must be an array/, '"streams" must be an array'); options = {name: 'foo', serializers: 'a string'}; t.throws(function () { bunyan.createLogger(options); }, - 'invalid options.serializers: must be an object', + /invalid options.serializers: must be an object/, '"serializers" cannot be a string'); options = {name: 'foo', serializers: [1, 2, 3]}; t.throws(function () { bunyan.createLogger(options); }, - 'invalid options.serializers: must be an object', + /invalid options.serializers: must be an object/, '"serializers" cannot be an array'); t.end(); @@ -107,27 +109,28 @@ test('ensure Logger child() options', function (t) { 'empty options should be fine too'); t.throws(function () { log.child({name: 'foo'}); }, - 'invalid options.name: child cannot set logger name', + /invalid options.name: child cannot set logger name/, 'child cannot change name'); var options = {stream: process.stdout, streams: []}; t.throws(function () { log.child(options); }, + /cannot mix "streams" and "stream" options/, 'cannot use "stream" and "streams"'); // https://github.com/trentm/node-bunyan/issues/3 options = {streams: {}}; t.throws(function () { log.child(options); }, - 'invalid options.streams: must be an array', + /invalid options.streams: must be an array/, '"streams" must be an array'); options = {serializers: 'a string'}; t.throws(function () { log.child(options); }, - 'invalid options.serializers: must be an object', + /invalid options.serializers: must be an object/, '"serializers" cannot be a string'); options = {serializers: [1, 2, 3]}; t.throws(function () { log.child(options); }, - 'invalid options.serializers: must be an object', + /invalid options.serializers: must be an object/, '"serializers" cannot be an array'); t.end();