From aca95f35829f3bf19d1160f1759f76d1919de3aa Mon Sep 17 00:00:00 2001 From: Robert Rossmann Date: Tue, 10 Jul 2018 12:31:22 +0200 Subject: [PATCH] =?UTF-8?q?test:=20epic=20improvement=20to=20test=20covera?= =?UTF-8?q?ge=20=F0=9F=92=AA?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../atlas/src/private/component-container.mjs | 8 ++--- packages/atlas/src/private/dispatch.mjs | 9 +++-- packages/atlas/test/aliasing.test.mjs | 26 ++++++++++++++ .../atlas/test/democonfig/serialisers.mjs | 2 +- packages/atlas/test/dispatch.test.mjs | 20 +++++++++++ packages/atlas/test/prepare.test.mjs | 4 +-- packages/cli/src/commands/start.mjs | 3 ++ packages/cli/test/commands/start.test.mjs | 36 +++++++++++++++++++ packages/component/test/api.test.mjs | 4 +-- packages/errors/test/api.test.mjs | 23 ++++++++++++ packages/koa/src/middleware.mjs | 4 +-- packages/koa/test/context/api.test.mjs | 2 +- packages/koa/test/context/testcontext.mjs | 5 +-- packages/koa/test/server/stop.test.mjs | 4 +-- packages/koa/test/websocket/api.test.mjs | 12 +++++++ .../mongoose/test/service/prepare.test.mjs | 21 +++++++---- packages/objection/src/service.mjs | 3 +- .../objection/test/service/models/index.mjs | 2 ++ .../objection/test/service/models/model-c.mjs | 5 +++ .../objection/test/service/prepare.test.mjs | 29 +++++++++++++-- .../objection/test/service/start.test.mjs | 4 +-- packages/repl/test/action/enter.test.mjs | 8 ++--- packages/sequelize/src/service.mjs | 2 +- .../test/migration/sequelizemock.mjs | 5 +-- .../sequelize/test/service/prepare.test.mjs | 16 +++++++-- 25 files changed, 211 insertions(+), 46 deletions(-) create mode 100644 packages/objection/test/service/models/model-c.mjs diff --git a/packages/atlas/src/private/component-container.mjs b/packages/atlas/src/private/component-container.mjs index 970b5da..499b570 100644 --- a/packages/atlas/src/private/component-container.mjs +++ b/packages/atlas/src/private/component-container.mjs @@ -113,7 +113,7 @@ class ComponentContainer { * @param {Object} options.catalog Atlas catalog of all components * @return {Promise} */ - async prepare(options = {}) { + async prepare(options) { this.component.log.trace('prepare:before') this::mkcatalog(this.#catalog, options) @@ -138,11 +138,11 @@ class ComponentContainer { /** * Start the component * - * @param {Object} opts={} Additional options + * @param {Object} opts Additional options * @param {Map} opts.hooks Hooks available in the application * @return {Promise} */ - async start(opts = {}) { + async start(opts) { if (this.started) { return this.component } @@ -224,7 +224,7 @@ class ComponentContainer { */ function mkobservers(observers, { hooks }) { for (const [alias, container] of hooks) { - const target = (container.aliases || {})[container.Component.observes] + const target = container.aliases[container.Component.observes] if (this.alias === target) { observers.set(alias, container) diff --git a/packages/atlas/src/private/dispatch.mjs b/packages/atlas/src/private/dispatch.mjs index cc49720..5629645 100644 --- a/packages/atlas/src/private/dispatch.mjs +++ b/packages/atlas/src/private/dispatch.mjs @@ -23,14 +23,13 @@ async function dispatch(event, subject) { } } - await Promise.all(Array.from(tasks.values())) - - // Ensure no uncaught error escapes from this place - // This utilises the fact that we can catch any Promise-related errors by attaching a .catch block - // to the promise, even if the promise body has already executed. 💡 for (const [hook, task] of tasks) { task.catch(err => void hook.component.log.error({ err, event }, 'hook:event:failure')) } + + // Ensure no uncaught error escapes from this place + await Promise.all(Array.from(tasks.values())) + .catch(() => {}) } export default dispatch diff --git a/packages/atlas/test/aliasing.test.mjs b/packages/atlas/test/aliasing.test.mjs index 5528e7e..865ae5d 100644 --- a/packages/atlas/test/aliasing.test.mjs +++ b/packages/atlas/test/aliasing.test.mjs @@ -124,4 +124,30 @@ describe('Atlas: cross-component communication', () => { expect(() => atlas.actions.dummy.ping('service:lolsvc')).to.throw(FrameworkError) }) + + it('declaring a component of invalid type throws', () => { + class DummyAction extends Action { + static requires = ['invalid:name'] + } + + atlas.action('dummy', DummyAction, { aliases: { 'invalid:name': 'dummy' } }) + + return expect(atlas.start()).to.be.eventually.rejectedWith( + FrameworkError, + /Invalid component type: invalid used in alias invalid:name/, + ) + }) + + it('resolving an alias into unknown component throws', () => { + class DummyAction extends Action { + static requires = ['service:dummy'] + } + + atlas.action('dummy', DummyAction, { aliases: { 'service:dummy': 'unknown' } }) + + return expect(atlas.start()).to.be.eventually.rejectedWith( + FrameworkError, + /Unable to find service unknown aliased as service:dummy/, + ) + }) }) diff --git a/packages/atlas/test/democonfig/serialisers.mjs b/packages/atlas/test/democonfig/serialisers.mjs index e0a27b6..ef16450 100644 --- a/packages/atlas/test/democonfig/serialisers.mjs +++ b/packages/atlas/test/democonfig/serialisers.mjs @@ -8,5 +8,5 @@ const stdSerializers = { export default { ...stdSerializers, - custom: () => {}, + custom: true, } diff --git a/packages/atlas/test/dispatch.test.mjs b/packages/atlas/test/dispatch.test.mjs index 8bfcfe4..02b9ab8 100644 --- a/packages/atlas/test/dispatch.test.mjs +++ b/packages/atlas/test/dispatch.test.mjs @@ -71,4 +71,24 @@ describe('Hook: custom events', () => { return expect(atlas.actions.dummy.trigger('handleOtherEvent')) .to.not.eventually.be.rejectedWith(Error) }) + + it('still resolves even if one of the hooks throws', async () => { + atlas = new Atlas({ + root: __dirname, + config: { + atlas: { log: { level: 'fatal' } }, + }, + }) + atlas.hook('dummy', DummyHook, { aliases: { 'action:dummy': 'dummy' } }) + atlas.hook('another', AnotherHook, { aliases: { 'action:dummy': 'dummy' } }) + atlas.action('dummy', DummyAction) + + await atlas.start() + + DummyHook.prototype.handleEvent = sinon.stub().resolves() + AnotherHook.prototype.handleEvent = sinon.stub().rejects(new Error('u-oh')) + + return expect(atlas.actions.dummy.trigger('handleEvent')) + .to.not.be.eventually.rejectedWith(Error) + }) }) diff --git a/packages/atlas/test/prepare.test.mjs b/packages/atlas/test/prepare.test.mjs index 31958b4..9bb2a00 100644 --- a/packages/atlas/test/prepare.test.mjs +++ b/packages/atlas/test/prepare.test.mjs @@ -16,7 +16,7 @@ class DummyHook extends Hook { } class DummyAction extends Action { - dummyMethod() {} + dummyMethod = sinon.stub() } describe('Atlas::prepare()', () => { @@ -154,7 +154,7 @@ describe('Atlas::prepare()', () => { atlas.hook('empty', Empty) - return expect(atlas.prepare()).to.be.rejectedWith( + return expect(atlas.prepare()).to.eventually.be.rejectedWith( FrameworkError, /does not have static 'observes' property/i, ) diff --git a/packages/cli/src/commands/start.mjs b/packages/cli/src/commands/start.mjs index 5c7c781..36130ff 100644 --- a/packages/cli/src/commands/start.mjs +++ b/packages/cli/src/commands/start.mjs @@ -57,6 +57,9 @@ class Start extends Command { * @return {void} */ function forcequit() { + process.removeListener('SIGINT', forcequit) + process.removeListener('SIGTERM', forcequit) + throw new Error('Forced quit') } diff --git a/packages/cli/test/commands/start.test.mjs b/packages/cli/test/commands/start.test.mjs index 3f90745..5f81d7a 100644 --- a/packages/cli/test/commands/start.test.mjs +++ b/packages/cli/test/commands/start.test.mjs @@ -89,5 +89,41 @@ describe('CLI: start', () => { expect(disconnect).to.have.callCount(1) }) + + it('throws an error to stop the process if a terminating signal is sent again', async () => { + await start.run() + start.doExit() + + // Sending SIGINT causes the test suite to exit with code 130, so just test SIGTERM 🤷‍♂️ + expect(() => process.emit('SIGTERM')).to.throw(Error, /Forced quit/) + + delete process.exitCode + }) + + it('kills the process after 10s if an error occurs during stop procedure', async function() { + this.sandbox.stub(process, 'exit') + this.sandbox.stub(console, 'error') + + const error = new Error('u-oh') + const clock = sinon.useFakeTimers({ + toFake: ['setTimeout'], + }) + + start.atlas.stop.rejects(error) + await start.doExit() + + // eslint-disable-next-line no-console + expect(console.error).to.have.been.calledWith(error.stack) + expect(process.exit).to.have.callCount(0) + + clock.runAll() + clock.restore() + + expect(process.exit).to.have.callCount(1) + expect(process.exitCode).to.equal(1) + expect(clock.now).to.equal(10000) + + delete process.exitCode + }) }) }) diff --git a/packages/component/test/api.test.mjs b/packages/component/test/api.test.mjs index ddd27ea..847b80b 100644 --- a/packages/component/test/api.test.mjs +++ b/packages/component/test/api.test.mjs @@ -44,7 +44,7 @@ describe('Component: basics and API', () => { }) it('saves the component function given on constructor to itself', () => { - const resolve = () => {} + const resolve = sinon.stub() const component = new Component({ component: resolve, }) @@ -53,7 +53,7 @@ describe('Component: basics and API', () => { }) it('saves the dispatch function given on constructor to itself', () => { - const dispatch = () => {} + const dispatch = sinon.stub() const component = new Component({ dispatch }) expect(component).itself.to.respondTo('dispatch') diff --git a/packages/errors/test/api.test.mjs b/packages/errors/test/api.test.mjs index a7fe0fd..90664af 100644 --- a/packages/errors/test/api.test.mjs +++ b/packages/errors/test/api.test.mjs @@ -10,4 +10,27 @@ describe('Errors', () => { expect(new errors.FrameworkError()).to.be.instanceOf(Error) }) }) + + describe('ValidationError', () => { + it('exists', () => { + expect(errors.ValidationError).to.be.a('function') + }) + + it('inherits from FrameworkError', () => { + expect(new errors.ValidationError()).to.be.instanceOf(errors.FrameworkError) + }) + + it('exposes the constructor argument as this.errors', () => { + const data = { error: '123' } + const err = new errors.ValidationError(data) + + expect(err.errors).to.eql(data) + }) + + it('works even if no errors are given to the constructor', () => { + const err = new errors.ValidationError() + + expect(err.errors).to.be.an('object') + }) + }) }) diff --git a/packages/koa/src/middleware.mjs b/packages/koa/src/middleware.mjs index 803ba10..5b8ba77 100644 --- a/packages/koa/src/middleware.mjs +++ b/packages/koa/src/middleware.mjs @@ -2,13 +2,13 @@ * Register the given middleware functions into the given Koa-compatible instance * * @param {Object} instance Koa-compatible instance. Must implement `.use()`. - * @param {Object} handlers={} Object where keys are the middlewares' names and the + * @param {Object} handlers Object where keys are the middlewares' names and the * values are the actual middleware functions. * @param {Object} config={} Configuration for individual middlewares. The keys should * match the middleware names. * @return {void} */ -export default function middleware(instance, handlers = {}, config = {}) { +export default function middleware(instance, handlers, config = {}) { for (const [name, handler] of Object.entries(handlers)) { instance.use(handler(config[name])) } diff --git a/packages/koa/test/context/api.test.mjs b/packages/koa/test/context/api.test.mjs index a218a60..f18509f 100644 --- a/packages/koa/test/context/api.test.mjs +++ b/packages/koa/test/context/api.test.mjs @@ -56,7 +56,7 @@ describe('Koa: ContextHook', () => { }) it('throws when the property already exists on koa.context', () => { - server.context.testmethod = () => {} + server.context.testmethod = sinon.stub() expect(() => hook.afterPrepare()) .to.throw(FrameworkError, /Unable to extend koa.context/) }) diff --git a/packages/koa/test/context/testcontext.mjs b/packages/koa/test/context/testcontext.mjs index 3d7d66c..37bddf8 100644 --- a/packages/koa/test/context/testcontext.mjs +++ b/packages/koa/test/context/testcontext.mjs @@ -1,4 +1,5 @@ +/* global sinon */ export default { - testmethod() {}, - anothermethod() {}, + testmethod: sinon.stub(), + anothermethod: sinon.stub(), } diff --git a/packages/koa/test/server/stop.test.mjs b/packages/koa/test/server/stop.test.mjs index 02ebb8d..cba77c2 100644 --- a/packages/koa/test/server/stop.test.mjs +++ b/packages/koa/test/server/stop.test.mjs @@ -53,9 +53,7 @@ describe('Koa::stop(instance)', () => { it('throws when called on an instance not yet prepared', () => { delete instance.server const msg = /Cannot stop a non-running server/ - service = new Koa({ - log: { info: () => {} }, - }) + service = new Koa({}) return expect(service.stop(instance)).to.eventually.be.rejectedWith(FrameworkError, msg) }) diff --git a/packages/koa/test/websocket/api.test.mjs b/packages/koa/test/websocket/api.test.mjs index 9665aa9..5ff6620 100644 --- a/packages/koa/test/websocket/api.test.mjs +++ b/packages/koa/test/websocket/api.test.mjs @@ -103,6 +103,12 @@ describe('Hook: WebsocketHook', () => { expect(args.first).to.eql({ firsttest: true }) expect(args.second).to.eql({ secondtest: true }) }) + + it('works without any middleware configured', async () => { + delete config.middleware + + await hook.afterPrepare() + }) }) @@ -164,5 +170,11 @@ describe('Hook: WebsocketHook', () => { await hook.beforeStop() expect(koa.ws.server.close).to.have.callCount(1) }) + + it('re-throws any errors returned from the koa.ws.server.close() method', () => { + koa.ws.server.close.callsArgWithAsync(0, new Error('u-oh')) + + return expect(hook.beforeStop()).to.eventually.be.rejectedWith(Error, /u-oh/) + }) }) }) diff --git a/packages/mongoose/test/service/prepare.test.mjs b/packages/mongoose/test/service/prepare.test.mjs index a467493..5328c96 100644 --- a/packages/mongoose/test/service/prepare.test.mjs +++ b/packages/mongoose/test/service/prepare.test.mjs @@ -8,7 +8,9 @@ describe('Mongoose::prepare()', () => { beforeEach(async () => { service = new Mongoose({ atlas: {}, - log: {}, + log: { + trace: sinon.stub(), + }, config: {}, }) @@ -24,13 +26,18 @@ describe('Mongoose::prepare()', () => { expect(await service.prepare()).to.be.instanceof(mongoose.Mongoose) }) - it('sets a debug function to log model events', async function() { - this.sandbox.stub(Object.getPrototypeOf(instance), 'set') - + it('sets a debug function to log model events', async () => { await service.prepare() - expect(instance.set).to.have.callCount(1) - expect(instance.set).to.have.been.calledWith('debug') - expect(instance.set.firstCall.args[1]).to.be.a('function') + expect(instance.options.debug).to.be.a('function') + + instance.options.debug('collection', 'method', 'arg1', 'arg2') + + expect(service.log.trace).to.have.callCount(1) + expect(service.log.trace).to.have.been.calledWith({ + collection: 'collection', + method: 'method', + args: ['arg1', 'arg2'], + }) }) }) diff --git a/packages/objection/src/service.mjs b/packages/objection/src/service.mjs index 6dcf9b1..2dcae93 100644 --- a/packages/objection/src/service.mjs +++ b/packages/objection/src/service.mjs @@ -39,7 +39,8 @@ class Objection extends Service { }, } - prepare() { + // eslint-disable-next-line require-await + async prepare() { const models = this.atlas.require(this.config.models) || {} const connection = knex(this.config.knex) const client = { diff --git a/packages/objection/test/service/models/index.mjs b/packages/objection/test/service/models/index.mjs index f28f4c1..bb3257e 100644 --- a/packages/objection/test/service/models/index.mjs +++ b/packages/objection/test/service/models/index.mjs @@ -1,7 +1,9 @@ import ModelA from './model-a' import ModelB from './model-b' +import ModelC from './model-c' export { ModelA, ModelB, + ModelC, } diff --git a/packages/objection/test/service/models/model-c.mjs b/packages/objection/test/service/models/model-c.mjs new file mode 100644 index 0000000..86ff1ab --- /dev/null +++ b/packages/objection/test/service/models/model-c.mjs @@ -0,0 +1,5 @@ +import { Model } from '../../..' + +export default class ModelC extends Model { + static tableName = 'modelc' +} diff --git a/packages/objection/test/service/prepare.test.mjs b/packages/objection/test/service/prepare.test.mjs index ca9f5df..4ba7fcb 100644 --- a/packages/objection/test/service/prepare.test.mjs +++ b/packages/objection/test/service/prepare.test.mjs @@ -1,4 +1,5 @@ import path from 'path' +import { errors } from '@atlas.js/atlas' import { Service as Objection } from '../..' import * as models from './models' @@ -42,11 +43,14 @@ describe('Objection::prepare()', () => { expect(instance.models).to.have.all.keys([ 'ModelA', 'ModelB', + 'ModelC', ]) }) it('resolves model names in relationship mappings to actual model classes', () => { - for (const Model of Object.values(instance.models)) { + void ['ModelA', 'ModelB'].forEach(name => { + const Model = instance.models[name] + // Sanity check expect(Model).to.have.property('relationMappings') expect(Object.keys(Model.relationMappings).length).to.be.greaterThan(0) @@ -54,7 +58,12 @@ describe('Objection::prepare()', () => { for (const relation of Object.values(Model.relationMappings)) { expect(relation.modelClass).to.be.a('function') } - } + }) + }) + + it('works with models with no relationship mappings', () => { + expect(instance.models.ModelC).to.be.a('function') + expect(instance.models.ModelC).not.to.have.property('relationshipMappings') }) it('exposes atlas instance as both static and instance property', () => { @@ -63,4 +72,20 @@ describe('Objection::prepare()', () => { expect(Model.prototype).to.have.property('atlas') } }) + + it('throws if a model relation references unknown model name', () => { + class ModelA { + static relationMappings = { + customRelation: { + modelClass: 'UnknownModel', + }, + } + } + service.atlas.require.returns({ ModelA }) + + return expect(service.prepare()).to.eventually.be.rejectedWith( + errors.FrameworkError, + /Unable to find relation UnknownModel defined in ModelA/, + ) + }) }) diff --git a/packages/objection/test/service/start.test.mjs b/packages/objection/test/service/start.test.mjs index 5db4bd0..bfa6760 100644 --- a/packages/objection/test/service/start.test.mjs +++ b/packages/objection/test/service/start.test.mjs @@ -44,7 +44,7 @@ describe('Objection::start()', () => { }) it('prefetches table metadata for all models', () => { - expect(Object.values(instance.models)).to.have.length(2) + expect(Object.values(instance.models)).to.have.length(Object.keys(models).length) for (const Model of Object.values(instance.models)) { expect(Model.fetchTableMetadata).to.have.callCount(1) @@ -62,7 +62,7 @@ describe('Objection::start()', () => { this.sandbox.stub(instance.connection, 'raw').resolves() // await service.start(instance) - expect(Object.values(instance.models)).to.have.length(2) + expect(Object.values(instance.models)).to.have.length(Object.keys(models).length) for (const Model of Object.values(instance.models)) { expect(Model.fetchTableMetadata).to.have.callCount(0) diff --git a/packages/repl/test/action/enter.test.mjs b/packages/repl/test/action/enter.test.mjs index f68718d..305be90 100644 --- a/packages/repl/test/action/enter.test.mjs +++ b/packages/repl/test/action/enter.test.mjs @@ -156,7 +156,7 @@ describe('Repl::enter()', () => { // Read whatever has been sent to the output const out = opts.output.read().toString('utf8') - const matches = out.match(new RegExp(opts.nl, 'g')) || [] + const matches = out.match(new RegExp(opts.nl, 'g')) // Make sure we get four lines separated by the \r\n escape sequence expect(matches).to.have.length(4) }) @@ -173,7 +173,7 @@ describe('Repl::enter()', () => { // Read whatever has been sent to the output const out = opts.output.read().toString('utf8') - const matches = out.match(new RegExp('', 'g')) || [] + const matches = out.match(new RegExp('', 'g')) // Make sure we get four lines separated by the escape sequence expect(matches).to.have.length(4) }) @@ -187,7 +187,7 @@ describe('Repl::enter()', () => { // Read whatever has been sent to the output const out = opts.output.read().toString('utf8') - const matches = out.match(/\r\n/g) || [] + const matches = out.match(/\r\n/g) // Make sure we get four lines separated by the \r\n escape sequence expect(matches).to.have.length(4) }) @@ -201,7 +201,7 @@ describe('Repl::enter()', () => { // Read whatever has been sent to the output const out = opts.output.read().toString('utf8') - const matches = out.match(/\n/g) || [] + const matches = out.match(/\n/g) // Make sure we get four lines separated by the \r\n escape sequence expect(matches).to.have.length(4) }) diff --git a/packages/sequelize/src/service.mjs b/packages/sequelize/src/service.mjs index 13b6c92..a52edbe 100644 --- a/packages/sequelize/src/service.mjs +++ b/packages/sequelize/src/service.mjs @@ -41,7 +41,7 @@ class Sequelize extends Service { function strace(sql) { // Remove the initial "Executing (default): prefix to keep only the SQL statement" - sql = sql.slice(sql.indexOf(':') + 1) + sql = sql.slice(sql.indexOf(':') + 1).trim() this.log.trace({ sql }, 'sequelize activity') } diff --git a/packages/sequelize/test/migration/sequelizemock.mjs b/packages/sequelize/test/migration/sequelizemock.mjs index 9d4c084..ed5fcbc 100644 --- a/packages/sequelize/test/migration/sequelizemock.mjs +++ b/packages/sequelize/test/migration/sequelizemock.mjs @@ -1,8 +1,5 @@ export default function mksequelizemock() { - const model = { - sync: () => Promise.resolve(model), - findAll: () => Promise.resolve([]), - } + const model = {} return { isDefined: () => true, diff --git a/packages/sequelize/test/service/prepare.test.mjs b/packages/sequelize/test/service/prepare.test.mjs index d44e28a..739789b 100644 --- a/packages/sequelize/test/service/prepare.test.mjs +++ b/packages/sequelize/test/service/prepare.test.mjs @@ -8,7 +8,9 @@ describe('Sequelize::prepare()', () => { beforeEach(async () => { service = new Database({ atlas: {}, - log: {}, + log: { + trace: sinon.stub(), + }, config: { uri: 'sqlite://test-db.sqlite', options: { @@ -34,7 +36,15 @@ describe('Sequelize::prepare()', () => { it('sets a debug function to log sequelize events', () => { expect(instance.options.logging).to.be.a('function') - // eslint-disable-next-line no-console - expect(instance.options.logging).to.not.equal(console.log) + }) + + it('prints the SQL statements Sequelize executes at trace level using the service logger', () => { + const example = 'Executing (default): SELECT 1 + 1' + const fn = instance.options.logging + + fn(example) + + expect(service.log.trace).to.have.callCount(1) + expect(service.log.trace).to.have.been.calledWith({ sql: 'SELECT 1 + 1' }) }) })