From 38f44df2d1fc1eadec360ca9c103b57719118a7a Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 17 Oct 2024 13:18:41 -0600 Subject: [PATCH] chore: fix a few flaky CSOT tests (#4278) --- ...resource-management-feature-integration.sh | 5 +- ...ient_side_operations_timeout.prose.test.ts | 2 +- .../node_csot.test.ts | 47 +++++++++------- .../crud/client_bulk_write.test.ts | 4 +- .../node-specific/abstract_cursor.test.ts | 54 +++++++++++++------ test/tools/utils.ts | 12 +++-- 6 files changed, 82 insertions(+), 42 deletions(-) diff --git a/.evergreen/run-resource-management-feature-integration.sh b/.evergreen/run-resource-management-feature-integration.sh index 093a4749d78..71756d96141 100644 --- a/.evergreen/run-resource-management-feature-integration.sh +++ b/.evergreen/run-resource-management-feature-integration.sh @@ -1,6 +1,9 @@ #! /bin/bash -source $DRIVERS_TOOLS/.evergreen/init-node-and-npm-env.sh +# source $DRIVERgit addS_TOOLS/.evergreen/init-node-and-npm-env.sh + +echo "node: $(node --version)" +echo "npm: $(npm --version)" echo "Building driver..." npm pack diff --git a/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts b/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts index 458447a437c..146a2585c52 100644 --- a/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts +++ b/test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts @@ -343,7 +343,7 @@ describe('CSOT spec prose tests', function () { client = this.configuration.newClient(undefined, { monitorCommands: true, - timeoutMS: 100, + timeoutMS: 150, minPoolSize: 20 }); await client.connect(); diff --git a/test/integration/client-side-operations-timeout/node_csot.test.ts b/test/integration/client-side-operations-timeout/node_csot.test.ts index a981a9113df..12b380d8f1a 100644 --- a/test/integration/client-side-operations-timeout/node_csot.test.ts +++ b/test/integration/client-side-operations-timeout/node_csot.test.ts @@ -361,7 +361,7 @@ describe('CSOT driver tests', metadata, () => { describe('Non-Tailable cursors', () => { let client: MongoClient; let internalClient: MongoClient; - let commandStarted: CommandStartedEvent[]; + let commandStarted: (CommandStartedEvent & { command: { maxTimeMS?: number } })[]; let commandSucceeded: CommandSucceededEvent[]; const failpoint: FailPoint = { configureFailPoint: 'failCommand', @@ -369,7 +369,7 @@ describe('CSOT driver tests', metadata, () => { data: { failCommands: ['find', 'getMore'], blockConnection: true, - blockTimeMS: 50 + blockTimeMS: 150 } }; @@ -435,7 +435,7 @@ describe('CSOT driver tests', metadata, () => { const cursor = client .db('db') .collection('coll') - .find({}, { batchSize: 1, timeoutMode: 'iteration', timeoutMS: 100 }) + .find({}, { batchSize: 1, timeoutMode: 'iteration', timeoutMS: 200 }) .project({ _id: 0 }); // Iterating over 3 documents in the collection, each artificially taking ~50 ms due to failpoint. If timeoutMS is not refreshed, then we'd expect to error @@ -457,20 +457,25 @@ describe('CSOT driver tests', metadata, () => { const cursor = client .db('db') .collection('coll') - .find({}, { batchSize: 1, timeoutMode: 'iteration', timeoutMS: 100 }) + .find({}, { batchSize: 1, timeoutMode: 'iteration', timeoutMS: 200 }) .project({ _id: 0 }); await cursor.toArray(); - expect(commandStarted).to.have.length.gte(3); // Find and 2 getMores - expect( - commandStarted.filter(ev => { - return ( - ev.command.find != null && - ev.command.getMore != null && - ev.command.maxTimeMS != null - ); - }) - ).to.have.lengthOf(0); + const commands = commandStarted.filter(c => + ['find', 'getMore'].includes(c.commandName) + ); + expect(commands).to.have.lengthOf(4); // Find and 2 getMores + + const [ + { command: aggregate }, + { command: getMore1 }, + { command: getMore2 }, + { command: getMore3 } + ] = commands; + expect(aggregate).not.to.have.property('maxTimeMS'); + expect(getMore1).not.to.have.property('maxTimeMS'); + expect(getMore2).not.to.have.property('maxTimeMS'); + expect(getMore3).not.to.have.property('maxTimeMS'); } ); }); @@ -644,7 +649,7 @@ describe('CSOT driver tests', metadata, () => { client = this.configuration.newClient(undefined, { monitorCommands: true, minPoolSize }); commandStarted = []; client.on('commandStarted', ev => commandStarted.push(ev)); - await client.connect(); + await waitUntilPoolsFilled(client, AbortSignal.timeout(30_000), minPoolSize); }); afterEach(async function () { @@ -685,11 +690,13 @@ describe('CSOT driver tests', metadata, () => { .db('db') .collection('coll') .find({}, { timeoutMS: 150, tailable: true, awaitData: true, batchSize: 1 }); - for (let i = 0; i < 5; i++) { - // Iterate cursor 5 times (server would have blocked for 500ms overall, but client - // should not throw - await cursor.next(); - } + // Iterate cursor 5 times (server would have blocked for 500ms overall, but client + // should not throw + await cursor.next(); + await cursor.next(); + await cursor.next(); + await cursor.next(); + await cursor.next(); }); it('does not use timeoutMS to compute maxTimeMS for getMores', metadata, async function () { diff --git a/test/integration/crud/client_bulk_write.test.ts b/test/integration/crud/client_bulk_write.test.ts index f5c6bf3b6df..ae7a1749b0e 100644 --- a/test/integration/crud/client_bulk_write.test.ts +++ b/test/integration/crud/client_bulk_write.test.ts @@ -321,7 +321,9 @@ describe('Client Bulk Write', function () { it( 'timeoutMS is refreshed to the timeoutMS passed to the bulk write for the killCursors command', - metadata, + { + requires: { ...metadata.requires, topology: '!load-balanced' } + }, async function () { const models = await makeMultiResponseBatchModelArray(this.configuration); const timeoutError = await client diff --git a/test/integration/node-specific/abstract_cursor.test.ts b/test/integration/node-specific/abstract_cursor.test.ts index 136e72a3499..8e154e1dc3e 100644 --- a/test/integration/node-specific/abstract_cursor.test.ts +++ b/test/integration/node-specific/abstract_cursor.test.ts @@ -7,6 +7,7 @@ import { inspect } from 'util'; import { AbstractCursor, type Collection, + type CommandStartedEvent, CursorTimeoutContext, CursorTimeoutMode, type FindCursor, @@ -17,7 +18,8 @@ import { MongoServerError, TimeoutContext } from '../../mongodb'; -import { type FailPoint } from '../../tools/utils'; +import { clearFailPoint, configureFailPoint } from '../../tools/utils'; +import { filterForCommands } from '../shared'; describe('class AbstractCursor', function () { describe('regression tests NODE-5372', function () { @@ -405,9 +407,11 @@ describe('class AbstractCursor', function () { let client: MongoClient; let collection: Collection; let context: CursorTimeoutContext; + const commands: CommandStartedEvent[] = []; beforeEach(async function () { - client = this.configuration.newClient(); + client = this.configuration.newClient({}, { monitorCommands: true }); + client.on('commandStarted', filterForCommands('killCursors', commands)); collection = client.db('abstract_cursor_integration').collection('test'); @@ -473,15 +477,17 @@ describe('class AbstractCursor', function () { }); describe('when the cursor refreshes the timeout for killCursors', function () { - it( - 'the provided timeoutContext is not modified', - { - requires: { - mongodb: '>=4.4' - } - }, - async function () { - await client.db('admin').command({ + let uri: string; + + before(function () { + uri = this.configuration.url({ useMultipleMongoses: false }); + }); + + beforeEach(async function () { + commands.length = 0; + await configureFailPoint( + this.configuration, + { configureFailPoint: 'failCommand', mode: { times: 1 }, data: { @@ -489,23 +495,41 @@ describe('class AbstractCursor', function () { blockConnection: true, blockTimeMS: 5000 } - } as FailPoint); + }, + uri + ); + }); + + afterEach(async function () { + await clearFailPoint(this.configuration, uri); + }); + it( + 'the provided timeoutContext is not modified', + { + requires: { + mongodb: '>=4.4', + topology: '!load-balanced' + } + }, + async function () { const cursor = collection.find( {}, { timeoutContext: context, - timeoutMS: 1000, + timeoutMS: 150, timeoutMode: CursorTimeoutMode.LIFETIME, batchSize: 1 } ); + const refresh = sinon.spy(context, 'refresh'); + const refreshed = sinon.spy(context, 'refreshed'); const error = await cursor.toArray().catch(e => e); expect(error).to.be.instanceof(MongoOperationTimeoutError); - // @ts-expect-error We know we have a CSOT timeout context but TS does not. - expect(context.timeoutContext.remainingTimeMS).to.be.lessThan(0); + expect(refresh.called).to.be.false; + expect(refreshed.called).to.be.true; } ); }); diff --git a/test/tools/utils.ts b/test/tools/utils.ts index 8ebc5e8f532..38c0da6c092 100644 --- a/test/tools/utils.ts +++ b/test/tools/utils.ts @@ -601,8 +601,12 @@ export async function waitUntilPoolsFilled( await Promise.all([wait$(), client.connect()]); } -export async function configureFailPoint(configuration: TestConfiguration, failPoint: FailPoint) { - const utilClient = configuration.newClient(); +export async function configureFailPoint( + configuration: TestConfiguration, + failPoint: FailPoint, + uri = configuration.url() +) { + const utilClient = configuration.newClient(uri); await utilClient.connect(); try { @@ -612,8 +616,8 @@ export async function configureFailPoint(configuration: TestConfiguration, failP } } -export async function clearFailPoint(configuration: TestConfiguration) { - const utilClient = configuration.newClient(); +export async function clearFailPoint(configuration: TestConfiguration, uri = configuration.url()) { + const utilClient = configuration.newClient(uri); await utilClient.connect(); try {