From 74916f297b6f8cc6a7056520cc3520ce2e9ca250 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 24 Jul 2024 16:25:01 +0200 Subject: [PATCH] test(NODE-6098): enable unified valid fail tests (#4181) --- .../unified_test_format.spec.test.ts | 6 +- .../entity-findCursor-malformed.json | 44 ++++++ .../entity-findCursor-malformed.yml | 31 ++++ .../valid-fail/entity-findCursor.json | 52 +++++++ .../valid-fail/entity-findCursor.yml | 31 ++++ .../ignoreResultAndError-malformed.json | 48 ++++++ .../ignoreResultAndError-malformed.yml | 34 ++++ .../valid-fail/ignoreResultAndError.json | 13 -- .../valid-fail/ignoreResultAndError.yml | 8 - .../valid-fail/operation-unsupported.json | 22 +++ .../valid-fail/operation-unsupported.yml | 13 ++ test/tools/unified-spec-runner/entities.ts | 26 ++-- .../entity_event_registry.ts | 8 +- test/tools/unified-spec-runner/operations.ts | 28 ++-- test/tools/unified-spec-runner/runner.ts | 30 +++- .../unified-spec-runner/unified-utils.ts | 69 ++++++-- test/unit/tools/unifed-utils.test.ts | 147 +++++++++--------- 17 files changed, 476 insertions(+), 134 deletions(-) create mode 100644 test/spec/unified-test-format/valid-fail/entity-findCursor-malformed.json create mode 100644 test/spec/unified-test-format/valid-fail/entity-findCursor-malformed.yml create mode 100644 test/spec/unified-test-format/valid-fail/entity-findCursor.json create mode 100644 test/spec/unified-test-format/valid-fail/entity-findCursor.yml create mode 100644 test/spec/unified-test-format/valid-fail/ignoreResultAndError-malformed.json create mode 100644 test/spec/unified-test-format/valid-fail/ignoreResultAndError-malformed.yml create mode 100644 test/spec/unified-test-format/valid-fail/operation-unsupported.json create mode 100644 test/spec/unified-test-format/valid-fail/operation-unsupported.yml diff --git a/test/integration/unified-test-format/unified_test_format.spec.test.ts b/test/integration/unified-test-format/unified_test_format.spec.test.ts index 4dd6ff80c0..2080fce98e 100644 --- a/test/integration/unified-test-format/unified_test_format.spec.test.ts +++ b/test/integration/unified-test-format/unified_test_format.spec.test.ts @@ -41,7 +41,11 @@ const filter: TestFilter = ({ description }) => { return false; }; -describe('Unified test format runner', function unifiedTestRunner() { +describe('Unified test format runner (valid-pass)', function unifiedTestRunner() { // Valid tests that should pass runUnifiedSuite(loadSpecTests('unified-test-format/valid-pass'), filter); }); + +describe('Unified test format runner (valid-fail)', function unifiedTestRunner() { + runUnifiedSuite(loadSpecTests('unified-test-format/valid-fail'), () => false, true); +}); diff --git a/test/spec/unified-test-format/valid-fail/entity-findCursor-malformed.json b/test/spec/unified-test-format/valid-fail/entity-findCursor-malformed.json new file mode 100644 index 0000000000..0956efa4c8 --- /dev/null +++ b/test/spec/unified-test-format/valid-fail/entity-findCursor-malformed.json @@ -0,0 +1,44 @@ +{ + "description": "entity-findCursor-malformed", + "schemaVersion": "1.3", + "createEntities": [ + { + "client": { + "id": "client0" + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "database0Name" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "coll0" + } + } + ], + "initialData": [ + { + "databaseName": "database0Name", + "collectionName": "coll0", + "documents": [] + } + ], + "tests": [ + { + "description": "createFindCursor fails if filter is not specified", + "operations": [ + { + "name": "createFindCursor", + "object": "collection0", + "saveResultAsEntity": "cursor0" + } + ] + } + ] +} diff --git a/test/spec/unified-test-format/valid-fail/entity-findCursor-malformed.yml b/test/spec/unified-test-format/valid-fail/entity-findCursor-malformed.yml new file mode 100644 index 0000000000..d7cf4856de --- /dev/null +++ b/test/spec/unified-test-format/valid-fail/entity-findCursor-malformed.yml @@ -0,0 +1,31 @@ +# This test is split out into a separate file to accommodate drivers that validate operation structure while decoding +# from JSON/YML. Such drivers fail to decode any files containing invalid operations. Combining this test in a file +# with other entity-findCursor valid-fail tests, which test failures that occur during test execution, would prevent +# such drivers from decoding the file and running any of the tests. +description: entity-findCursor-malformed + +schemaVersion: '1.3' + +createEntities: + - client: + id: &client0 client0 + - database: + id: &database0 database0 + client: *client0 + databaseName: &database0Name database0Name + - collection: + id: &collection0 collection0 + database: *database0 + collectionName: &collection0Name coll0 + +initialData: + - databaseName: *database0Name + collectionName: *collection0Name + documents: [] + +tests: + - description: createFindCursor fails if filter is not specified + operations: + - name: createFindCursor + object: *collection0 + saveResultAsEntity: &cursor0 cursor0 diff --git a/test/spec/unified-test-format/valid-fail/entity-findCursor.json b/test/spec/unified-test-format/valid-fail/entity-findCursor.json new file mode 100644 index 0000000000..389e448c06 --- /dev/null +++ b/test/spec/unified-test-format/valid-fail/entity-findCursor.json @@ -0,0 +1,52 @@ +{ + "description": "entity-findCursor", + "schemaVersion": "1.3", + "createEntities": [ + { + "client": { + "id": "client0" + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "database0Name" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "coll0" + } + } + ], + "initialData": [ + { + "databaseName": "database0Name", + "collectionName": "coll0", + "documents": [] + } + ], + "tests": [ + { + "description": "iterateUntilDocumentOrError fails if it references a nonexistent entity", + "operations": [ + { + "name": "iterateUntilDocumentOrError", + "object": "cursor0" + } + ] + }, + { + "description": "close fails if it references a nonexistent entity", + "operations": [ + { + "name": "close", + "object": "cursor0" + } + ] + } + ] +} diff --git a/test/spec/unified-test-format/valid-fail/entity-findCursor.yml b/test/spec/unified-test-format/valid-fail/entity-findCursor.yml new file mode 100644 index 0000000000..d796c132d2 --- /dev/null +++ b/test/spec/unified-test-format/valid-fail/entity-findCursor.yml @@ -0,0 +1,31 @@ +description: entity-findCursor + +schemaVersion: '1.3' + +createEntities: + - client: + id: &client0 client0 + - database: + id: &database0 database0 + client: *client0 + databaseName: &database0Name database0Name + - collection: + id: &collection0 collection0 + database: *database0 + collectionName: &collection0Name coll0 + +initialData: + - databaseName: *database0Name + collectionName: *collection0Name + documents: [] + +tests: + - description: iterateUntilDocumentOrError fails if it references a nonexistent entity + operations: + - name: iterateUntilDocumentOrError + object: cursor0 + + - description: close fails if it references a nonexistent entity + operations: + - name: close + object: cursor0 diff --git a/test/spec/unified-test-format/valid-fail/ignoreResultAndError-malformed.json b/test/spec/unified-test-format/valid-fail/ignoreResultAndError-malformed.json new file mode 100644 index 0000000000..b64779c723 --- /dev/null +++ b/test/spec/unified-test-format/valid-fail/ignoreResultAndError-malformed.json @@ -0,0 +1,48 @@ +{ + "description": "ignoreResultAndError-malformed", + "schemaVersion": "1.3", + "createEntities": [ + { + "client": { + "id": "client0", + "useMultipleMongoses": true + } + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "database0Name" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "coll0" + } + } + ], + "initialData": [ + { + "collectionName": "coll0", + "databaseName": "database0Name", + "documents": [] + } + ], + "tests": [ + { + "description": "malformed operation fails if ignoreResultAndError is true", + "operations": [ + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "foo": "bar" + }, + "ignoreResultAndError": true + } + ] + } + ] +} diff --git a/test/spec/unified-test-format/valid-fail/ignoreResultAndError-malformed.yml b/test/spec/unified-test-format/valid-fail/ignoreResultAndError-malformed.yml new file mode 100644 index 0000000000..4822bbe625 --- /dev/null +++ b/test/spec/unified-test-format/valid-fail/ignoreResultAndError-malformed.yml @@ -0,0 +1,34 @@ +# This test is split out into a separate file to accommodate drivers that validate operation structure while decoding +# from JSON/YML. Such drivers fail to decode any files containing invalid operations. Combining this test in a file +# with other ignoreResultAndError valid-fail tests, which test failures that occur during test execution, would prevent +# such drivers from decoding the file and running any of the tests. +description: ignoreResultAndError-malformed + +schemaVersion: '1.3' + +createEntities: + - client: + id: &client0 client0 + useMultipleMongoses: true + - database: + id: &database0 database0 + client: *client0 + databaseName: &database0Name database0Name + - collection: + id: &collection0 collection0 + database: *database0 + collectionName: &collection0Name coll0 + +initialData: + - collectionName: *collection0Name + databaseName: *database0Name + documents: [] + +tests: + - description: malformed operation fails if ignoreResultAndError is true + operations: + - name: insertOne + object: *collection0 + arguments: + foo: bar + ignoreResultAndError: true diff --git a/test/spec/unified-test-format/valid-fail/ignoreResultAndError.json b/test/spec/unified-test-format/valid-fail/ignoreResultAndError.json index 4457040b4f..01b2421a9f 100644 --- a/test/spec/unified-test-format/valid-fail/ignoreResultAndError.json +++ b/test/spec/unified-test-format/valid-fail/ignoreResultAndError.json @@ -54,19 +54,6 @@ "ignoreResultAndError": false } ] - }, - { - "description": "malformed operation fails if ignoreResultAndError is true", - "operations": [ - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "foo": "bar" - }, - "ignoreResultAndError": true - } - ] } ] } diff --git a/test/spec/unified-test-format/valid-fail/ignoreResultAndError.yml b/test/spec/unified-test-format/valid-fail/ignoreResultAndError.yml index 52498627a5..802c601f92 100644 --- a/test/spec/unified-test-format/valid-fail/ignoreResultAndError.yml +++ b/test/spec/unified-test-format/valid-fail/ignoreResultAndError.yml @@ -33,11 +33,3 @@ tests: # Insert the same document to force a DuplicateKey error. document: *insertDocument ignoreResultAndError: false - - - description: malformed operation fails if ignoreResultAndError is true - operations: - - name: insertOne - object: *collection0 - arguments: - foo: bar - ignoreResultAndError: true diff --git a/test/spec/unified-test-format/valid-fail/operation-unsupported.json b/test/spec/unified-test-format/valid-fail/operation-unsupported.json new file mode 100644 index 0000000000..d8ef5ab1c8 --- /dev/null +++ b/test/spec/unified-test-format/valid-fail/operation-unsupported.json @@ -0,0 +1,22 @@ +{ + "description": "operation-unsupported", + "schemaVersion": "1.0", + "createEntities": [ + { + "client": { + "id": "client0" + } + } + ], + "tests": [ + { + "description": "Unsupported operation", + "operations": [ + { + "name": "unsupportedOperation", + "object": "client0" + } + ] + } + ] +} diff --git a/test/spec/unified-test-format/valid-fail/operation-unsupported.yml b/test/spec/unified-test-format/valid-fail/operation-unsupported.yml new file mode 100644 index 0000000000..ba311637c0 --- /dev/null +++ b/test/spec/unified-test-format/valid-fail/operation-unsupported.yml @@ -0,0 +1,13 @@ +description: "operation-unsupported" + +schemaVersion: "1.0" + +createEntities: + - client: + id: &client0 client0 + +tests: + - description: "Unsupported operation" + operations: + - name: unsupportedOperation + object: *client0 diff --git a/test/tools/unified-spec-runner/entities.ts b/test/tools/unified-spec-runner/entities.ts index 57256370ed..07a3c36330 100644 --- a/test/tools/unified-spec-runner/entities.ts +++ b/test/tools/unified-spec-runner/entities.ts @@ -1,4 +1,4 @@ -import { expect } from 'chai'; +import { AssertionError, expect } from 'chai'; import { EventEmitter } from 'events'; import { @@ -303,7 +303,7 @@ export class UnifiedMongoClient extends MongoClient { case 'all': return [...this.commandEvents, ...this.cmapEvents, ...this.sdamEvents]; default: - throw new Error(`Unknown eventType: ${eventType}`); + throw new AssertionError(`Unknown eventType: ${eventType}`); } } @@ -490,7 +490,7 @@ export class EntitiesMap extends Map { mapOf(type: EntityTypeId): EntitiesMap { const ctor = ENTITY_CTORS.get(type); if (!ctor) { - throw new Error(`Unknown type ${type}`); + throw new AssertionError(`Unknown type ${type}`); } return new EntitiesMap(Array.from(this.entries()).filter(([, e]) => e instanceof ctor)); } @@ -522,7 +522,7 @@ export class EntitiesMap extends Map { getEntity(type: EntityTypeId, key: string, assertExists = true): Entity | undefined { const entity = this.get(key); if (!entity) { - if (assertExists) throw new Error(`Entity '${key}' does not exist`); + if (assertExists) throw new AssertionError(`Entity '${key}' does not exist`); return undefined; } if (NO_INSTANCE_CHECK.includes(type)) { @@ -531,10 +531,10 @@ export class EntitiesMap extends Map { } const ctor = ENTITY_CTORS.get(type); if (!ctor) { - throw new Error(`Unknown type ${type}`); + throw new AssertionError(`Unknown type ${type}`); } if (!(entity instanceof ctor)) { - throw new Error(`${key} is not an instance of ${type}`); + throw new AssertionError(`${key} is not an instance of ${type}`); } return entity; } @@ -586,11 +586,17 @@ export class EntitiesMap extends Map { uri = makeConnectionString(config.url({ useMultipleMongoses }), entity.client.uriOptions); } const client = new UnifiedMongoClient(uri, entity.client); - new EntityEventRegistry(client, entity.client, map).register(); try { + new EntityEventRegistry(client, entity.client, map).register(); await client.connect(); } catch (error) { console.error(ejson`failed to connect entity ${entity}`); + // In the case where multiple clients are defined in the test and any one of them failed + // to connect, but others did succeed, we need to ensure all open clients are closed. + const clients = map.mapOf('client'); + for (const clientEntry of clients.values()) { + await clientEntry.close(); + } throw error; } map.set(entity.client.id, client); @@ -666,13 +672,13 @@ export class EntitiesMap extends Map { } else if ('thread' in entity) { map.set(entity.thread.id, new UnifiedThread(entity.thread.id)); } else if ('stream' in entity) { - throw new Error(`Unsupported Entity ${JSON.stringify(entity)}`); + throw new AssertionError(`Unsupported Entity ${JSON.stringify(entity)}`); } else if ('clientEncryption' in entity) { - const clientEncryption = createClientEncryption(map, entity.clientEncryption); + const clientEncryption = await createClientEncryption(map, entity.clientEncryption); map.set(entity.clientEncryption.id, clientEncryption); } else { - throw new Error(`Unsupported Entity ${JSON.stringify(entity)}`); + throw new AssertionError(`Unsupported Entity ${JSON.stringify(entity)}`); } } return map; diff --git a/test/tools/unified-spec-runner/entity_event_registry.ts b/test/tools/unified-spec-runner/entity_event_registry.ts index 25c9c862a9..d62e106988 100644 --- a/test/tools/unified-spec-runner/entity_event_registry.ts +++ b/test/tools/unified-spec-runner/entity_event_registry.ts @@ -1,3 +1,5 @@ +import { AssertionError } from 'chai'; + import { COMMAND_FAILED, COMMAND_STARTED, @@ -60,11 +62,15 @@ export class EntityEventRegistry { register(): void { if (this.clientEntity.storeEventsAsEntities) { for (const { id, events } of this.clientEntity.storeEventsAsEntities) { + if (this.entitiesMap.has(id) || this.clientEntity.id === id) { + throw new AssertionError(`Duplicate id ${id} found while storing events as entities`); + } this.entitiesMap.set(id, []); for (const eventName of events) { // Need to map the event names to the Node event names. this.client.on(MAPPINGS[eventName], () => { - this.entitiesMap.getEntity('events', id).push({ + const events = this.entitiesMap.getEntity('events', id); + events.push({ name: eventName, observedAt: Date.now() }); diff --git a/test/tools/unified-spec-runner/operations.ts b/test/tools/unified-spec-runner/operations.ts index 27e93a21cd..045016c08e 100644 --- a/test/tools/unified-spec-runner/operations.ts +++ b/test/tools/unified-spec-runner/operations.ts @@ -44,9 +44,11 @@ type RunOperationFn = ( ) => Promise; export const operations = new Map(); +export class MalformedOperationError extends AssertionError {} + operations.set('createEntities', async ({ entities, operation, testConfig }) => { if (!operation.arguments?.entities) { - throw new Error('encountered createEntities operation without entities argument'); + throw new AssertionError('encountered createEntities operation without entities argument'); } await EntitiesMap.createEntities(testConfig, null, operation.arguments.entities!, entities); }); @@ -59,7 +61,7 @@ operations.set('abortTransaction', async ({ entities, operation }) => { operations.set('aggregate', async ({ entities, operation }) => { const dbOrCollection = entities.get(operation.object) as Db | Collection; if (!(dbOrCollection instanceof Db || dbOrCollection instanceof Collection)) { - throw new Error(`Operation object '${operation.object}' must be a db or collection`); + throw new AssertionError(`Operation object '${operation.object}' must be a db or collection`); } const { pipeline, ...opts } = operation.arguments!; const cursor = dbOrCollection.aggregate(pipeline, opts); @@ -228,7 +230,7 @@ operations.set('close', async ({ entities, operation }) => { } catch {} /* eslint-enable no-empty */ - throw new Error(`No closable entity with key ${operation.object}`); + throw new AssertionError(`No closable entity with key ${operation.object}`); }); operations.set('commitTransaction', async ({ entities, operation }) => { @@ -239,7 +241,7 @@ operations.set('commitTransaction', async ({ entities, operation }) => { operations.set('createChangeStream', async ({ entities, operation }) => { const watchable = entities.get(operation.object); if (watchable == null || !('watch' in watchable)) { - throw new Error(`Entity ${operation.object} must be watchable`); + throw new AssertionError(`Entity ${operation.object} must be watchable`); } const { pipeline, ...args } = operation.arguments!; @@ -356,6 +358,9 @@ operations.set('failPoint', async ({ entities, operation }) => { operations.set('insertOne', async ({ entities, operation }) => { const collection = entities.getEntity('collection', operation.object); const { document, ...opts } = operation.arguments!; + if (!document) { + throw new MalformedOperationError('No document defined in the arguments for insertOne'); + } // Looping exposes the fact that we can generate _ids for inserted // documents and we don't want the original operation to get modified // and use the same _id for each insert. @@ -582,7 +587,7 @@ operations.set('waitForEvent', async ({ entities, operation }) => { eventPromise, sleep(10000).then(() => Promise.reject( - new Error( + new AssertionError( `Timed out waiting for ${eventName}; captured [${mongoClient .getCapturedEvents('all') .map(e => e.constructor.name) @@ -677,7 +682,7 @@ operations.set('waitForPrimaryChange', async ({ entities, operation }) => { await Promise.race([ newPrimaryPromise, sleep(timeoutMS ?? 10000).then(() => - Promise.reject(new Error(`Timed out waiting for primary change on ${client}`)) + Promise.reject(new AssertionError(`Timed out waiting for primary change on ${client}`)) ) ]); }); @@ -697,7 +702,9 @@ operations.set('waitForThread', async ({ entities, operation }) => { const thread = entities.getEntity('thread', threadId, true); await Promise.race([ thread.finish(), - sleep(10000).then(() => Promise.reject(new Error(`Timed out waiting for thread: ${threadId}`))) + sleep(10000).then(() => + Promise.reject(new AssertionError(`Timed out waiting for thread: ${threadId}`)) + ) ]); }); @@ -750,10 +757,11 @@ operations.set('estimatedDocumentCount', async ({ entities, operation }) => { operations.set('runCommand', async ({ entities, operation }: OperationFunctionParams) => { const db = entities.getEntity('db', operation.object); - if (operation.arguments?.command == null) throw new Error('runCommand requires a command'); + if (operation.arguments?.command == null) + throw new AssertionError('runCommand requires a command'); const { command } = operation.arguments; - if (operation.arguments.timeoutMS != null) throw new Error('timeoutMS not supported'); + if (operation.arguments.timeoutMS != null) throw new AssertionError('timeoutMS not supported'); const options = { readPreference: operation.arguments.readPreference, @@ -949,7 +957,7 @@ export async function executeOperationAndCheck( if (operation.expectError) { expectErrorCheck(error, operation.expectError, entities); return; - } else if (!operation.ignoreResultAndError) { + } else if (!operation.ignoreResultAndError || error instanceof MalformedOperationError) { throw error; } } diff --git a/test/tools/unified-spec-runner/runner.ts b/test/tools/unified-spec-runner/runner.ts index ab4a5cd897..e27d072f19 100644 --- a/test/tools/unified-spec-runner/runner.ts +++ b/test/tools/unified-spec-runner/runner.ts @@ -1,9 +1,16 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ -import { expect } from 'chai'; +import { AssertionError, expect } from 'chai'; import { gte as semverGte, satisfies as semverSatisfies } from 'semver'; import type { MongoClient } from '../../mongodb'; -import { MONGODB_ERROR_CODES, ns, ReadPreference, TopologyType } from '../../mongodb'; +import { + MONGODB_ERROR_CODES, + MongoParseError, + MongoServerError, + ns, + ReadPreference, + TopologyType +} from '../../mongodb'; import { ejson } from '../utils'; import { AstrolabeResultsWriter } from './astrolabe_results_writer'; import { EntitiesMap, type UnifiedMongoClient } from './entities'; @@ -306,13 +313,28 @@ async function runUnifiedTest( */ export function runUnifiedSuite( specTests: uni.UnifiedSuite[], - skipFilter: uni.TestFilter = () => false + skipFilter: uni.TestFilter = () => false, + expectRuntimeError = false ): void { for (const unifiedSuite of specTests) { context(String(unifiedSuite.description), function () { for (const [index, test] of unifiedSuite.tests.entries()) { it(String(test.description === '' ? `Test ${index}` : test.description), async function () { - await runUnifiedTest(this, unifiedSuite, test, skipFilter); + if (expectRuntimeError) { + const error = await runUnifiedTest(this, unifiedSuite, test, skipFilter).catch( + error => error + ); + expect(error).to.satisfy(value => { + return ( + value instanceof AssertionError || + value instanceof MongoServerError || + value instanceof TypeError || + value instanceof MongoParseError + ); + }); + } else { + await runUnifiedTest(this, unifiedSuite, test, skipFilter); + } }); } }); diff --git a/test/tools/unified-spec-runner/unified-utils.ts b/test/tools/unified-spec-runner/unified-utils.ts index 2f36409875..25a5115a6d 100644 --- a/test/tools/unified-spec-runner/unified-utils.ts +++ b/test/tools/unified-spec-runner/unified-utils.ts @@ -1,5 +1,5 @@ import { EJSON } from 'bson'; -import { expect } from 'chai'; +import { AssertionError, expect } from 'chai'; import ConnectionString from 'mongodb-connection-string-url'; import { gte as semverGte, lte as semverLte } from 'semver'; import { isDeepStrictEqual } from 'util'; @@ -12,11 +12,13 @@ import { type DbOptions, type Document, getMongoDBClientEncryption, - type MongoClient + type MongoClient, + ReturnDocument } from '../../mongodb'; import { shouldRunServerlessTest } from '../../tools/utils'; import type { CmapEvent, CommandEvent, EntitiesMap, SdamEvent } from './entities'; import { matchesEvents } from './match'; +import { MalformedOperationError } from './operations'; import type { ClientEncryptionEntity, CollectionOrDatabaseOptions, @@ -51,7 +53,7 @@ export async function topologySatisfies( }[config.topologyType]; if (!Array.isArray(r.topologies)) { - throw new Error('Topology specification must be an array'); + throw new AssertionError('Topology specification must be an array'); } if (r.topologies.includes('sharded-replicaset') && topologyType === 'sharded') { @@ -61,7 +63,7 @@ export async function topologySatisfies( skipReason = `requires sharded-replicaset but shards.length=${shards.length}`; } } else { - if (!topologyType) throw new Error(`Topology undiscovered: ${config.topologyType}`); + if (!topologyType) throw new AssertionError(`Topology undiscovered: ${config.topologyType}`); ok &&= r.topologies.includes(topologyType); if (!ok && skipReason == null) { skipReason = `requires ${r.topologies} but discovered a ${topologyType} topology`; @@ -83,7 +85,8 @@ export async function topologySatisfies( } if (r.serverParameters) { - if (!config.parameters) throw new Error('Configuration does not have server parameters'); + if (!config.parameters) + throw new AssertionError('Configuration does not have server parameters'); for (const [name, value] of Object.entries(r.serverParameters)) { if (name in config.parameters) { ok &&= isDeepStrictEqual(config.parameters[name], value); @@ -199,7 +202,13 @@ export function patchCollectionOptions( export function translateOptions(options: Document): Document { const translatedOptions = { ...options }; if (options.returnDocument) { - translatedOptions.returnDocument = options.returnDocument.toLowerCase(); + const returnDocument = options.returnDocument.toLowerCase(); + if (![ReturnDocument.BEFORE, ReturnDocument.AFTER].includes(returnDocument)) { + throw new MalformedOperationError( + 'Return document must be specified as either "before" or "after"' + ); + } + translatedOptions.returnDocument = returnDocument; } return translatedOptions as Document; } @@ -250,7 +259,7 @@ export function getCSFLETestDataFromEnvironment(environment: Record { getMongoDBClientEncryption(); const { clientEncryptionOpts } = entity; @@ -503,7 +538,7 @@ export function createClientEncryption( const clientEntity = map.getEntity('client', keyVaultClient, false); if (!clientEntity) { - throw new Error( + throw new AssertionError( 'unable to get client entity required by client encryption entity in unified test' ); } @@ -537,7 +572,13 @@ export function createClientEncryption( }, {}); } - const kmsProviders = mergeKMSProviders(kmsProvidersFromTest, kmsProvidersFromEnvironment); + let kmsProviders; + try { + kmsProviders = mergeKMSProviders(kmsProvidersFromTest, kmsProvidersFromEnvironment); + } catch (error) { + await clientEntity.close(); + throw error; + } const autoEncryptionOptions: AutoEncryptionOptions = { keyVaultClient: clientEntity, diff --git a/test/unit/tools/unifed-utils.test.ts b/test/unit/tools/unifed-utils.test.ts index ea8513940f..9745f1c570 100644 --- a/test/unit/tools/unifed-utils.test.ts +++ b/test/unit/tools/unifed-utils.test.ts @@ -4,19 +4,16 @@ import { mergeKMSProviders } from '../../tools/unified-spec-runner/unified-utils describe('parseOptions', function () { context('aws providers', function () { - it('does not configure the provider if none is given', function () { - const parsedProviders = mergeKMSProviders({}, {}); - expect(parsedProviders).not.to.have.property('aws'); + it('throws if none is given', function () { + expect(() => { + mergeKMSProviders({}, {}); + }).to.throw(); }); - it('configures the provider without credentials if an empty object is supplied', function () { - const parsedProviders = mergeKMSProviders( - { - aws: {} - }, - {} - ); - expect(parsedProviders.aws).deep.equal({}); + it('throws if an empty object is supplied', function () { + expect(() => { + mergeKMSProviders({ aws: {} }, {}); + }).to.throw(); }); it('replaces a $$placeholder value with the value from the environment', function () { @@ -37,18 +34,19 @@ describe('parseOptions', function () { }); }); - it('omits required fields if the field is not present in the kmsProviders', function () { - const parsedProviders = mergeKMSProviders( - { - aws: { - accessKeyId: { $$placeholder: 1 } + it('throws if required field is not present in the kmsProviders', function () { + expect(() => { + mergeKMSProviders( + { + aws: { + accessKeyId: { $$placeholder: 1 } + } + }, + { + aws: { accessKeyId: 'accessKeyId' } } - }, - { - aws: { accessKeyId: 'accessKeyId' } - } - ); - expect(parsedProviders.aws).not.to.have.property('secretAccessKey'); + ); + }).to.throw(); }); it('configures the provider with the exact credentials from the test', function () { @@ -76,9 +74,10 @@ describe('parseOptions', function () { }); }); context('local providers', function () { - it('does not configure the provider if none is given', function () { - const parsedProviders = mergeKMSProviders({}, {}); - expect(parsedProviders).not.to.have.property('local'); + it('throws if none is given', function () { + expect(() => { + mergeKMSProviders({}, {}); + }).to.throw(); }); it('configures the provider without credentials if an empty object is supplied', function () { @@ -127,19 +126,16 @@ describe('parseOptions', function () { }); context('azure', function () { - it('does not configure the provider if none is given', function () { - const parsedProviders = mergeKMSProviders({}, {}); - expect(parsedProviders).not.to.have.property('azure'); + it('throws if none is given', function () { + expect(() => { + mergeKMSProviders({}, {}); + }).to.throw(); }); - it('configures the provider without credentials if an empty object is supplied', function () { - const parsedProviders = mergeKMSProviders( - { - azure: {} - }, - {} - ); - expect(parsedProviders.azure).deep.equal({}); + it('throws if an empty object is supplied', function () { + expect(() => { + mergeKMSProviders({ azure: {} }, {}); + }).to.throw(); }); it('replaces a $$placeholder value with the value from the environment', function () { @@ -166,18 +162,19 @@ describe('parseOptions', function () { }); }); - it('omits required fields if the field is not present in the kmsProviders', function () { - const parsedProviders = mergeKMSProviders( - { - azure: { - tenantId: 'tenantId', - clientSecret: 'clientSecret', - identityPlatformEndpoint: 'identifyPlatformEndpoint' - } - }, - {} - ); - expect(parsedProviders.azure).not.to.have.property('clientId'); + it('throws if required field is not present in the kmsProviders', function () { + expect(() => { + mergeKMSProviders( + { + azure: { + tenantId: 'tenantId', + clientSecret: 'clientSecret', + identityPlatformEndpoint: 'identifyPlatformEndpoint' + } + }, + {} + ); + }).to.throw(); }); it('configures the provider with the exact credentials from the test otherwise', function () { @@ -209,19 +206,21 @@ describe('parseOptions', function () { }); context('gcp', function () { - it('does not configure the provider if none is given', function () { - const parsedProviders = mergeKMSProviders({}, {}); - expect(parsedProviders).not.to.have.property('gcp'); + it('throws if none is given', function () { + expect(() => { + mergeKMSProviders({}, {}); + }).throw(); }); - it('configures the provider without credentials if an empty object is supplied', function () { - const parsedProviders = mergeKMSProviders( - { - gcp: {} - }, - {} - ); - expect(parsedProviders.gcp).deep.equal({}); + it('throws if an empty object is supplied', function () { + expect(() => { + mergeKMSProviders( + { + gcp: {} + }, + {} + ); + }).to.throw(); }); it('replaces a $$placeholder value with the value from the environment', function () { @@ -246,17 +245,18 @@ describe('parseOptions', function () { }); }); - it('omits required fields if the field is not present in the kmsProviders', function () { - const parsedProviders = mergeKMSProviders( - { - gcp: { - email: 'email', - endPoint: 'endPoint' - } - }, - {} - ); - expect(parsedProviders.gcp).not.to.have.property('privateKey'); + it('throws if required field is not present in the kmsProviders', function () { + expect(() => { + mergeKMSProviders( + { + gcp: { + email: 'email', + endPoint: 'endPoint' + } + }, + {} + ); + }).to.throw(); }); it('configures the provider with the exact credentials from the test otherwise', function () { @@ -285,9 +285,10 @@ describe('parseOptions', function () { }); context('kmip', function () { - it('does not configure the provider if none is given', function () { - const parsedProviders = mergeKMSProviders({}, {}); - expect(parsedProviders).not.to.have.property('kmip'); + it('throws if none is given', function () { + expect(() => { + mergeKMSProviders({}, {}); + }).to.throw(); }); it('configures the provider without credentials if an empty object is supplied', function () {