From 441381877d56ed5803e5c26aadb329f6343870f1 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 26 Mar 2025 11:33:33 -0400 Subject: [PATCH 01/11] test(NODE-6492): add integration tests for transaction write concern behavior --- .../transactions/transactions.prose.test.ts | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 test/integration/transactions/transactions.prose.test.ts diff --git a/test/integration/transactions/transactions.prose.test.ts b/test/integration/transactions/transactions.prose.test.ts new file mode 100644 index 00000000000..52fdd409580 --- /dev/null +++ b/test/integration/transactions/transactions.prose.test.ts @@ -0,0 +1,49 @@ +import { expect } from 'chai'; + +import { type MongoClient } from '../../mongodb'; + +const metadata: MongoDBMetadataUI = { + requires: { + topology: '!single' + } +}; + +describe('Transactions Spec Prose', function () { + let client: MongoClient; + const started = []; + + beforeEach(async function () { + started.length = 0; + client = this.configuration.newClient({}, { monitorCommands: true }); + client.on('commandStarted', ev => started.push(ev)); + }); + + afterEach(async function () { + await client.close(); + }); + + describe('Options Inside Transaction', function () { + it( + '1.0 Write concern not inherited from collection object inside transaction.', + metadata, + async () => { + await client.withSession(async session => { + session.startTransaction(); + + const collection = client.db().collection('txn-test', { writeConcern: { w: 0 } }); + + await collection.insertOne({ n: 1 }, { session }); + + await session.commitTransaction(); + }); + + const insertStarted = started.find(ev => ev.commandName === 'insert'); + expect(insertStarted).to.not.have.nested.property('command.writeConcern'); + + // not in asked by the spec test but good to check, this is where the WC would be if it wasn't ignored. + const commitTransactionStarted = started.find(ev => ev.commandName === 'commitTransaction'); + expect(commitTransactionStarted).to.not.have.nested.property('command.writeConcern'); + } + ); + }); +}); From f52a918a2444c94b9364e3db2d6e73e54ebf9477 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 26 Mar 2025 12:41:47 -0400 Subject: [PATCH 02/11] test: reset transaction test collection before each test --- .../transactions/transactions.prose.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/integration/transactions/transactions.prose.test.ts b/test/integration/transactions/transactions.prose.test.ts index 52fdd409580..f841b11f612 100644 --- a/test/integration/transactions/transactions.prose.test.ts +++ b/test/integration/transactions/transactions.prose.test.ts @@ -15,10 +15,23 @@ describe('Transactions Spec Prose', function () { beforeEach(async function () { started.length = 0; client = this.configuration.newClient({}, { monitorCommands: true }); + + await client + .db() + .collection('txn-test') + .drop() + .catch(() => null); + await client.db().createCollection('txn-test'); + client.on('commandStarted', ev => started.push(ev)); }); afterEach(async function () { + await client + .db() + .collection('txn-test') + .drop() + .catch(() => null); await client.close(); }); From c6f83b374a0b682ba19553978a5ee38a0b7573f3 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 26 Mar 2025 13:50:56 -0400 Subject: [PATCH 03/11] test: skip transactions on sharded clusters for MongoDB versions < 4.2 --- .../transactions/transactions.prose.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/integration/transactions/transactions.prose.test.ts b/test/integration/transactions/transactions.prose.test.ts index f841b11f612..99e42675582 100644 --- a/test/integration/transactions/transactions.prose.test.ts +++ b/test/integration/transactions/transactions.prose.test.ts @@ -1,4 +1,5 @@ import { expect } from 'chai'; +import * as semver from 'semver'; import { type MongoClient } from '../../mongodb'; @@ -13,6 +14,16 @@ describe('Transactions Spec Prose', function () { const started = []; beforeEach(async function () { + if ( + semver.satisfies(this.configuration.version, '>4.2') && + this.configuration.topologyType === 'Sharded' + ) { + if (this.currentTest) { + this.currentTest.skipReason = + 'Transactions on sharded clusters are only supported after 4.2'; + } + this.skip(); + } started.length = 0; client = this.configuration.newClient({}, { monitorCommands: true }); From 13bda33fe296c722d631e10c817710088b429322 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 26 Mar 2025 14:12:59 -0400 Subject: [PATCH 04/11] test: skip currentTest --- test/integration/transactions/transactions.prose.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/transactions/transactions.prose.test.ts b/test/integration/transactions/transactions.prose.test.ts index 99e42675582..f10908cfeb2 100644 --- a/test/integration/transactions/transactions.prose.test.ts +++ b/test/integration/transactions/transactions.prose.test.ts @@ -21,8 +21,8 @@ describe('Transactions Spec Prose', function () { if (this.currentTest) { this.currentTest.skipReason = 'Transactions on sharded clusters are only supported after 4.2'; + this.currentTest.skip(); } - this.skip(); } started.length = 0; client = this.configuration.newClient({}, { monitorCommands: true }); From 0df4118b652e96c0b4dbf7f540d70e213070f531 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 26 Mar 2025 15:42:44 -0400 Subject: [PATCH 05/11] test: update version check for sharded cluster transactions to include MongoDB <= 4.2 --- test/integration/transactions/transactions.prose.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/transactions/transactions.prose.test.ts b/test/integration/transactions/transactions.prose.test.ts index f10908cfeb2..eadd63dd12b 100644 --- a/test/integration/transactions/transactions.prose.test.ts +++ b/test/integration/transactions/transactions.prose.test.ts @@ -15,7 +15,7 @@ describe('Transactions Spec Prose', function () { beforeEach(async function () { if ( - semver.satisfies(this.configuration.version, '>4.2') && + semver.satisfies(this.configuration.version, '<=4.2') && this.configuration.topologyType === 'Sharded' ) { if (this.currentTest) { From 664cfe99fad252022da41e5b339ea25397abd07e Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 26 Mar 2025 16:28:12 -0400 Subject: [PATCH 06/11] test: fix skipping tests and add optional chaining for client methods --- test/integration/transactions/transactions.prose.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/transactions/transactions.prose.test.ts b/test/integration/transactions/transactions.prose.test.ts index eadd63dd12b..2110137fdb7 100644 --- a/test/integration/transactions/transactions.prose.test.ts +++ b/test/integration/transactions/transactions.prose.test.ts @@ -21,7 +21,7 @@ describe('Transactions Spec Prose', function () { if (this.currentTest) { this.currentTest.skipReason = 'Transactions on sharded clusters are only supported after 4.2'; - this.currentTest.skip(); + this.skip(); } } started.length = 0; @@ -39,11 +39,11 @@ describe('Transactions Spec Prose', function () { afterEach(async function () { await client - .db() + ?.db() .collection('txn-test') .drop() .catch(() => null); - await client.close(); + await client?.close(); }); describe('Options Inside Transaction', function () { From fd5a174c317506ab264f80e26766f9d476a7f7b5 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 27 Mar 2025 13:25:54 -0400 Subject: [PATCH 07/11] test: ensure transaction test waits for result --- .../transactions/transactions.prose.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/integration/transactions/transactions.prose.test.ts b/test/integration/transactions/transactions.prose.test.ts index 2110137fdb7..753e7db55c6 100644 --- a/test/integration/transactions/transactions.prose.test.ts +++ b/test/integration/transactions/transactions.prose.test.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import * as semver from 'semver'; -import { type MongoClient } from '../../mongodb'; +import { type MongoClient, type ObjectId } from '../../mongodb'; const metadata: MongoDBMetadataUI = { requires: { @@ -51,16 +51,18 @@ describe('Transactions Spec Prose', function () { '1.0 Write concern not inherited from collection object inside transaction.', metadata, async () => { + let _id: ObjectId; + const collection = client.db().collection('txn-test', { writeConcern: { w: 0 } }); + await client.withSession(async session => { session.startTransaction(); - - const collection = client.db().collection('txn-test', { writeConcern: { w: 0 } }); - - await collection.insertOne({ n: 1 }, { session }); - + _id = (await collection.insertOne({ n: 1 }, { session })).insertedId; await session.commitTransaction(); }); + // keep finding until we get a result, otherwise the test will timeout. + while ((await collection.findOne({ _id })) == null); + const insertStarted = started.find(ev => ev.commandName === 'insert'); expect(insertStarted).to.not.have.nested.property('command.writeConcern'); From fc9f0e301f58dea19082e46585cae57bd5b5d48b Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 31 Mar 2025 12:37:13 -0400 Subject: [PATCH 08/11] test: update transaction test to assert document existence instead of using a loop --- test/integration/transactions/transactions.prose.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/transactions/transactions.prose.test.ts b/test/integration/transactions/transactions.prose.test.ts index 753e7db55c6..d1b3404d354 100644 --- a/test/integration/transactions/transactions.prose.test.ts +++ b/test/integration/transactions/transactions.prose.test.ts @@ -61,7 +61,7 @@ describe('Transactions Spec Prose', function () { }); // keep finding until we get a result, otherwise the test will timeout. - while ((await collection.findOne({ _id })) == null); + expect(await collection.findOne({ _id })).to.have.property('n', 1); const insertStarted = started.find(ev => ev.commandName === 'insert'); expect(insertStarted).to.not.have.nested.property('command.writeConcern'); From bf315dba2604f11906fd5cde1e7297b2e07099d4 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 31 Mar 2025 16:13:10 -0400 Subject: [PATCH 09/11] chore: add a maxServerVersion to CS test failing on latest --- .../unified/change-streams-disambiguatedPaths.json | 1 + .../change-streams/unified/change-streams-disambiguatedPaths.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json index a8667b5436c..fccec0caea9 100644 --- a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json +++ b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json @@ -26,6 +26,7 @@ "runOnRequirements": [ { "minServerVersion": "6.1.0", + "maxServerVersion": "8.1.0", "topologies": [ "replicaset", "load-balanced", diff --git a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml index acd05cec4dc..1a657680728 100644 --- a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml +++ b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml @@ -15,6 +15,7 @@ createEntities: runOnRequirements: - minServerVersion: "6.1.0" + maxServerVersion: "8.1.0" topologies: [ replicaset, sharded-replicaset, load-balanced, sharded ] serverless: forbid From bc8202b304a3794c7b0b57ba10f17adb5d55e330 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 2 Apr 2025 11:22:29 -0400 Subject: [PATCH 10/11] remove changes --- .../change-streams-disambiguatedPaths.json | 66 ++++++++++++++++++- .../change-streams-disambiguatedPaths.yml | 27 +++++++- .../unified/change-streams.json | 12 +--- .../change-streams/unified/change-streams.yml | 4 +- 4 files changed, 93 insertions(+), 16 deletions(-) diff --git a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json index fccec0caea9..91d8e66da20 100644 --- a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json +++ b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json @@ -26,9 +26,9 @@ "runOnRequirements": [ { "minServerVersion": "6.1.0", - "maxServerVersion": "8.1.0", "topologies": [ "replicaset", + "sharded-replicaset", "load-balanced", "sharded" ], @@ -43,6 +43,70 @@ } ], "tests": [ + { + "description": "disambiguatedPaths is not present when showExpandedEvents is false/unset", + "operations": [ + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 1, + "a": { + "1": 1 + } + } + } + }, + { + "name": "createChangeStream", + "object": "collection0", + "arguments": { + "pipeline": [] + }, + "saveResultAsEntity": "changeStream0" + }, + { + "name": "updateOne", + "object": "collection0", + "arguments": { + "filter": { + "_id": 1 + }, + "update": { + "$set": { + "a.1": 2 + } + } + } + }, + { + "name": "iterateUntilDocumentOrError", + "object": "changeStream0", + "expectResult": { + "operationType": "update", + "ns": { + "db": "database0", + "coll": "collection0" + }, + "updateDescription": { + "updatedFields": { + "$$exists": true + }, + "removedFields": { + "$$exists": true + }, + "truncatedArrays": { + "$$exists": true + }, + "disambiguatedPaths": { + "$$exists": false + } + } + } + } + ] + }, { "description": "disambiguatedPaths is present on updateDescription when an ambiguous path is present", "operations": [ diff --git a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml index 1a657680728..a2524d23cee 100644 --- a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml +++ b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml @@ -15,7 +15,6 @@ createEntities: runOnRequirements: - minServerVersion: "6.1.0" - maxServerVersion: "8.1.0" topologies: [ replicaset, sharded-replicaset, load-balanced, sharded ] serverless: forbid @@ -25,6 +24,32 @@ initialData: documents: [] tests: + - description: "disambiguatedPaths is not present when showExpandedEvents is false/unset" + operations: + - name: insertOne + object: *collection0 + arguments: + document: { _id: 1, 'a': { '1': 1 } } + - name: createChangeStream + object: *collection0 + arguments: { pipeline: [] } + saveResultAsEntity: &changeStream0 changeStream0 + - name: updateOne + object: *collection0 + arguments: + filter: { _id: 1 } + update: { $set: { 'a.1': 2 } } + - name: iterateUntilDocumentOrError + object: *changeStream0 + expectResult: + operationType: "update" + ns: { db: *database0, coll: *collection0 } + updateDescription: + updatedFields: { $$exists: true } + removedFields: { $$exists: true } + truncatedArrays: { $$exists: true } + disambiguatedPaths: { $$exists: false } + - description: "disambiguatedPaths is present on updateDescription when an ambiguous path is present" operations: - name: insertOne diff --git a/test/spec/change-streams/unified/change-streams.json b/test/spec/change-streams/unified/change-streams.json index a155d85b6e4..c8b60ed4e25 100644 --- a/test/spec/change-streams/unified/change-streams.json +++ b/test/spec/change-streams/unified/change-streams.json @@ -181,12 +181,7 @@ "field": "array", "newSize": 2 } - ], - "disambiguatedPaths": { - "$$unsetOrMatches": { - "$$exists": true - } - } + ] } } } @@ -1413,11 +1408,6 @@ "$$unsetOrMatches": { "$$exists": true } - }, - "disambiguatedPaths": { - "$$unsetOrMatches": { - "$$exists": true - } } } } diff --git a/test/spec/change-streams/unified/change-streams.yml b/test/spec/change-streams/unified/change-streams.yml index 7f824623a67..3235533b5d8 100644 --- a/test/spec/change-streams/unified/change-streams.yml +++ b/test/spec/change-streams/unified/change-streams.yml @@ -115,8 +115,7 @@ tests: "field": "array", "newSize": 2 } - ], - disambiguatedPaths: { $$unsetOrMatches: { $$exists: true } } + ] } } @@ -723,7 +722,6 @@ tests: updatedFields: { x: 2 } removedFields: [] truncatedArrays: { $$unsetOrMatches: { $$exists: true } } - disambiguatedPaths: { $$unsetOrMatches: { $$exists: true } } - name: iterateUntilDocumentOrError object: *changeStream0 expectResult: From 80f0c6369366145e23ef94c4918bd909dcf702e3 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 2 Apr 2025 11:23:18 -0400 Subject: [PATCH 11/11] chore: remove changes --- .../change-streams-disambiguatedPaths.json | 65 ------------------- .../change-streams-disambiguatedPaths.yml | 26 -------- .../unified/change-streams.json | 12 +++- .../change-streams/unified/change-streams.yml | 4 +- 4 files changed, 14 insertions(+), 93 deletions(-) diff --git a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json index 91d8e66da20..a8667b5436c 100644 --- a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json +++ b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.json @@ -28,7 +28,6 @@ "minServerVersion": "6.1.0", "topologies": [ "replicaset", - "sharded-replicaset", "load-balanced", "sharded" ], @@ -43,70 +42,6 @@ } ], "tests": [ - { - "description": "disambiguatedPaths is not present when showExpandedEvents is false/unset", - "operations": [ - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 1, - "a": { - "1": 1 - } - } - } - }, - { - "name": "createChangeStream", - "object": "collection0", - "arguments": { - "pipeline": [] - }, - "saveResultAsEntity": "changeStream0" - }, - { - "name": "updateOne", - "object": "collection0", - "arguments": { - "filter": { - "_id": 1 - }, - "update": { - "$set": { - "a.1": 2 - } - } - } - }, - { - "name": "iterateUntilDocumentOrError", - "object": "changeStream0", - "expectResult": { - "operationType": "update", - "ns": { - "db": "database0", - "coll": "collection0" - }, - "updateDescription": { - "updatedFields": { - "$$exists": true - }, - "removedFields": { - "$$exists": true - }, - "truncatedArrays": { - "$$exists": true - }, - "disambiguatedPaths": { - "$$exists": false - } - } - } - } - ] - }, { "description": "disambiguatedPaths is present on updateDescription when an ambiguous path is present", "operations": [ diff --git a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml index a2524d23cee..acd05cec4dc 100644 --- a/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml +++ b/test/spec/change-streams/unified/change-streams-disambiguatedPaths.yml @@ -24,32 +24,6 @@ initialData: documents: [] tests: - - description: "disambiguatedPaths is not present when showExpandedEvents is false/unset" - operations: - - name: insertOne - object: *collection0 - arguments: - document: { _id: 1, 'a': { '1': 1 } } - - name: createChangeStream - object: *collection0 - arguments: { pipeline: [] } - saveResultAsEntity: &changeStream0 changeStream0 - - name: updateOne - object: *collection0 - arguments: - filter: { _id: 1 } - update: { $set: { 'a.1': 2 } } - - name: iterateUntilDocumentOrError - object: *changeStream0 - expectResult: - operationType: "update" - ns: { db: *database0, coll: *collection0 } - updateDescription: - updatedFields: { $$exists: true } - removedFields: { $$exists: true } - truncatedArrays: { $$exists: true } - disambiguatedPaths: { $$exists: false } - - description: "disambiguatedPaths is present on updateDescription when an ambiguous path is present" operations: - name: insertOne diff --git a/test/spec/change-streams/unified/change-streams.json b/test/spec/change-streams/unified/change-streams.json index c8b60ed4e25..a155d85b6e4 100644 --- a/test/spec/change-streams/unified/change-streams.json +++ b/test/spec/change-streams/unified/change-streams.json @@ -181,7 +181,12 @@ "field": "array", "newSize": 2 } - ] + ], + "disambiguatedPaths": { + "$$unsetOrMatches": { + "$$exists": true + } + } } } } @@ -1408,6 +1413,11 @@ "$$unsetOrMatches": { "$$exists": true } + }, + "disambiguatedPaths": { + "$$unsetOrMatches": { + "$$exists": true + } } } } diff --git a/test/spec/change-streams/unified/change-streams.yml b/test/spec/change-streams/unified/change-streams.yml index 3235533b5d8..7f824623a67 100644 --- a/test/spec/change-streams/unified/change-streams.yml +++ b/test/spec/change-streams/unified/change-streams.yml @@ -115,7 +115,8 @@ tests: "field": "array", "newSize": 2 } - ] + ], + disambiguatedPaths: { $$unsetOrMatches: { $$exists: true } } } } @@ -722,6 +723,7 @@ tests: updatedFields: { x: 2 } removedFields: [] truncatedArrays: { $$unsetOrMatches: { $$exists: true } } + disambiguatedPaths: { $$unsetOrMatches: { $$exists: true } } - name: iterateUntilDocumentOrError object: *changeStream0 expectResult: