Skip to content

Commit 93a24b2

Browse files
nbbeekennbbeeken
committed
chore(NODE-6844): modernize defer and remove incorrect usages
1 parent d9c7afc commit 93a24b2

File tree

4 files changed

+157
-159
lines changed

4 files changed

+157
-159
lines changed

‎global.d.ts

+2
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ declare global {
8484

8585
interface Context {
8686
configuration: TestConfiguration;
87+
/** @deprecated Please use afterEach hooks instead */
88+
defer(fn: () => Promise<unknown>): void;
8789
}
8890

8991
interface Test {

‎test/integration/change-streams/change_stream.test.ts

+73-75
Original file line numberDiff line numberDiff line change
@@ -209,75 +209,68 @@ describe('Change Streams', function () {
209209
}
210210
});
211211

212-
it('should support creating multiple simultaneous ChangeStreams', {
213-
metadata: { requires: { topology: 'replicaset' } },
214-
215-
test: function (done) {
212+
it(
213+
'should support creating multiple simultaneous ChangeStreams',
214+
{ requires: { topology: 'replicaset' } },
215+
async function () {
216216
const configuration = this.configuration;
217217
const client = configuration.newClient();
218218

219-
client.connect((err, client) => {
220-
expect(err).to.not.exist;
221-
this.defer(() => client.close());
219+
await client.connect();
222220

223-
const database = client.db('integration_tests');
224-
const collection1 = database.collection('simultaneous1');
225-
const collection2 = database.collection('simultaneous2');
221+
this.defer(() => client.close());
226222

227-
const changeStream1 = collection1.watch([{ $addFields: { changeStreamNumber: 1 } }]);
228-
this.defer(() => changeStream1.close());
229-
const changeStream2 = collection2.watch([{ $addFields: { changeStreamNumber: 2 } }]);
230-
this.defer(() => changeStream2.close());
231-
const changeStream3 = collection2.watch([{ $addFields: { changeStreamNumber: 3 } }]);
232-
this.defer(() => changeStream3.close());
223+
const database = client.db('integration_tests');
224+
const collection1 = database.collection('simultaneous1');
225+
const collection2 = database.collection('simultaneous2');
233226

234-
setTimeout(() => {
235-
this.defer(
236-
collection1.insertMany([{ a: 1 }]).then(() => collection2.insertMany([{ a: 1 }]))
237-
);
238-
}, 50);
239-
240-
Promise.resolve()
241-
.then(() =>
242-
Promise.all([changeStream1.hasNext(), changeStream2.hasNext(), changeStream3.hasNext()])
243-
)
244-
.then(function (hasNexts) {
245-
// Check all the Change Streams have a next item
246-
assert.ok(hasNexts[0]);
247-
assert.ok(hasNexts[1]);
248-
assert.ok(hasNexts[2]);
249-
250-
return Promise.all([changeStream1.next(), changeStream2.next(), changeStream3.next()]);
251-
})
252-
.then(function (changes) {
253-
// Check the values of the change documents are correct
254-
assert.equal(changes[0].operationType, 'insert');
255-
assert.equal(changes[1].operationType, 'insert');
256-
assert.equal(changes[2].operationType, 'insert');
257-
258-
expect(changes[0]).to.have.nested.property('fullDocument.a', 1);
259-
expect(changes[1]).to.have.nested.property('fullDocument.a', 1);
260-
expect(changes[2]).to.have.nested.property('fullDocument.a', 1);
261-
262-
expect(changes[0]).to.have.nested.property('ns.db', 'integration_tests');
263-
expect(changes[1]).to.have.nested.property('ns.db', 'integration_tests');
264-
expect(changes[2]).to.have.nested.property('ns.db', 'integration_tests');
265-
266-
expect(changes[0]).to.have.nested.property('ns.coll', 'simultaneous1');
267-
expect(changes[1]).to.have.nested.property('ns.coll', 'simultaneous2');
268-
expect(changes[2]).to.have.nested.property('ns.coll', 'simultaneous2');
269-
270-
expect(changes[0]).to.have.nested.property('changeStreamNumber', 1);
271-
expect(changes[1]).to.have.nested.property('changeStreamNumber', 2);
272-
expect(changes[2]).to.have.nested.property('changeStreamNumber', 3);
273-
})
274-
.then(
275-
() => done(),
276-
err => done(err)
277-
);
278-
});
227+
const changeStream1 = collection1.watch([{ $addFields: { changeStreamNumber: 1 } }]);
228+
this.defer(() => changeStream1.close());
229+
const changeStream2 = collection2.watch([{ $addFields: { changeStreamNumber: 2 } }]);
230+
this.defer(() => changeStream2.close());
231+
const changeStream3 = collection2.watch([{ $addFields: { changeStreamNumber: 3 } }]);
232+
this.defer(() => changeStream3.close());
233+
234+
setTimeout(() => {
235+
collection1.insertMany([{ a: 1 }]).then(() => collection2.insertMany([{ a: 1 }]));
236+
}, 50);
237+
238+
await Promise.resolve()
239+
.then(() =>
240+
Promise.all([changeStream1.hasNext(), changeStream2.hasNext(), changeStream3.hasNext()])
241+
)
242+
.then(function (hasNexts) {
243+
// Check all the Change Streams have a next item
244+
assert.ok(hasNexts[0]);
245+
assert.ok(hasNexts[1]);
246+
assert.ok(hasNexts[2]);
247+
248+
return Promise.all([changeStream1.next(), changeStream2.next(), changeStream3.next()]);
249+
})
250+
.then(function (changes) {
251+
// Check the values of the change documents are correct
252+
assert.equal(changes[0].operationType, 'insert');
253+
assert.equal(changes[1].operationType, 'insert');
254+
assert.equal(changes[2].operationType, 'insert');
255+
256+
expect(changes[0]).to.have.nested.property('fullDocument.a', 1);
257+
expect(changes[1]).to.have.nested.property('fullDocument.a', 1);
258+
expect(changes[2]).to.have.nested.property('fullDocument.a', 1);
259+
260+
expect(changes[0]).to.have.nested.property('ns.db', 'integration_tests');
261+
expect(changes[1]).to.have.nested.property('ns.db', 'integration_tests');
262+
expect(changes[2]).to.have.nested.property('ns.db', 'integration_tests');
263+
264+
expect(changes[0]).to.have.nested.property('ns.coll', 'simultaneous1');
265+
expect(changes[1]).to.have.nested.property('ns.coll', 'simultaneous2');
266+
expect(changes[2]).to.have.nested.property('ns.coll', 'simultaneous2');
267+
268+
expect(changes[0]).to.have.nested.property('changeStreamNumber', 1);
269+
expect(changes[1]).to.have.nested.property('changeStreamNumber', 2);
270+
expect(changes[2]).to.have.nested.property('changeStreamNumber', 3);
271+
});
279272
}
280-
});
273+
);
281274

282275
it('should properly close ChangeStream cursor', {
283276
metadata: { requires: { topology: 'replicaset' } },
@@ -807,22 +800,27 @@ describe('Change Streams', function () {
807800

808801
it('when invoked with promises', {
809802
metadata: { requires: { topology: 'replicaset' } },
810-
test: function () {
811-
const read = () => {
812-
return Promise.resolve()
813-
.then(() => changeStream.next())
814-
.then(() => changeStream.next())
815-
.then(() => {
816-
this.defer(lastWrite());
817-
const nextP = changeStream.next();
818-
return changeStream.close().then(() => nextP);
819-
});
803+
test: async function () {
804+
const read = async () => {
805+
await changeStream.next();
806+
await changeStream.next();
807+
808+
const write = lastWrite();
809+
810+
const nextP = changeStream.next();
811+
812+
await changeStream.close();
813+
814+
await write;
815+
await nextP;
820816
};
821817

822-
return Promise.all([read(), write()]).then(
823-
() => Promise.reject(new Error('Expected operation to fail with error')),
824-
err => expect(err.message).to.equal('ChangeStream is closed')
818+
const error = await Promise.all([read(), write()]).then(
819+
() => null,
820+
error => error
825821
);
822+
823+
expect(error.message).to.equal('ChangeStream is closed');
826824
}
827825
});
828826

‎test/integration/change-streams/change_streams.prose.test.ts

+39-36
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,9 @@ const initIteratorMode = async (cs: ChangeStream) => {
8080
};
8181

8282
/** Waits for a change stream to start */
83-
function waitForStarted(changeStream, callback) {
84-
changeStream.cursor.once('init', () => {
85-
callback();
86-
});
83+
async function waitForStarted(changeStream, callback) {
84+
await once(changeStream.cursor, 'init');
85+
await callback();
8786
}
8887

8988
// Define the pipeline processing changes
@@ -844,42 +843,48 @@ describe('Change Stream prose tests', function () {
844843
describe('Change Stream prose 17-18', function () {
845844
let client: MongoClient;
846845
let coll: Collection;
847-
let startAfter;
846+
let startAfter: unknown;
848847

849848
function recordEvent(events, e) {
850849
if (e.commandName !== 'aggregate') return;
851850
events.push({ $changeStream: e.command.pipeline[0].$changeStream });
852851
}
853852

854-
beforeEach(function (done) {
853+
beforeEach('get startAfter token', async function () {
855854
const configuration = this.configuration;
856-
client = configuration.newClient({ monitorCommands: true });
857-
client.connect(err => {
858-
expect(err).to.not.exist;
859-
coll = client.db('integration_tests').collection('setupAfterTest');
860-
const changeStream = coll.watch();
861-
changeStream.on('error', done);
862-
waitForStarted(changeStream, () => {
863-
coll.insertOne({ x: 1 }, { writeConcern: { w: 'majority', j: true } }, err => {
864-
expect(err).to.not.exist;
855+
const utilClient = configuration.newClient();
856+
await utilClient.connect();
865857

866-
coll.drop(err => {
867-
expect(err).to.not.exist;
868-
});
869-
});
870-
});
858+
const coll = utilClient.db('integration_tests').collection('setupAfterTest');
859+
const changeStream = coll.watch();
871860

872-
changeStream.on('change', change => {
873-
if (change.operationType === 'invalidate') {
874-
startAfter = change._id;
875-
changeStream.close(done);
876-
}
877-
});
878-
});
861+
const willInit = once(changeStream.cursor, 'init');
862+
863+
await changeStream.tryNext();
864+
await willInit;
865+
866+
await coll.insertOne({ x: 1 }, { writeConcern: { w: 'majority', j: true } });
867+
await coll.drop();
868+
869+
for await (const change of changeStream) {
870+
if (change.operationType === 'invalidate') {
871+
startAfter = change._id;
872+
break;
873+
}
874+
}
875+
876+
await changeStream.close();
877+
878+
await utilClient.close();
879879
});
880880

881-
afterEach(function (done) {
882-
client.close(done);
881+
beforeEach(async function () {
882+
client = this.configuration.newClient({}, { monitorCommands: true });
883+
coll = client.db('integration_tests').collection('setupAfterTest');
884+
});
885+
886+
afterEach(async function () {
887+
await client.close();
883888
});
884889

885890
// 17. $changeStream stage for ChangeStream started with startAfter against a server >=4.1.1
@@ -894,8 +899,8 @@ describe('Change Stream prose tests', function () {
894899
client.on('commandStarted', e => recordEvent(events, e));
895900
const changeStream = coll.watch([], { startAfter });
896901

897-
changeStream.on('error', async e => {
898-
await changeStream.close(e);
902+
changeStream.on('error', async () => {
903+
await changeStream.close();
899904
});
900905

901906
const changePromise = once(changeStream, 'change');
@@ -955,11 +960,9 @@ describe('Change Stream prose tests', function () {
955960
});
956961

957962
waitForStarted(changeStream, () =>
958-
this.defer(
959-
coll
960-
.insertOne({ x: 2 }, { writeConcern: { w: 'majority', j: true } })
961-
.then(() => coll.insertOne({ x: 3 }, { writeConcern: { w: 'majority', j: true } }))
962-
)
963+
coll
964+
.insertOne({ x: 2 }, { writeConcern: { w: 'majority', j: true } })
965+
.then(() => coll.insertOne({ x: 3 }, { writeConcern: { w: 'majority', j: true } }))
963966
);
964967
}
965968
});

‎test/tools/runner/plugins/deferred.js

+43-48
Original file line numberDiff line numberDiff line change
@@ -1,57 +1,52 @@
11
'use strict';
2+
23
const kDeferred = Symbol('deferred');
4+
const mocha = require('mocha');
35

4-
(mocha => {
5-
const Context = mocha.Context;
6-
function makeExecuteDeferred(test) {
7-
return () => {
8-
const deferredActions = test[kDeferred];
9-
10-
// process actions LIFO
11-
const promises = Array.from(deferredActions).reverse();
12-
const result = promises.reduce((p, action) => {
13-
if (action.length > 0) {
14-
// assume these are async methods with provided `done`
15-
const actionPromise = new Promise((resolve, reject) => {
16-
function done(err) {
17-
if (err) return reject(err);
18-
resolve();
19-
}
20-
21-
action(done);
22-
});
23-
24-
return p.then(actionPromise);
25-
}
26-
27-
return p.then(action);
28-
}, Promise.resolve());
29-
30-
return result.then(
31-
() => test[kDeferred].clear(),
32-
err => {
33-
test[kDeferred].clear();
34-
return Promise.reject(err);
35-
}
36-
);
37-
};
38-
}
6+
const { Context } = mocha;
397

40-
Context.prototype.defer = function (fn) {
41-
const test = this.test;
42-
if (test[kDeferred] == null) {
43-
test[kDeferred] = new Set();
8+
function makeExecuteDeferred(test) {
9+
return async function () {
10+
/** @type {Array<() => Promise<void>>} */
11+
const deferredActions = test[kDeferred];
4412

45-
const parentSuite = test.parent;
46-
const afterEachHooks = parentSuite._afterEach;
47-
if (afterEachHooks[0] == null || afterEachHooks[0].title !== kDeferred) {
48-
const deferredHook = parentSuite._createHook('"deferred" hook', makeExecuteDeferred(test));
13+
// process actions LIFO
14+
const actions = Array.from(deferredActions).reverse();
4915

50-
afterEachHooks.unshift(deferredHook);
16+
try {
17+
for (const fn of actions) {
18+
await fn();
5119
}
20+
} finally {
21+
test[kDeferred].length = 0;
5222
}
53-
54-
test[kDeferred].add(fn);
55-
return this;
5623
};
57-
})(require('mocha'));
24+
}
25+
26+
Context.prototype.defer = function defer(fn) {
27+
const test = this.test;
28+
29+
if (typeof fn !== 'function') {
30+
throw new Error('defer is meant to take a function that returns a promise');
31+
}
32+
33+
if (test[kDeferred] == null) {
34+
test[kDeferred] = [];
35+
36+
const parentSuite = test.parent;
37+
const afterEachHooks = parentSuite._afterEach;
38+
if (afterEachHooks[0] == null || afterEachHooks[0].title !== kDeferred) {
39+
const deferredHook = parentSuite._createHook('"deferred" hook', makeExecuteDeferred(test));
40+
41+
// defer runs after test but before afterEach(s)
42+
afterEachHooks.unshift(deferredHook);
43+
}
44+
}
45+
46+
if (test[kDeferred].includes(fn)) {
47+
throw new Error('registered the same deferred action more than once');
48+
}
49+
50+
test[kDeferred].push(fn);
51+
return this;
52+
};

0 commit comments

Comments
 (0)