OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Added DomainEvents.allSettled utility method (#16075)

Browse Source 
no issue

With the increased usage of DomainEvents, it gets harder to build
reliable tests without having to resort to timeouts. This utility method
allows us to wait for all events to be processed before continuing with
the test.

This change should speed up tests and make them more reliable.

It only adds extra code when running tests and shouldn't impact
production.
This commit is contained in:
Simon Backx 2023-01-04 14:30:35 +01:00 committed by GitHub
parent e2e9a56583
commit 913ad18b71
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 157 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
ghost/core/test/e2e-api/admin/members.test.js
View File

@ -1,4 +1,4 @@
const {agentProvider, mockManager, fixtureManager, matchers, sleep} = require('../../utils/e2e-framework');
const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework');
const {anyEtag, anyObjectId, anyUuid, anyISODateTime, anyISODate, anyString, anyArray, anyLocationFor, anyContentLength, anyErrorId, anyObject} = matchers;
const ObjectId = require('bson-objectid').default;
@ -17,6 +17,7 @@ const memberAttributionService = require('../../../core/server/services/member-a
const urlService = require('../../../core/server/services/url');
const urlUtils = require('../../../core/shared/url-utils');
const settingsCache = require('../../../core/shared/settings-cache');
const DomainEvents = require('@tryghost/domain-events');
/**
* Assert that haystack and needles match, ignoring the order.
@ -732,7 +733,7 @@ describe('Members API', function () {
});
const memberPassVerification = passBody.members[0];
await sleep(100);
await DomainEvents.allSettled();
assert.ok(!settingsCache.get('email_verification_required'), 'Email verification should NOT be required');
const memberFailLimit = {
@ -753,7 +754,7 @@ describe('Members API', function () {
});
const memberFailVerification = failBody.members[0];
await sleep(100);
await DomainEvents.allSettled();
assert.ok(settingsCache.get('email_verification_required'), 'Email verification should be required');
mockManager.assert.sentEmail({
subject: 'Email needs verification'

9
ghost/core/test/e2e-api/members-comments/comments.test.js
View File

@ -1,5 +1,5 @@
const assert = require('assert');
const {agentProvider, mockManager, fixtureManager, matchers, configUtils, sleep} = require('../../utils/e2e-framework');
const {agentProvider, mockManager, fixtureManager, matchers, configUtils} = require('../../utils/e2e-framework');
const {anyEtag, anyObjectId, anyLocationFor, anyISODateTime, anyErrorId, anyUuid, anyNumber, anyBoolean} = matchers;
const should = require('should');
const models = require('../../../core/server/models');
@ -7,6 +7,7 @@ const moment = require('moment-timezone');
const settingsCache = require('../../../core/shared/settings-cache');
const sinon = require('sinon');
const settingsService = require('../../../core/server/services/settings/settings-service');
const DomainEvents = require('@tryghost/domain-events');
let membersAgent, membersAgent2, postId, postTitle, commentId;
@ -83,7 +84,7 @@ async function testCanCommentOnPost(member) {
});
// Wait for the dispatched events (because this happens async)
await sleep(200);
await DomainEvents.allSettled();
// Check last_updated_at changed?
member = await models.Member.findOne({id: member.id});
@ -126,7 +127,7 @@ async function testCanReply(member, emailMatchers = {}) {
});
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
// Check last_updated_at changed?
member = await models.Member.findOne({id: member.id});
@ -348,7 +349,7 @@ describe('Comments API', function () {
});
// Wait for the dispatched events (because this happens async)
await sleep(200);
await DomainEvents.allSettled();
// Check last updated_at is not changed?
member = await models.Member.findOne({id: member.id});

5
ghost/core/test/e2e-api/members/send-magic-link.test.js
View File

@ -1,6 +1,7 @@
const {agentProvider, mockManager, fixtureManager, matchers, sleep} = require('../../utils/e2e-framework');
const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework');
const should = require('should');
const settingsCache = require('../../../core/shared/settings-cache');
const DomainEvents = require('@tryghost/domain-events');
const {anyErrorId} = matchers;
let membersAgent, membersService;
@ -197,7 +198,7 @@ describe('sendMagicLink', function () {
const data = await membersService.api.getMemberDataFromMagicLinkToken(token);
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
// Check member alert is sent to site owners
mockManager.assert.sentEmail({
to: 'jbloggs@example.com',

5
ghost/core/test/e2e-api/members/webhooks.test.js
View File

@ -4,10 +4,11 @@ const nock = require('nock');
const should = require('should');
const stripe = require('stripe');
const {Product} = require('../../../core/server/models/product');
const {agentProvider, mockManager, fixtureManager, matchers, sleep} = require('../../utils/e2e-framework');
const {agentProvider, mockManager, fixtureManager, matchers} = require('../../utils/e2e-framework');
const models = require('../../../core/server/models');
const urlService = require('../../../core/server/services/url');
const urlUtils = require('../../../core/shared/url-utils');
const DomainEvents = require('@tryghost/domain-events');
const {anyEtag, anyObjectId, anyUuid, anyISODateTime, anyISODate, anyString, anyArray, anyLocationFor, anyErrorId, anyObject} = matchers;
let membersAgent;
@ -703,7 +704,7 @@ describe('Members API', function () {
});
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmail({
subject: '💸 Paid subscription started: checkout-webhook-test@email.com',

43
ghost/core/test/integration/services/email-service/email-event-storage.test.js
View File

@ -1,9 +1,10 @@
const sinon = require('sinon');
const {agentProvider, fixtureManager, sleep} = require('../../../utils/e2e-framework');
const {agentProvider, fixtureManager} = require('../../../utils/e2e-framework');
const assert = require('assert');
const domainEvents = require('@tryghost/domain-events');
const MailgunClient = require('@tryghost/mailgun-client');
const {EmailDeliveredEvent} = require('@tryghost/email-events');
const DomainEvents = require('@tryghost/domain-events');
async function resetFailures(models, emailId) {
await models.EmailRecipientFailure.destroy({
@ -80,8 +81,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if status has changed to delivered, with correct timestamp
const updatedEmailRecipient = await models.EmailRecipient.findOne({
@ -134,8 +135,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if status has changed to delivered, with correct timestamp
const updatedEmailRecipient = await models.EmailRecipient.findOne({
@ -185,8 +186,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if status has changed to delivered, with correct timestamp
const updatedEmailRecipient = await models.EmailRecipient.findOne({
@ -273,8 +274,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if status has changed to delivered, with correct timestamp
const updatedEmailRecipient = await models.EmailRecipient.findOne({
@ -377,8 +378,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if status has changed to delivered, with correct timestamp
const updatedEmailRecipient = await models.EmailRecipient.findOne({
@ -496,8 +497,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if status has changed to delivered, with correct timestamp
const updatedEmailRecipient = await models.EmailRecipient.findOne({
@ -603,8 +604,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if status has changed to delivered, with correct timestamp
const updatedEmailRecipient = await models.EmailRecipient.findOne({
@ -710,8 +711,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if status has changed to delivered, with correct timestamp
const updatedEmailRecipient = await models.EmailRecipient.findOne({
@ -782,8 +783,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if event exists
const {body: {events: eventsAfter}} = await agent.get(eventsURI);
@ -840,8 +841,8 @@ describe('EmailEventStorage', function () {
assert.deepEqual(result.emailIds, [emailId]);
assert.deepEqual(result.memberIds, [memberId]);
// Now wait for events processed
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if unsubscribed
const member = await membersService.api.members.get({id: memberId}, {withRelated: ['newsletters']});

10
ghost/core/test/integration/services/mega.test.js
View File

@ -1,10 +1,11 @@
require('should');
const {agentProvider, fixtureManager, mockManager, sleep} = require('../../utils/e2e-framework');
const {agentProvider, fixtureManager, mockManager} = require('../../utils/e2e-framework');
const moment = require('moment');
const ObjectId = require('bson-objectid').default;
const models = require('../../../core/server/models');
const sinon = require('sinon');
const assert = require('assert');
const DomainEvents = require('@tryghost/domain-events');
let agent;
async function createPublishedPostEmail() {
@ -198,7 +199,7 @@ describe('MEGA', function () {
const exclude = '%recipient.unsubscribe_url%';
let firstLink;
for (const el of $('a').toArray()) {
const href = $(el).attr('href');
@ -227,9 +228,8 @@ describe('MEGA', function () {
.expect('Location', link.to.href)
.expect(302);
// Since this is all event based, we need to wait a coulple of ms
// in the future we should wait for all dispatched events to be completed.
await sleep(200);
// Since this is all event based we should wait for all dispatched events to be completed.
await DomainEvents.allSettled();
// Check if click was tracked and associated with the correct member
const member = await models.Member.findOne({uuid: memberUuid});

2
ghost/core/test/regression/api/admin/members-importer.test.js
View File

@ -10,7 +10,7 @@ const settingsCache = require('../../../../core/shared/settings-cache');
const models = require('../../../../core/server/models');
const jobManager = require('../../../../core/server/services/jobs/job-service');
const {mockManager, sleep} = require('../../../utils/e2e-framework');
const {mockManager} = require('../../../utils/e2e-framework');
const assert = require('assert');
let request;

20
ghost/core/test/unit/server/services/staff/index.test.js
View File

@ -3,7 +3,7 @@ const sinon = require('sinon');
const staffService = require('../../../../../core/server/services/staff');
const DomainEvents = require('@tryghost/domain-events');
const {mockManager, sleep} = require('../../../../utils/e2e-framework');
const {mockManager} = require('../../../../utils/e2e-framework');
const models = require('../../../../../core/server/models');
const {SubscriptionCreatedEvent, SubscriptionCancelledEvent, MemberCreatedEvent} = require('@tryghost/member-events');
@ -81,7 +81,7 @@ describe('Staff Service:', function () {
}));
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmail({
to: 'owner@ghost.org',
subject: /🥳 Free member signup: Jamie/
@ -97,7 +97,7 @@ describe('Staff Service:', function () {
}));
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmail({
to: 'owner@ghost.org',
subject: /🥳 Free member signup: Jamie/
@ -113,7 +113,7 @@ describe('Staff Service:', function () {
}));
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmailCount(0);
});
});
@ -138,7 +138,7 @@ describe('Staff Service:', function () {
}));
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmail({
to: 'owner@ghost.org',
subject: /💸 Paid subscription started: Jamie/
@ -154,7 +154,7 @@ describe('Staff Service:', function () {
}));
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmail({
to: 'owner@ghost.org',
subject: /💸 Paid subscription started: Jamie/
@ -170,7 +170,7 @@ describe('Staff Service:', function () {
}));
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmailCount(0);
});
});
@ -190,7 +190,7 @@ describe('Staff Service:', function () {
}, new Date()));
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmail({
to: 'owner@ghost.org',
subject: /⚠️ Cancellation: Jamie/
@ -206,7 +206,7 @@ describe('Staff Service:', function () {
}));
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmail({
to: 'owner@ghost.org',
subject: /⚠️ Cancellation: Jamie/
@ -222,7 +222,7 @@ describe('Staff Service:', function () {
}));
// Wait for the dispatched events (because this happens async)
await sleep(250);
await DomainEvents.allSettled();
mockManager.assert.sentEmailCount(0);
});
});

36
ghost/domain-events/lib/DomainEvents.js
View File

@ -36,6 +36,9 @@ class DomainEvents {
logging.error('Unhandled error in event handler for event: ' + Event.name);
logging.error(e);
}
if (this.#trackingEnabled) {
this.#onProcessed();
}
});
}
@ -56,8 +59,41 @@ class DomainEvents {
* @returns {void}
*/
static dispatchRaw(name, data) {
if (this.#trackingEnabled) {
this.#dispatchCount += DomainEvents.ee.listenerCount(name);
}
DomainEvents.ee.emit(name, data);
}
// Methods and properties below are only for testing purposes
static #awaitQueue = [];
static #dispatchCount = 0;
static #processedCount = 0;
static #trackingEnabled = process.env.NODE_ENV.startsWith('test');
/**
* Waits for all the events in the queue to be dispatched and fully processed (async).
*/
static allSettled() {
return new Promise((resolve) => {
if (this.#processedCount >= this.#dispatchCount) {
// Resolve immediately if there are no events in the queue
resolve();
return;
}
this.#awaitQueue.push({resolve});
});
}
static #onProcessed() {
this.#processedCount += 1;
if (this.#processedCount >= this.#dispatchCount) {
for (const item of this.#awaitQueue) {
item.resolve();
}
this.#awaitQueue = [];
}
}
}
module.exports = DomainEvents;

2
ghost/domain-events/package.json
View File

@ -8,7 +8,7 @@
"types": "types",
"scripts": {
"dev": "echo \"Implement me!\"",
"test:unit": "NODE_ENV=testing c8 --all --reporter text --reporter cobertura --check-coverage mocha './test/**/*.test.js'",
"test:unit": "NODE_ENV=testing c8 --all --reporter text --reporter cobertura --check-coverage --100 mocha './test/**/*.test.js'",
"test": "yarn test:unit",
"lint": "eslint . --ext .js --cache"
},

79
ghost/domain-events/test/DomainEvents.test.js
View File

@ -1,5 +1,7 @@
const should = require('should');
const DomainEvents = require('../');
const assert = require('assert');
const sinon = require('sinon');
const logging = require('@tryghost/logging');
class TestEvent {
/**
@ -13,37 +15,88 @@ class TestEvent {
}
}
const sleep = ms => new Promise((resolve) => {
setTimeout(resolve, ms);
});
describe('DomainEvents', function () {
it('Will call multiple subscribers with the event when it is dispatched', function (done) {
afterEach(function () {
sinon.restore();
DomainEvents.ee.removeAllListeners();
});
it('Will call multiple subscribers with the event when it is dispatched', async function () {
const event = new TestEvent('Hello, world!');
let called = 0;
let events = [];
/**
* @param {TestEvent} receivedEvent
*/
function handler1(receivedEvent) {
should.equal(receivedEvent, event);
called += 1;
if (called === 2) {
done();
}
// Do not add assertions here, they are caught by DomainEvents
events.push(receivedEvent);
}
/**
* @param {TestEvent} receivedEvent
*/
function handler2(receivedEvent) {
should.equal(receivedEvent, event);
called += 1;
if (called === 2) {
done();
}
// Do not add assertions here, they are caught by DomainEvents
events.push(receivedEvent);
}
DomainEvents.subscribe(TestEvent, handler1);
DomainEvents.subscribe(TestEvent, handler2);
DomainEvents.dispatch(event);
await DomainEvents.allSettled();
assert.equal(events.length, 2);
assert.equal(events[0], event);
assert.equal(events[1], event);
});
it('Catches async errors in handlers', async function () {
const event = new TestEvent('Hello, world!');
const stub = sinon.stub(logging, 'error').returns();
/**
* @param {TestEvent} receivedEvent
*/
async function handler1() {
await sleep(10);
throw new Error('Test error');
}
DomainEvents.subscribe(TestEvent, handler1);
DomainEvents.dispatch(event);
await DomainEvents.allSettled();
assert.equal(stub.calledTwice, true);
});
describe('allSettled', function () {
it('Resolves when there are no events', async function () {
await DomainEvents.allSettled();
assert(true);
});
it('Waits for all listeners', async function () {
let counter = 0;
DomainEvents.subscribe(TestEvent, async () => {
await sleep(20);
counter += 1;
});
DomainEvents.subscribe(TestEvent, async () => {
await sleep(40);
counter += 1;
});
DomainEvents.dispatch(new TestEvent('Hello, world!'));
await DomainEvents.allSettled();
assert.equal(counter, 2);
});
});
});

9
ghost/members-events-service/test/last-seen-at-updater.test.js
View File

@ -10,12 +10,6 @@ const {MemberPageViewEvent, MemberCommentEvent} = require('@tryghost/member-even
const moment = require('moment');
const {EmailOpenedEvent} = require('@tryghost/email-events');
async function sleep(ms) {
return new Promise((resolve) => {
setTimeout(resolve, ms);
});
}
describe('LastSeenAtUpdater', function () {
it('Calls updateLastSeenAt on MemberPageViewEvents', async function () {
const now = moment('2022-02-28T18:00:00Z').utc();
@ -72,8 +66,7 @@ describe('LastSeenAtUpdater', function () {
sinon.spy(updater, 'updateLastSeenAt');
sinon.spy(updater, 'updateLastSeenAtWithoutKnownLastSeen');
DomainEvents.dispatch(EmailOpenedEvent.create({memberId: '1', emailRecipientId: '1', emailId: '1', timestamp: now.toDate()}));
// Wait for next tick
await sleep(50);
await DomainEvents.allSettled();
assert(updater.updateLastSeenAtWithoutKnownLastSeen.calledOnceWithExactly('1', now.toDate()));
assert(db.update.calledOnce);
});