From 36744f93560576fd6a011c4201dac9bc1d58c51c Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 25 Jan 2024 18:03:53 -0500 Subject: [PATCH 1/6] ready for review --- .../transactions/transactions.spec.test.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/integration/transactions/transactions.spec.test.js b/test/integration/transactions/transactions.spec.test.js index 05b829324a4..a2b0c08a5d5 100644 --- a/test/integration/transactions/transactions.spec.test.js +++ b/test/integration/transactions/transactions.spec.test.js @@ -90,7 +90,16 @@ const SKIP_TESTS = [ // TODO(NODE-2034): Will be implemented as part of NODE-2034 'Client side error in command starting transaction', - 'Client side error when transaction is in progress' + 'Client side error when transaction is in progress', + + // TODO(NODE-5855): Unskip Transactions Spec Unified Tests mongos-unpin.unpin + 'unpin after TransientTransactionError error on commit', + 'unpin on successful abort', + 'unpin after non-transient error on abort', + 'unpin after TransientTransactionError error on abort', + 'unpin when a new transaction is started', + 'unpin when a non-transaction write operation uses a session', + 'unpin when a non-transaction read operation uses a session' ]; describe('Transactions Spec Legacy Tests', function () { From b04f402676ecb914e4ee2deb2a6c54713faca5ec Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Mon, 29 Jan 2024 14:18:07 -0500 Subject: [PATCH 2/6] PR requested changes --- .../transactions/transactions.spec.test.js | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/test/integration/transactions/transactions.spec.test.js b/test/integration/transactions/transactions.spec.test.js index a2b0c08a5d5..1dad7c6807f 100644 --- a/test/integration/transactions/transactions.spec.test.js +++ b/test/integration/transactions/transactions.spec.test.js @@ -1,5 +1,7 @@ 'use strict'; +import { gte } from 'semver'; + const path = require('path'); const { expect } = require('chai'); const { TestRunnerContext, generateTopologyTests } = require('../../tools/spec-runner'); @@ -90,16 +92,15 @@ const SKIP_TESTS = [ // TODO(NODE-2034): Will be implemented as part of NODE-2034 'Client side error in command starting transaction', - 'Client side error when transaction is in progress', + 'Client side error when transaction is in progress' +]; +const LATEST_SKIP_TESTS = [ // TODO(NODE-5855): Unskip Transactions Spec Unified Tests mongos-unpin.unpin 'unpin after TransientTransactionError error on commit', 'unpin on successful abort', 'unpin after non-transient error on abort', - 'unpin after TransientTransactionError error on abort', - 'unpin when a new transaction is started', - 'unpin when a non-transaction write operation uses a session', - 'unpin when a non-transaction read operation uses a session' + 'unpin after TransientTransactionError error on abort' ]; describe('Transactions Spec Legacy Tests', function () { @@ -122,8 +123,11 @@ describe('Transactions Spec Legacy Tests', function () { return testContext.setup(this.configuration); }); - function testFilter(spec) { - return SKIP_TESTS.indexOf(spec.description) === -1; + function testFilter(spec, configuration) { + return ( + SKIP_TESTS.indexOf(spec.description) === -1 && + (!gte(configuration.version, '7.1.0') || LATEST_SKIP_TESTS.indexOf(spec.description) === -1) + ); } generateTopologyTests(testSuites, testContext, testFilter); From 23f110722310147eb0da6fb71e17483cd8417e79 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 31 Jan 2024 15:55:09 -0500 Subject: [PATCH 3/6] test fix --- test/integration/transactions/transactions.spec.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/transactions/transactions.spec.test.js b/test/integration/transactions/transactions.spec.test.js index 1dad7c6807f..5afc281cbaf 100644 --- a/test/integration/transactions/transactions.spec.test.js +++ b/test/integration/transactions/transactions.spec.test.js @@ -126,7 +126,7 @@ describe('Transactions Spec Legacy Tests', function () { function testFilter(spec, configuration) { return ( SKIP_TESTS.indexOf(spec.description) === -1 && - (!gte(configuration.version, '7.1.0') || LATEST_SKIP_TESTS.indexOf(spec.description) === -1) + (!gte(configuration.version, '7.9.9') || LATEST_SKIP_TESTS.indexOf(spec.description) === -1) ); } From 7185bfcc05ffaa0e97369b1ece89495acbfc421c Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 31 Jan 2024 17:40:48 -0500 Subject: [PATCH 4/6] test fix actual --- .../transactions/transactions.spec.test.js | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/integration/transactions/transactions.spec.test.js b/test/integration/transactions/transactions.spec.test.js index 5afc281cbaf..66bdcfaefa5 100644 --- a/test/integration/transactions/transactions.spec.test.js +++ b/test/integration/transactions/transactions.spec.test.js @@ -80,11 +80,22 @@ class TransactionsRunnerContext extends TestRunnerContext { } } +const LATEST_UNIFIED_SKIP_TESTS = [ + 'unpin after TransientTransactionError error on commit', + 'unpin on successful abort', + 'unpin after non-transient error on abort', + 'unpin after TransientTransactionError error on abort' +]; + describe('Transactions Spec Unified Tests', function () { - runUnifiedSuite(loadSpecTests(path.join('transactions', 'unified'))); + runUnifiedSuite(loadSpecTests(path.join('transactions', 'unified')), (test, ctx) => + gte(ctx.version, '8.0.0') && LATEST_UNIFIED_SKIP_TESTS.includes(test.description) + ? 'TODO(NODE-5855): Unskip Transactions Spec Unified Tests mongos-unpin.unpin' + : false + ); }); -const SKIP_TESTS = [ +const LEGACY_SKIP_TESTS = [ // TODO(NODE-3943): Investigate these commit test failures // OLD COMMENT: commitTransaction retry seems to be swallowed by mongos in these two cases 'commitTransaction retry fails on new mongos', @@ -95,19 +106,11 @@ const SKIP_TESTS = [ 'Client side error when transaction is in progress' ]; -const LATEST_SKIP_TESTS = [ - // TODO(NODE-5855): Unskip Transactions Spec Unified Tests mongos-unpin.unpin - 'unpin after TransientTransactionError error on commit', - 'unpin on successful abort', - 'unpin after non-transient error on abort', - 'unpin after TransientTransactionError error on abort' -]; - describe('Transactions Spec Legacy Tests', function () { const testContext = new TransactionsRunnerContext(); if (process.env.SERVERLESS) { // TODO(NODE-3550): these tests should pass on serverless but currently fail - SKIP_TESTS.push( + LEGACY_SKIP_TESTS.push( 'abortTransaction only performs a single retry', 'abortTransaction does not retry after Interrupted', 'abortTransaction does not retry after WriteConcernError Interrupted', @@ -123,11 +126,8 @@ describe('Transactions Spec Legacy Tests', function () { return testContext.setup(this.configuration); }); - function testFilter(spec, configuration) { - return ( - SKIP_TESTS.indexOf(spec.description) === -1 && - (!gte(configuration.version, '7.9.9') || LATEST_SKIP_TESTS.indexOf(spec.description) === -1) - ); + function testFilter(spec) { + return LEGACY_SKIP_TESTS.indexOf(spec.description) === -1; } generateTopologyTests(testSuites, testContext, testFilter); From 7ff5aabf2a89c158c3c6ee1d223c716e37844e74 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 1 Feb 2024 11:42:58 -0500 Subject: [PATCH 5/6] Add transaction started to skipped tests --- test/integration/transactions/transactions.spec.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/transactions/transactions.spec.test.js b/test/integration/transactions/transactions.spec.test.js index 66bdcfaefa5..9eb206cb803 100644 --- a/test/integration/transactions/transactions.spec.test.js +++ b/test/integration/transactions/transactions.spec.test.js @@ -84,7 +84,8 @@ const LATEST_UNIFIED_SKIP_TESTS = [ 'unpin after TransientTransactionError error on commit', 'unpin on successful abort', 'unpin after non-transient error on abort', - 'unpin after TransientTransactionError error on abort' + 'unpin after TransientTransactionError error on abort', + 'unpin when a new transaction is started' ]; describe('Transactions Spec Unified Tests', function () { From 08c990d68031bb95ce02cbc864e589db7400dffa Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 1 Feb 2024 13:41:09 -0500 Subject: [PATCH 6/6] add read/write into skip as well (all remaining transacation tests --- test/integration/transactions/transactions.spec.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/integration/transactions/transactions.spec.test.js b/test/integration/transactions/transactions.spec.test.js index 9eb206cb803..d1fe27ef786 100644 --- a/test/integration/transactions/transactions.spec.test.js +++ b/test/integration/transactions/transactions.spec.test.js @@ -85,7 +85,9 @@ const LATEST_UNIFIED_SKIP_TESTS = [ 'unpin on successful abort', 'unpin after non-transient error on abort', 'unpin after TransientTransactionError error on abort', - 'unpin when a new transaction is started' + 'unpin when a new transaction is started', + 'unpin when a non-transaction write operation uses a session', + 'unpin when a non-transaction read operation uses a session' ]; describe('Transactions Spec Unified Tests', function () {