From 36c04aad070c5648d07497bdd44fdfcda08fdd90 Mon Sep 17 00:00:00 2001 From: Carmine DiMascio Date: Wed, 9 Oct 2019 20:25:57 -0400 Subject: [PATCH] refactor tests --- src/middlewares/openapi.response.validator.ts | 27 +------------- test/additional.props.spec.ts | 25 ++++++------- test/coercion.spec.ts | 25 ++++++------- test/common/app.common.ts | 4 +-- test/headers.spec.ts | 4 +-- test/multipart.spec.ts | 30 +++++++--------- test/nullable.spec.ts | 25 ++++++------- test/path.level.parameters.spec.ts | 22 ++++++------ test/query.params.spec.ts | 17 ++++----- test/resources/response.validation.yaml | 2 +- test/response.validation.spec.ts | 35 +++++++++---------- 11 files changed, 89 insertions(+), 127 deletions(-) diff --git a/src/middlewares/openapi.response.validator.ts b/src/middlewares/openapi.response.validator.ts index a9e54f82..bb8cc287 100644 --- a/src/middlewares/openapi.response.validator.ts +++ b/src/middlewares/openapi.response.validator.ts @@ -18,8 +18,7 @@ export class ResponseValidator { constructor(openApiSpec, options: any = {}) { this.spec = openApiSpec; this.ajv = createResponseAjv(openApiSpec, options); - (mung).onError = function(err, req, res, next) { - // monkey patch mung to rethrow exception + (mung).onError = (err, req, res, next) => { return next(err); }; } @@ -129,28 +128,4 @@ export class ResponseValidator { } return validators; } - - private validateBody(body) {} - - private toOpenapiValidationError(error: Ajv.ErrorObject) { - const validationError = { - path: `instance${error.dataPath}`, - errorCode: `${error.keyword}.openapi.responseValidation`, - message: error.message, - }; - - validationError.path = validationError.path.replace( - /^instance\.(?:response\.)?/, - '', - ); - - validationError.message = - validationError.path + ' ' + validationError.message; - - if (validationError.path === 'response') { - delete validationError.path; - } - - return validationError; - } } diff --git a/test/additional.props.spec.ts b/test/additional.props.spec.ts index 2311af21..284a97f5 100644 --- a/test/additional.props.spec.ts +++ b/test/additional.props.spec.ts @@ -8,7 +8,6 @@ const packageJson = require('../package.json'); describe(packageJson.name, () => { let app = null; - let basePath = null; before(async () => { // Set up the express app @@ -17,16 +16,14 @@ describe(packageJson.name, () => { 'resources', 'additional.properties.yaml', ); - app = await createApp({ apiSpec }, 3005); - basePath = app.basePath; - - // Define new coercion routes - app.use( - `${basePath}/additional_props`, - express - .Router() - .post(`/false`, (req, res) => res.json(req.body)) - .post(`/true`, (req, res) => res.json(req.body)), + app = await createApp({ apiSpec }, 3005, app => + app.use( + `${app.basePath}/additional_props`, + express + .Router() + .post(`/false`, (req, res) => res.json(req.body)) + .post(`/true`, (req, res) => res.json(req.body)), + ), ); }); @@ -36,14 +33,14 @@ describe(packageJson.name, () => { it('should return 400 if additionalProperties=false, but extra props sent', async () => request(app) - .post(`${basePath}/additional_props/false`) + .post(`${app.basePath}/additional_props/false`) .send({ name: 'test', extra_prop: 'test', }) .expect(400) .then(r => { - expect(r.body.errors).to.be.an('array') + expect(r.body.errors).to.be.an('array'); expect(r.body.errors).to.have.length(1); const message = r.body.errors[0].message; expect(message).to.equal('should NOT have additional properties'); @@ -51,7 +48,7 @@ describe(packageJson.name, () => { it('should return 200 if additonalProperities=true and extra props are sent', async () => request(app) - .post(`${basePath}/additional_props/true`) + .post(`${app.basePath}/additional_props/true`) .send({ name: 'test', extra_prop: 'test', diff --git a/test/coercion.spec.ts b/test/coercion.spec.ts index c1364155..5b71d9a1 100644 --- a/test/coercion.spec.ts +++ b/test/coercion.spec.ts @@ -8,21 +8,18 @@ const packageJson = require('../package.json'); describe(packageJson.name, () => { let app = null; - let basePath = null; before(async () => { // Set up the express app const apiSpec = path.join('test', 'resources', 'coercion.yaml'); - app = await createApp({ apiSpec }, 3005); - basePath = app.basePath; - - // Define new coercion routes - app.use( - `${basePath}/coercion`, - express - .Router() - .post(`/pets`, (req, res) => res.json(req.body)) - .post(`/pets_string_boolean`, (req, res) => res.json(req.body)), + app = await createApp({ apiSpec }, 3005, app => + app.use( + `${app.basePath}/coercion`, + express + .Router() + .post(`/pets`, (req, res) => res.json(req.body)) + .post(`/pets_string_boolean`, (req, res) => res.json(req.body)), + ), ); }); @@ -32,7 +29,7 @@ describe(packageJson.name, () => { it('should coerce is_cat to boolean since it is defined as a boolean in the spec', async () => request(app) - .post(`${basePath}/coercion/pets`) + .post(`${app.basePath}/coercion/pets`) .send({ name: 'test', is_cat: 'true', @@ -45,7 +42,7 @@ describe(packageJson.name, () => { it('should keep is_cat as boolean', async () => request(app) - .post(`${basePath}/coercion/pets`) + .post(`${app.basePath}/coercion/pets`) .send({ name: 'test', is_cat: true, @@ -58,7 +55,7 @@ describe(packageJson.name, () => { it('should coerce a is_cat from boolean to string since it is defined as such in the spec', async () => request(app) - .post(`${basePath}/coercion/pets_string_boolean`) + .post(`${app.basePath}/coercion/pets_string_boolean`) .send({ name: 'test', is_cat: true, diff --git a/test/common/app.common.ts b/test/common/app.common.ts index 2478f102..8d72b033 100644 --- a/test/common/app.common.ts +++ b/test/common/app.common.ts @@ -1,13 +1,11 @@ import * as http from 'http'; import * as express from 'express'; -const BASE_PATH = '/v1'; export function startServer(app, port): Promise { return new Promise((resolve, reject) => { const http = require('http'); const server = http.createServer(app); app.server = server; - app.basePath = BASE_PATH; server.listen(port, () => { console.log(`Listening on port ${port}`); resolve(server); @@ -16,7 +14,7 @@ export function startServer(app, port): Promise { } export function routes(app) { - const basePath = BASE_PATH; + const basePath = app.basePath; const router1 = express .Router() .post('/', function(req, res, next) { diff --git a/test/headers.spec.ts b/test/headers.spec.ts index e97fc657..ae123482 100644 --- a/test/headers.spec.ts +++ b/test/headers.spec.ts @@ -6,13 +6,11 @@ import * as packageJson from '../package.json'; describe(packageJson.name, () => { let app = null; - let basePath = null; before(() => { const apiSpec = path.join('test', 'resources', 'openapi.yaml'); return createApp({ apiSpec }, 3004).then(a => { app = a; - basePath = (app).basePath; }); }); @@ -22,7 +20,7 @@ describe(packageJson.name, () => { it('should throw 400 if required header is missing', async () => request(app) - .get(`${basePath}/pets/10/attributes`) + .get(`${app.basePath}/pets/10/attributes`) .set('Accept', 'application/json') .expect('Content-Type', /json/) .expect(400) diff --git a/test/multipart.spec.ts b/test/multipart.spec.ts index f5ca85d2..939a33ce 100644 --- a/test/multipart.spec.ts +++ b/test/multipart.spec.ts @@ -5,25 +5,21 @@ import { createApp } from './common/app'; import * as packageJson from '../package.json'; describe(packageJson.name, () => { - let app = null; - let basePath = null; + describe(`GET .../pets/:id/photos`, () => { + let app = null; - before(() => { - const apiSpec = path.join('test', 'resources', 'openapi.yaml'); - return createApp({ apiSpec }, 3003).then(a => { - app = a; - basePath = (app).basePath; + before(async () => { + const apiSpec = path.join('test', 'resources', 'openapi.yaml'); + app = await createApp({ apiSpec }, 3003); }); - }); - after(() => { - (app).server.close(); - }); + after(() => { + (app).server.close(); + }); - describe(`GET ${basePath}/pets/:id/photos`, () => { it('should throw 400 when required multipart file field', async () => request(app) - .post(`${basePath}/pets/10/photos`) + .post(`${app.basePath}/pets/10/photos`) .set('Content-Type', 'multipart/form-data') .set('Accept', 'application/json') .expect(400) @@ -38,7 +34,7 @@ describe(packageJson.name, () => { it('should throw 400 when required form field is missing during multipart upload', async () => request(app) - .post(`${basePath}/pets/10/photos`) + .post(`${app.basePath}/pets/10/photos`) .set('Content-Type', 'multipart/form-data') .set('Accept', 'application/json') .attach('file', 'package.json') @@ -46,7 +42,7 @@ describe(packageJson.name, () => { it('should validate multipart file and metadata', async () => request(app) - .post(`${basePath}/pets/10/photos`) + .post(`${app.basePath}/pets/10/photos`) .set('Content-Type', 'multipart/form-data') .set('Accept', 'application/json') .attach('file', 'package.json') @@ -65,7 +61,7 @@ describe(packageJson.name, () => { it('should throw 405 get method not allowed', async () => request(app) - .get(`${basePath}/pets/10/photos`) + .get(`${app.basePath}/pets/10/photos`) .set('Content-Type', 'multipart/form-data') .set('Accept', 'application/json') .expect('Content-Type', /json/) @@ -75,7 +71,7 @@ describe(packageJson.name, () => { it('should throw 415 unsupported media type', async () => request(app) - .post(`${basePath}/pets/10/photos`) + .post(`${app.basePath}/pets/10/photos`) .send({ test: 'test' }) .set('Content-Type', 'application/json') .expect('Content-Type', /json/) diff --git a/test/nullable.spec.ts b/test/nullable.spec.ts index 3ff36ce4..ce7d8b6c 100644 --- a/test/nullable.spec.ts +++ b/test/nullable.spec.ts @@ -13,12 +13,13 @@ describe(packageJson.name, () => { before(async () => { // Set up the express app const apiSpec = path.join('test', 'resources', 'nullable.yaml'); - app = await createApp({ apiSpec, coerceTypes: false }, 3005); - basePath = app.basePath; - - app.use( - `${basePath}`, - express.Router().post(`/pets/nullable`, (req, res) => res.json(req.body)), + app = await createApp({ apiSpec, coerceTypes: false }, 3005, app => + app.use( + `${app.basePath}`, + express + .Router() + .post(`/pets/nullable`, (req, res) => res.json(req.body)), + ), ); }); @@ -28,7 +29,7 @@ describe(packageJson.name, () => { it('should allow null to be set (name: nullable true)', async () => request(app) - .post(`${basePath}/pets/nullable`) + .post(`${app.basePath}/pets/nullable`) .send({ name: null, }) @@ -39,7 +40,7 @@ describe(packageJson.name, () => { it('should not fill an explicity null with default when coerceTypes is false', async () => request(app) - .post(`${basePath}/pets`) + .post(`${app.basePath}/pets`) .send({ name: null, }) @@ -47,7 +48,7 @@ describe(packageJson.name, () => { it('should fill unspecified field with default when coerceTypes is false', async () => request(app) - .post(`${basePath}/pets`) + .post(`${app.basePath}/pets`) .send({ name: 'name', }) @@ -58,7 +59,7 @@ describe(packageJson.name, () => { it('should fail if required and not provided (nullable true)', async () => request(app) - .post(`${basePath}/pets/nullable`) + .post(`${app.basePath}/pets/nullable`) .send({}) .expect(400) .then(r => { @@ -67,7 +68,7 @@ describe(packageJson.name, () => { it('should fail if required and not provided (nullable false', async () => request(app) - .post(`${basePath}/pets`) + .post(`${app.basePath}/pets`) .send({}) .expect(400) .then(r => { @@ -76,7 +77,7 @@ describe(packageJson.name, () => { it('should fail if required and provided as null when nullable is false', async () => request(app) - .post(`${basePath}/pets`) + .post(`${app.basePath}/pets`) .send({ name: null, }) diff --git a/test/path.level.parameters.spec.ts b/test/path.level.parameters.spec.ts index fa751212..3bc60054 100644 --- a/test/path.level.parameters.spec.ts +++ b/test/path.level.parameters.spec.ts @@ -17,13 +17,13 @@ describe(packageJson.name, () => { 'resources', 'path.level.parameters.yaml', ); - app = await createApp({ apiSpec }, 3005); - basePath = app.basePath; - - // Define new coercion routes - app.use( - `${basePath}`, - express.Router().get(`/path_level_parameters`, (_req, res) => res.send()), + app = await createApp({ apiSpec }, 3005, app => + app.use( + `${app.basePath}`, + express + .Router() + .get(`/path_level_parameters`, (_req, res) => res.send()), + ), ); }); @@ -33,7 +33,7 @@ describe(packageJson.name, () => { it('should return 400 if pathLevel query parameter is not provided', async () => request(app) - .get(`${basePath}/path_level_parameters?operationLevel=123`) + .get(`${app.basePath}/path_level_parameters?operationLevel=123`) .send() .expect(400) .then(r => { @@ -45,7 +45,7 @@ describe(packageJson.name, () => { it('should return 400 if operationLevel query parameter is not provided', async () => request(app) - .get(`${basePath}/path_level_parameters?pathLevel=123`) + .get(`${app.basePath}/path_level_parameters?pathLevel=123`) .send() .expect(400) .then(r => { @@ -59,7 +59,7 @@ describe(packageJson.name, () => { it('should return 400 if neither operationLevel, nor pathLevel query parameters are provided', async () => request(app) - .get(`${basePath}/path_level_parameters`) + .get(`${app.basePath}/path_level_parameters`) .send() .expect(400) .then(r => { @@ -74,7 +74,7 @@ describe(packageJson.name, () => { it('should return 200 if both pathLevel and operationLevel query parameter are provided', async () => request(app) - .get(`${basePath}/path_level_parameters?operationLevel=123&pathLevel=123`) + .get(`${app.basePath}/path_level_parameters?operationLevel=123&pathLevel=123`) .send() .expect(200)); }); diff --git a/test/query.params.spec.ts b/test/query.params.spec.ts index 89cd3af2..946a7866 100644 --- a/test/query.params.spec.ts +++ b/test/query.params.spec.ts @@ -13,12 +13,13 @@ describe(packageJson.name, () => { before(async () => { // Set up the express app const apiSpec = path.join('test', 'resources', 'query.params.yaml'); - app = await createApp({ apiSpec }, 3005); - basePath = app.basePath; - - app.use( - `${basePath}`, - express.Router().post(`/pets/nullable`, (req, res) => res.json(req.body)), + app = await createApp({ apiSpec }, 3005, app => + app.use( + `${app.basePath}`, + express + .Router() + .post(`/pets/nullable`, (req, res) => res.json(req.body)), + ), ); }); @@ -28,7 +29,7 @@ describe(packageJson.name, () => { it('should pass if known query params are specified', async () => request(app) - .get(`${basePath}/pets`) + .get(`${app.basePath}/pets`) .query({ tags: 'one,two,three', limit: 10, @@ -39,7 +40,7 @@ describe(packageJson.name, () => { it('should fail if unknown query param is specified', async () => request(app) - .get(`${basePath}/pets`) + .get(`${app.basePath}/pets`) .query({ tags: 'one,two,three', limit: 10, diff --git a/test/resources/response.validation.yaml b/test/resources/response.validation.yaml index 7437c443..26274a53 100644 --- a/test/resources/response.validation.yaml +++ b/test/resources/response.validation.yaml @@ -12,7 +12,7 @@ info: name: Apache 2.0 url: https://www.apache.org/licenses/LICENSE-2.0.html servers: - - url: /v1/ + - url: /v1 paths: /pets: description: endpoints for pets diff --git a/test/response.validation.spec.ts b/test/response.validation.spec.ts index 80746715..84cb37c8 100644 --- a/test/response.validation.spec.ts +++ b/test/response.validation.spec.ts @@ -9,30 +9,29 @@ const apiSpecPath = path.join('test', 'resources', 'response.validation.yaml'); describe(packageJson.name, () => { let app = null; - let basePath = null; before(async () => { - // Set up the express app + // set up express app app = await createApp( { apiSpec: apiSpecPath, validateResponses: true }, 3005, + app => { + app.get(`${app.basePath}/pets`, (req, res) => { + let json = {}; + if ((req.query.mode = 'bad_type')) { + json = [{ id: 'bad_id', name: 'name', tag: 'tag' }]; + } + return res.json(json); + }); + app.use((err, req, res, next) => { + res.status(err.status || 500).json({ + message: err.message, + code: err.status || 500, + }); + }); + }, false, ); - basePath = app.basePath; - app.get(`${basePath}/pets`, (req, res) => { - let json = {}; - if ((req.query.mode = 'bad_type')) { - json = [{ id: 'bad_id', name: 'name', tag: 'tag' }]; - } - return res.json(json); - }); - // Register error handler - app.use((err, req, res, next) => { - res.status(err.status || 500).json({ - message: err.message, - code: err.status, - }); - }); }); after(() => { @@ -41,7 +40,7 @@ describe(packageJson.name, () => { it('should fail if response field has a value of incorrect type', async () => request(app) - .get(`${basePath}/pets?mode=bad_type`) + .get(`${app.basePath}/pets?mode=bad_type`) .expect(500) .then((r: any) => { expect(r.body.message).to.contain('should be integer');