From f350206990e04373066b6aa3492d576daa2de34b Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 6 Oct 2017 12:54:37 +0000 Subject: [PATCH 1/8] Strict check --- test/support/assert.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/support/assert.js b/test/support/assert.js index c5e22e93..76380833 100644 --- a/test/support/assert.js +++ b/test/support/assert.js @@ -137,7 +137,7 @@ assert.response = function(server, req, res, callback) { // Assert response status if (typeof status === 'number') { - if (response.statusCode != status) { + if (response.statusCode !== status) { return callback(response, new Error(colorize( '[red]{Invalid response status code.}\n' + ' Expected: [green]{' + status + '}\n' + From 6c0e6210d6ca4ccd7d8b03332721584dd1cdc97a Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 6 Oct 2017 15:15:16 +0200 Subject: [PATCH 2/8] Split response validation --- test/support/assert.js | 106 +++++++++++++++++++++++------------------ 1 file changed, 59 insertions(+), 47 deletions(-) diff --git a/test/support/assert.js b/test/support/assert.js index 76380833..fc56a839 100644 --- a/test/support/assert.js +++ b/test/support/assert.js @@ -106,7 +106,6 @@ assert.response = function(server, req, res, callback) { // jshint maxcomplexity:9 function onServerListening() { - var status = res.status || res.statusCode; var requestParams = { url: 'http://' + host + ':' + port + req.url, method: req.method || 'GET', @@ -122,57 +121,70 @@ assert.response = function(server, req, res, callback) { request(requestParams, function assert$response$requestHandler(error, response, body) { listener.close(function() { response.body = response.body || body; - - // Assert response body - if (res.body) { - var eql = res.body instanceof RegExp ? res.body.test(response.body) : res.body === response.body; - if (!eql) { - return callback(response, new Error(colorize( - '[red]{Invalid response body.}\n' + - ' Expected: [green]{' + res.body + '}\n' + - ' Got: [red]{' + response.body + '}')) - ); - } - } - - // Assert response status - if (typeof status === 'number') { - if (response.statusCode !== status) { - return callback(response, new Error(colorize( - '[red]{Invalid response status code.}\n' + - ' Expected: [green]{' + status + '}\n' + - ' Got: [red]{' + response.statusCode + '}\n' + - ' Body: ' + response.body)) - ); - } - } - - // Assert response headers - if (res.headers) { - var keys = Object.keys(res.headers); - for (var i = 0, len = keys.length; i < len; ++i) { - var name = keys[i], - actual = response.headers[name.toLowerCase()], - expected = res.headers[name], - headerEql = expected instanceof RegExp ? expected.test(actual) : expected === actual; - if (!headerEql) { - return callback(response, new Error(colorize( - 'Invalid response header [bold]{' + name + '}.\n' + - ' Expected: [green]{' + expected + '}\n' + - ' Got: [red]{' + actual + '}')) - ); - } - } - } - - // Callback - callback(response); + var err = validateResponse(response, res); + return callback(response, err); }); }); } }; -// jshint maxcomplexity:6 + +function validateResponseBody(response, expected) { + if (expected.body) { + var eql = expected.body instanceof RegExp ? + expected.body.test(response.body) : + expected.body === response.body; + if (!eql) { + return new Error(colorize( + '[red]{Invalid response body.}\n' + + ' Expected: [green]{' + expected.body + '}\n' + + ' Got: [red]{' + response.body + '}') + ); + } + } +} + +function validateResponseStatus(response, expected) { + var status = expected.status || expected.statusCode; + // Assert response status + if (typeof status === 'number') { + if (response.statusCode !== status) { + return new Error(colorize( + '[red]{Invalid response status code.}\n' + + ' Expected: [green]{' + status + '}\n' + + ' Got: [red]{' + response.statusCode + '}\n' + + ' Body: ' + response.body) + ); + } + } +} + +function validateResponseHeaders(response, expected) { + // Assert response headers + if (expected.headers) { + var keys = Object.keys(expected.headers); + for (var i = 0, len = keys.length; i < len; ++i) { + var name = keys[i], + actual = response.headers[name.toLowerCase()], + expectedHeader = expected.headers[name], + headerEql = expectedHeader instanceof RegExp ? expectedHeader.test(actual) : expectedHeader === actual; + if (!headerEql) { + return new Error(colorize( + 'Invalid response header [bold]{' + name + '}.\n' + + ' Expected: [green]{' + expectedHeader + '}\n' + + ' Got: [red]{' + actual + '}') + ); + } + } + } +} + +function validateResponse(response, expected) { + // Assert response body + return validateResponseBody(response, expected) || + validateResponseStatus(response, expected) || + validateResponseHeaders(response, expected); +} // @param tolerance number of tolerated grid cell differences // jshint maxcomplexity:9 From 38c50e0bec4ba711df0f3f5c287037038329f6e7 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 6 Oct 2017 15:15:33 +0200 Subject: [PATCH 3/8] Fix jshint hint --- test/support/assert.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/support/assert.js b/test/support/assert.js index fc56a839..44309d89 100644 --- a/test/support/assert.js +++ b/test/support/assert.js @@ -187,8 +187,8 @@ function validateResponse(response, expected) { } // @param tolerance number of tolerated grid cell differences -// jshint maxcomplexity:9 assert.utfgridEqualsFile = function(buffer, file_b, tolerance, callback) { + // jshint maxcomplexity:9 fs.writeFileSync('/tmp/grid.json', buffer, 'binary'); // <-- to debug/update var expected_json = JSON.parse(fs.readFileSync(file_b, 'utf8')); From 664892bba9d01441eaa16e7be4d8f859c36591d0 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 6 Oct 2017 15:15:43 +0200 Subject: [PATCH 4/8] Complexity already fixed --- test/support/assert.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/support/assert.js b/test/support/assert.js index 44309d89..bcb186aa 100644 --- a/test/support/assert.js +++ b/test/support/assert.js @@ -77,7 +77,6 @@ function randomImagePath() { return path.resolve('test/results/png/image-test-' + Date.now() + '.png'); } -// jshint maxcomplexity:9 assert.response = function(server, req, res, callback) { if (!callback) { callback = res; From 5d750f3b9858921ae596b7c490fd58c7db273d94 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 6 Oct 2017 15:24:58 +0200 Subject: [PATCH 5/8] Several jshint fixes --- test/support/test-client.js | 38 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 3e9c26f3..832d8e44 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -22,7 +22,7 @@ const MAPNIK_SUPPORTED_FORMATS = { 'grid.json': true, 'geojson': true, 'mvt': true -} +}; function TestClient(config, apiKey) { this.mapConfig = isMapConfig(config) ? config : null; @@ -115,7 +115,7 @@ module.exports.CARTOCSS = { module.exports.SQL = { EMPTY: 'select 1 as cartodb_id, null::geometry as the_geom_webmercator', ONE_POINT: 'select 1 as cartodb_id, \'SRID=3857;POINT(0 0)\'::geometry the_geom_webmercator' -} +}; TestClient.prototype.getWidget = function(widgetName, params, callback) { var self = this; @@ -479,7 +479,6 @@ TestClient.prototype.getFeatureAttributes = function(featureId, layerId, params, } }; - var layergroupId; step( function createLayergroup() { var next = this; @@ -568,7 +567,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { var layergroupId; if (params.layergroupid) { - layergroupId = params.layergroupid + layergroupId = params.layergroupid; } step( @@ -583,7 +582,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { return next(new Error('apiKey param is mandatory to create a new template')); } - params.placeholders = params.placeholders || {}; + params.placeholders = params.placeholders || {}; assert.response(self.server, { @@ -616,7 +615,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { return next(null, layergroupId); } - var data = templateId ? params.placeholders : self.mapConfig + var data = templateId ? params.placeholders : self.mapConfig; var path = templateId ? urlNamed + '/' + templateId + '?' + qs.stringify({api_key: self.apiKey}) : url; @@ -646,6 +645,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { ); }, function getTileResult(err, _layergroupId) { + // jshint maxcomplexity:12 assert.ifError(err); layergroupId = _layergroupId; @@ -744,7 +744,7 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { body = JSON.parse(res.body); break; default: - body = res.body + body = res.body; break; } @@ -784,10 +784,11 @@ TestClient.prototype.getLayergroup = function(expectedResponse, callback) { }, expectedResponse, function(res, err) { + var parsedBody; // If there is a response, we are still interested in catching the created keys // to be able to delete them on the .drain() method. if (res) { - var parsedBody = JSON.parse(res.body); + parsedBody = JSON.parse(res.body); if (parsedBody.layergroupid) { self.keysToDelete['map_cfg|' + LayergroupToken.parse(parsedBody.layergroupid).token] = 0; self.keysToDelete['user:localhost:mapviews:global'] = 5; @@ -805,7 +806,7 @@ TestClient.prototype.getLayergroup = function(expectedResponse, callback) { TestClient.prototype.getStaticCenter = function (params, callback) { var self = this; - let { layergroupid, z, lat, lng, width, height, format } = params + let { layergroupid, z, lat, lng, width, height, format } = params; var url = `/api/v1/map/`; @@ -821,7 +822,7 @@ TestClient.prototype.getStaticCenter = function (params, callback) { return next(null, layergroupid); } - var data = self.mapConfig + var data = self.mapConfig; var path = url; assert.response(self.server, @@ -851,13 +852,11 @@ TestClient.prototype.getStaticCenter = function (params, callback) { function getStaticResult(err, _layergroupid) { assert.ifError(err); - var next = this; - layergroupid = _layergroupid; self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupid).token] = 0; self.keysToDelete['user:localhost:mapviews:global'] = 5; - url = `/api/v1/map/static/center/${layergroupid}/${z}/${lat}/${lng}/${width}/${height}.${format}` + url = `/api/v1/map/static/center/${layergroupid}/${z}/${lat}/${lng}/${width}/${height}.${format}`; if (self.apiKey) { url += '?' + qs.stringify({api_key: self.apiKey}); @@ -895,7 +894,7 @@ TestClient.prototype.getStaticCenter = function (params, callback) { body = JSON.parse(res.body); break; default: - body = res.body + body = res.body; break; } @@ -995,11 +994,11 @@ TestClient.prototype.getAttributes = function(params, callback) { var self = this; if (!Number.isFinite(params.featureId)) { - throw new Error('featureId param must be a number') + throw new Error('featureId param must be a number'); } if (!Number.isFinite(params.layer)) { - throw new Error('layer param must be a number') + throw new Error('layer param must be a number'); } var url = '/api/v1/map'; @@ -1011,7 +1010,7 @@ TestClient.prototype.getAttributes = function(params, callback) { var layergroupid; if (params.layergroupid) { - layergroupid = params.layergroupid + layergroupid = params.layergroupid; } step( @@ -1149,12 +1148,11 @@ TestClient.prototype.setUserRenderTimeoutLimit = function (user, userTimeoutLimi TestClient.prototype.setUserDatabaseTimeoutLimit = function (timeoutLimit, callback) { const dbname = _.template(global.environment.postgres_auth_user, { user_id: 1 }) + '_db'; - const dbuser = _.template(global.environment.postgres_auth_user, { user_id: 1 }) - const pass = _.template(global.environment.postgres_auth_pass, { user_id: 1 }) + const dbuser = _.template(global.environment.postgres_auth_user, { user_id: 1 }); const publicuser = global.environment.postgres.user; // we need to guarantee all new connections have the new settings - helper.cleanPGPoolConnections() + helper.cleanPGPoolConnections(); const psql = new PSQL({ user: 'postgres', From 64fe070ab26643456994c8d03ccbeff1bf4ff773 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 6 Oct 2017 15:27:03 +0200 Subject: [PATCH 6/8] Put layergroupId handling close --- test/support/test-client.js | 43 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/test/support/test-client.js b/test/support/test-client.js index 832d8e44..3a3375cf 100644 --- a/test/support/test-client.js +++ b/test/support/test-client.js @@ -130,7 +130,6 @@ TestClient.prototype.getWidget = function(widgetName, params, callback) { url += '?' + qs.stringify({ filters: JSON.stringify(params.filters) }); } - var layergroupId; step( function createLayergroup() { var next = this; @@ -176,11 +175,12 @@ TestClient.prototype.getWidget = function(widgetName, params, callback) { } ); }, - function getWidgetResult(err, _layergroupId) { + function getWidgetResult(err, layergroupId) { assert.ifError(err); var next = this; - layergroupId = _layergroupId; + self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; + self.keysToDelete['user:localhost:mapviews:global'] = 5; var urlParams = { own_filter: params.hasOwnProperty('own_filter') ? params.own_filter : 1 @@ -217,8 +217,6 @@ TestClient.prototype.getWidget = function(widgetName, params, callback) { ); }, function finish(err, res) { - self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; - self.keysToDelete['user:localhost:mapviews:global'] = 5; var widget; if (!err && res.body) { widget = JSON.parse(res.body); @@ -241,7 +239,6 @@ TestClient.prototype.widgetSearch = function(widgetName, userQuery, params, call url += '?' + qs.stringify({ filters: JSON.stringify(params.filters) }); } - var layergroupId; step( function createLayergroup() { var next = this; @@ -287,11 +284,12 @@ TestClient.prototype.widgetSearch = function(widgetName, userQuery, params, call } ); }, - function getWidgetSearchResult(err, _layergroupId) { + function getWidgetSearchResult(err, layergroupId) { assert.ifError(err); var next = this; - layergroupId = _layergroupId; + self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; + self.keysToDelete['user:localhost:mapviews:global'] = 5; var urlParams = { q: userQuery, @@ -326,8 +324,6 @@ TestClient.prototype.widgetSearch = function(widgetName, userQuery, params, call ); }, function finish(err, res) { - self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; - self.keysToDelete['user:localhost:mapviews:global'] = 5; var searchResult; if (!err && res.body) { searchResult = JSON.parse(res.body); @@ -365,7 +361,6 @@ TestClient.prototype.getDataview = function(dataviewName, params, callback) { } }; - var layergroupId; step( function createLayergroup() { var next = this; @@ -401,11 +396,10 @@ TestClient.prototype.getDataview = function(dataviewName, params, callback) { } ); }, - function getDataviewResult(err, _layergroupId) { + function getDataviewResult(err, layergroupId) { assert.ifError(err); var next = this; - layergroupId = _layergroupId; self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; self.keysToDelete['user:localhost:mapviews:global'] = 5; @@ -644,11 +638,10 @@ TestClient.prototype.getTile = function(z, x, y, params, callback) { } ); }, - function getTileResult(err, _layergroupId) { + function getTileResult(err, layergroupId) { // jshint maxcomplexity:12 assert.ifError(err); - layergroupId = _layergroupId; self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; self.keysToDelete['user:localhost:mapviews:global'] = 5; @@ -849,14 +842,13 @@ TestClient.prototype.getStaticCenter = function (params, callback) { } ); }, - function getStaticResult(err, _layergroupid) { + function getStaticResult(err, layergroupId) { assert.ifError(err); - layergroupid = _layergroupid; - self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupid).token] = 0; + self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; self.keysToDelete['user:localhost:mapviews:global'] = 5; - url = `/api/v1/map/static/center/${layergroupid}/${z}/${lat}/${lng}/${width}/${height}.${format}`; + url = `/api/v1/map/static/center/${layergroupId}/${z}/${lat}/${lng}/${width}/${height}.${format}`; if (self.apiKey) { url += '?' + qs.stringify({api_key: self.apiKey}); @@ -912,7 +904,6 @@ TestClient.prototype.getNodeStatus = function(nodeName, callback) { url += '?' + qs.stringify({api_key: this.apiKey}); } - var layergroupId; var nodes = {}; step( function createLayergroup() { @@ -951,10 +942,9 @@ TestClient.prototype.getNodeStatus = function(nodeName, callback) { } ); }, - function getNodeStatusResult(err, _layergroupId) { + function getNodeStatusResult(err, layergroupId) { assert.ifError(err); - layergroupId = _layergroupId; self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; self.keysToDelete['user:localhost:mapviews:global'] = 5; @@ -1047,14 +1037,13 @@ TestClient.prototype.getAttributes = function(params, callback) { } ); }, - function getAttributes(err, _layergroupid) { + function getAttributes(err, layergroupId) { assert.ifError(err); - layergroupid = _layergroupid; - self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupid).token] = 0; + self.keysToDelete['map_cfg|' + LayergroupToken.parse(layergroupId).token] = 0; self.keysToDelete['user:localhost:mapviews:global'] = 5; - url = `/api/v1/map/${layergroupid}/${params.layer}/attributes/${params.featureId}`; + url = `/api/v1/map/${layergroupId}/${params.layer}/attributes/${params.featureId}`; if (self.apiKey) { url += '?' + qs.stringify({api_key: self.apiKey}); @@ -1126,7 +1115,7 @@ module.exports.getStaticMap = function getStaticMap(templateName, params, callba // this could be removed once named maps are invalidated, otherwise you hits the cache var server = new CartodbWindshaft(serverOptions); - assert.response(self.server, requestOptions, expectedResponse, function (res, err) { + assert.response(server, requestOptions, expectedResponse, function (res, err) { helper.deleteRedisKeys({'user:localhost:mapviews:global': 5}, function() { return callback(err, mapnik.Image.fromBytes(new Buffer(res.body, 'binary'))); }); From b9c511ee60b4c1e7ba5599832437b0530c797526 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 6 Oct 2017 15:27:48 +0200 Subject: [PATCH 7/8] Remove unused file --- test/support/config.js | 21 --------------------- 1 file changed, 21 deletions(-) delete mode 100644 test/support/config.js diff --git a/test/support/config.js b/test/support/config.js deleted file mode 100644 index 4b8102f7..00000000 --- a/test/support/config.js +++ /dev/null @@ -1,21 +0,0 @@ -var _ = require('underscore'); - - -require(__dirname + '/test_helper'); - -module.exports = function(opts) { - - var config = { - redis_pool: { - max: 10, - idleTimeoutMillis: 1, - reapIntervalMillis: 1, - port: global.environment.redis.port - } - } - - _.extend(config, opts || {}); - - return config; -}(); - From d4015085c7fd3dde8b826bbdfc4db5218c33a9ee Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Fri, 6 Oct 2017 15:28:01 +0200 Subject: [PATCH 8/8] Include test/support as part of jshint validation --- .jshintignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.jshintignore b/.jshintignore index d781eace..39e650b4 100644 --- a/.jshintignore +++ b/.jshintignore @@ -1,4 +1,3 @@ test/results/ test/monkey/ test/benchmark.js -test/support/