Mobrovac has uploaded a new change for review. ( 
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/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/page.js
M test/features/v1/siteinfo.js
M test/utils/server.js
13 files changed, 296 insertions(+), 264 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/chromium-render 
refs/changes/60/392060/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 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/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 863fdd0..c2dd183 100644
--- a/package.json
+++ b/package.json
@@ -30,27 +30,28 @@
   "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",
-    "nsp": "^2.6.3",
+    "nsp": "^2.8.1",
     "mocha-eslint": "^3.0.1",
     "eslint": "^3.12.0",
     "eslint-config-node-services": "^2.0.2",
@@ -60,7 +61,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/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: newchange
Gerrit-Change-Id: I155142886f439ecec811698cf2cf2abf29396dbe
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/chromium-render
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

Reply via email to