Pmiazga has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/392060 )
Change subject: Update to service-template-node v0.5.3
......................................................................
Update to service-template-node v0.5.3
Bug: T180800
Change-Id: I155142886f439ecec811698cf2cf2abf29396dbe
---
M .travis.yml
M app.js
M lib/api-util.js
M lib/queue.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/features/ex/errors.js
M test/features/info/info.js
M test/features/v1/html2pdf.js
M test/features/v1/page.js
M test/features/v1/siteinfo.js
M test/utils/server.js
15 files changed, 302 insertions(+), 265 deletions(-)
Approvals:
Pmiazga: Verified; Looks good to me, approved
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 ae8cc51..9a5a618 100644
--- a/app.js
+++ b/app.js
@@ -1,16 +1,17 @@
'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 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');
/**
@@ -21,7 +22,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
@@ -30,22 +31,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;
@@ -54,35 +55,35 @@
}
// 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);
// 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,
@@ -90,18 +91,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');
@@ -121,11 +122,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);
@@ -135,45 +136,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
@@ -186,28 +188,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);
});
@@ -227,9 +230,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 00e8e38..8e12e8d 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/queue.js b/lib/queue.js
index 22e7022..84a3e98 100644
--- a/lib/queue.js
+++ b/lib/queue.js
@@ -45,7 +45,7 @@
/**
* Return number of waiting/in progress jobs
- * @return {Number}
+ * @return {number}
*/
_countJobsInQueue() {
const queue = this._queueObject;
diff --git a/lib/util.js b/lib/util.js
index 6e974ab..1452505 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 8da44e9..c9a5628 100644
--- a/package.json
+++ b/package.json
@@ -31,28 +31,29 @@
"homepage":
"https://github.com/wikimedia/mediawiki-services-chromium-render";,
"dependencies": {
"async": "^2.6.0",
- "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",
+ "compression": "^1.7.1",
+ "domino": "^1.0.30",
+ "express": "^4.16.2",
+ "js-yaml": "^3.10.0",
+ "preq": "^0.5.3",
"puppeteer": "^0.11.0",
- "service-runner": "^2.3.0",
- "swagger-router": "^0.5.6",
- "swagger-ui": "git+https://github.com/wikimedia/swagger-ui.git#master";
+ "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",
+ "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",
"json" :"9.0.6",
- "nsp": "^2.6.3",
+ "nsp": "^2.8.1",
"mocha-eslint": "^3.0.1",
"eslint": "^3.12.0",
"eslint-config-node-services": "^2.0.2",
@@ -62,7 +63,7 @@
},
"deploy": {
"target": "debian",
- "node": "6.9.1",
+ "node": "6.11.1",
"dependencies": {
"debian": [
"gconf-service",
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..3cef837 100644
--- a/test/features/app/app.js
+++ b/test/features/app/app.js
@@ -1,33 +1,37 @@
'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 +39,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 +56,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..1eeb1c6 100644
--- a/test/features/app/spec.js
+++ b/test/features/app/spec.js
@@ -1,25 +1,29 @@
'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 +33,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 +70,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 +91,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 +130,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 +187,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 +229,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 +249,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 +274,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/features/ex/errors.js b/test/features/ex/errors.js
index 4e7ec44..e4152c9 100644
--- a/test/features/ex/errors.js
+++ b/test/features/ex/errors.js
@@ -5,6 +5,10 @@
var assert = require('../../utils/assert.js');
var server = require('../../utils/server.js');
+if (!server.stopHookAdded) {
+ server.stopHookAdded = true;
+ after(() => server.stop());
+}
describe('errors', function() {
@@ -84,6 +88,5 @@
assert.deepEqual(err.body.type, 'unauthorized');
});
});
-
});
diff --git a/test/features/info/info.js b/test/features/info/info.js
index a866dfb..65b8551 100644
--- a/test/features/info/info.js
+++ b/test/features/info/info.js
@@ -1,58 +1,62 @@
'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('service information', function() {
this.timeout(20000);
- before(function () { return server.start(); });
+ before(() => { return server.start(); });
// common URI prefix for info tests
- var infoUri = server.config.uri + '_info/';
+ const infoUri = `${server.config.uri}_info/`;
// common function used for generating requests
// and checking their return values
function checkRet(fieldName) {
return preq.get({
uri: infoUri + fieldName
- }).then(function(res) {
+ }).then((res) => {
// check the returned Content-Type header
assert.contentType(res, 'application/json');
// the status as well
assert.status(res, 200);
// finally, check the body has the specified field
assert.notDeepEqual(res.body, undefined, 'No body returned!');
- assert.notDeepEqual(res.body[fieldName], undefined, 'No ' +
fieldName + ' field returned!');
+ assert.notDeepEqual(res.body[fieldName], undefined, `No
${fieldName} field returned!`);
});
}
- it('should get the service name', function() {
+ it('should get the service name', () => {
return checkRet('name');
});
- it('should get the service version', function() {
+ it('should get the service version', () => {
return checkRet('version');
});
- it('should redirect to the service home page', function() {
+ it('should redirect to the service home page', () => {
return preq.get({
- uri: infoUri + 'home',
+ uri: `${infoUri}home`,
followRedirect: false
- }).then(function(res) {
+ }).then((res) => {
// check the status
assert.status(res, 301);
});
});
- it('should get the service info', function() {
+ it('should get the service info', () => {
return preq.get({
uri: infoUri
- }).then(function(res) {
+ }).then((res) => {
// check the status
assert.status(res, 200);
// check the returned Content-Type header
@@ -65,6 +69,5 @@
assert.notDeepEqual(res.body.home, undefined, 'No home field
returned!');
});
});
-
});
diff --git a/test/features/v1/html2pdf.js b/test/features/v1/html2pdf.js
index 38099ff..6a23a6f 100644
--- a/test/features/v1/html2pdf.js
+++ b/test/features/v1/html2pdf.js
@@ -4,6 +4,11 @@
const assert = require('../../utils/assert.js');
const server = require('../../utils/server.js');
+if (!server.stopHookAdded) {
+ server.stopHookAdded = true;
+ after(() => server.stop());
+}
+
describe('html2pdf', function() {
this.timeout(20000);
before(function () { return server.start(); });
diff --git a/test/features/v1/page.js b/test/features/v1/page.js
index a6064a6..26e0cc7 100644
--- a/test/features/v1/page.js
+++ b/test/features/v1/page.js
@@ -5,6 +5,10 @@
var assert = require('../../utils/assert.js');
var server = require('../../utils/server.js');
+if (!server.stopHookAdded) {
+ server.stopHookAdded = true;
+ after(() => server.stop());
+}
describe('page gets', function() {
@@ -64,6 +68,5 @@
assert.deepEqual(err.status, 404);
});
});
-
});
diff --git a/test/features/v1/siteinfo.js b/test/features/v1/siteinfo.js
index 8284d40..415f5e7 100644
--- a/test/features/v1/siteinfo.js
+++ b/test/features/v1/siteinfo.js
@@ -5,6 +5,10 @@
var assert = require('../../utils/assert.js');
var server = require('../../utils/server.js');
+if (!server.stopHookAdded) {
+ server.stopHookAdded = true;
+ after(() => server.stop());
+}
describe('wiki site info', function() {
@@ -66,6 +70,5 @@
assert.deepEqual(err.status, 504);
});
});
-
});
diff --git a/test/utils/server.js b/test/utils/server.js
index ba0d718..8a67d3f 100644
--- a/test/utils/server.js
+++ b/test/utils/server.js
@@ -5,24 +5,24 @@
/* 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 +33,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 +45,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 +70,6 @@
} else {
return BBPromise.resolve();
}
-
}
module.exports.config = config;
--
To view, visit https://gerrit.wikimedia.org/r/392060
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I155142886f439ecec811698cf2cf2abf29396dbe
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/services/chromium-render
Gerrit-Branch: master
Gerrit-Owner: Mobrovac <mobro...@wikimedia.org>
Gerrit-Reviewer: Bmansurov <bmansu...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Pmiazga <pmia...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
