From 7762e31356241719a6f0720b9ade18722f6cd3d5 Mon Sep 17 00:00:00 2001 From: Frank Date: Tue, 11 Nov 2014 13:12:50 +0100 Subject: [PATCH 01/12] tests: add trace tests * app.instrument * app._callTracers * res.trace --- test/app.calltracers.js | 34 ++++++++++++++++++++++++++++++++++ test/app.instrument.js | 23 +++++++++++++++++++++++ test/res.trace.js | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 test/app.calltracers.js create mode 100644 test/app.instrument.js create mode 100644 test/res.trace.js diff --git a/test/app.calltracers.js b/test/app.calltracers.js new file mode 100644 index 00000000000..420e513ba36 --- /dev/null +++ b/test/app.calltracers.js @@ -0,0 +1,34 @@ + +var express = require('../'); + +describe('app', function(){ + describe('._callTracers(res, event, args)', function(){ + it('should activate all tracers', function(){ + var app = express(); + app.id = 'app-1'; + var log = ''; + + function tracer(traceApp, req, res, event, date, args){ + (traceApp === app).should.be.ok; + req.url.should.be.equal('/'); + res.id.should.be.equal('1'); + event.should.be.equal('new:event'); + } + + function otherTracer(traceApp, req, res, event, date, args){ + log = event; + } + + app.instrument(tracer); + app.instrument(otherTracer); + + var res = Object.create(app.response); + res.req = Object.create(app.request); + res.req.url = '/'; + res.id = '1'; + app._callTracers(res, 'new:event', []); + + log.should.be.equal('new:event'); + }); + }) +}) diff --git a/test/app.instrument.js b/test/app.instrument.js new file mode 100644 index 00000000000..9fdba7b001a --- /dev/null +++ b/test/app.instrument.js @@ -0,0 +1,23 @@ + +var express = require('../'); + +describe('app', function(){ + describe('.instrument(fn)', function(){ + it('should add function to the tracer list', function(){ + var app = express(); + function debug (app, req, res, event, args) { + console.log(event); + } + app.instrument(debug); + debug.should.be.equal(app.tracers[0]); + }); + + it('should throw err if tracer is not a function', function(){ + var app = express(); + + (function(){ + app.instrument('notatracer'); + }).should.throw('instrument expects a function'); + }); + }) +}) diff --git a/test/res.trace.js b/test/res.trace.js new file mode 100644 index 00000000000..fa7441713d4 --- /dev/null +++ b/test/res.trace.js @@ -0,0 +1,35 @@ + +var express = require('../'); +var request = require('supertest'); + +describe('res', function(){ + describe('.trace(event)', function(){ + it('should activate all tracers set at application level', function(done){ + var app = express(); + + app.instrument(function(traceApp, req, res, event, date, args){ + (traceApp === traceApp).should.be.ok; + req.url.should.be.equal('/'); + res.id.should.be.equal('1'); + 'one:event'.should.be.equal(event); + 'info'.should.be.equal(args[0]); + }); + + app.use(function(req, res, next){ + res.id = '1'; + res.trace('one:event', 'info'); + next(); + }); + + app.get('/', function(req, res){ + res.send('ok'); + }); + + request(app) + .get('/') + .end(function(err, res) { + done(); + }); + }) + }) +}) From 94c12a8d8726771e6bf1bd73d2c7f196501d07ac Mon Sep 17 00:00:00 2001 From: Frank Date: Tue, 11 Nov 2014 13:13:33 +0100 Subject: [PATCH 02/12] add trace capabilities to express * allow to add tracer to the app level * activate tracer from the response object --- lib/application.js | 38 ++++++++++++++++++++++++++++++++++++++ lib/response.js | 26 ++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/lib/application.js b/lib/application.js index 0ee4def3890..a6451814645 100644 --- a/lib/application.js +++ b/lib/application.js @@ -57,6 +57,7 @@ app.init = function init() { this.cache = {}; this.engines = {}; this.settings = {}; + this.tracers = []; this.defaultConfiguration(); }; @@ -617,6 +618,43 @@ app.listen = function listen() { return server.listen.apply(server, arguments); }; +/* + * Set a tracer at the application level. Tracer will be activated on + * The tracer function must have the following signature: + * + * function (app, req, res, event, date, args) ... + * + * @param {function} The tracer to set. + * @return {app} for chaining. + * @public + */ + +app.instrument = function(tracer){ + if (tracer === undefined || typeof tracer !== 'function') + throw new Error('instrument expects a function'); + this.tracers.push(tracer); + return this; +}; + +/* + * Call all tracers set on the application. Add context information: + * running app, request, response and date. + * + * @param {Response} Response that fires the tracing event. + * @param {Event} The event to trace. + * @param {Array} Arguments to transmit to the tracker. + * + * @private + */ + +app._callTracers = function(res, event, args){ + var date = new Date(); + var app = this; + this.tracers.forEach(function(tracer){ + tracer(app, res.req, res, event, date, args); + }); +}; + /** * Log error using console.error. * diff --git a/lib/response.js b/lib/response.js index 6128f450a94..10c7589f07d 100644 --- a/lib/response.js +++ b/lib/response.js @@ -1051,6 +1051,32 @@ function sendfile(res, file, options, callback) { file.pipe(res); } +/** + * Run every tracers set at the application level for given events + * and arguments. To set a tracer on the application use the `instrument` + * method. + * + * @param {function} event The event to trace. + * @return {ServerResponse} for chaining + * @public + */ + +res.trace = function(event) { + var args = []; + switch (arguments.length) { + case 0: + throw new Error('No event defined!'); + case 1: break; + case 2: + args = [arguments[1]]; + break; + default: + args = [].slice.call(arguments, 1); + } + this.app._callTracers(this, event, args); + return this; +}; + /** * Stringify JSON, like JSON.stringify, but v8 optimized. * @private From 1526713144c439f8538a1a52a8b3aa53a19aa877 Mon Sep 17 00:00:00 2001 From: Frank Date: Tue, 11 Nov 2014 13:15:45 +0100 Subject: [PATCH 03/12] examples: add trace example --- examples/trace/index.js | 47 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 examples/trace/index.js diff --git a/examples/trace/index.js b/examples/trace/index.js new file mode 100644 index 00000000000..5ae8cb3261f --- /dev/null +++ b/examples/trace/index.js @@ -0,0 +1,47 @@ +var debug = require('debug')('trace-example'); +var express = require('../../'); + + +function random() { + return Math.random().toString(36).slice(2); +}; + + +var app = express(); + +app.use(function (req, res, next) { + res.start = Date.now() + res.id = random(); + res.trace('start', new Date()); + next(); +}); + +app.use(function (req, res, next) { + res.trace('wait:before'); + setTimeout(function () { + res.trace('wait:after'); + next(); + }, Math.random() * 100); +}) + +app.get('/', function (req, res, next) { + res.trace('user.id', random()); + res.trace('some.event', random(), random()); + res.trace('another.event', random(), random()); + res.set('X-Response-Time', (Date.now() - res.start) + 'ms'); + res.send('Hello world!'); + next(); +}); + +app.use(function (req, res, next) { + res.trace('finish'); +}); + +app.instrument(function (app, req, res, event, date, args) { + debug(res.id, event, args.join(' ')); +}); + +var port = process.env.PORT || 3210; + +app.listen(port); +console.log('Trace example for Express listening on port %s', port); From 46a64696ab294ca3beddbd6ec371ab148ae04aa8 Mon Sep 17 00:00:00 2001 From: Frank Date: Thu, 15 Jan 2015 15:41:48 +0100 Subject: [PATCH 04/12] improve _callTracers function performances _callTrackers runs a for loop on the application tracer list. Because tracing should impact performances as least as possible, this commit optimizes this for loop. Opitimization performed: * Usage of traditional for loop instead of forEach * Caching of tracer array length that is used to test the end of the loop. Here are the benchmarks that inspired these optimizations: * http://jsperf.com/array-foreach-vs-for-loop * http://jsperf.com/for-vs-foreacay --- lib/application.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/application.js b/lib/application.js index a6451814645..8e7581198ba 100644 --- a/lib/application.js +++ b/lib/application.js @@ -650,9 +650,13 @@ app.instrument = function(tracer){ app._callTracers = function(res, event, args){ var date = new Date(); var app = this; - this.tracers.forEach(function(tracer){ + + var tracers = this.tracers; + var length = tracers.length; + for (var i = 0; i < length; i++) { + var tracer = tracers[i]; tracer(app, res.req, res, event, date, args); - }); + } }; /** From 38ee25b00fdc6c9ac434962151c5f65cddaacd04 Mon Sep 17 00:00:00 2001 From: Frank Date: Thu, 15 Jan 2015 16:32:29 +0100 Subject: [PATCH 05/12] change call arguments of instrument tracer. Tracers instrumented at application level now handles a single option object as argument. --- examples/trace/index.js | 4 ++-- lib/application.js | 31 ++++++++++++++++++++++++------- test/app.calltracers.js | 20 +++++++++++--------- test/app.instrument.js | 4 ++-- test/res.trace.js | 18 ++++++++---------- 5 files changed, 47 insertions(+), 30 deletions(-) diff --git a/examples/trace/index.js b/examples/trace/index.js index 5ae8cb3261f..c2e1dbb940e 100644 --- a/examples/trace/index.js +++ b/examples/trace/index.js @@ -37,8 +37,8 @@ app.use(function (req, res, next) { res.trace('finish'); }); -app.instrument(function (app, req, res, event, date, args) { - debug(res.id, event, args.join(' ')); +app.instrument(function (options) { + debug(options.res.id, options.event, options.args.join(' ')); }); var port = process.env.PORT || 3210; diff --git a/lib/application.js b/lib/application.js index 8e7581198ba..8be9b6b8001 100644 --- a/lib/application.js +++ b/lib/application.js @@ -619,10 +619,17 @@ app.listen = function listen() { }; /* - * Set a tracer at the application level. Tracer will be activated on - * The tracer function must have the following signature: - * - * function (app, req, res, event, date, args) ... + * Set a tracer at the application level. Tracer will be activated when a + * tracing call will be fired by a response. + * The tracer function takes an option object as argument with the given + * attributes: + * + * - app: current application + * - res: response that fires the tracer + * - req: request related to above response + * - event: string describing the event to trace + * - date: when the tracing occurs + * - args: allow to freely add information * * @param {function} The tracer to set. * @return {app} for chaining. @@ -638,7 +645,8 @@ app.instrument = function(tracer){ /* * Call all tracers set on the application. Add context information: - * running app, request, response and date. + * running app, request, response and date. The date is the when the tracing + * was fired (now). * * @param {Response} Response that fires the tracing event. * @param {Event} The event to trace. @@ -650,12 +658,21 @@ app.instrument = function(tracer){ app._callTracers = function(res, event, args){ var date = new Date(); var app = this; - var tracers = this.tracers; var length = tracers.length; + + var options = { + app: app, + res: res, + req: res.req, + event: event, + date: date, + args: args + }; + for (var i = 0; i < length; i++) { var tracer = tracers[i]; - tracer(app, res.req, res, event, date, args); + tracer(options); } }; diff --git a/test/app.calltracers.js b/test/app.calltracers.js index 420e513ba36..f572086a992 100644 --- a/test/app.calltracers.js +++ b/test/app.calltracers.js @@ -2,21 +2,23 @@ var express = require('../'); describe('app', function(){ - describe('._callTracers(res, event, args)', function(){ + describe('._callTracers(options)', function(){ it('should activate all tracers', function(){ var app = express(); app.id = 'app-1'; var log = ''; - function tracer(traceApp, req, res, event, date, args){ - (traceApp === app).should.be.ok; - req.url.should.be.equal('/'); - res.id.should.be.equal('1'); - event.should.be.equal('new:event'); + function tracer(options){ + (options.app === app).should.be.ok; + options.req.url.should.be.equal('/'); + options.res.id.should.be.equal('1'); + options.event.should.be.equal('new:event'); + options.date.should.exist; + (options.args[0] === 'arg1').should.be.ok } - function otherTracer(traceApp, req, res, event, date, args){ - log = event; + function otherTracer(options){ + log = options.event; } app.instrument(tracer); @@ -26,7 +28,7 @@ describe('app', function(){ res.req = Object.create(app.request); res.req.url = '/'; res.id = '1'; - app._callTracers(res, 'new:event', []); + app._callTracers(res, 'new:event', ['arg1']); log.should.be.equal('new:event'); }); diff --git a/test/app.instrument.js b/test/app.instrument.js index 9fdba7b001a..6a3994aecc1 100644 --- a/test/app.instrument.js +++ b/test/app.instrument.js @@ -5,8 +5,8 @@ describe('app', function(){ describe('.instrument(fn)', function(){ it('should add function to the tracer list', function(){ var app = express(); - function debug (app, req, res, event, args) { - console.log(event); + function debug (options) { + console.log(options.event); } app.instrument(debug); debug.should.be.equal(app.tracers[0]); diff --git a/test/res.trace.js b/test/res.trace.js index fa7441713d4..903a474bdee 100644 --- a/test/res.trace.js +++ b/test/res.trace.js @@ -4,15 +4,15 @@ var request = require('supertest'); describe('res', function(){ describe('.trace(event)', function(){ - it('should activate all tracers set at application level', function(done){ + it('should call all tracers set at application level', function(done){ var app = express(); - app.instrument(function(traceApp, req, res, event, date, args){ - (traceApp === traceApp).should.be.ok; - req.url.should.be.equal('/'); - res.id.should.be.equal('1'); - 'one:event'.should.be.equal(event); - 'info'.should.be.equal(args[0]); + app.instrument(function(options){ + (options.app === app).should.be.ok; + options.req.url.should.be.equal('/'); + options.res.id.should.be.equal('1'); + 'one:event'.should.be.equal(options.event); + 'info'.should.be.equal(options.args[0]); }); app.use(function(req, res, next){ @@ -27,9 +27,7 @@ describe('res', function(){ request(app) .get('/') - .end(function(err, res) { - done(); - }); + .expect(200, done); }) }) }) From fc978b776992c3d541d2d1ec814da918487e272e Mon Sep 17 00:00:00 2001 From: Frank Date: Wed, 21 Jan 2015 23:36:53 +0100 Subject: [PATCH 06/12] code style improvement for tracer module --- examples/trace/index.js | 34 ++++++++++++++++++++-------------- lib/application.js | 7 +++---- test/app.calltracers.js | 2 +- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/examples/trace/index.js b/examples/trace/index.js index c2e1dbb940e..f5d143fa9ac 100644 --- a/examples/trace/index.js +++ b/examples/trace/index.js @@ -1,22 +1,18 @@ var debug = require('debug')('trace-example'); var express = require('../../'); - -function random() { - return Math.random().toString(36).slice(2); -}; - - var app = express(); -app.use(function (req, res, next) { +// Add a middleware that runs trace function. +app.use(function(req, res, next){ res.start = Date.now() res.id = random(); res.trace('start', new Date()); next(); }); -app.use(function (req, res, next) { +// Add another middleware that runs trace after an asynchronous call. +app.use(function(req, res, next){ res.trace('wait:before'); setTimeout(function () { res.trace('wait:after'); @@ -24,7 +20,8 @@ app.use(function (req, res, next) { }, Math.random() * 100); }) -app.get('/', function (req, res, next) { +// Add a route to show request traces. +app.get('/', function(req, res, next){ res.trace('user.id', random()); res.trace('some.event', random(), random()); res.trace('another.event', random(), random()); @@ -33,15 +30,24 @@ app.get('/', function (req, res, next) { next(); }); -app.use(function (req, res, next) { +// Add an after request processing middleware that runs trace. +app.use(function(req, res, next){ res.trace('finish'); }); -app.instrument(function (options) { +// Configure tracer. +app.instrument(function(options){ debug(options.res.id, options.event, options.args.join(' ')); }); -var port = process.env.PORT || 3210; +// Helpers to generate random ids. +function random(){ + return Math.random().toString(36).slice(2); +}; + -app.listen(port); -console.log('Trace example for Express listening on port %s', port); +/* istanbul ignore next */ +if (!module.parent) { + app.listen(3000); + console.log('Express started on port 3000'); +} diff --git a/lib/application.js b/lib/application.js index 8be9b6b8001..64306039c42 100644 --- a/lib/application.js +++ b/lib/application.js @@ -637,8 +637,8 @@ app.listen = function listen() { */ app.instrument = function(tracer){ - if (tracer === undefined || typeof tracer !== 'function') - throw new Error('instrument expects a function'); + if (tracer === undefined || typeof tracer !== 'function') + throw new TypeError('instrument expects a function'); this.tracers.push(tracer); return this; }; @@ -671,8 +671,7 @@ app._callTracers = function(res, event, args){ }; for (var i = 0; i < length; i++) { - var tracer = tracers[i]; - tracer(options); + tracers[i](options); } }; diff --git a/test/app.calltracers.js b/test/app.calltracers.js index f572086a992..6f3611f1475 100644 --- a/test/app.calltracers.js +++ b/test/app.calltracers.js @@ -14,7 +14,7 @@ describe('app', function(){ options.res.id.should.be.equal('1'); options.event.should.be.equal('new:event'); options.date.should.exist; - (options.args[0] === 'arg1').should.be.ok + (options.args[0] === 'arg1').should.be.ok; } function otherTracer(options){ From 2abd43c4075eb3d4a3728129b08f82a277d1a839 Mon Sep 17 00:00:00 2001 From: Frank Date: Sun, 8 Mar 2015 23:07:08 +0100 Subject: [PATCH 07/12] test for parent tracer firing --- test/res.trace.js | 64 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/test/res.trace.js b/test/res.trace.js index 903a474bdee..ab237be473a 100644 --- a/test/res.trace.js +++ b/test/res.trace.js @@ -30,4 +30,66 @@ describe('res', function(){ .expect(200, done); }) }) -}) + + describe('.trace(event) with parents', function(){ + it('should call parent tracers too', function(done){ + + var app = express(); + var appParent1 = express(); + var appParent2 = express(); + + var parentTraces1 = []; + var parentTraces2 = []; + + appParent1.instrument(function(options){ + options.res.id.should.be.equal('1'); + parentTraces1.push(options.event + ':parent1'); + }); + + appParent2.instrument(function(options){ + options.res.id.should.be.equal('1'); + parentTraces2.push(options.event + ':parent2'); + }); + + appParent1.use('/childof1', app); + appParent2.use('/childof2', app); + app.instrument(function(options){ + 'one:event'.should.be.equal(options.event); + 'info'.should.be.equal(options.args[0]); + }); + + app.use(function(req, res, next){ + res.id = '1'; + next(); + }); + + app.get('/', function(req, res){ + res.trace('one:event', 'info'); + res.send('ok'); + }); + + request(appParent1) + .get('/childof1') + .expect(200, function(){ + + parentTraces1.length.should.be.equal(1); + parentTraces2.length.should.be.equal(1); + parentTraces1[0].should.be.equal('one:event:parent1'); + parentTraces2[0].should.be.equal('one:event:parent2'); + + request(appParent2) + .get('/childof2') + .expect(200, function(){ + + parentTraces1.length.should.be.equal(2); + parentTraces2.length.should.be.equal(2); + parentTraces1[0].should.be.equal('one:event:parent1'); + parentTraces2[0].should.be.equal('one:event:parent2'); + parentTraces1[1].should.be.equal('one:event:parent1'); + parentTraces2[1].should.be.equal('one:event:parent2'); + done(); + }); + }); + }); + }) +}); From 786785b77061bffff90c7b710475af24a35e7ff5 Mon Sep 17 00:00:00 2001 From: Frank Date: Sun, 8 Mar 2015 23:08:26 +0100 Subject: [PATCH 08/12] link subapp tracer to parent tracer The operation is performed when the subapp is mounted on the parent app. A tracer that calls the parent tracers is added to the sub app. --- lib/application.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/application.js b/lib/application.js index 64306039c42..35eea5d0da6 100644 --- a/lib/application.js +++ b/lib/application.js @@ -99,6 +99,10 @@ app.defaultConfiguration = function defaultConfiguration() { this.response.__proto__ = parent.response; this.engines.__proto__ = parent.engines; this.settings.__proto__ = parent.settings; + + this.instrument(function callParentTracer(options) { + parent._callTracers(options.res, options.event, options.args); + }); }); // setup locals From f48f54b610a5bee42f2826181785ac5ad622e6a9 Mon Sep 17 00:00:00 2001 From: Frank Rousseau Date: Fri, 11 Nov 2016 21:14:11 +0100 Subject: [PATCH 09/12] Proper usage of supertest expect in tracing tests Supertest lib provides a method called expect to test the result of a request. This method requires a callback that takes an error object as first parametr. If this error object is not inspected, the validation performed by expect are not checked. Tracing tests didn't do that checking. This commit add checking to trace tests to ensure that no error is returned when expect method is called. --- test/res.trace.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/res.trace.js b/test/res.trace.js index ab237be473a..8c927d07c89 100644 --- a/test/res.trace.js +++ b/test/res.trace.js @@ -70,7 +70,9 @@ describe('res', function(){ request(appParent1) .get('/childof1') - .expect(200, function(){ + .expect(200, function(err){ + + if (err) return done(err); parentTraces1.length.should.be.equal(1); parentTraces2.length.should.be.equal(1); @@ -79,7 +81,9 @@ describe('res', function(){ request(appParent2) .get('/childof2') - .expect(200, function(){ + .expect(200, function(err){ + + if (err) return done(err); parentTraces1.length.should.be.equal(2); parentTraces2.length.should.be.equal(2); From aed9cb20859a7b6d33b85561980dc1937882ab1b Mon Sep 17 00:00:00 2001 From: Frank Rousseau Date: Fri, 11 Nov 2016 21:38:18 +0100 Subject: [PATCH 10/12] In tracing tests, replace should by assert --- test/res.trace.js | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/test/res.trace.js b/test/res.trace.js index 8c927d07c89..c21366177b1 100644 --- a/test/res.trace.js +++ b/test/res.trace.js @@ -1,6 +1,7 @@ var express = require('../'); var request = require('supertest'); +var assert = require('assert'); describe('res', function(){ describe('.trace(event)', function(){ @@ -8,11 +9,11 @@ describe('res', function(){ var app = express(); app.instrument(function(options){ - (options.app === app).should.be.ok; - options.req.url.should.be.equal('/'); - options.res.id.should.be.equal('1'); - 'one:event'.should.be.equal(options.event); - 'info'.should.be.equal(options.args[0]); + assert.equal(options.app, app) + assert.equal(options.req.url, '/'); + assert.equal(options.res.id, '1'); + assert.equal(options.event, 'one:event'); + assert.equal(options.args[0], 'info'); }); app.use(function(req, res, next){ @@ -42,24 +43,24 @@ describe('res', function(){ var parentTraces2 = []; appParent1.instrument(function(options){ - options.res.id.should.be.equal('1'); + assert.equal(options.req.id, '1'); parentTraces1.push(options.event + ':parent1'); }); appParent2.instrument(function(options){ - options.res.id.should.be.equal('1'); + assert.equal(options.req.id, '1'); parentTraces2.push(options.event + ':parent2'); }); appParent1.use('/childof1', app); appParent2.use('/childof2', app); app.instrument(function(options){ - 'one:event'.should.be.equal(options.event); - 'info'.should.be.equal(options.args[0]); + assert.equal(options.event, 'one:event'); + assert.equal(options.args[0], 'info'); }); app.use(function(req, res, next){ - res.id = '1'; + req.id = '1'; next(); }); @@ -71,26 +72,24 @@ describe('res', function(){ request(appParent1) .get('/childof1') .expect(200, function(err){ - if (err) return done(err); - parentTraces1.length.should.be.equal(1); - parentTraces2.length.should.be.equal(1); - parentTraces1[0].should.be.equal('one:event:parent1'); - parentTraces2[0].should.be.equal('one:event:parent2'); + assert.equal(parentTraces1.length, 1); + assert.equal(parentTraces2.length, 1); + assert.equal(parentTraces1[0], 'one:event:parent1'); + assert.equal(parentTraces2[0], 'one:event:parent2'); request(appParent2) .get('/childof2') .expect(200, function(err){ - if (err) return done(err); - parentTraces1.length.should.be.equal(2); - parentTraces2.length.should.be.equal(2); - parentTraces1[0].should.be.equal('one:event:parent1'); - parentTraces2[0].should.be.equal('one:event:parent2'); - parentTraces1[1].should.be.equal('one:event:parent1'); - parentTraces2[1].should.be.equal('one:event:parent2'); + assert.equal(parentTraces1.length, 2); + assert.equal(parentTraces2.length, 2); + assert.equal(parentTraces1[0], 'one:event:parent1'); + assert.equal(parentTraces2[0], 'one:event:parent2'); + assert.equal(parentTraces1[1], 'one:event:parent1'); + assert.equal(parentTraces2[1], 'one:event:parent2'); done(); }); }); From 6b7ee9cb4126c7757624de6f6cb8c4a3cc916f5a Mon Sep 17 00:00:00 2001 From: Frank Rousseau Date: Fri, 11 Nov 2016 21:39:36 +0100 Subject: [PATCH 11/12] In trace function, do not init args to empty array --- lib/response.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/response.js b/lib/response.js index 10c7589f07d..73393606ffd 100644 --- a/lib/response.js +++ b/lib/response.js @@ -1062,11 +1062,13 @@ function sendfile(res, file, options, callback) { */ res.trace = function(event) { - var args = []; + var args; switch (arguments.length) { case 0: throw new Error('No event defined!'); - case 1: break; + case 1: + args = []; + break; case 2: args = [arguments[1]]; break; From 72b57aeaf5a06c53f60dee25397cd6202293f067 Mon Sep 17 00:00:00 2001 From: Frank Rousseau Date: Fri, 11 Nov 2016 21:40:32 +0100 Subject: [PATCH 12/12] Remove _callTracers tests Private methods do not need to be tested --- test/app.calltracers.js | 36 ------------------------------------ 1 file changed, 36 deletions(-) delete mode 100644 test/app.calltracers.js diff --git a/test/app.calltracers.js b/test/app.calltracers.js deleted file mode 100644 index 6f3611f1475..00000000000 --- a/test/app.calltracers.js +++ /dev/null @@ -1,36 +0,0 @@ - -var express = require('../'); - -describe('app', function(){ - describe('._callTracers(options)', function(){ - it('should activate all tracers', function(){ - var app = express(); - app.id = 'app-1'; - var log = ''; - - function tracer(options){ - (options.app === app).should.be.ok; - options.req.url.should.be.equal('/'); - options.res.id.should.be.equal('1'); - options.event.should.be.equal('new:event'); - options.date.should.exist; - (options.args[0] === 'arg1').should.be.ok; - } - - function otherTracer(options){ - log = options.event; - } - - app.instrument(tracer); - app.instrument(otherTracer); - - var res = Object.create(app.response); - res.req = Object.create(app.request); - res.req.url = '/'; - res.id = '1'; - app._callTracers(res, 'new:event', ['arg1']); - - log.should.be.equal('new:event'); - }); - }) -})