From b2a5bc980e94b7289b08ce7dc37bdc11e3541337 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe <github.erisds@gmail.com> Date: Mon, 7 Feb 2022 17:28:53 +0000 Subject: [PATCH] Abstracted sentEmail assertions as part of mock utils The idea here is to keep the concept of _what_ we're asserting (that an email was sent) away from the implementation of how we check that assertion This means that tests can have a consistent api like .sentEmail(), and if we change how that function works, we only have to update the assertion function in the MockManager Much more cleanup to come behind the scenes, but the aim is to make the tests as clean as possible --- .../api/admin/authentication.test.js | 15 +++++--- test/regression/api/admin/members.test.js | 10 +++--- test/utils/e2e-framework-mock-utils.js | 36 +++++++++++++++++-- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/test/regression/api/admin/authentication.test.js b/test/regression/api/admin/authentication.test.js index cb2a3da557..05e792a486 100644 --- a/test/regression/api/admin/authentication.test.js +++ b/test/regression/api/admin/authentication.test.js @@ -409,11 +409,16 @@ describe('Authentication API canary', function () { const sessions = await models.Session.fetchAll(); expect(sessions.length).to.equal(0); - expect(emailStub.callCount).to.equal(2); - expect(emailStub.firstCall.args[0].subject).to.equal('Reset Password'); - expect(emailStub.secondCall.args[0].subject).to.equal('Reset Password'); - expect(emailStub.firstCall.args[0].to).to.equal('jbloggs@example.com'); - expect(emailStub.secondCall.args[0].to).to.equal('ghost-author@example.com'); + mockManager.assert.sentEmailCount(2); + + mockManager.assert.sentEmail({ + subject: 'Reset Password', + to: 'jbloggs@example.com' + }); + mockManager.assert.sentEmail({ + subject: 'Reset Password', + to: 'ghost-author@example.com' + }); }); }); }); diff --git a/test/regression/api/admin/members.test.js b/test/regression/api/admin/members.test.js index ece8bc4813..956cbc7eb7 100644 --- a/test/regression/api/admin/members.test.js +++ b/test/regression/api/admin/members.test.js @@ -1,14 +1,10 @@ -const path = require('path'); const querystring = require('querystring'); const should = require('should'); -const supertest = require('supertest'); const sinon = require('sinon'); const testUtils = require('../../../utils'); const {agentProvider, mockManager, fixtureManager} = require('../../../utils/e2e-framework'); const localUtils = require('./utils'); -const config = require('../../../../core/shared/config'); const labs = require('../../../../core/shared/labs'); -const mailService = require('../../../../core/server/services/mail'); let agent; let emailStub; @@ -65,8 +61,10 @@ describe('Members API', function () { should.exist(res.headers.location); res.headers.location.should.equal(`http://127.0.0.1:2369${localUtils.API.getApiQuery('members/')}${res.body.members[0].id}/`); - mailService.GhostMailer.prototype.send.called.should.be.true(); - mailService.GhostMailer.prototype.send.args[0][0].to.should.equal('member_getting_confirmation@test.com'); + mockManager.assert.sentEmail({ + subject: '🙌 Complete your sign up to Ghost!', + to: 'member_getting_confirmation@test.com' + }); await agent .delete(`members/${jsonResponse.members[0].id}/`) diff --git a/test/utils/e2e-framework-mock-utils.js b/test/utils/e2e-framework-mock-utils.js index ac3002851f..3913ff37d6 100644 --- a/test/utils/e2e-framework-mock-utils.js +++ b/test/utils/e2e-framework-mock-utils.js @@ -1,7 +1,9 @@ const errors = require('@tryghost/errors'); const sinon = require('sinon'); +const assert = require('node:assert'); let mocks = {}; +let emailCount = 0; const mailService = require('../../core/server/services/mail/index'); @@ -13,20 +15,50 @@ const mockMail = () => { return mocks.mail; }; -const assertMailSentTo = (email) => { +const sentEmailCount = (count) => { if (!mocks.mail) { throw new errors.IncorrectUsageError({ message: 'Cannot assert on mail when mail has not been mocked' }); } + + sinon.assert.callCount(mocks.mail, count); +}; + +const sentEmail = (matchers) => { + if (!mocks.mail) { + throw new errors.IncorrectUsageError({ + message: 'Cannot assert on mail when mail has not been mocked' + }); + } + + let spyCall = mocks.mail.getCall(emailCount); + + // We increment here so that the messaging has an index of 1, whilst getting the call has an index of 0 + emailCount += 1; + + sinon.assert.called(mocks.mail); + + Object.keys(matchers).forEach((key) => { + let value = matchers[key]; + + // We use assert, rather than sinon.assert.calledWith, as we end up with much better error messaging + assert.notEqual(spyCall.args[0][key], undefined, `Expected email to have property ${key}`); + assert.equal(spyCall.args[0][key], value, `Expected Email ${emailCount} to have ${key} of ${value}`); + }); }; const restore = () => { sinon.restore(); mocks = {}; + emailCount = 0; }; module.exports = { mockMail, - restore + restore, + assert: { + sentEmailCount, + sentEmail + } };