From dd214aa67c838f52c7c7a8064057d9022481410e Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Mon, 6 May 2024 11:30:44 +0200 Subject: [PATCH] Refactored `@tryghost/mw-error-handler` to assert - removes should as our preferred assertion lib is `assert` - removes empty test utils, these won't be needed --- ghost/mw-error-handler/package.json | 1 - .../test/mw-error-handler.test.js | 132 +++++++++--------- .../mw-error-handler/test/utils/assertions.js | 11 -- ghost/mw-error-handler/test/utils/index.js | 11 -- .../mw-error-handler/test/utils/overrides.js | 10 -- 5 files changed, 65 insertions(+), 100 deletions(-) delete mode 100644 ghost/mw-error-handler/test/utils/assertions.js delete mode 100644 ghost/mw-error-handler/test/utils/index.js delete mode 100644 ghost/mw-error-handler/test/utils/overrides.js diff --git a/ghost/mw-error-handler/package.json b/ghost/mw-error-handler/package.json index a8c567d1c4..f74d594d77 100644 --- a/ghost/mw-error-handler/package.json +++ b/ghost/mw-error-handler/package.json @@ -20,7 +20,6 @@ "devDependencies": { "c8": "8.0.1", "mocha": "10.2.0", - "should": "13.2.3", "sinon": "15.2.0" }, "dependencies": { diff --git a/ghost/mw-error-handler/test/mw-error-handler.test.js b/ghost/mw-error-handler/test/mw-error-handler.test.js index 074644e226..8a64e347a0 100644 --- a/ghost/mw-error-handler/test/mw-error-handler.test.js +++ b/ghost/mw-error-handler/test/mw-error-handler.test.js @@ -1,9 +1,7 @@ -// Switch these lines once there are useful utils -// const testUtils = require('./utils'); -require('./utils'); const path = require('path'); -const should = require('should'); const assert = require('assert/strict'); +const sinon = require('sinon'); + const {InternalServerError, NotFoundError} = require('@tryghost/errors'); const {cacheControlValues} = require('@tryghost/http-cache-utils'); const { @@ -22,12 +20,12 @@ describe('Prepare Error', function () { prepareError(new Error('test!'), {}, { set: () => {} }, (err) => { - err.statusCode.should.eql(500); - err.name.should.eql('InternalServerError'); - err.message.should.eql('An unexpected error occurred, please try again.'); - err.context.should.eql('test!'); - err.code.should.eql('UNEXPECTED_ERROR'); - err.stack.should.startWith('Error: test!'); + assert.equal(err.statusCode, 500); + assert.equal(err.name, 'InternalServerError'); + assert.equal(err.message, 'An unexpected error occurred, please try again.'); + assert.equal(err.context, 'test!'); + assert.equal(err.code, 'UNEXPECTED_ERROR'); + assert.ok(err.stack.startsWith('Error: test!')); done(); }); }); @@ -36,11 +34,11 @@ describe('Prepare Error', function () { prepareError(new InternalServerError({message: 'Handled Error', context: 'Details'}), {}, { set: () => {} }, (err) => { - err.statusCode.should.eql(500); - err.name.should.eql('InternalServerError'); - err.message.should.eql('Handled Error'); - err.context.should.eql('Details'); - err.stack.should.startWith('InternalServerError: Handled Error'); + assert.equal(err.statusCode, 500); + assert.equal(err.name, 'InternalServerError'); + assert.equal(err.message, 'Handled Error'); + assert.equal(err.context, 'Details'); + assert.ok(err.stack.startsWith('InternalServerError: Handled Error')); done(); }); }); @@ -51,10 +49,10 @@ describe('Prepare Error', function () { prepareError(error, {}, { set: () => {} }, (err) => { - err.statusCode.should.eql(404); - err.name.should.eql('NotFoundError'); - err.stack.should.startWith('NotFoundError: Resource could not be found'); - err.hideStack.should.eql(true); + assert.equal(err.statusCode, 404); + assert.equal(err.name, 'NotFoundError'); + assert.ok(err.stack.startsWith('NotFoundError: Resource could not be found')); + assert.equal(err.hideStack, true); done(); }); }); @@ -63,9 +61,9 @@ describe('Prepare Error', function () { prepareError([new Error('test!')], {}, { set: () => {} }, (err) => { - err.statusCode.should.eql(500); - err.name.should.eql('InternalServerError'); - err.stack.should.startWith('Error: test!'); + assert.equal(err.statusCode, 500); + assert.equal(err.name, 'InternalServerError'); + assert.ok(err.stack.startsWith('Error: test!')); done(); }); }); @@ -79,11 +77,11 @@ describe('Prepare Error', function () { prepareError(error, {}, { set: () => {} }, (err) => { - err.statusCode.should.eql(400); - err.name.should.eql('IncorrectUsageError'); + assert.equal(err.statusCode, 400); + assert.equal(err.name, 'IncorrectUsageError'); // TODO: consider if the message should be trusted here - err.message.should.eql('obscure handlebars message!'); - err.stack.should.startWith('Error: obscure handlebars message!'); + assert.equal(err.message, 'obscure handlebars message!'); + assert.ok(err.stack.startsWith('Error: obscure handlebars message!')); done(); }); }); @@ -97,10 +95,10 @@ describe('Prepare Error', function () { prepareError(error, {}, { set: () => {} }, (err) => { - err.statusCode.should.eql(400); - err.name.should.eql('IncorrectUsageError'); - err.message.should.eql('obscure express-hbs message!'); - err.stack.should.startWith('Error: obscure express-hbs message!'); + assert.equal(err.statusCode, 400); + assert.equal(err.name, 'IncorrectUsageError'); + assert.equal(err.message, 'obscure express-hbs message!'); + assert.ok(err.stack.startsWith('Error: obscure express-hbs message!')); done(); }); }); @@ -117,13 +115,13 @@ describe('Prepare Error', function () { prepareError(error, {}, { set: () => {} }, (err) => { - err.statusCode.should.eql(500); - err.name.should.eql('InternalServerError'); - err.message.should.eql('An unexpected error occurred, please try again.'); - err.code.should.eql('UNEXPECTED_ERROR'); - err.sqlErrorCode.should.eql('ER_WRONG_VALUE'); - err.sql.should.eql('select anything from anywhere where something = anything;'); - err.sqlMessage.should.eql('Incorrect DATETIME value: 3234234234'); + assert.equal(err.statusCode, 500); + assert.equal(err.name, 'InternalServerError'); + assert.equal(err.message, 'An unexpected error occurred, please try again.'); + assert.equal(err.code, 'UNEXPECTED_ERROR'); + assert.equal(err.sqlErrorCode, 'ER_WRONG_VALUE'); + assert.equal(err.sql, 'select anything from anywhere where something = anything;'); + assert.equal(err.sqlMessage, 'Incorrect DATETIME value: 3234234234'); done(); }); }); @@ -133,7 +131,7 @@ describe('Prepare Stack', function () { it('Correctly prepares the stack for an error', function (done) { prepareStack(new Error('test!'), {}, {}, (err) => { // Includes "Stack Trace" text prepending human readable trace - err.stack.should.startWith('Error: test!\nStack Trace:'); + assert.ok(err.stack.startsWith('Error: test!\nStack Trace:')); done(); }); }); @@ -201,8 +199,8 @@ describe('Error renderers', function () { it('Renders JSON', function (done) { jsonErrorRenderer(new Error('test!'), {}, { json: (data) => { - data.errors.length.should.eql(1); - data.errors[0].message.should.eql('test!'); + assert.equal(data.errors.length, 1); + assert.equal(data.errors[0].message, 'test!'); done(); } }, () => {}); @@ -216,9 +214,9 @@ describe('Error renderers', function () { } }, { json: (data) => { - data.errors.length.should.eql(1); - data.errors[0].message.should.eql('Unknown error - RangeError, cannot read oembed.'); - data.errors[0].context.should.eql('test!'); + assert.equal(data.errors.length, 1); + assert.equal(data.errors[0].message, 'Unknown error - RangeError, cannot read oembed.'); + assert.equal(data.errors[0].context, 'test!'); done(); } }, () => {}); @@ -234,9 +232,9 @@ describe('Error renderers', function () { } }, { json: (data) => { - data.errors.length.should.eql(1); - data.errors[0].message.should.eql('Internal server error, cannot list blog.'); - data.errors[0].context.should.eql('test!'); + assert.equal(data.errors.length, 1); + assert.equal(data.errors[0].message, 'Internal server error, cannot list blog.'); + assert.equal(data.errors[0].context, 'test!'); done(); } }, () => {}); @@ -253,9 +251,9 @@ describe('Error renderers', function () { } }, { json: (data) => { - data.errors.length.should.eql(1); - data.errors[0].message.should.eql('Internal server error, cannot upload image.'); - data.errors[0].context.should.eql('test! Image was too large.'); + assert.equal(data.errors.length, 1); + assert.equal(data.errors[0].message, 'Internal server error, cannot upload image.'); + assert.equal(data.errors[0].context, 'test! Image was too large.'); done(); } }, () => {}); @@ -266,7 +264,7 @@ describe('Error renderers', function () { errorHandler: () => {} }); - renderer.length.should.eql(4); + assert.equal(renderer.length, 4); }); it('Exports the JSON renderer', function () { @@ -274,15 +272,15 @@ describe('Error renderers', function () { errorHandler: () => {} }); - renderer.length.should.eql(5); + assert.equal(renderer.length, 5); }); }); describe('Resource Not Found', function () { it('Returns 404 Not Found Error for a generic case', function (done) { resourceNotFound({}, {}, (error) => { - should.equal(error.statusCode, 404); - should.equal(error.message, 'Resource not found'); + assert.equal(error.statusCode, 404); + assert.equal(error.message, 'Resource not found'); done(); }); }); @@ -301,10 +299,10 @@ describe('Resource Not Found', function () { }; resourceNotFound(req, res, (error) => { - should.equal(error.statusCode, 406); - should.equal(error.message, 'Request could not be served, the endpoint was not found.'); - should.equal(error.context, 'Provided client accept-version v3.9 is behind current Ghost version v4.3.'); - should.equal(error.help, 'Try upgrading your Ghost API client.'); + assert.equal(error.statusCode, 406); + assert.equal(error.message, 'Request could not be served, the endpoint was not found.'); + assert.equal(error.context, 'Provided client accept-version v3.9 is behind current Ghost version v4.3.'); + assert.equal(error.help, 'Try upgrading your Ghost API client.'); done(); }); }); @@ -323,10 +321,10 @@ describe('Resource Not Found', function () { }; resourceNotFound(req, res, (error) => { - should.equal(error.statusCode, 406); - should.equal(error.message, 'Request could not be served, the endpoint was not found.'); - should.equal(error.context, 'Provided client accept-version v4.8 is ahead of current Ghost version v4.3.'); - should.equal(error.help, 'Try upgrading your Ghost install.'); + assert.equal(error.statusCode, 406); + assert.equal(error.message, 'Request could not be served, the endpoint was not found.'); + assert.equal(error.context, 'Provided client accept-version v4.8 is ahead of current Ghost version v4.3.'); + assert.equal(error.help, 'Try upgrading your Ghost install.'); done(); }); }); @@ -345,8 +343,8 @@ describe('Resource Not Found', function () { }; resourceNotFound(req, res, (error) => { - should.equal(error.statusCode, 404); - should.equal(error.message, 'Resource not found'); + assert.equal(error.statusCode, 404); + assert.equal(error.message, 'Resource not found'); done(); }); }); @@ -354,16 +352,16 @@ describe('Resource Not Found', function () { describe('pageNotFound', function () { it('returns 404 with special message when message not set', function (done) { pageNotFound({}, {}, (error) => { - should.equal(error.statusCode, 404); - should.equal(error.message, 'Page not found'); + assert.equal(error.statusCode, 404); + assert.equal(error.message, 'Page not found'); done(); }); }); it('returns 404 with special message even if message is set', function (done) { pageNotFound({message: 'uh oh'}, {}, (error) => { - should.equal(error.statusCode, 404); - should.equal(error.message, 'Page not found'); + assert.equal(error.statusCode, 404); + assert.equal(error.message, 'Page not found'); done(); }); }); diff --git a/ghost/mw-error-handler/test/utils/assertions.js b/ghost/mw-error-handler/test/utils/assertions.js deleted file mode 100644 index 7364ee8aa1..0000000000 --- a/ghost/mw-error-handler/test/utils/assertions.js +++ /dev/null @@ -1,11 +0,0 @@ -/** - * Custom Should Assertions - * - * Add any custom assertions to this file. - */ - -// Example Assertion -// should.Assertion.add('ExampleAssertion', function () { -// this.params = {operator: 'to be a valid Example Assertion'}; -// this.obj.should.be.an.Object; -// }); diff --git a/ghost/mw-error-handler/test/utils/index.js b/ghost/mw-error-handler/test/utils/index.js deleted file mode 100644 index 0d67d86ff8..0000000000 --- a/ghost/mw-error-handler/test/utils/index.js +++ /dev/null @@ -1,11 +0,0 @@ -/** - * Test Utilities - * - * Shared utils for writing tests - */ - -// Require overrides - these add globals for tests -require('./overrides'); - -// Require assertions - adds custom should assertions -require('./assertions'); diff --git a/ghost/mw-error-handler/test/utils/overrides.js b/ghost/mw-error-handler/test/utils/overrides.js deleted file mode 100644 index 90203424ee..0000000000 --- a/ghost/mw-error-handler/test/utils/overrides.js +++ /dev/null @@ -1,10 +0,0 @@ -// This file is required before any test is run - -// Taken from the should wiki, this is how to make should global -// Should is a global in our eslint test config -global.should = require('should').noConflict(); -should.extend(); - -// Sinon is a simple case -// Sinon is a global in our eslint test config -global.sinon = require('sinon');