From 363e02c195be160acbae56087702ff4886ee6dce Mon Sep 17 00:00:00 2001 From: Krasiyan Date: Mon, 9 Jan 2017 22:54:04 +0200 Subject: [PATCH 1/8] wip - use functions for routes moved building the final request url from the 'suites-manager' to the 'request-agent' wip fixing tests --- lib/request-agent.js | 5 ++++- lib/request-handler.js | 10 +++++++--- lib/suites-manager.js | 7 +++---- test/unit/request-agent.js | 7 +++++++ test/unit/request-handler.js | 8 +++++--- 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/lib/request-agent.js b/lib/request-agent.js index 596410d..2b7b2e2 100644 --- a/lib/request-agent.js +++ b/lib/request-agent.js @@ -1,6 +1,7 @@ 'use strict'; var _ = require('underscore'); +var url = require('url'); module.exports = function(agent){ this.agent = agent; @@ -12,9 +13,11 @@ module.exports = function(agent){ this.make = function(options, callback){ var data = evalIfFunction(options.data) || {}, query = evalIfFunction(options.query) || {}, + route = evalIfFunction(options.route) || '', headers = evalIfFunction(options.headers) || {}, method = options.method === 'delete' ? 'del' : options.method, - request = this.agent[method](options.url); + requestUrl = url.resolve(options.service, route), + request = this.agent[method](requestUrl); if(!_.isEmpty(data)) { request.send(data); diff --git a/lib/request-handler.js b/lib/request-handler.js index 75cfec4..e1dafee 100644 --- a/lib/request-handler.js +++ b/lib/request-handler.js @@ -34,9 +34,9 @@ module.exports = { } }); }, - setup: function(suiteName, suiteHref, suite, requestAgent){ + setup: function(suiteName, service, suite, requestAgent){ var self = this, - req = _.extend(_.clone(suite.endpoint), { url: suiteHref }), + req = _.extend(_.clone(suite.endpoint), { service: service }), suiteOptions = { expectedStatusCode: suite.endpoint.expectedStatusCode, maxMean: suite.endpoint.maxMean, @@ -45,6 +45,10 @@ module.exports = { }, suiteRequest = {}; + if(!!suite.endpoint.route){ + suiteRequest.route = _.isFunction(suite.endpoint.route) ? 'Dynamic route' : suite.endpoint.route; + } + if(!!suite.endpoint.headers){ suiteRequest.headers = _.isFunction(suite.endpoint.headers) ? 'Dynamic headers' : suite.endpoint.headers; } @@ -57,7 +61,7 @@ module.exports = { suiteRequest.query = _.isFunction(suite.endpoint.query) ? 'Dynamic query' : suite.endpoint.query; } - suite.runner.add(suiteName, suiteHref, suiteOptions, suiteRequest, function(done){ + suite.runner.add(suiteName, suiteRequest.route, suiteOptions, suiteRequest, function(done){ self.make(req, suite, suiteName, requestAgent, done); }); } diff --git a/lib/suites-manager.js b/lib/suites-manager.js index 3068cda..6030fcb 100644 --- a/lib/suites-manager.js +++ b/lib/suites-manager.js @@ -7,7 +7,7 @@ var ResultsHandler = require('./results-handler'); var Runner = require('./runner'); var sanitise = require('./sanitise'); var settings = require('./settings'); -var url = require('url'); + var validator = require('./validator'); var _ = require('underscore'); @@ -45,11 +45,10 @@ module.exports = function(agent, debugHelper){ _.forEach(this.services, function(service, serviceName){ _.forEach(this.routes, function(route){ - var routeHref = url.resolve(service, route.endpoint.route), - routeName = serviceName + '/' + route.name, + var routeName = serviceName + '/' + route.name, self = this; - requestHandler.setup(routeName, routeHref, route, self.requestAgent); + requestHandler.setup(routeName, service, route, self.requestAgent); }, this); }, this); diff --git a/test/unit/request-agent.js b/test/unit/request-agent.js index 82a36f2..7bf4b1a 100644 --- a/test/unit/request-agent.js +++ b/test/unit/request-agent.js @@ -18,6 +18,7 @@ describe('requestAgent.make function', function(){ it('should correctly handle null data', function(done){ requestAgent.make({ + service: 'root', route: '/post', method: 'post', data: null @@ -29,6 +30,7 @@ describe('requestAgent.make function', function(){ it('should correctly handle undefined data', function(done){ requestAgent.make({ + service: 'root', route: '/post', method: 'post', data: undefined @@ -48,6 +50,7 @@ describe('requestAgent.make function', function(){ }; var request = { + service: 'root', route: '/post', method: 'post', data: dataFunc @@ -72,6 +75,7 @@ describe('requestAgent.make function', function(){ }; var request = { + service: 'root', route: '/get', method: 'get', query: queryFunc @@ -88,6 +92,7 @@ describe('requestAgent.make function', function(){ it('should correctly handle cookies', function(done){ requestAgent.make({ + service: 'root', route: '/get', method: 'get', headers: { @@ -101,6 +106,7 @@ describe('requestAgent.make function', function(){ it('should correctly handle headers', function(done){ requestAgent.make({ + service: 'root', route: '/get', method: 'get', headers: { @@ -124,6 +130,7 @@ describe('requestAgent.make function', function(){ }; var request = { + service: 'root', route: '/get', method: 'get', headers: headersFunc diff --git a/test/unit/request-handler.js b/test/unit/request-handler.js index 1e3815a..185b534 100644 --- a/test/unit/request-handler.js +++ b/test/unit/request-handler.js @@ -17,7 +17,8 @@ describe('requestHandler.setup function', function(){ var suiteObj = { endpoint: { - aProperty: 'value' + aProperty: 'value', + route: 'someRoute' }, runner: { add: function(suiteName, suiteHref, suiteOptions, suiteRequest, callback){ @@ -26,11 +27,12 @@ describe('requestHandler.setup function', function(){ } }; - requestHandler.setup('suiteName', 'suiteHref', suiteObj, fakeAgent); + requestHandler.setup('suiteName', 'serviceRoot', suiteObj, fakeAgent); var req = fakeAgentStack[0][0]; - req.url.should.be.eql('suiteHref'); + req.route.should.be.eql('someRoute'); + req.service.should.be.eql('serviceRoot'); req.aProperty.should.be.eql('value'); done(); From 69beea28851dbb1286c36ffef79aa48bacb409da Mon Sep 17 00:00:00 2001 From: Krasiyan Date: Fri, 20 Jan 2017 17:23:41 +0200 Subject: [PATCH 2/8] added the service root to the 'Dynamic route' message in the request-hanlder --- lib/request-handler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/request-handler.js b/lib/request-handler.js index e1dafee..48fda9d 100644 --- a/lib/request-handler.js +++ b/lib/request-handler.js @@ -46,7 +46,7 @@ module.exports = { suiteRequest = {}; if(!!suite.endpoint.route){ - suiteRequest.route = _.isFunction(suite.endpoint.route) ? 'Dynamic route' : suite.endpoint.route; + suiteRequest.route = _.isFunction(suite.endpoint.route) ? 'Dynamic route on ' + service : suite.endpoint.route; } if(!!suite.endpoint.headers){ From 70c6eaa3b4b8cf34ccd894324b2f298247ce143b Mon Sep 17 00:00:00 2001 From: Krasiyan Date: Fri, 20 Jan 2017 17:40:39 +0200 Subject: [PATCH 3/8] added test on the request-handler handling dymanic routes (functions) --- test/unit/request-handler.js | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/unit/request-handler.js b/test/unit/request-handler.js index 185b534..b13b906 100644 --- a/test/unit/request-handler.js +++ b/test/unit/request-handler.js @@ -37,6 +37,41 @@ describe('requestHandler.setup function', function(){ done(); }); + + it('should properly handle dynamic routes', function(done){ + + var fakeAgentStack = []; + + var fakeAgent = { + make: function(){ + fakeAgentStack.push(arguments); + } + }; + + var suiteObj = { + endpoint: { + aProperty: 'value', + route: function () { + return 'someDynamicRoute'; + } + }, + runner: { + add: function(suiteName, suiteHref, suiteOptions, suiteRequest, callback){ + suiteRequest.route.should.be.eql('Dynamic route on serviceRoot'); + callback(); + } + } + }; + + requestHandler.setup('suiteName', 'serviceRoot', suiteObj, fakeAgent); + + var req = fakeAgentStack[0][0]; + req.route.should.be.a.Function(); + req.service.should.be.eql('serviceRoot'); + req.aProperty.should.be.eql('value'); + + done(); + }); }); describe('requestHandler.make function', function(){ From 7b5ed1041a1123539323c83f515bebb72e162893 Mon Sep 17 00:00:00 2001 From: Krasiyan Date: Fri, 20 Jan 2017 17:49:14 +0200 Subject: [PATCH 4/8] resolving the route on the service in 'request-handler' if the route is not a function --- lib/request-handler.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/request-handler.js b/lib/request-handler.js index 48fda9d..a0673f2 100644 --- a/lib/request-handler.js +++ b/lib/request-handler.js @@ -3,6 +3,7 @@ var format = require('./format'); var settings = require('./settings'); var _ = require('underscore'); +var url = require('url'); module.exports = { make: function(req, suite, suiteName, requestAgent, callback){ @@ -46,7 +47,7 @@ module.exports = { suiteRequest = {}; if(!!suite.endpoint.route){ - suiteRequest.route = _.isFunction(suite.endpoint.route) ? 'Dynamic route on ' + service : suite.endpoint.route; + suiteRequest.route = _.isFunction(suite.endpoint.route) ? 'Dynamic route on ' + service : url.resolve(service, suite.endpoint.route); } if(!!suite.endpoint.headers){ From eacec0cff719463825ae9f0ff172e4a0189687b8 Mon Sep 17 00:00:00 2001 From: Krasiyan Date: Fri, 20 Jan 2017 18:14:21 +0200 Subject: [PATCH 5/8] improved the 'request-agent' unit test to test the 'route as a function' scenario including the proper resolving of the route in the service --- lib/request-agent.js | 5 +++-- test/fixtures/test-agent.js | 9 ++++++--- test/unit/request-agent.js | 38 ++++++++++++++++++++++++++++++------- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/lib/request-agent.js b/lib/request-agent.js index 2b7b2e2..525a901 100644 --- a/lib/request-agent.js +++ b/lib/request-agent.js @@ -16,12 +16,13 @@ module.exports = function(agent){ route = evalIfFunction(options.route) || '', headers = evalIfFunction(options.headers) || {}, method = options.method === 'delete' ? 'del' : options.method, - requestUrl = url.resolve(options.service, route), + service = options.service || '', + requestUrl = url.resolve(service, route), request = this.agent[method](requestUrl); if(!_.isEmpty(data)) { request.send(data); - } + } if(!_.isEmpty(query)) { request.query(query); diff --git a/test/fixtures/test-agent.js b/test/fixtures/test-agent.js index 1b32916..6745b94 100644 --- a/test/fixtures/test-agent.js +++ b/test/fixtures/test-agent.js @@ -1,20 +1,23 @@ 'use strict'; -module.exports.FakeAgent = function(){ +module.exports.FakeAgent = function(){ this.end = function(callback){ callback(null, { + url: this.url, data: this.data, headers: this.headers, query: this.queryData }); }; - this.get = function(request){ + this.get = function(url){ + this.url = url; return this; }; - this.post = function(request){ + this.post = function(url){ + this.url = url; return this; }; diff --git a/test/unit/request-agent.js b/test/unit/request-agent.js index 7bf4b1a..72cb32a 100644 --- a/test/unit/request-agent.js +++ b/test/unit/request-agent.js @@ -18,7 +18,7 @@ describe('requestAgent.make function', function(){ it('should correctly handle null data', function(done){ requestAgent.make({ - service: 'root', + service: 'http://service/', route: '/post', method: 'post', data: null @@ -30,7 +30,7 @@ describe('requestAgent.make function', function(){ it('should correctly handle undefined data', function(done){ requestAgent.make({ - service: 'root', + service: 'http://service/', route: '/post', method: 'post', data: undefined @@ -40,6 +40,30 @@ describe('requestAgent.make function', function(){ }); }); + it('should correctly handle route as a function', function(done){ + + var i = 0; + + var routeFunc = function(){ + i++; + return '/endpoint'+i; + }; + + var request = { + service: 'http://service/', + route: routeFunc, + method: 'post' + }; + + requestAgent.make(request, function(err, fakeResults){ + fakeResults.url.should.be.eql('http://service/endpoint1'); + requestAgent.make(request, function(err, fakeResults){ + fakeResults.url.should.be.eql('http://service/endpoint2'); + done(); + }); + }); + }); + it('should correctly handle data as a function', function(done){ var i = 0; @@ -50,7 +74,7 @@ describe('requestAgent.make function', function(){ }; var request = { - service: 'root', + service: 'http://service/', route: '/post', method: 'post', data: dataFunc @@ -75,7 +99,7 @@ describe('requestAgent.make function', function(){ }; var request = { - service: 'root', + service: 'http://service/', route: '/get', method: 'get', query: queryFunc @@ -92,7 +116,7 @@ describe('requestAgent.make function', function(){ it('should correctly handle cookies', function(done){ requestAgent.make({ - service: 'root', + service: 'http://service/', route: '/get', method: 'get', headers: { @@ -106,7 +130,7 @@ describe('requestAgent.make function', function(){ it('should correctly handle headers', function(done){ requestAgent.make({ - service: 'root', + service: 'http://service/', route: '/get', method: 'get', headers: { @@ -130,7 +154,7 @@ describe('requestAgent.make function', function(){ }; var request = { - service: 'root', + service: 'http://service/', route: '/get', method: 'get', headers: headersFunc From d1a59a9dfc7fafcb036d73eefc258aee7986322e Mon Sep 17 00:00:00 2001 From: Krasiyan Date: Fri, 20 Jan 2017 18:15:58 +0200 Subject: [PATCH 6/8] updated README.md added note on the possible to have 'route' as a function --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4f3ec99..213e54c 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ apiBenchmark.measure(service, routes, function(err, results){ (String, default 'get'): Http verb. #### route - (String): the route to benchmark + (String): the route to benchmark. In case of function (that has to return an string) it will be evaulated for each request. #### headers (Object): the headers to send. In case of function (that has to return an object) it will be evaulated for each request. From 936d867804eb79148861201d4ecffcba07b28fc0 Mon Sep 17 00:00:00 2001 From: Krasiyan Date: Fri, 20 Jan 2017 18:29:35 +0200 Subject: [PATCH 7/8] updated the report template to display dynamic routes as text instead of hyperlinks --- templates/report.html | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/templates/report.html b/templates/report.html index de8b969..80b1f6b 100644 --- a/templates/report.html +++ b/templates/report.html @@ -224,9 +224,15 @@ '
  • 99% Percentile: ' + fixNumber(routeData.stats.p99, 6) + '
    • ' + '
  • 99.9% Percentile: ' + fixNumber(routeData.stats.p999, 6) + '
    • ' + '
    ' + - '
    Request: ' + routeData.options.method.toUpperCase() + - ' ' + - routeData.href + '
    Concurrency level: ' + routeData.options.concurrencyLevel + '
    '; + '
    Request: ' + routeData.options.method.toUpperCase(); + + if (routeData.href.indexOf('Dynamic route') > -1) { + template += routeData.href; + } else { + template += '' + routeData.href + ''; + } + + template += '
    Concurrency level: ' + routeData.options.concurrencyLevel + '
    '; if(routeData.options.delay) template += 'Delay between bench cycles: ' + routeData.options.delay + '
    '; From 9afaadeb84dc542c6c05f03733e8a8fc0f380d95 Mon Sep 17 00:00:00 2001 From: Krasiyan Date: Fri, 20 Jan 2017 18:33:48 +0200 Subject: [PATCH 8/8] html report styling fix - added missed out space between the request method and request href --- templates/report.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/report.html b/templates/report.html index 80b1f6b..c5de509 100644 --- a/templates/report.html +++ b/templates/report.html @@ -224,7 +224,7 @@ '
  • 99% Percentile: ' + fixNumber(routeData.stats.p99, 6) + '
    • ' + '
  • 99.9% Percentile: ' + fixNumber(routeData.stats.p999, 6) + '
    • ' + '
    ' + - '
    Request: ' + routeData.options.method.toUpperCase(); + '
    Request: ' + routeData.options.method.toUpperCase() + ' '; if (routeData.href.indexOf('Dynamic route') > -1) { template += routeData.href;