Mobrovac has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/398020 )
Change subject: Update to service-template-node v0.5.4
......................................................................
Update to service-template-node v0.5.4
Bug: T181564
Bug: T180676
Change-Id: Id65e4383aff42192f500efaa70e6045d3ecf939f
---
M .travis.yml
M app.js
M lib/api-util.js
M lib/swagger-ui.js
M lib/util.js
M package.json
M server.js
M test/features/app/app.js
M test/features/app/spec.js
M test/utils/assert.js
M test/utils/logStream.js
M test/utils/server.js
12 files changed, 359 insertions(+), 331 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/services/recommendation-api
refs/changes/20/398020/1
diff --git a/.travis.yml b/.travis.yml
index df202fa..b6a9e5d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,3 +5,5 @@
node_js:
- "4"
- "6"
+ - "8"
+ - "node"
diff --git a/app.js b/app.js
index 236a497..eb51fb3 100644
--- a/app.js
+++ b/app.js
@@ -1,17 +1,18 @@
'use strict';
-var http = require('http');
-var BBPromise = require('bluebird');
-var express = require('express');
-var compression = require('compression');
-var bodyParser = require('body-parser');
-var fs = BBPromise.promisifyAll(require('fs'));
-var sUtil = require('./lib/util');
-var apiUtil = require('./lib/api-util');
-var translation = require('./lib/translation');
-var packageInfo = require('./package.json');
-var yaml = require('js-yaml');
+const http = require('http');
+const BBPromise = require('bluebird');
+const express = require('express');
+const compression = require('compression');
+const bodyParser = require('body-parser');
+const fs = BBPromise.promisifyAll(require('fs'));
+const sUtil = require('./lib/util');
+const apiUtil = require('./lib/api-util');
+const packageInfo = require('./package.json');
+const yaml = require('js-yaml');
+const addShutdown = require('http-shutdown');
+const translation = require('./lib/translation');
/**
@@ -22,7 +23,7 @@
function initApp(options) {
// the main application object
- var app = express();
+ const app = express();
// get the options and make them available in the app
app.logger = options.logger; // the logging device
@@ -31,22 +32,22 @@
app.info = packageInfo; // this app's package info
// ensure some sane defaults
- if(!app.conf.port) { app.conf.port = 8888; }
- if(!app.conf.interface) { app.conf.interface = '0.0.0.0'; }
- if(app.conf.compression_level === undefined) { app.conf.compression_level
= 3; }
- if(app.conf.cors === undefined) { app.conf.cors = '*'; }
- if(app.conf.csp === undefined) {
- app.conf.csp =
- "default-src 'self'; object-src 'none'; media-src *; img-src *;
style-src *; frame-ancestors 'self'";
+ if (!app.conf.port) { app.conf.port = 8888; }
+ if (!app.conf.interface) { app.conf.interface = '0.0.0.0'; }
+ if (app.conf.compression_level === undefined) { app.conf.compression_level
= 3; }
+ if (app.conf.cors === undefined) { app.conf.cors = '*'; }
+ if (app.conf.csp === undefined) {
+ // eslint-disable-next-line max-len
+ app.conf.csp = "default-src 'self'; object-src 'none'; media-src *;
img-src *; style-src *; frame-ancestors 'self'";
}
// set outgoing proxy
- if(app.conf.proxy) {
+ if (app.conf.proxy) {
process.env.HTTP_PROXY = app.conf.proxy;
// if there is a list of domains which should
// not be proxied, set it
- if(app.conf.no_proxy_list) {
- if(Array.isArray(app.conf.no_proxy_list)) {
+ if (app.conf.no_proxy_list) {
+ if (Array.isArray(app.conf.no_proxy_list)) {
process.env.NO_PROXY = app.conf.no_proxy_list.join(',');
} else {
process.env.NO_PROXY = app.conf.no_proxy_list;
@@ -55,15 +56,15 @@
}
// set up header whitelisting for logging
- if(!app.conf.log_header_whitelist) {
+ if (!app.conf.log_header_whitelist) {
app.conf.log_header_whitelist = [
- 'cache-control', 'content-type', 'content-length', 'if-match',
- 'user-agent', 'x-request-id'
+ 'cache-control', 'content-type', 'content-length', 'if-match',
+ 'user-agent', 'x-request-id'
];
}
- app.conf.log_header_whitelist = new RegExp('^(?:' +
app.conf.log_header_whitelist.map(function(item) {
+ app.conf.log_header_whitelist = new
RegExp(`^(?:${app.conf.log_header_whitelist.map((item) => {
return item.trim();
- }).join('|') + ')$', 'i');
+ }).join('|')})$`, 'i');
// set up the request templates for the APIs
apiUtil.setupApiTemplates(app);
@@ -71,21 +72,21 @@
translation.setupTemplates(app);
// set up the spec
- if(!app.conf.spec) {
- app.conf.spec = __dirname + '/spec.yaml';
+ if (!app.conf.spec) {
+ app.conf.spec = `${__dirname}/spec.yaml`;
}
- if(app.conf.spec.constructor !== Object) {
+ if (app.conf.spec.constructor !== Object) {
try {
app.conf.spec = yaml.safeLoad(fs.readFileSync(app.conf.spec));
- } catch(e) {
- app.logger.log('warn/spec', 'Could not load the spec: ' + e);
+ } catch (e) {
+ app.logger.log('warn/spec', `Could not load the spec: ${e}`);
app.conf.spec = {};
}
}
- if(!app.conf.spec.swagger) {
+ if (!app.conf.spec.swagger) {
app.conf.spec.swagger = '2.0';
}
- if(!app.conf.spec.info) {
+ if (!app.conf.spec.info) {
app.conf.spec.info = {
version: app.info.version,
title: app.info.name,
@@ -93,18 +94,18 @@
};
}
app.conf.spec.info.version = app.info.version;
- if(!app.conf.spec.paths) {
+ if (!app.conf.spec.paths) {
app.conf.spec.paths = {};
}
// set the CORS and CSP headers
- app.all('*', function(req, res, next) {
- if(app.conf.cors !== false) {
+ app.all('*', (req, res, next) => {
+ if (app.conf.cors !== false) {
res.header('access-control-allow-origin', app.conf.cors);
res.header('access-control-allow-headers', 'accept,
x-requested-with, content-type');
res.header('access-control-expose-headers', 'etag');
}
- if(app.conf.csp !== false) {
+ if (app.conf.csp !== false) {
res.header('x-xss-protection', '1; mode=block');
res.header('x-content-type-options', 'nosniff');
res.header('x-frame-options', 'SAMEORIGIN');
@@ -124,11 +125,11 @@
// disable the ETag header - users should provide them!
app.set('etag', false);
// enable compression
- app.use(compression({level: app.conf.compression_level}));
+ app.use(compression({ level: app.conf.compression_level }));
// use the JSON body parser
app.use(bodyParser.json());
// use the application/x-www-form-urlencoded parser
- app.use(bodyParser.urlencoded({extended: true}));
+ app.use(bodyParser.urlencoded({ extended: true }));
return BBPromise.resolve(app);
@@ -138,45 +139,46 @@
/**
* Loads all routes declared in routes/ into the app
* @param {Application} app the application object to load routes into
- * @returns {bluebird} a promise resolving to the app object
+ * @return {bluebird} a promise resolving to the app object
*/
-function loadRoutes (app) {
+function loadRoutes(app) {
// get the list of files in routes/
- return fs.readdirAsync(__dirname + '/routes').map(function(fname) {
- return BBPromise.try(function() {
+ return fs.readdirAsync(`${__dirname}/routes`).map((fname) => {
+ return BBPromise.try(() => {
// ... and then load each route
// but only if it's a js file
- if(!/\.js$/.test(fname)) {
+ if (!/\.js$/.test(fname)) {
return undefined;
}
// import the route file
- var route = require(__dirname + '/routes/' + fname);
+ const route = require(`${__dirname}/routes/${fname}`);
return route(app);
- }).then(function(route) {
- if(route === undefined) {
+ }).then((route) => {
+ if (route === undefined) {
return undefined;
}
// check that the route exports the object we need
- if(route.constructor !== Object || !route.path || !route.router ||
!(route.api_version || route.skip_domain)) {
- throw new TypeError('routes/' + fname + ' does not export the
correct object!');
+ if (route.constructor !== Object || !route.path || !route.router
+ || !(route.api_version || route.skip_domain)) {
+ throw new TypeError(`routes/${fname} does not export the
correct object!`);
}
// normalise the path to be used as the mount point
- if(route.path[0] !== '/') {
- route.path = '/' + route.path;
+ if (route.path[0] !== '/') {
+ route.path = `/${route.path}`;
}
- if(route.path[route.path.length - 1] !== '/') {
- route.path = route.path + '/';
+ if (route.path[route.path.length - 1] !== '/') {
+ route.path = `${route.path}/`;
}
- if(!route.skip_domain) {
- route.path = '/:domain/v' + route.api_version + route.path;
+ if (!route.skip_domain) {
+ route.path = `/:domain/v${route.api_version}${route.path}`;
}
// wrap the route handlers with Promise.try() blocks
sUtil.wrapRouteHandlers(route, app);
// all good, use that route
app.use(route.path, route.router);
});
- }).then(function () {
+ }).then(() => {
// catch errors
sUtil.setErrorHandler(app);
// route loading is now complete, return the app object
@@ -189,28 +191,29 @@
/**
* Creates and start the service's web server
* @param {Application} app the app object to use in the service
- * @returns {bluebird} a promise creating the web server
+ * @return {bluebird} a promise creating the web server
*/
function createServer(app) {
// return a promise which creates an HTTP server,
// attaches the app to it, and starts accepting
// incoming client requests
- var server;
- return new BBPromise(function (resolve) {
+ let server;
+ return new BBPromise((resolve) => {
server = http.createServer(app).listen(
app.conf.port,
app.conf.interface,
resolve
);
- }).then(function () {
+ server = addShutdown(server);
+ }).then(() => {
app.logger.log('info',
- 'Worker ' + process.pid + ' listening on ' + (app.conf.interface
|| '*') + ':' + app.conf.port);
+ `Worker ${process.pid} listening on ${app.conf.interface ||
'*'}:${app.conf.port}`);
// Don't delay incomplete packets for 40ms (Linux default) on
// pipelined HTTP sockets. We write in large chunks or buffers, so
// lack of coalescing should not be an issue here.
- server.on("connection", function(socket) {
+ server.on("connection", (socket) => {
socket.setNoDelay(true);
});
@@ -230,9 +233,9 @@
return initApp(options)
.then(loadRoutes)
- .then(function(app) {
+ .then((app) => {
// serve static files from static/
- app.use('/static', express.static(__dirname + '/static'));
+ app.use('/static', express.static(`${__dirname}/static`));
return app;
}).then(createServer);
diff --git a/lib/api-util.js b/lib/api-util.js
index 48f5bd5..90e0e6d 100644
--- a/lib/api-util.js
+++ b/lib/api-util.js
@@ -9,10 +9,10 @@
/**
* Calls the MW API with the supplied query as its body
- * @param {Object} app the application object
+ * @param {!Object} app the application object
* @param {string} domain the domain to issue the request to
- * @param {Object} query an object with all the query parameters for the MW API
- * @return {Promise} a promise resolving as the response object from the MW API
+ * @param {?Object} query an object with all the query parameters for the MW
API
+ * @return {!Promise} a promise resolving as the response object from the MW
API
*/
function mwApiGet(app, domain, query) {
@@ -45,15 +45,15 @@
/**
* Calls the REST API with the supplied domain, path and request parameters
- * @param {Object} app the application object
+ * @param {!Object} app the application object
* @param {string} domain the domain to issue the request for
- * @param {string} path the REST API path to contact without the leading slash
- * @param {Object} [restReq={}] the object containing the REST request details
- * @param {string} [restReq.method=get] the request method
- * @param {Object} [restReq.query={}] the query string to send, if any
- * @param {Object} [restReq.headers={}] the request headers to send
- * @param {Object} [restReq.body=null] the body of the request, if any
- * @return {Promise} a promise resolving as the response object from the REST
API
+ * @param {!string} path the REST API path to contact without the leading slash
+ * @param {?Object} [restReq={}] the object containing the REST request details
+ * @param {?string} [restReq.method=get] the request method
+ * @param {?Object} [restReq.query={}] the query string to send, if any
+ * @param {?Object} [restReq.headers={}] the request headers to send
+ * @param {?Object} [restReq.body=null] the body of the request, if any
+ * @return {!Promise} a promise resolving as the response object from the REST
API
*
*/
function restApiGet(app, domain, path, restReq) {
@@ -78,7 +78,7 @@
/**
* Sets up the request templates for MW and RESTBase API requests
- * @param {Application} app the application object
+ * @param {!Application} app the application object
*/
function setupApiTemplates(app) {
diff --git a/lib/swagger-ui.js b/lib/swagger-ui.js
index a4e41c2..a85985c 100644
--- a/lib/swagger-ui.js
+++ b/lib/swagger-ui.js
@@ -33,13 +33,13 @@
.replace(/((?:src|href)=['"])/g, '$1?doc&path=')
// Some self-promotion
.replace(/<a id="logo".*?<\/a>/,
- `<a id="logo"
href="${app.info.homepage}">${app.info.name}</a>`)
+ `<a id="logo"
href="${app.info.homepage}">${app.info.name}</a>`)
.replace(/<title>[^<]*<\/title>/,
`<title>${app.info.name}</title>`)
// Replace the default url with ours, switch off validation &
// limit the size of documents to apply syntax highlighting to
- .replace(/Sorter: "alpha"/, 'Sorter: "alpha", validatorUrl:
null, ' +
- 'highlightSizeThreshold: 10000')
- .replace(/docExpansion: "none"/, 'docExpansion: "list"')
+ .replace(/docExpansion: "none"/, 'docExpansion: "list", '
+ + 'validatorUrl: null, '
+ + 'highlightSizeThreshold: 10000')
.replace(/ url: url,/, 'url: "/?spec",');
}
diff --git a/lib/util.js b/lib/util.js
index ee44883..cb3d587 100644
--- a/lib/util.js
+++ b/lib/util.js
@@ -38,9 +38,9 @@
/**
* Generates an object suitable for logging out of a request object
- * @param {Request} req the request
- * @param {RegExp} whitelistRE the RegExp used to filter headers
- * @return {Object} an object containing the key components of the request
+ * @param {!Request} req the request
+ * @param {?RegExp} whitelistRE the RegExp used to filter headers
+ * @return {!Object} an object containing the key components of the request
*/
function reqForLog(req, whitelistRE) {
@@ -69,9 +69,9 @@
/**
- * Serialises an error object in a form suitable for logging
- * @param {Error} err error to serialise
- * @return {Object} the serialised version of the error
+ * Serialises an error object in a form suitable for logging.
+ * @param {!Error} err error to serialise
+ * @return {!Object} the serialised version of the error
*/
function errForLog(err) {
@@ -90,8 +90,8 @@
}
/**
- * Generates a unique request ID
- * @return {string} the generated request ID
+ * Generates a unique request ID.
+ * @return {!String} the generated request ID
*/
function generateRequestId() {
@@ -105,8 +105,8 @@
* promised try blocks so as to allow catching all errors,
* regardless of whether a handler returns/uses promises
* or not.
- * @param {Object} route the object containing the router and path to bind it
to
- * @param {Application} app the application object
+ * @param {!Object} route the object containing the router and path to bind it
to
+ * @param {!Application} app the application object
*/
function wrapRouteHandlers(route, app) {
@@ -143,9 +143,8 @@
/**
- * Generates an error handler for the given applications
- * and installs it. Usage:
- * @param {Application} app the application object to add the handler to
+ * Generates an error handler for the given applications and installs it.
+ * @param {!Application} app the application object to add the handler to
*/
function setErrorHandler(app) {
@@ -224,8 +223,8 @@
/**
* Creates a new router with some default options.
- * @param {Object} [opts] additional options to pass to express.Router()
- * @return {Router} a new router object
+ * @param {?Object} [opts] additional options to pass to express.Router()
+ * @return {!Router} a new router object
*/
function createRouter(opts) {
@@ -243,9 +242,9 @@
/**
- * Adds logger to the request and logs it
- * @param {*} req request object
- * @param {Application} app application object
+ * Adds logger to the request and logs it.
+ * @param {!*} req request object
+ * @param {!Application} app application object
*/
function initAndLogRequest(req, app) {
req.headers = req.headers || {};
diff --git a/package.json b/package.json
index 2a614a7..25d40c7 100644
--- a/package.json
+++ b/package.json
@@ -1,11 +1,12 @@
{
"name": "recommendation-api",
- "version": "0.4.0",
+ "version": "0.4.1",
"description": "Provides recommendations in Wikimedia projects",
"main": "./app.js",
"scripts": {
"start": "service-runner",
"test": "PREQ_CONNECT_TIMEOUT=15 mocha && nsp check",
+ "lint": "eslint --cache --max-warnings 0 --ext .js --ext .json .",
"docker-start": "service-runner docker-start",
"docker-test": "service-runner docker-test",
"test-build": "service-runner docker-test && service-runner build
--deploy-repo --force",
@@ -29,30 +30,32 @@
},
"homepage": "https://meta.wikimedia.org/wiki/Recommendation_API";,
"dependencies": {
- "bluebird": "^3.5.0",
- "body-parser": "^1.17.1",
- "bunyan": "^1.8.9",
+ "bluebird": "^3.5.1",
+ "body-parser": "^1.18.2",
+ "bunyan": "^1.8.12",
"cassandra-uuid": "^0.0.2",
- "compression": "^1.6.2",
- "domino": "^1.0.28",
- "express": "^4.15.2",
- "js-yaml": "^3.8.2",
- "preq": "^0.5.2",
- "service-runner": "^2.2.5",
- "swagger-router": "^0.5.6",
- "swagger-ui": "git+https://github.com/wikimedia/swagger-ui#master";
+ "compression": "^1.7.1",
+ "domino": "^1.0.30",
+ "express": "^4.16.2",
+ "js-yaml": "^3.10.0",
+ "preq": "^0.5.3",
+ "service-runner": "^2.4.2",
+ "swagger-router": "^0.7.1",
+ "swagger-ui": "git+https://github.com/wikimedia/swagger-ui#master";,
+ "http-shutdown": "^1.2.0"
},
"devDependencies": {
- "extend": "^3.0.0",
+ "ajv": "^5.5.0",
+ "extend": "^3.0.1",
"istanbul": "^0.4.5",
- "mocha": "^3.2.0",
+ "mocha": "^4.0.1",
"mocha-jshint": "^2.3.1",
"mocha-lcov-reporter": "^1.3.0",
- "nsp": "^2.6.3",
- "mocha-eslint":"^3.0.1",
- "eslint": "^3.12.0",
- "eslint-config-node-services": "^2.0.2",
- "eslint-config-wikimedia": "^0.4.0",
+ "nsp": "^2.8.1",
+ "mocha-eslint": "^4.1.0",
+ "eslint": "^4.12.0",
+ "eslint-config-node-services": "^2.2.5",
+ "eslint-config-wikimedia": "^0.5.0",
"eslint-plugin-json": "^1.2.0",
"eslint-plugin-jsdoc": "^3.0.0"
},
diff --git a/server.js b/server.js
index a45b9ce..2a5ac56 100755
--- a/server.js
+++ b/server.js
@@ -8,5 +8,5 @@
// (config.yaml by default, specify other path with -c). It requires the
// module(s) specified in the config 'services' section (app.js in this
// example).
-var ServiceRunner = require('service-runner');
+const ServiceRunner = require('service-runner');
new ServiceRunner().start();
diff --git a/test/features/app/app.js b/test/features/app/app.js
index d30b94e..70755da 100644
--- a/test/features/app/app.js
+++ b/test/features/app/app.js
@@ -1,33 +1,39 @@
+/* global describe, it, before, after */
+
'use strict';
-var preq = require('preq');
-var assert = require('../../utils/assert.js');
-var server = require('../../utils/server.js');
+const preq = require('preq');
+const assert = require('../../utils/assert.js');
+const server = require('../../utils/server.js');
+if (!server.stopHookAdded) {
+ server.stopHookAdded = true;
+ after(() => server.stop());
+}
describe('express app', function() {
- this.timeout(20000);
+ this.timeout(20000); // eslint-disable-line no-invalid-this
- before(function () { return server.start(); });
+ before(() => { return server.start(); });
- it('should get robots.txt', function() {
+ it('should get robots.txt', () => {
return preq.get({
- uri: server.config.uri + 'robots.txt'
- }).then(function(res) {
+ uri: `${server.config.uri}robots.txt`
+ }).then((res) => {
assert.deepEqual(res.status, 200);
- assert.deepEqual(res.headers['disallow'], '/');
+ assert.deepEqual(res.headers.disallow, '/');
});
});
- it('should set CORS headers', function() {
- if(server.config.service.conf.cors === false) {
+ it('should set CORS headers', () => {
+ if (server.config.service.conf.cors === false) {
return true;
}
return preq.get({
- uri: server.config.uri + 'robots.txt'
- }).then(function(res) {
+ uri: `${server.config.uri}robots.txt`
+ }).then((res) => {
assert.deepEqual(res.status, 200);
assert.deepEqual(res.headers['access-control-allow-origin'], '*');
assert.deepEqual(!!res.headers['access-control-allow-headers'],
true);
@@ -35,13 +41,13 @@
});
});
- it('should set CSP headers', function() {
- if(server.config.service.conf.csp === false) {
+ it('should set CSP headers', () => {
+ if (server.config.service.conf.csp === false) {
return true;
}
return preq.get({
- uri: server.config.uri + 'robots.txt'
- }).then(function(res) {
+ uri: `${server.config.uri}robots.txt`
+ }).then((res) => {
assert.deepEqual(res.status, 200);
assert.deepEqual(res.headers['x-xss-protection'], '1; mode=block');
assert.deepEqual(res.headers['x-content-type-options'], 'nosniff');
@@ -52,29 +58,29 @@
});
});
- it.skip('should get static content gzipped', function() {
+ it.skip('should get static content gzipped', () => {
return preq.get({
- uri: server.config.uri + 'static/index.html',
+ uri: `${server.config.uri}static/index.html`,
headers: {
'accept-encoding': 'gzip, deflate'
}
- }).then(function(res) {
+ }).then((res) => {
// check that the response is gzip-ed
assert.deepEqual(res.headers['content-encoding'], 'gzip',
'Expected gzipped contents!');
});
});
- it('should get static content uncompressed', function() {
+ it('should get static content uncompressed', () => {
return preq.get({
- uri: server.config.uri + 'static/index.html',
+ uri: `${server.config.uri}static/index.html`,
headers: {
'accept-encoding': ''
}
- }).then(function(res) {
+ }).then((res) => {
// check that the response is gzip-ed
- assert.deepEqual(res.headers['content-encoding'], undefined, 'Did
not expect gzipped contents!');
+ const contentEncoding = res.headers['content-encoding'];
+ assert.deepEqual(contentEncoding, undefined, 'Did not expect
gzipped contents!');
});
});
-
});
diff --git a/test/features/app/spec.js b/test/features/app/spec.js
index 570fb40..eb80baa 100644
--- a/test/features/app/spec.js
+++ b/test/features/app/spec.js
@@ -1,25 +1,31 @@
+/* global describe, it, before, after */
+
'use strict';
-var preq = require('preq');
-var assert = require('../../utils/assert.js');
-var server = require('../../utils/server.js');
-var URI = require('swagger-router').URI;
-var yaml = require('js-yaml');
-var fs = require('fs');
+const preq = require('preq');
+const assert = require('../../utils/assert.js');
+const server = require('../../utils/server.js');
+const URI = require('swagger-router').URI;
+const yaml = require('js-yaml');
+const fs = require('fs');
+if (!server.stopHookAdded) {
+ server.stopHookAdded = true;
+ after(() => server.stop());
+}
function staticSpecLoad() {
- var spec;
- var myService =
server.config.conf.services[server.config.conf.services.length - 1].conf;
- var specPath = __dirname + '/../../../' + (myService.spec ? myService.spec
: 'spec.yaml');
+ let spec;
+ const myService =
server.config.conf.services[server.config.conf.services.length - 1].conf;
+ const specPath = `${__dirname}/../../../${myService.spec ? myService.spec
: 'spec.yaml'}`;
try {
spec = yaml.safeLoad(fs.readFileSync(specPath));
- } catch(e) {
+ } catch (e) {
// this error will be detected later, so ignore it
- spec = {paths: {}, 'x-default-params': {}};
+ spec = { paths: {}, 'x-default-params': {} };
}
return spec;
@@ -29,30 +35,32 @@
function validateExamples(pathStr, defParams, mSpec) {
- var uri = new URI(pathStr, {}, true);
+ const uri = new URI(pathStr, {}, true);
- if(!mSpec) {
+ if (!mSpec) {
try {
uri.expand(defParams);
return true;
- } catch(e) {
- throw new Error('Missing parameter for route ' + pathStr + ' : ' +
e.message);
+ } catch (e) {
+ throw new Error(`Missing parameter for route ${pathStr} :
${e.message}`);
}
}
- if(!Array.isArray(mSpec)) {
- throw new Error('Route ' + pathStr + ' : x-amples must be an array!');
+ if (!Array.isArray(mSpec)) {
+ throw new Error(`Route ${pathStr} : x-amples must be an array!`);
}
- mSpec.forEach(function(ex, idx) {
- if(!ex.title) {
- throw new Error('Route ' + pathStr + ', example ' + idx + ': title
missing!');
+ mSpec.forEach((ex, idx) => {
+ if (!ex.title) {
+ throw new Error(`Route ${pathStr}, example ${idx}: title
missing!`);
}
ex.request = ex.request || {};
try {
uri.expand(Object.assign({}, defParams, ex.request.params || {}));
- } catch(e) {
- throw new Error('Route ' + pathStr + ', example ' + idx + ' (' +
ex.title + '): missing parameter: ' + e.message);
+ } catch (e) {
+ throw new Error(
+ `Route ${pathStr}, example ${idx} (${ex.title}): missing
parameter: ${e.message}`
+ );
}
});
@@ -64,10 +72,10 @@
function constructTestCase(title, path, method, request, response) {
return {
- title: title,
+ title,
request: {
uri: server.config.uri + (path[0] === '/' ? path.substr(1) : path),
- method: method,
+ method,
headers: request.headers || {},
query: request.query,
body: request.body,
@@ -85,31 +93,30 @@
function constructTests(paths, defParams) {
- var ret = [];
+ const ret = [];
- Object.keys(paths).forEach(function(pathStr) {
- Object.keys(paths[pathStr]).forEach(function(method) {
- var p = paths[pathStr][method];
- var uri;
- if(p.hasOwnProperty('x-monitor') && !p['x-monitor']) {
+ Object.keys(paths).forEach((pathStr) => {
+ Object.keys(paths[pathStr]).forEach((method) => {
+ const p = paths[pathStr][method];
+ if ({}.hasOwnProperty.call(p, 'x-monitor') && !p['x-monitor']) {
return;
}
- uri = new URI(pathStr, {}, true);
- if(!p['x-amples']) {
+ const uri = new URI(pathStr, {}, true);
+ if (!p['x-amples']) {
ret.push(constructTestCase(
pathStr,
- uri.toString({params: defParams}),
+ uri.toString({ params: defParams }),
method,
{},
{}
));
return;
}
- p['x-amples'].forEach(function(ex) {
+ p['x-amples'].forEach((ex) => {
ex.request = ex.request || {};
ret.push(constructTestCase(
ex.title,
- uri.toString({params: Object.assign({}, defParams,
ex.request.params || {})}),
+ uri.toString({ params: Object.assign({}, defParams,
ex.request.params || {}) }),
method,
ex.request,
ex.response || {}
@@ -125,51 +132,52 @@
function cmp(result, expected, errMsg) {
- if(expected === null || expected === undefined) {
+ if (expected === null || expected === undefined) {
// nothing to expect, so we can return
return true;
}
- if(result === null || result === undefined) {
+ if (result === null || result === undefined) {
result = '';
}
- if(expected.constructor === Object) {
- Object.keys(expected).forEach(function(key) {
- var val = expected[key];
- assert.deepEqual(result.hasOwnProperty(key), true, 'Body field ' +
key + ' not found in response!');
- cmp(result[key], val, key + ' body field mismatch!');
+ if (expected.constructor === Object) {
+ Object.keys(expected).forEach((key) => {
+ const val = expected[key];
+ assert.deepEqual({}.hasOwnProperty.call(result, key), true,
+ `Body field ${key} not found in response!`);
+ cmp(result[key], val, `${key} body field mismatch!`);
});
return true;
- } else if(expected.constructor === Array) {
- if(result.constructor !== Array) {
+ } else if (expected.constructor === Array) {
+ if (result.constructor !== Array) {
assert.deepEqual(result, expected, errMsg);
return true;
}
// only one item in expected - compare them all
- if(expected.length === 1 && result.length > 1) {
- result.forEach(function(item) {
+ if (expected.length === 1 && result.length > 1) {
+ result.forEach((item) => {
cmp(item, expected[0], errMsg);
});
return true;
}
// more than one item expected, check them one by one
- if(expected.length !== result.length) {
+ if (expected.length !== result.length) {
assert.deepEqual(result, expected, errMsg);
return true;
}
- expected.forEach(function(item, idx) {
+ expected.forEach((item, idx) => {
cmp(result[idx], item, errMsg);
});
return true;
}
- if(expected.length > 1 && expected[0] === '/' && expected[expected.length
- 1] === '/') {
- if((new RegExp(expected.slice(1, -1))).test(result)) {
+ if (expected.length > 1 && expected[0] === '/' && expected[expected.length
- 1] === '/') {
+ if ((new RegExp(expected.slice(1, -1))).test(result)) {
return true;
}
- } else if(expected.length === 0 && result.length === 0) {
+ } else if (expected.length === 0 && result.length === 0) {
return true;
- } else if(result === expected || result.startsWith(expected)) {
+ } else if (result === expected || result.startsWith(expected)) {
return true;
}
@@ -181,32 +189,35 @@
function validateTestResponse(testCase, res) {
- var expRes = testCase.response;
+ const expRes = testCase.response;
// check the status
assert.status(res, expRes.status);
// check the headers
- Object.keys(expRes.headers).forEach(function(key) {
- var val = expRes.headers[key];
- assert.deepEqual(res.headers.hasOwnProperty(key), true, 'Header ' +
key + ' not found in response!');
- cmp(res.headers[key], val, key + ' header mismatch!');
+ Object.keys(expRes.headers).forEach((key) => {
+ const val = expRes.headers[key];
+ assert.deepEqual({}.hasOwnProperty.call(res.headers, key), true,
+ `Header ${key} not found in response!`);
+ cmp(res.headers[key], val, `${key} header mismatch!`);
});
// check the body
- if(!expRes.body) {
+ if (!expRes.body) {
return true;
}
res.body = res.body || '';
- if(Buffer.isBuffer(res.body)) { res.body = res.body.toString(); }
- if(expRes.body.constructor !== res.body.constructor) {
- if(expRes.body.constructor === String) {
+ if (Buffer.isBuffer(res.body)) { res.body = res.body.toString(); }
+ if (expRes.body.constructor !== res.body.constructor) {
+ if (expRes.body.constructor === String) {
res.body = JSON.stringify(res.body);
} else {
res.body = JSON.parse(res.body);
}
}
// check that the body type is the same
- if(expRes.body.constructor !== res.body.constructor) {
- throw new Error('Expected a body of type ' + expRes.body.constructor +
' but gotten ' + res.body.constructor);
+ if (expRes.body.constructor !== res.body.constructor) {
+ throw new Error(
+ `Expected body type ${expRes.body.constructor} but got
${res.body.constructor}`
+ );
}
// compare the bodies
@@ -220,19 +231,19 @@
describe('Swagger spec', function() {
// the variable holding the spec
- var spec = staticSpecLoad();
+ let spec = staticSpecLoad();
// default params, if given
- var defParams = spec['x-default-params'] || {};
+ let defParams = spec['x-default-params'] || {};
- this.timeout(20000);
+ this.timeout(20000); // eslint-disable-line no-invalid-this
- before(function () {
+ before(() => {
return server.start();
});
- it('get the spec', function() {
- return preq.get(server.config.uri + '?spec')
- .then(function(res) {
+ it('get the spec', () => {
+ return preq.get(`${server.config.uri}?spec`)
+ .then((res) => {
assert.status(200);
assert.contentType(res, 'application/json');
assert.notDeepEqual(res.body, undefined, 'No body received!');
@@ -240,25 +251,24 @@
});
});
- it('spec validation', function() {
- if(spec['x-default-params']) {
+ it('spec validation', () => {
+ if (spec['x-default-params']) {
defParams = spec['x-default-params'];
}
// check the high-level attributes
- ['info', 'swagger', 'paths'].forEach(function(prop) {
- assert.deepEqual(!!spec[prop], true, 'No ' + prop + ' field
present!');
+ ['info', 'swagger', 'paths'].forEach((prop) => {
+ assert.deepEqual(!!spec[prop], true, `No ${prop} field present!`);
});
// no paths - no love
assert.deepEqual(!!Object.keys(spec.paths), true, 'No paths given in
the spec!');
// now check each path
- Object.keys(spec.paths).forEach(function(pathStr) {
- var path;
+ Object.keys(spec.paths).forEach((pathStr) => {
assert.deepEqual(!!pathStr, true, 'A path cannot have a length of
zero!');
- path = spec.paths[pathStr];
- assert.deepEqual(!!Object.keys(path), true, 'No methods defined
for path: ' + pathStr);
- Object.keys(path).forEach(function(method) {
- var mSpec = path[method];
- if(mSpec.hasOwnProperty('x-monitor') && !mSpec['x-monitor']) {
+ const path = spec.paths[pathStr];
+ assert.deepEqual(!!Object.keys(path), true, `No methods defined
for path: ${pathStr}`);
+ Object.keys(path).forEach((method) => {
+ const mSpec = path[method];
+ if ({}.hasOwnProperty.call(mSpec, 'x-monitor') &&
!mSpec['x-monitor']) {
return;
}
validateExamples(pathStr, defParams, mSpec['x-amples']);
@@ -266,20 +276,19 @@
});
});
- describe('routes', function() {
+ describe('routes', () => {
- constructTests(spec.paths, defParams).forEach(function(testCase) {
- it(testCase.title, function() {
+ constructTests(spec.paths, defParams).forEach((testCase) => {
+ it(testCase.title, () => {
return preq(testCase.request)
- .then(function(res) {
+ .then((res) => {
validateTestResponse(testCase, res);
- }, function(err) {
+ }, (err) => {
validateTestResponse(testCase, err);
});
});
});
});
-
});
diff --git a/test/utils/assert.js b/test/utils/assert.js
index ed030ea..dfbcdef 100644
--- a/test/utils/assert.js
+++ b/test/utils/assert.js
@@ -1,7 +1,26 @@
+/* eslint-disable no-console */
+
'use strict';
-var assert = require('assert');
+const assert = require('assert');
+
+
+function deepEqual(result, expected, message) {
+
+ try {
+ if (typeof expected === 'string') {
+ assert.ok(result === expected || (new
RegExp(expected).test(result)));
+ } else {
+ assert.deepEqual(result, expected, message);
+ }
+ } catch (e) {
+ console.log(`Expected:\n${JSON.stringify(expected, null, 2)}`);
+ console.log(`Result:\n${JSON.stringify(result, null, 2)}`);
+ throw e;
+ }
+
+}
/**
@@ -10,7 +29,7 @@
function status(res, expected) {
deepEqual(res.status, expected,
- 'Expected status to be ' + expected + ', but was ' + res.status);
+ `Expected status to be ${expected}, but was ${res.status}`);
}
@@ -20,9 +39,9 @@
*/
function contentType(res, expected) {
- var actual = res.headers['content-type'];
+ const actual = res.headers['content-type'];
deepEqual(actual, expected,
- 'Expected content-type to be ' + expected + ', but was ' + actual);
+ `Expected content-type to be ${expected}, but was ${actual}`);
}
@@ -43,30 +62,13 @@
}
-function deepEqual(result, expected, message) {
-
- try {
- if (typeof expected === 'string') {
- assert.ok(result === expected || (new
RegExp(expected).test(result)));
- } else {
- assert.deepEqual(result, expected, message);
- }
- } catch (e) {
- console.log('Expected:\n' + JSON.stringify(expected, null, 2));
- console.log('Result:\n' + JSON.stringify(result, null, 2));
- throw e;
- }
-
-}
-
-
function notDeepEqual(result, expected, message) {
try {
assert.notDeepEqual(result, expected, message);
} catch (e) {
- console.log('Not expected:\n' + JSON.stringify(expected, null, 2));
- console.log('Result:\n' + JSON.stringify(result, null, 2));
+ console.log(`Not expected:\n${JSON.stringify(expected, null, 2)}`);
+ console.log(`Result:\n${JSON.stringify(result, null, 2)}`);
throw e;
}
@@ -75,7 +77,7 @@
function fails(promise, onRejected) {
- var failed = false;
+ let failed = false;
function trackFailure(e) {
failed = true;
diff --git a/test/utils/logStream.js b/test/utils/logStream.js
index f8e292d..3872fb7 100644
--- a/test/utils/logStream.js
+++ b/test/utils/logStream.js
@@ -1,68 +1,72 @@
+/* eslint-disable no-console */
+
'use strict';
-var bunyan = require('bunyan');
+const bunyan = require('bunyan');
function logStream(logStdout) {
- var log = [];
- var parrot = bunyan.createLogger({
- name: 'test-logger',
- level: 'warn'
- });
+ const log = [];
+ const parrot = bunyan.createLogger({
+ name: 'test-logger',
+ level: 'warn'
+ });
- function write(chunk, encoding, callback) {
- try {
- var entry = JSON.parse(chunk);
- var levelMatch = /^(\w+)/.exec(entry.levelPath);
- if (logStdout && levelMatch) {
- var level = levelMatch[1];
- if (parrot[level]) {
- parrot[level](entry);
+ function write(chunk, encoding, callback) {
+ try {
+ const entry = JSON.parse(chunk);
+ const levelMatch = /^(\w+)/.exec(entry.levelPath);
+ if (logStdout && levelMatch) {
+ const level = levelMatch[1];
+ if (parrot[level]) {
+ parrot[level](entry);
+ }
}
+ } catch (e) {
+ console.error('something went wrong trying to parrot a log entry',
e, chunk);
}
- } catch (e) {
- console.error('something went wrong trying to parrot a log entry', e,
chunk);
+
+ log.push(chunk);
}
- log.push(chunk);
- }
-
- // to implement the stream writer interface
- function end(chunk, encoding, callback) {
- }
-
- function get() {
- return log;
- }
-
- function slice() {
-
- var begin = log.length;
- var end = null;
-
- function halt() {
- if (end === null) {
- end = log.length;
- }
+ // to implement the stream writer interface
+ function end(chunk, encoding, callback) {
}
function get() {
- return log.slice(begin, end);
+ return log;
+ }
+
+ function slice() {
+
+ const begin = log.length;
+ let end = null;
+
+ function halt() {
+ if (end === null) {
+ end = log.length;
+ }
+ }
+
+ function get() {
+ return log.slice(begin, end);
+ }
+
+ /* Disable eslint object-shorthand until Node 4 support is dropped */
+ /* eslint-disable object-shorthand */
+ return {
+ halt: halt,
+ get: get
+ };
+
}
return {
- halt: halt,
- get: get
+ write: write,
+ end: end,
+ slice: slice,
+ get: get
};
-
- }
-
- return {
- write: write,
- end: end,
- slice: slice,
- get: get
- };
}
module.exports = logStream;
diff --git a/test/utils/server.js b/test/utils/server.js
index ba0d718..f9d0127 100644
--- a/test/utils/server.js
+++ b/test/utils/server.js
@@ -1,28 +1,24 @@
'use strict';
-// mocha defines to avoid JSHint breakage
-/* global describe, it, before, beforeEach, after, afterEach */
-
-
-var BBPromise = require('bluebird');
-var ServiceRunner = require('service-runner');
-var logStream = require('./logStream');
-var fs = require('fs');
-var assert = require('./assert');
-var yaml = require('js-yaml');
-var extend = require('extend');
+const BBPromise = require('bluebird');
+const ServiceRunner = require('service-runner');
+const logStream = require('./logStream');
+const fs = require('fs');
+const assert = require('./assert');
+const yaml = require('js-yaml');
+const extend = require('extend');
// set up the configuration
-var config = {
- conf: yaml.safeLoad(fs.readFileSync(__dirname + '/../../config.yaml'))
+let config = {
+ conf: yaml.safeLoad(fs.readFileSync(`${__dirname}/../../config.yaml`))
};
// build the API endpoint URI by supposing the actual service
// is the last one in the 'services' list in the config file
-var myServiceIdx = config.conf.services.length - 1;
-var myService = config.conf.services[myServiceIdx];
-config.uri = 'http://localhost:' + myService.conf.port + '/';
+const myServiceIdx = config.conf.services.length - 1;
+const myService = config.conf.services[myServiceIdx];
+config.uri = `http://localhost:${myService.conf.port}/`;
config.service = myService;
// no forking, run just one process when testing
config.conf.num_workers = 0;
@@ -33,11 +29,11 @@
stream: logStream()
};
// make a deep copy of it for later reference
-var origConfig = extend(true, {}, config);
+const origConfig = extend(true, {}, config);
-var stop = function() { return BBPromise.resolve(); };
-var options = null;
-var runner = new ServiceRunner();
+module.exports.stop = () => { return BBPromise.resolve(); };
+let options = null;
+const runner = new ServiceRunner();
function start(_options) {
@@ -45,18 +41,23 @@
_options = _options || {};
if (!assert.isDeepEqual(options, _options)) {
- console.log('server options changed; restarting');
- return stop().then(function() {
+ console.log('starting test server'); // eslint-disable-line no-console
+ return module.exports.stop().then(() => {
options = _options;
// set up the config
config = extend(true, {}, origConfig);
extend(true, config.conf.services[myServiceIdx].conf, options);
return runner.start(config.conf)
- .then(function() {
- stop = function () {
- console.log('stopping test server');
- return runner.stop().then(function() {
- stop = function() { return BBPromise.resolve(); };
+ .then((serviceReturns) => {
+ module.exports.stop = () => {
+ console.log('stopping test server'); //
eslint-disable-line no-console
+ serviceReturns.forEach(servers =>
+ servers.forEach(server =>
+ server.shutdown()));
+ return runner.stop().then(() => {
+ module.exports.stop = () => {
+ return BBPromise.resolve();
+ };
});
};
return true;
@@ -65,7 +66,6 @@
} else {
return BBPromise.resolve();
}
-
}
module.exports.config = config;
--
To view, visit https://gerrit.wikimedia.org/r/398020
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id65e4383aff42192f500efaa70e6045d3ecf939f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/recommendation-api
Gerrit-Branch: master
Gerrit-Owner: Mobrovac <mobro...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
