From 7009338f52148342c03894df121ee51703a1852a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Tue, 17 Sep 2024 20:13:07 +0200 Subject: [PATCH 01/14] Refactored TestShell to ensure killAll is called --- packages/e2e-tests/test/e2e-analytics.spec.ts | 4 +- packages/e2e-tests/test/e2e-auth.spec.ts | 8 +- packages/e2e-tests/test/e2e-aws.spec.ts | 6 +- packages/e2e-tests/test/e2e-banners.spec.ts | 4 +- packages/e2e-tests/test/e2e-bson.spec.ts | 6 +- packages/e2e-tests/test/e2e-direct.spec.ts | 9 +- packages/e2e-tests/test/e2e-editor.spec.ts | 5 +- packages/e2e-tests/test/e2e-fle.spec.ts | 5 +- packages/e2e-tests/test/e2e-oidc.spec.ts | 5 +- packages/e2e-tests/test/e2e-proxy.spec.ts | 4 +- packages/e2e-tests/test/e2e-snapshot.spec.ts | 5 +- packages/e2e-tests/test/e2e-snippet.spec.ts | 5 +- packages/e2e-tests/test/e2e-tls.spec.ts | 9 +- packages/e2e-tests/test/e2e.spec.ts | 5 +- packages/e2e-tests/test/test-shell.ts | 85 +++++++++++++------ 15 files changed, 98 insertions(+), 67 deletions(-) diff --git a/packages/e2e-tests/test/e2e-analytics.spec.ts b/packages/e2e-tests/test/e2e-analytics.spec.ts index f660c5d84e..500da6a00c 100644 --- a/packages/e2e-tests/test/e2e-analytics.spec.ts +++ b/packages/e2e-tests/test/e2e-analytics.spec.ts @@ -1,7 +1,7 @@ import { expect } from 'chai'; import { startTestCluster } from '../../../testing/integration-testing-hooks'; import { eventually } from '../../../testing/eventually'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; describe('e2e Analytics Node', function () { const replSetName = 'replicaSet'; @@ -13,7 +13,7 @@ describe('e2e Analytics Node', function () { { args: ['--replSet', replSetName] } ); - afterEach(TestShell.cleanup); + cleanTestShellsAfterEach(); before(async function () { if (process.env.MONGOSH_TEST_FORCE_API_STRICT) { diff --git a/packages/e2e-tests/test/e2e-auth.spec.ts b/packages/e2e-tests/test/e2e-auth.spec.ts index f9fdc52a26..ecbcd8be84 100644 --- a/packages/e2e-tests/test/e2e-auth.spec.ts +++ b/packages/e2e-tests/test/e2e-auth.spec.ts @@ -2,7 +2,7 @@ import { expect } from 'chai'; import type { Db, Document, MongoClientOptions } from 'mongodb'; import { MongoClient } from 'mongodb'; import { eventually } from '../../../testing/eventually'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import { skipIfApiStrict, startSharedTestServer, @@ -107,6 +107,8 @@ describe('Auth e2e', function () { let examplePrivilege2: Document; describe('with regular URI', function () { + cleanTestShellsAfterEach(); + beforeEach(async function () { const connectionString = await testServer.connectionString(); dbName = `test-${Date.now()}`; @@ -137,7 +139,6 @@ describe('Auth e2e', function () { await client.close(); }); - afterEach(TestShell.cleanup); describe('user management', function () { describe('createUser', function () { @@ -1144,6 +1145,7 @@ describe('Auth e2e', function () { await client.close(); }); - afterEach(TestShell.cleanup); + + cleanTestShellsAfterEach(); }); }); diff --git a/packages/e2e-tests/test/e2e-aws.spec.ts b/packages/e2e-tests/test/e2e-aws.spec.ts index 989fc153f7..bf2c099053 100644 --- a/packages/e2e-tests/test/e2e-aws.spec.ts +++ b/packages/e2e-tests/test/e2e-aws.spec.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; import { spawnSync } from 'child_process'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; function assertEnvVariable(variableName: string): string { if (process.env.MONGOSH_TEST_FORCE_API_STRICT) { @@ -84,6 +84,8 @@ function getConnectionString(username?: string, password?: string): string { } describe('e2e AWS AUTH', function () { + cleanTestShellsAfterEach(); + this.timeout(60_000); // AWS auth tests can take longer than the default timeout in CI let expectedAssumedRole: string; @@ -117,8 +119,6 @@ describe('e2e AWS AUTH', function () { ).replace('arn:aws:iam::', 'arn:aws:sts::')}/*`; }); - afterEach(TestShell.cleanup); - context('without environment variables being present', function () { context('specifying explicit parameters', function () { it('connects with access key and secret', async function () { diff --git a/packages/e2e-tests/test/e2e-banners.spec.ts b/packages/e2e-tests/test/e2e-banners.spec.ts index fb6a520cbe..6ffed51134 100644 --- a/packages/e2e-tests/test/e2e-banners.spec.ts +++ b/packages/e2e-tests/test/e2e-banners.spec.ts @@ -2,11 +2,11 @@ import { skipIfApiStrict, startSharedTestServer, } from '../../../testing/integration-testing-hooks'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; describe('e2e startup banners', function () { skipIfApiStrict(); - afterEach(TestShell.cleanup); + cleanTestShellsAfterEach(); const testServer = startSharedTestServer(); diff --git a/packages/e2e-tests/test/e2e-bson.spec.ts b/packages/e2e-tests/test/e2e-bson.spec.ts index e69408a4d9..40d8108acd 100644 --- a/packages/e2e-tests/test/e2e-bson.spec.ts +++ b/packages/e2e-tests/test/e2e-bson.spec.ts @@ -2,10 +2,12 @@ import { expect } from 'chai'; import type { Db } from 'mongodb'; import { MongoClient } from 'mongodb'; import { bson } from '@mongosh/service-provider-core'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import { startSharedTestServer } from '../../../testing/integration-testing-hooks'; describe('BSON e2e', function () { + cleanTestShellsAfterEach(); + const testServer = startSharedTestServer(); let db: Db; let client: MongoClient; @@ -30,7 +32,7 @@ describe('BSON e2e', function () { await client.close(); }); - afterEach(TestShell.cleanup); + describe('printed BSON', function () { const outputDoc = { ObjectId: "ObjectId('5f16b8bebe434dc98cdfc9ca')", diff --git a/packages/e2e-tests/test/e2e-direct.spec.ts b/packages/e2e-tests/test/e2e-direct.spec.ts index 6c11882393..8b5d461aab 100644 --- a/packages/e2e-tests/test/e2e-direct.spec.ts +++ b/packages/e2e-tests/test/e2e-direct.spec.ts @@ -5,11 +5,11 @@ import { } from '../../../testing/integration-testing-hooks'; import { eventually } from '../../../testing/eventually'; import { expect } from 'chai'; -import { TestShell } from './test-shell'; +import { TestShell, cleanTestShellsAfterEach } from './test-shell'; describe('e2e direct connection', function () { skipIfApiStrict(); - afterEach(TestShell.cleanup); + cleanTestShellsAfterEach(); const tabtab = async (shell: TestShell) => { await new Promise((resolve) => setTimeout(resolve, 400)); @@ -82,11 +82,6 @@ describe('e2e direct connection', function () { await shell.executeLine('db.testcollection.insertOne({})'); shell.writeInputLine('exit'); }); - after(async function () { - const shell = TestShell.start({ args: [await rs0.connectionString()] }); - await shell.executeLine(`db.getSiblingDB("${dbname}").dropDatabase()`); - shell.writeInputLine('exit'); - }); context('connecting to secondary members directly', function () { it('works when specifying a connection string', async function () { diff --git a/packages/e2e-tests/test/e2e-editor.spec.ts b/packages/e2e-tests/test/e2e-editor.spec.ts index 6e29cc0abe..8d5c63f786 100644 --- a/packages/e2e-tests/test/e2e-editor.spec.ts +++ b/packages/e2e-tests/test/e2e-editor.spec.ts @@ -2,7 +2,7 @@ import { expect } from 'chai'; import path from 'path'; import { promises as fs } from 'fs'; import { eventually } from '../../../testing/eventually'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import { useTmpdir, fakeExternalEditor, @@ -15,6 +15,8 @@ describe('external editor e2e', function () { let env: Record; let shell: TestShell; + cleanTestShellsAfterEach(); + beforeEach(async function () { const homeInfo = setTemporaryHomeDirectory(); @@ -42,7 +44,6 @@ describe('external editor e2e', function () { }); afterEach(async function () { - await TestShell.killall.call(this); try { await fs.rm(homedir, { recursive: true, force: true }); } catch (err: any) { diff --git a/packages/e2e-tests/test/e2e-fle.spec.ts b/packages/e2e-tests/test/e2e-fle.spec.ts index 49dae636e3..60711aa48d 100644 --- a/packages/e2e-tests/test/e2e-fle.spec.ts +++ b/packages/e2e-tests/test/e2e-fle.spec.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; import { MongoClient } from 'mongodb'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import { eventually } from '../../../testing/eventually'; import { startTestServer, @@ -16,6 +16,8 @@ import { inspect } from 'util'; import path from 'path'; describe('FLE tests', function () { + cleanTestShellsAfterEach(); + const testServer = startTestServer('e2e-fle', { topology: 'replset', secondaries: 0, @@ -56,7 +58,6 @@ describe('FLE tests', function () { await client.db(dbname).dropDatabase(); await client.close(); }); - afterEach(TestShell.cleanup); function* awsTestCases() { for (const useApiStrict of [false, true]) { diff --git a/packages/e2e-tests/test/e2e-oidc.spec.ts b/packages/e2e-tests/test/e2e-oidc.spec.ts index 0c38fe2319..70ffdcaae3 100644 --- a/packages/e2e-tests/test/e2e-oidc.spec.ts +++ b/packages/e2e-tests/test/e2e-oidc.spec.ts @@ -6,7 +6,7 @@ import { import { promises as fs } from 'fs'; import type { OIDCMockProviderConfig } from '@mongodb-js/oidc-mock-provider'; import { OIDCMockProvider } from '@mongodb-js/oidc-mock-provider'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import path from 'path'; import { expect } from 'chai'; import { createServer as createHTTPSServer } from 'https'; @@ -30,6 +30,7 @@ import { * happens in isolation. */ describe('OIDC auth e2e', function () { + cleanTestShellsAfterEach(); skipIfApiStrict(); // connectionStatus is unversioned. let getTokenPayload: typeof oidcMockProviderConfig.getTokenPayload; @@ -164,8 +165,6 @@ describe('OIDC auth e2e', function () { ]); }); - afterEach(TestShell.cleanup); - async function verifyUser( shell: TestShell, username: string, diff --git a/packages/e2e-tests/test/e2e-proxy.spec.ts b/packages/e2e-tests/test/e2e-proxy.spec.ts index c8623c3ad3..98c94ad330 100644 --- a/packages/e2e-tests/test/e2e-proxy.spec.ts +++ b/packages/e2e-tests/test/e2e-proxy.spec.ts @@ -11,7 +11,7 @@ import { startSharedTestServer, startTestServer, } from '../../../testing/integration-testing-hooks'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import type { Server as HTTPSServer } from 'https'; import { createServer as createHTTPSServer } from 'https'; import { @@ -40,7 +40,7 @@ describe('e2e proxy support', function () { skipIfApiStrict(); skipIfEnvServerVersion('< 7.0'); - afterEach(TestShell.cleanup); + cleanTestShellsAfterEach(); const tmpdir = useTmpdir(); const testServer = startSharedTestServer(); diff --git a/packages/e2e-tests/test/e2e-snapshot.spec.ts b/packages/e2e-tests/test/e2e-snapshot.spec.ts index e4caa28fc6..0454d88a98 100644 --- a/packages/e2e-tests/test/e2e-snapshot.spec.ts +++ b/packages/e2e-tests/test/e2e-snapshot.spec.ts @@ -2,7 +2,7 @@ import { skipIfApiStrict, startSharedTestServer, } from '../../../testing/integration-testing-hooks'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import { expect } from 'chai'; const setDifference = (a: T[], b: T[]) => a.filter((e) => !b.includes(e)); @@ -17,7 +17,8 @@ const commonPrefix = (a: string, b: string): string => describe('e2e snapshot support', function () { skipIfApiStrict(); - afterEach(TestShell.cleanup); + + cleanTestShellsAfterEach(); const testServer = startSharedTestServer(); diff --git a/packages/e2e-tests/test/e2e-snippet.spec.ts b/packages/e2e-tests/test/e2e-snippet.spec.ts index 3b1c329ed4..e4ec8fc5c6 100644 --- a/packages/e2e-tests/test/e2e-snippet.spec.ts +++ b/packages/e2e-tests/test/e2e-snippet.spec.ts @@ -1,11 +1,13 @@ import { promises as fs } from 'fs'; import path from 'path'; import { expect } from 'chai'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import { useTmpdir } from './repl-helpers'; import { eventually } from '../../../testing/eventually'; describe('snippet integration tests', function () { + cleanTestShellsAfterEach(); + this.timeout(120_000); const tmpdir = useTmpdir(); @@ -44,7 +46,6 @@ describe('snippet integration tests', function () { { recursive: true } ); }); - afterEach(TestShell.cleanup); it('allows managing snippets', async function () { shell.writeInputLine('snippet install analyze-schema'); diff --git a/packages/e2e-tests/test/e2e-tls.spec.ts b/packages/e2e-tests/test/e2e-tls.spec.ts index 573fa85e05..5794435a3e 100644 --- a/packages/e2e-tests/test/e2e-tls.spec.ts +++ b/packages/e2e-tests/test/e2e-tls.spec.ts @@ -9,7 +9,7 @@ import { getCertPath, connectionStringWithLocalhost, } from './repl-helpers'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; const CA_CERT = getCertPath('ca.crt'); const NON_CA_CERT = getCertPath('non-ca.crt'); @@ -30,6 +30,8 @@ const CRL_INCLUDING_SERVER = getCertPath('ca-server.crl'); * and compliance with user-specified behavior that is specific to TLS connectivity. */ describe('e2e TLS', function () { + cleanTestShellsAfterEach(); + let homedir: string; let env: Record; let logBasePath: string; @@ -64,7 +66,6 @@ describe('e2e TLS', function () { console.error('Could not remove fake home directory:', err); } }); - afterEach(TestShell.cleanup); context( 'connecting without client cert to server with valid cert', @@ -87,7 +88,6 @@ describe('e2e TLS', function () { shell.kill(); await shell.waitForExit(); }); - afterEach(TestShell.cleanup); const server = startTestServer('e2e-tls-no-cli-valid-srv', { args: [ @@ -323,8 +323,6 @@ describe('e2e TLS', function () { await shell.executeLine('db.shutdownServer({ force: true })'); shell.kill(); await shell.waitForExit(); - - await TestShell.cleanup.call(this); }); const server = startTestServer('e2e-tls-valid-cli-valid-srv', { @@ -625,7 +623,6 @@ describe('e2e TLS', function () { }); await shell.waitForPrompt(); await shell.executeLine('db.shutdownServer({ force: true })'); - await TestShell.killall(); }); const server = startTestServer('e2e-tls-invalid-srv', { diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts index e084014a43..751aeba0bc 100644 --- a/packages/e2e-tests/test/e2e.spec.ts +++ b/packages/e2e-tests/test/e2e.spec.ts @@ -3,7 +3,7 @@ import { expect } from 'chai'; import type { Db } from 'mongodb'; import { MongoClient } from 'mongodb'; import { eventually } from '../../../testing/eventually'; -import { TestShell } from './test-shell'; +import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import { skipIfServerVersion, startSharedTestServer, @@ -29,7 +29,7 @@ const jsContextFlagCombinations: `--jsContext=${'plain-vm' | 'repl'}`[][] = [ describe('e2e', function () { const testServer = startSharedTestServer(); - afterEach(TestShell.cleanup); + cleanTestShellsAfterEach(); describe('--version', function () { it('shows version', async function () { @@ -1378,7 +1378,6 @@ describe('e2e', function () { }); afterEach(async function () { - await TestShell.killall.call(this); try { await fs.rm(homedir, { recursive: true, force: true }); } catch (err: any) { diff --git a/packages/e2e-tests/test/test-shell.ts b/packages/e2e-tests/test/test-shell.ts index ada66d234d..ad1b1e8dce 100644 --- a/packages/e2e-tests/test/test-shell.ts +++ b/packages/e2e-tests/test/test-shell.ts @@ -12,6 +12,8 @@ import stripAnsi from 'strip-ansi'; import { EJSON } from 'bson'; import { eventually } from '../../../testing/eventually'; +/* eslint-disable mocha/no-exports -- This file export hooks wrapping Mocha's Hooks APIs */ + export type TestShellStartupResult = | { state: 'prompt' } | { state: 'exit'; exitCode: number }; @@ -31,6 +33,50 @@ function matches(str: string, pattern: string | RegExp): boolean { : pattern.test(str); } +/** + * Toggle used to ensure an appropriate hook is registered, to clean up test shells. + * NOTE: This is a local variable of the module instead of a static on {@link TestShell} to allow the hooks to toggle it. + */ +let testShellEnabled = false; + +/** + * Enables the TestShell and kill all shells after all tests of the current suite. + */ +export function cleanTestShellsAfter() { + before('enabled TestShell', function () { + assert( + !testShellEnabled, + 'TestShell is already enabled, use only one cleanTestShellsAfter or cleanTestShellsAfterEach' + ); + testShellEnabled = true; + }); + + after('kill all TestShell instances', async function (this: Mocha.Context) { + assert(testShellEnabled, 'Expected TestShell to be enabled'); + testShellEnabled = false; + await TestShell.killAll(); + }); +} + +/** + * Enables the TestShell and kill all shells after each test + * NOTE: This also registers {@link cleanTestShellsAfter} hook internally, to allow `after` hooks to start TestShell instances. + */ +export function cleanTestShellsAfterEach() { + cleanTestShellsAfter(); + + afterEach( + 'kill all TestShell instances', + async function (this: Mocha.Context) { + assert(testShellEnabled, 'Expected TestShell to be enabled'); + if (this.currentTest?.state === 'failed') { + TestShell.printShells(); + } + await TestShell.killAll(); + } + ); +} + export interface TestShellOptions { args: string[]; env?: Record; @@ -47,6 +93,11 @@ export class TestShell { private static _openShells: TestShell[] = []; static start(options: TestShellOptions = { args: [] }): TestShell { + assert( + testShellEnabled, + 'Expected TestShell to be enabled, did you call cleanTestShellsAfter or cleanTestShellsAfterEach? Or did you call TestShell.start in an after hook?' + ); + let shellProcess: ChildProcessWithoutNullStreams; let env = options.env || process.env; @@ -105,14 +156,13 @@ export class TestShell { return shell.output; } - static async killall(): Promise { - const exitPromises: Promise[] = []; - while (TestShell._openShells.length) { - const shell = TestShell._openShells.pop()!; - shell.kill(); - exitPromises.push(shell.waitForExit()); - } - await Promise.all(exitPromises); + static async killAll(): Promise { + await Promise.all( + TestShell._openShells.map((shell) => { + shell.kill(); + return shell.waitForExit(); + }) + ); } debugInformation() { @@ -125,24 +175,10 @@ export class TestShell { }; } - static async cleanupAfterAll(): Promise { - let foundOpenShells = false; + static printShells() { for (const shell of TestShell._openShells) { - foundOpenShells = true; console.error(shell.debugInformation()); } - await TestShell.killall(); - if (foundOpenShells) - throw new Error('Open shells at end of test discovered!'); - } - - static async cleanup(this: Mocha.Context): Promise { - if (this.currentTest?.state === 'failed') { - for (const shell of TestShell._openShells) { - console.error(shell.debugInformation()); - } - } - await TestShell.killall(); } private _process: ChildProcessWithoutNullStreams; @@ -336,6 +372,3 @@ export class TestShell { return match.groups!.logId; } } - -// If any shells remain at the end of the script, print their full output -globalThis?.after('ensure no hanging shells', TestShell.cleanupAfterAll); From f87070f2707ff5a9ffa77211fea2ed9cda9e5fc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 18 Sep 2024 10:24:18 +0200 Subject: [PATCH 02/14] Avoid asserting test shell enabled in after as some "before" hook might have failed --- packages/e2e-tests/test/test-shell.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/e2e-tests/test/test-shell.ts b/packages/e2e-tests/test/test-shell.ts index ad1b1e8dce..298aea2a8d 100644 --- a/packages/e2e-tests/test/test-shell.ts +++ b/packages/e2e-tests/test/test-shell.ts @@ -52,7 +52,6 @@ export function cleanTestShellsAfter() { }); after('kill all TestShell instances', async function (this: Mocha.Context) { - assert(testShellEnabled, 'Expected TestShell to be enabled'); testShellEnabled = false; await TestShell.killAll(); }); From d2541a34c3cbe6628a913ba0b1dd3862ccfd95b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 18 Sep 2024 12:17:34 +0200 Subject: [PATCH 03/14] Remove open shells as they get killed --- packages/e2e-tests/test/test-shell.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test/test-shell.ts b/packages/e2e-tests/test/test-shell.ts index 298aea2a8d..202bd09a7b 100644 --- a/packages/e2e-tests/test/test-shell.ts +++ b/packages/e2e-tests/test/test-shell.ts @@ -156,8 +156,10 @@ export class TestShell { } static async killAll(): Promise { + // Using splice to mutate the array of open shells in-place + const openShells = TestShell._openShells.splice(0); await Promise.all( - TestShell._openShells.map((shell) => { + openShells.map((shell) => { shell.kill(); return shell.waitForExit(); }) From 186b2ec001eb59d6693ed5ec9a8218415265f275 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Wed, 25 Sep 2024 20:54:45 +0200 Subject: [PATCH 04/14] Kill shells before removing homedir --- packages/e2e-tests/test/e2e.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts index 751aeba0bc..96f534d54d 100644 --- a/packages/e2e-tests/test/e2e.spec.ts +++ b/packages/e2e-tests/test/e2e.spec.ts @@ -1378,6 +1378,7 @@ describe('e2e', function () { }); afterEach(async function () { + await TestShell.killAll(); try { await fs.rm(homedir, { recursive: true, force: true }); } catch (err: any) { From 8365e4d39d514e02bf37d8cf29c17ee187a57cd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 08:53:51 +0200 Subject: [PATCH 05/14] Add a startTestShell onto the Mocha.Context --- packages/e2e-tests/test/test-shell.spec.ts | 34 ++++ packages/e2e-tests/test/test-shell.ts | 179 ++++++++++++++------- 2 files changed, 151 insertions(+), 62 deletions(-) create mode 100644 packages/e2e-tests/test/test-shell.spec.ts diff --git a/packages/e2e-tests/test/test-shell.spec.ts b/packages/e2e-tests/test/test-shell.spec.ts new file mode 100644 index 0000000000..919dfbe649 --- /dev/null +++ b/packages/e2e-tests/test/test-shell.spec.ts @@ -0,0 +1,34 @@ +import { TestShell } from './test-shell'; + +describe('TestShell', function () { + context('sub-suite', function () { + before(async function () { + const shell = this.startTestShell({ args: ['--nodb'] }); + await shell.waitForPrompt(); + }); + + beforeEach(async function () { + const shell = this.startTestShell({ args: ['--nodb'] }); + await shell.waitForPrompt(); + }); + + after(async function () { + const shell = this.startTestShell({ args: ['--nodb'] }); + await shell.waitForPrompt(); + }); + + afterEach(async function () { + const shell = this.startTestShell({ args: ['--nodb'] }); + await shell.waitForPrompt(); + }); + + it("doesn't explode", async function () { + const shell = this.startTestShell({ args: ['--nodb'] }); + await shell.waitForPrompt(); + }); + }); + + after(function () { + TestShell.assertNoOpenShells(); + }); +}); diff --git a/packages/e2e-tests/test/test-shell.ts b/packages/e2e-tests/test/test-shell.ts index 202bd09a7b..07e0b2cc8b 100644 --- a/packages/e2e-tests/test/test-shell.ts +++ b/packages/e2e-tests/test/test-shell.ts @@ -1,4 +1,4 @@ -import type Mocha from 'mocha'; +import Mocha from 'mocha'; import assert from 'assert'; import type { ChildProcess, @@ -33,49 +33,6 @@ function matches(str: string, pattern: string | RegExp): boolean { : pattern.test(str); } -/** - * Toggle used to ensure an appropriate hook is registered, to clean up test shells. - * NOTE: This is a local variable of the module instead of a static on {@link TestShell} to allow the hooks to toggle it. - */ -let testShellEnabled = false; - -/** - * Enables the TestShell and kill all shells after all tests of the current suite. - */ -export function cleanTestShellsAfter() { - before('enabled TestShell', function () { - assert( - !testShellEnabled, - 'TestShell is already enabled, use only one cleanTestShellsAfter or cleanTestShellsAfterEach' - ); - testShellEnabled = true; - }); - - after('kill all TestShell instances', async function (this: Mocha.Context) { - testShellEnabled = false; - await TestShell.killAll(); - }); -} - -/** - * Enables the TestShell and kill all shells after each test - * NOTE: This also registers {@link cleanTestShellsAfter} hook internally, to allow `after` hooks to start TestShell instances. - */ -export function cleanTestShellsAfterEach() { - cleanTestShellsAfter(); - - afterEach( - 'kill all TestShell instances', - async function (this: Mocha.Context) { - assert(testShellEnabled, 'Expected TestShell to be enabled'); - if (this.currentTest?.state === 'failed') { - TestShell.printShells(); - } - await TestShell.killAll(); - } - ); -} - export interface TestShellOptions { args: string[]; env?: Record; @@ -89,14 +46,12 @@ export interface TestShellOptions { * Test shell helper class. */ export class TestShell { - private static _openShells: TestShell[] = []; + private static _openShells: Set = new Set(); + /** + * @deprecated Use the {@link Mocha.Context.startTestShell} hook instead + */ static start(options: TestShellOptions = { args: [] }): TestShell { - assert( - testShellEnabled, - 'Expected TestShell to be enabled, did you call cleanTestShellsAfter or cleanTestShellsAfterEach? Or did you call TestShell.start in an after hook?' - ); - let shellProcess: ChildProcessWithoutNullStreams; let env = options.env || process.env; @@ -141,7 +96,10 @@ export class TestShell { } const shell = new TestShell(shellProcess, options.consumeStdio); - TestShell._openShells.push(shell); + TestShell._openShells.add(shell); + void shell.waitForExit().then(() => { + TestShell._openShells.delete(shell); + }); return shell; } @@ -155,17 +113,6 @@ export class TestShell { return shell.output; } - static async killAll(): Promise { - // Using splice to mutate the array of open shells in-place - const openShells = TestShell._openShells.splice(0); - await Promise.all( - openShells.map((shell) => { - shell.kill(); - return shell.waitForExit(); - }) - ); - } - debugInformation() { return { pid: this.process.pid, @@ -182,6 +129,21 @@ export class TestShell { } } + static assertNoOpenShells() { + const debugInformation = [...TestShell._openShells].map((shell) => + shell.debugInformation() + ); + assert.strictEqual( + TestShell._openShells.size, + 0, + `Expected no open shells, found: ${JSON.stringify( + debugInformation, + null, + 2 + )}` + ); + } + private _process: ChildProcessWithoutNullStreams; private _output: string; @@ -373,3 +335,96 @@ export class TestShell { return match.groups!.logId; } } + +// Context extension to manage TestShell lifetime + +declare module 'mocha' { + interface Context { + /** + * Starts a test shell and registers a hook to kill it after the test + */ + startTestShell(options?: TestShellOptions): TestShell; + } +} + +const TEST_SHELLS_AFTER_ALL = Symbol('test-shells-after-all'); +const TEST_SHELLS_AFTER_EACH = Symbol('test-shells-after-each'); + +type AfterAllInjectedSuite = { + [TEST_SHELLS_AFTER_ALL]: Set; +}; + +type AfterEachInjectedSuite = { + [TEST_SHELLS_AFTER_EACH]: Set; +}; + +/** + * Registers an after (all or each) hook to kill test shells started during the hooks or tests + */ +function ensureAfterHook( + hookName: 'afterEach', + suite: Mocha.Suite +): asserts suite is AfterEachInjectedSuite & Mocha.Suite; +function ensureAfterHook( + hookName: 'afterAll', + suite: Mocha.Suite +): asserts suite is AfterAllInjectedSuite & Mocha.Suite; +function ensureAfterHook( + hookName: 'afterEach' | 'afterAll', + suite: Partial & Mocha.Suite +): void { + const symbol = + hookName === 'afterAll' ? TEST_SHELLS_AFTER_ALL : TEST_SHELLS_AFTER_EACH; + if (!suite[symbol]) { + // Store the set of shells to kill afterwards + const shells = new Set(); + suite[symbol] = shells; + suite[hookName](async () => { + const shellsToKill = [...shells]; + shells.clear(); + await Promise.all( + shellsToKill.map((shell) => { + // TODO: Consider if it's okay to kill those that are already killed? + shell.kill(); + return shell.waitForExit(); + }) + ); + }); + } +} + +Mocha.Context.prototype.startTestShell = function ( + this: Mocha.Context, + options: TestShellOptions +) { + const { test: runnable } = this; + assert(runnable, 'Expected a runnable / test'); + const { parent } = runnable; + assert(parent, 'Expected runnable to have a parent'); + // Start the shell + const shell = TestShell.start(options); + // Register a hook to kill the shell + if (runnable instanceof Mocha.Hook) { + if ( + runnable.originalTitle === '"before each" hook' || + runnable.originalTitle === '"after each" hook' + ) { + ensureAfterHook('afterEach', parent); + parent[TEST_SHELLS_AFTER_EACH].add(shell); + } else if ( + runnable.originalTitle === '"before all" hook' || + runnable.originalTitle === '"after all" hook' + ) { + ensureAfterHook('afterAll', parent); + parent[TEST_SHELLS_AFTER_ALL].add(shell); + } else { + throw new Error(`Unexpected ${runnable.originalTitle || runnable.title}`); + } + } else if (runnable instanceof Mocha.Test) { + ensureAfterHook('afterEach', parent); + parent[TEST_SHELLS_AFTER_EACH].add(shell); + } else { + throw new Error('Unexpected Runnable: Expected a Hook or a Test'); + } + return shell; +}; From 6bb4a8e4c21a9af681318373f9ce2402dd35bec3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 09:00:00 +0200 Subject: [PATCH 06/14] Require test-shell.ts before tests --- packages/e2e-tests/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/package.json b/packages/e2e-tests/package.json index fb446e7ede..c35fbbb7e5 100644 --- a/packages/e2e-tests/package.json +++ b/packages/e2e-tests/package.json @@ -11,7 +11,7 @@ "url": "git://github.com/mongodb-js/mongosh.git" }, "scripts": { - "test": "mocha -r \"../../scripts/import-expansions.js\" --timeout 15000 --colors -r ts-node/register \"./test/e2e*.spec.ts\"", + "test": "mocha -r ts-node/register -r \"../../scripts/import-expansions.js\" -r \"./test/test-shell.ts\" --timeout 15000 --colors \"./test/*.spec.ts\"", "test-ci": "node ../../scripts/run-if-package-requested.js npm test", "test-coverage": "nyc --no-clean --cwd ../.. --reporter=none npm run test", "test-ci-coverage": "nyc --no-clean --cwd ../.. --reporter=none npm run test-ci", From ce2f446f9a2b9caf8df6a0db3695d7dcc40494a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 09:13:27 +0200 Subject: [PATCH 07/14] Update tests to start shells from context --- packages/e2e-tests/test/e2e-analytics.spec.ts | 11 +- packages/e2e-tests/test/e2e-auth.spec.ts | 28 +++-- packages/e2e-tests/test/e2e-aws.spec.ts | 19 ++-- packages/e2e-tests/test/e2e-banners.spec.ts | 9 +- packages/e2e-tests/test/e2e-bson.spec.ts | 6 +- packages/e2e-tests/test/e2e-direct.spec.ts | 56 +++++----- packages/e2e-tests/test/e2e-editor.spec.ts | 6 +- packages/e2e-tests/test/e2e-fle.spec.ts | 42 ++++---- packages/e2e-tests/test/e2e-oidc.spec.ts | 33 +++--- packages/e2e-tests/test/e2e-proxy.spec.ts | 31 +++--- packages/e2e-tests/test/e2e-snapshot.spec.ts | 4 +- packages/e2e-tests/test/e2e-snippet.spec.ts | 6 +- packages/e2e-tests/test/e2e-tls.spec.ts | 53 +++++---- packages/e2e-tests/test/e2e.spec.ts | 102 +++++++++--------- 14 files changed, 190 insertions(+), 216 deletions(-) diff --git a/packages/e2e-tests/test/e2e-analytics.spec.ts b/packages/e2e-tests/test/e2e-analytics.spec.ts index 500da6a00c..e32bf15117 100644 --- a/packages/e2e-tests/test/e2e-analytics.spec.ts +++ b/packages/e2e-tests/test/e2e-analytics.spec.ts @@ -1,7 +1,6 @@ import { expect } from 'chai'; import { startTestCluster } from '../../../testing/integration-testing-hooks'; import { eventually } from '../../../testing/eventually'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; describe('e2e Analytics Node', function () { const replSetName = 'replicaSet'; @@ -13,8 +12,6 @@ describe('e2e Analytics Node', function () { { args: ['--replSet', replSetName] } ); - cleanTestShellsAfterEach(); - before(async function () { if (process.env.MONGOSH_TEST_FORCE_API_STRICT) { return this.skip(); @@ -36,7 +33,7 @@ describe('e2e Analytics Node', function () { ], }; - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs0.connectionString()], }); await shell.waitForPrompt(); @@ -55,7 +52,7 @@ describe('e2e Analytics Node', function () { context('without readPreference', function () { it('a direct connection ends up at primary', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs0.connectionString()], }); await shell.waitForPrompt(); @@ -68,13 +65,13 @@ describe('e2e Analytics Node', function () { context('specifying readPreference and tags', function () { it('ends up at the ANALYTICS node', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ `${await rs0.connectionString()}?replicaSet=${replSetName}&readPreference=secondary&readPreferenceTags=nodeType:ANALYTICS`, ], }); - const directConnectionToAnalyticsShell = TestShell.start({ + const directConnectionToAnalyticsShell = this.startTestShell({ args: [`${await rs3.connectionString()}?directConnection=true`], }); await Promise.all([ diff --git a/packages/e2e-tests/test/e2e-auth.spec.ts b/packages/e2e-tests/test/e2e-auth.spec.ts index ecbcd8be84..70c7bc83b2 100644 --- a/packages/e2e-tests/test/e2e-auth.spec.ts +++ b/packages/e2e-tests/test/e2e-auth.spec.ts @@ -2,7 +2,7 @@ import { expect } from 'chai'; import type { Db, Document, MongoClientOptions } from 'mongodb'; import { MongoClient } from 'mongodb'; import { eventually } from '../../../testing/eventually'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; +import type { TestShell } from './test-shell'; import { skipIfApiStrict, startSharedTestServer, @@ -107,12 +107,10 @@ describe('Auth e2e', function () { let examplePrivilege2: Document; describe('with regular URI', function () { - cleanTestShellsAfterEach(); - beforeEach(async function () { const connectionString = await testServer.connectionString(); dbName = `test-${Date.now()}`; - shell = TestShell.start({ args: [connectionString] }); + shell = this.startTestShell({ args: [connectionString] }); client = await MongoClient.connect(connectionString, {}); @@ -880,7 +878,7 @@ describe('Auth e2e', function () { pathname: `/${dbName}`, } ); - shell = TestShell.start({ args: [authConnectionString] }); + shell = this.startTestShell({ args: [authConnectionString] }); await shell.waitForPrompt(); shell.assertNoErrors(); await shell.executeLine(`use ${dbName}`); @@ -904,7 +902,7 @@ describe('Auth e2e', function () { pathname: `/${dbName}`, } ); - shell = TestShell.start({ args: [authConnectionString] }); + shell = this.startTestShell({ args: [authConnectionString] }); await shell.waitForPrompt(); shell.assertNoErrors(); await shell.executeLine(`use ${dbName}`); @@ -931,7 +929,7 @@ describe('Auth e2e', function () { }); it('can auth when there is -u and -p', async function () { const connectionString = await testServer.connectionString(); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ connectionString, '-u', @@ -966,7 +964,7 @@ describe('Auth e2e', function () { return this.skip(); // No SCRAM-SHA-1 in FIPS mode } const connectionString = await testServer.connectionString(); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ connectionString, '-u', @@ -990,7 +988,7 @@ describe('Auth e2e', function () { // This test is not particularly meaningful if we're using the system OpenSSL installation // and it is not properly configured for FIPS to begin with. This is the case on e.g. // Ubuntu 22.04 in evergreen CI. - const preTestShell = TestShell.start({ + const preTestShell = this.startTestShell({ args: [ '--quiet', '--nodb', @@ -1010,7 +1008,7 @@ describe('Auth e2e', function () { } const connectionString = await testServer.connectionString(); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ connectionString, '--tlsFIPSMode', @@ -1035,7 +1033,7 @@ describe('Auth e2e', function () { }); it('can auth with SCRAM-SHA-256', async function () { const connectionString = await testServer.connectionString(); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ connectionString, '-u', @@ -1056,7 +1054,7 @@ describe('Auth e2e', function () { }); it('cannot auth when authenticationMechanism mismatches (sha256 -> sha1)', async function () { const connectionString = await testServer.connectionString(); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ connectionString, '-u', @@ -1077,7 +1075,7 @@ describe('Auth e2e', function () { }); it('cannot auth when authenticationMechanism mismatches (sha1 -> sha256)', async function () { const connectionString = await testServer.connectionString(); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ connectionString, '-u', @@ -1098,7 +1096,7 @@ describe('Auth e2e', function () { }); it('does not fail with kerberos not found for GSSAPI', async function () { const connectionString = await testServer.connectionString(); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ connectionString, '-u', @@ -1145,7 +1143,5 @@ describe('Auth e2e', function () { await client.close(); }); - - cleanTestShellsAfterEach(); }); }); diff --git a/packages/e2e-tests/test/e2e-aws.spec.ts b/packages/e2e-tests/test/e2e-aws.spec.ts index bf2c099053..421a723fb9 100644 --- a/packages/e2e-tests/test/e2e-aws.spec.ts +++ b/packages/e2e-tests/test/e2e-aws.spec.ts @@ -1,6 +1,5 @@ import { expect } from 'chai'; import { spawnSync } from 'child_process'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; function assertEnvVariable(variableName: string): string { if (process.env.MONGOSH_TEST_FORCE_API_STRICT) { @@ -84,8 +83,6 @@ function getConnectionString(username?: string, password?: string): string { } describe('e2e AWS AUTH', function () { - cleanTestShellsAfterEach(); - this.timeout(60_000); // AWS auth tests can take longer than the default timeout in CI let expectedAssumedRole: string; @@ -122,7 +119,7 @@ describe('e2e AWS AUTH', function () { context('without environment variables being present', function () { context('specifying explicit parameters', function () { it('connects with access key and secret', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ getConnectionString(), '--username', @@ -142,7 +139,7 @@ describe('e2e AWS AUTH', function () { it('connects with access key, secret, and session token for IAM role', async function () { const tokenDetails = generateIamSessionToken(); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ getConnectionString(), '--username', @@ -165,7 +162,7 @@ describe('e2e AWS AUTH', function () { context('specifying connection string parameters', function () { it('connects with access key and secret', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [getConnectionString(AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY)], }); const result = await shell.waitForPromptOrExit(); @@ -179,7 +176,7 @@ describe('e2e AWS AUTH', function () { it('connects with access key, secret, and session token for IAM role', async function () { const tokenDetails = generateIamSessionToken(); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ `${getConnectionString( tokenDetails.key, @@ -203,7 +200,7 @@ describe('e2e AWS AUTH', function () { context('with AWS environment variables', function () { context('without any other parameters', function () { it('connects for the IAM user', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [getConnectionString()], env: { ...process.env, @@ -222,7 +219,7 @@ describe('e2e AWS AUTH', function () { it('connects for the IAM role session', async function () { const tokenDetails = generateIamSessionToken(); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [getConnectionString()], env: { ...process.env, @@ -243,7 +240,7 @@ describe('e2e AWS AUTH', function () { context('with invalid environment but valid parameters', function () { it('connects for the IAM user', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ getConnectionString(), '--username', @@ -268,7 +265,7 @@ describe('e2e AWS AUTH', function () { it('connects for the IAM role session', async function () { const tokenDetails = generateIamSessionToken(); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ getConnectionString(), '--username', diff --git a/packages/e2e-tests/test/e2e-banners.spec.ts b/packages/e2e-tests/test/e2e-banners.spec.ts index 6ffed51134..14929f1bff 100644 --- a/packages/e2e-tests/test/e2e-banners.spec.ts +++ b/packages/e2e-tests/test/e2e-banners.spec.ts @@ -2,17 +2,16 @@ import { skipIfApiStrict, startSharedTestServer, } from '../../../testing/integration-testing-hooks'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; +import type { TestShell } from './test-shell'; describe('e2e startup banners', function () { skipIfApiStrict(); - cleanTestShellsAfterEach(); const testServer = startSharedTestServer(); context('without special configuration', function () { it('shows startup warnings', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString()], }); await shell.waitForPrompt(); @@ -30,7 +29,7 @@ describe('e2e startup banners', function () { let helperShell: TestShell; beforeEach(async function () { - helperShell = TestShell.start({ + helperShell = this.startTestShell({ args: [await testServer.connectionString()], }); await helperShell.waitForPrompt(); @@ -47,7 +46,7 @@ describe('e2e startup banners', function () { }); it('shows automation notices', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString()], }); await shell.waitForPrompt(); diff --git a/packages/e2e-tests/test/e2e-bson.spec.ts b/packages/e2e-tests/test/e2e-bson.spec.ts index 40d8108acd..a31e0b81a8 100644 --- a/packages/e2e-tests/test/e2e-bson.spec.ts +++ b/packages/e2e-tests/test/e2e-bson.spec.ts @@ -2,12 +2,10 @@ import { expect } from 'chai'; import type { Db } from 'mongodb'; import { MongoClient } from 'mongodb'; import { bson } from '@mongosh/service-provider-core'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; +import type { TestShell } from './test-shell'; import { startSharedTestServer } from '../../../testing/integration-testing-hooks'; describe('BSON e2e', function () { - cleanTestShellsAfterEach(); - const testServer = startSharedTestServer(); let db: Db; let client: MongoClient; @@ -17,7 +15,7 @@ describe('BSON e2e', function () { beforeEach(async function () { const connectionString = await testServer.connectionString(); dbName = `test-${Date.now()}`; - shell = TestShell.start({ args: [connectionString] }); + shell = this.startTestShell({ args: [connectionString] }); client = await MongoClient.connect(connectionString, {}); diff --git a/packages/e2e-tests/test/e2e-direct.spec.ts b/packages/e2e-tests/test/e2e-direct.spec.ts index 8b5d461aab..e4f5e0aec4 100644 --- a/packages/e2e-tests/test/e2e-direct.spec.ts +++ b/packages/e2e-tests/test/e2e-direct.spec.ts @@ -5,12 +5,10 @@ import { } from '../../../testing/integration-testing-hooks'; import { eventually } from '../../../testing/eventually'; import { expect } from 'chai'; -import { TestShell, cleanTestShellsAfterEach } from './test-shell'; +import type { TestShell } from './test-shell'; describe('e2e direct connection', function () { skipIfApiStrict(); - cleanTestShellsAfterEach(); - const tabtab = async (shell: TestShell) => { await new Promise((resolve) => setTimeout(resolve, 400)); shell.writeInput('\u0009'); @@ -33,7 +31,7 @@ describe('e2e direct connection', function () { { server: rs2, name: 'rs2' }, ].forEach(({ server, name }) => { it(`works when connecting to node ${name} of uninitialized set`, async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await server.connectionString()], }); await shell.waitForPrompt(); @@ -58,7 +56,9 @@ describe('e2e direct connection', function () { ], }; - const shell = TestShell.start({ args: [await rs0.connectionString()] }); + const shell = this.startTestShell({ + args: [await rs0.connectionString()], + }); await shell.waitForPrompt(); await shell.executeLine( `rs.initiate(${JSON.stringify(replSetConfig)})` @@ -85,7 +85,7 @@ describe('e2e direct connection', function () { context('connecting to secondary members directly', function () { it('works when specifying a connection string', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString()], }); await shell.waitForPrompt(); @@ -96,7 +96,7 @@ describe('e2e direct connection', function () { }); it('works when specifying just host and port', async function () { - const shell = TestShell.start({ args: [await rs1.hostport()] }); + const shell = this.startTestShell({ args: [await rs1.hostport()] }); await shell.waitForPrompt(); await shell.executeLine('db.isMaster()'); shell.assertContainsOutput('ismaster: false'); @@ -105,7 +105,7 @@ describe('e2e direct connection', function () { }); it('lists collections without explicit readPreference', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [`${await rs1.connectionString()}`], }); await shell.waitForPrompt(); @@ -115,7 +115,7 @@ describe('e2e direct connection', function () { }); it('lists collections when an incompatible readPreference is provided', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [`${await rs1.connectionString()}`], }); await shell.waitForPrompt(); @@ -127,7 +127,7 @@ describe('e2e direct connection', function () { }); it('lists collections when readPreference is set via Mongo', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString()], }); await shell.waitForPrompt(); @@ -140,7 +140,7 @@ describe('e2e direct connection', function () { }); it('slists databases without explicit readPreference', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString()], }); await shell.waitForPrompt(); @@ -150,7 +150,7 @@ describe('e2e direct connection', function () { }); it('lists databases when an incompatible readPreference is provided', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString()], }); await shell.waitForPrompt(); @@ -162,7 +162,7 @@ describe('e2e direct connection', function () { }); it('lists databases when readPreference is set via Mongo', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString()], }); await shell.waitForPrompt(); @@ -175,7 +175,7 @@ describe('e2e direct connection', function () { }); it('lists collections and dbs using show by default', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString()], }); await shell.waitForPrompt(); @@ -190,7 +190,7 @@ describe('e2e direct connection', function () { if (process.arch === 's390x') { return this.skip(); // https://jira.mongodb.org/browse/MONGOSH-746 } - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString({}, { pathname: `/${dbname}` })], forceTerminal: true, }); @@ -206,7 +206,7 @@ describe('e2e direct connection', function () { skipIfServerVersion(rs0, '< 4.2'); it('allows aggregate with $merge with secondary readpref', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await rs1.connectionString( { @@ -238,7 +238,7 @@ describe('e2e direct connection', function () { context('connecting to primary', function () { it('when specifying replicaSet', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString({ replicaSet: replSetId })], }); await shell.waitForPrompt(); @@ -248,7 +248,7 @@ describe('e2e direct connection', function () { shell.assertContainsOutput(`setName: '${replSetId}'`); }); it('when setting directConnection to false', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString({ directConnection: 'false' })], }); await shell.waitForPrompt(); @@ -265,7 +265,7 @@ describe('e2e direct connection', function () { (await rs1.hostport()) + ',' + (await rs0.hostport()); - const shell = TestShell.start({ args: [connectionString] }); + const shell = this.startTestShell({ args: [connectionString] }); await shell.waitForPrompt(); await shell.executeLine('db.isMaster()'); shell.assertContainsOutput('ismaster: true'); @@ -279,7 +279,7 @@ describe('e2e direct connection', function () { (await rs1.hostport()) + ',' + (await rs0.hostport()); - const shell = TestShell.start({ args: ['--host', hostlist] }); + const shell = this.startTestShell({ args: ['--host', hostlist] }); await shell.waitForPrompt(); await shell.executeLine('db.isMaster()'); await shell.executeLine('({ dbname: db.getName() })'); @@ -297,7 +297,7 @@ describe('e2e direct connection', function () { (await rs1.hostport()) + ',' + (await rs0.hostport()); - const shell = TestShell.start({ args: ['--host', hostlist] }); + const shell = this.startTestShell({ args: ['--host', hostlist] }); await shell.waitForPrompt(); await shell.executeLine('db.isMaster()'); await shell.executeLine('({ dbname: db.getName() })'); @@ -315,7 +315,7 @@ describe('e2e direct connection', function () { (await rs1.hostport()) + ',' + (await rs0.hostport()); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--host', hostlist, 'admin'], }); await shell.waitForPrompt(); @@ -334,7 +334,7 @@ describe('e2e direct connection', function () { (await rs1.hostport()) + ',' + (await rs0.hostport()); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--host', hostlist, 'admin'], }); await shell.waitForExit(); @@ -342,7 +342,7 @@ describe('e2e direct connection', function () { }); it('lists collections and dbs using show by default', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [`${await rs1.connectionString()}`], }); await shell.waitForPrompt(); @@ -356,7 +356,7 @@ describe('e2e direct connection', function () { if (process.arch === 's390x') { return this.skip(); // https://jira.mongodb.org/browse/MONGOSH-746 } - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs1.connectionString({}, { pathname: `/${dbname}` })], forceTerminal: true, }); @@ -376,7 +376,7 @@ describe('e2e direct connection', function () { (await rs1.hostport()) + ',' + (await rs0.hostport()); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ '--host', hostlist, @@ -391,7 +391,7 @@ describe('e2e direct connection', function () { shell.assertContainsOutput("user: 'anna'"); }); it('drops indexes even if a read preference is specified in the connection url', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await rs0.connectionString({ readPreference: 'secondary' })], }); @@ -405,7 +405,7 @@ describe('e2e direct connection', function () { describe('fail-fast connections', function () { it('does not fail fast for ECONNREFUSED errors when one host is reachable', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ `mongodb://${await rs1.hostport()},127.0.0.1:1/?replicaSet=${replSetId}&readPreference=secondary`, ], diff --git a/packages/e2e-tests/test/e2e-editor.spec.ts b/packages/e2e-tests/test/e2e-editor.spec.ts index 8d5c63f786..a7cbdeefd7 100644 --- a/packages/e2e-tests/test/e2e-editor.spec.ts +++ b/packages/e2e-tests/test/e2e-editor.spec.ts @@ -2,7 +2,7 @@ import { expect } from 'chai'; import path from 'path'; import { promises as fs } from 'fs'; import { eventually } from '../../../testing/eventually'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; +import type { TestShell } from './test-shell'; import { useTmpdir, fakeExternalEditor, @@ -15,15 +15,13 @@ describe('external editor e2e', function () { let env: Record; let shell: TestShell; - cleanTestShellsAfterEach(); - beforeEach(async function () { const homeInfo = setTemporaryHomeDirectory(); homedir = homeInfo.homedir; env = homeInfo.env; - shell = TestShell.start({ + shell = this.startTestShell({ args: ['--nodb'], forceTerminal: true, env, diff --git a/packages/e2e-tests/test/e2e-fle.spec.ts b/packages/e2e-tests/test/e2e-fle.spec.ts index 60711aa48d..e022a587d2 100644 --- a/packages/e2e-tests/test/e2e-fle.spec.ts +++ b/packages/e2e-tests/test/e2e-fle.spec.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; import { MongoClient } from 'mongodb'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; +import type { TestShell } from './test-shell'; import { eventually } from '../../../testing/eventually'; import { startTestServer, @@ -16,8 +16,6 @@ import { inspect } from 'util'; import path from 'path'; describe('FLE tests', function () { - cleanTestShellsAfterEach(); - const testServer = startTestServer('e2e-fle', { topology: 'replset', secondaries: 0, @@ -93,8 +91,8 @@ describe('FLE tests', function () { const accessKeyId = 'SxHpYMUtB1CEVg9tX0N1'; const secretAccessKey = '44mjXTk34uMUmORma3w1viIAx4RCUv78bzwDY0R7'; const sessionToken = 'WXWHMnniSqij0CH27KK7H'; - async function makeTestShell(): Promise { - const shell = TestShell.start({ + async function makeTestShell(ctx: Mocha.Context): Promise { + const shell = ctx.startTestShell({ args: [ `--cryptSharedLibPath=${cryptLibrary}`, ...(withEnvVarCredentials @@ -148,7 +146,7 @@ describe('FLE tests', function () { } it('passes through command line options', async function () { - const shell = await makeTestShell(); + const shell = await makeTestShell(this); await shell.executeLine(`db.keyVault.insertOne({ _id: UUID("e7b4abe7-ff70-48c3-9d3a-3526e18c2646"), keyMaterial: new Binary(Buffer.from("010202007888b7b9089f9cf816059c4c02edf139d50227528b2a74a5c9910c89095d45a9d10133bd4c047f2ba610d7ad4efcc945f863000000c23081bf06092a864886f70d010706a081b13081ae0201003081a806092a864886f70d010701301e060960864801650304012e3011040cf406b9ccb00f83dd632e76e9020110807b9c2b3a676746e10486ec64468d45ec89cac30f59812b711fc24530188166c481f4f4ab376c258f8f54affdc8523468fdd07b84e77b21a14008a23fb6d111c05eb4287b7b973f3a60d5c7d87074119b424477366cbe72c31da8fc76b8f72e31f609c3b423c599d3e4a59c21e4a0fe227ebe1aa53038cb94f79c457b", "hex"), 0), @@ -196,7 +194,7 @@ describe('FLE tests', function () { }); it('forwards command line options to the main Mongo instance', async function () { - const shell = await makeTestShell(); + const shell = await makeTestShell(this); await shell.executeLine( 'keyId = db.getMongo().getKeyVault().createKey("aws", {' + 'region: "us-east-2", key: "arn:aws:kms:us-east-2:398471984214:key/174b7c1d-3651-4517-7521-21988befd8cb" });' @@ -218,7 +216,7 @@ describe('FLE tests', function () { } it('works when the original shell was started with --nodb', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb'], }); await shell.waitForPrompt(); @@ -249,7 +247,7 @@ describe('FLE tests', function () { }); it('works when a schemaMap option has been passed', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', `--cryptSharedLibPath=${cryptLibrary}`], }); await shell.waitForPrompt(); @@ -295,7 +293,7 @@ describe('FLE tests', function () { }); it('skips automatic encryption when a bypassQueryAnalysis option has been passed', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', `--cryptSharedLibPath=${cryptLibrary}`], }); const uri = JSON.stringify(await testServer.connectionString()); @@ -374,7 +372,7 @@ describe('FLE tests', function () { }); it('does not allow compactStructuredEncryptionData command when mongo instance configured without auto encryption', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString()], }); await shell.waitForPrompt(); @@ -393,7 +391,7 @@ describe('FLE tests', function () { it('can read existing QEv1 data', async function () { const uri = await testServer.connectionString(); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [uri, `--cryptSharedLibPath=${cryptLibrary}`], }); await shell.waitForPrompt(); @@ -504,7 +502,7 @@ describe('FLE tests', function () { it('allows explicit enryption with bypassQueryAnalysis', async function () { // No --cryptSharedLibPath since bypassQueryAnalysis is also a community edition feature - const shell = TestShell.start({ args: ['--nodb'] }); + const shell = this.startTestShell({ args: ['--nodb'] }); const uri = JSON.stringify(await testServer.connectionString()); await shell.waitForPrompt(); @@ -566,7 +564,7 @@ describe('FLE tests', function () { }); it('drops fle2 collection with all helper collections when encryptedFields options are in listCollections', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', `--cryptSharedLibPath=${cryptLibrary}`], }); const uri = JSON.stringify(await testServer.connectionString()); @@ -641,7 +639,7 @@ describe('FLE tests', function () { }); it('creates an encrypted collection and generates data encryption keys automatically per encrypted fields', async function () { - const shell = TestShell.start({ args: ['--nodb'] }); + const shell = this.startTestShell({ args: ['--nodb'] }); const uri = JSON.stringify(await testServer.connectionString()); await shell.waitForPrompt(); await shell.executeLine( @@ -695,7 +693,7 @@ describe('FLE tests', function () { }`; it('allows compactStructuredEncryptionData command when mongo instance configured with auto encryption', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', `--cryptSharedLibPath=${cryptLibrary}`], }); const uri = JSON.stringify(await testServer.connectionString()); @@ -740,7 +738,7 @@ describe('FLE tests', function () { }); it('creates an encrypted collection and generates data encryption keys automatically per encrypted fields', async function () { - const shell = TestShell.start({ args: ['--nodb'] }); + const shell = this.startTestShell({ args: ['--nodb'] }); const uri = JSON.stringify(await testServer.connectionString()); await shell.waitForPrompt(); await shell.executeLine( @@ -782,7 +780,7 @@ describe('FLE tests', function () { it('allows explicit range encryption with bypassQueryAnalysis', async function () { // No --cryptSharedLibPath since bypassQueryAnalysis is also a community edition feature - const shell = TestShell.start({ args: ['--nodb'] }); + const shell = this.startTestShell({ args: ['--nodb'] }); const uri = JSON.stringify(await testServer.connectionString()); await shell.waitForPrompt(); @@ -864,7 +862,7 @@ describe('FLE tests', function () { return this.skip(); } - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', `--cryptSharedLibPath=${cryptLibrary}`], }); const uri = JSON.stringify(await testServer.connectionString()); @@ -920,7 +918,7 @@ describe('FLE tests', function () { skipIfServerVersion(testServer, '>= 6.0'); // FLE2 available on 6.0+ it('provides a good error message when createCollection fails due to low server version', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ `--cryptSharedLibPath=${cryptLibrary}`, await testServer.connectionString(), @@ -934,7 +932,7 @@ describe('FLE tests', function () { }); it('provides a good error message when createCollection fails due to low FCV', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ `--cryptSharedLibPath=${cryptLibrary}`, await testServer.connectionString(), @@ -951,7 +949,7 @@ describe('FLE tests', function () { }); it('performs KeyVault data key management as expected', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await testServer.connectionString(), `--cryptSharedLibPath=${cryptLibrary}`, diff --git a/packages/e2e-tests/test/e2e-oidc.spec.ts b/packages/e2e-tests/test/e2e-oidc.spec.ts index 70ffdcaae3..a8ddaa7f37 100644 --- a/packages/e2e-tests/test/e2e-oidc.spec.ts +++ b/packages/e2e-tests/test/e2e-oidc.spec.ts @@ -6,7 +6,7 @@ import { import { promises as fs } from 'fs'; import type { OIDCMockProviderConfig } from '@mongodb-js/oidc-mock-provider'; import { OIDCMockProvider } from '@mongodb-js/oidc-mock-provider'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; +import type { TestShell } from './test-shell'; import path from 'path'; import { expect } from 'chai'; import { createServer as createHTTPSServer } from 'https'; @@ -30,7 +30,6 @@ import { * happens in isolation. */ describe('OIDC auth e2e', function () { - cleanTestShellsAfterEach(); skipIfApiStrict(); // connectionStatus is unversioned. let getTokenPayload: typeof oidcMockProviderConfig.getTokenPayload; @@ -182,7 +181,7 @@ describe('OIDC auth e2e', function () { } it('can successfully authenticate using OIDC Auth Code Flow', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString(), '--authenticationMechanism=MONGODB-OIDC', @@ -197,7 +196,7 @@ describe('OIDC auth e2e', function () { }); it('can successfully authenticate using OIDC Auth Code Flow when a username is specified', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString(), '--username=testuser', @@ -213,7 +212,7 @@ describe('OIDC auth e2e', function () { }); it('can successfully authenticate using OIDC Device Auth Flow', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString(), '--authenticationMechanism=MONGODB-OIDC', @@ -231,7 +230,7 @@ describe('OIDC auth e2e', function () { }); it('hints the user to use Device Auth Flow if starting a browser fails', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString(), '--authenticationMechanism=MONGODB-OIDC', @@ -253,7 +252,7 @@ describe('OIDC auth e2e', function () { payload: (await originalGetPayload(metadata)).payload, }; }; - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString({ maxIdleTimeMS: '1', @@ -275,7 +274,7 @@ describe('OIDC auth e2e', function () { it('keeps authentication state when resetting connection options', async function () { const cs = await testServer.connectionString(); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ cs, '--authenticationMechanism=MONGODB-OIDC', @@ -299,7 +298,7 @@ describe('OIDC auth e2e', function () { it('re-authenticates when connecting to a different endpoint from the same shell', async function () { const urlOptions = { username: 'testuser' }; // Make sure these match between the two connections - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString({}, urlOptions), '--authenticationMechanism=MONGODB-OIDC', @@ -323,7 +322,7 @@ describe('OIDC auth e2e', function () { }); it('can share state with another shell', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString(), '--authenticationMechanism=MONGODB-OIDC', @@ -351,7 +350,7 @@ describe('OIDC auth e2e', function () { handle = handle.slice(0, -1); } - const shell2 = TestShell.start({ + const shell2 = this.startTestShell({ args: [ await testServer.connectionString(), '--authenticationMechanism=MONGODB-OIDC', @@ -378,7 +377,7 @@ describe('OIDC auth e2e', function () { path.join(tmpdir.path, 'certs', 'somefilename.crt') ); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer2.connectionString( {}, @@ -408,7 +407,7 @@ describe('OIDC auth e2e', function () { path.join(tmpdir.path, 'certs', 'somefilename.crt') ); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer2.connectionString( {}, @@ -450,7 +449,7 @@ describe('OIDC auth e2e', function () { }; // Consistency check: ID token is *not* used by default - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer3.connectionString(), '--authenticationMechanism=MONGODB-OIDC', @@ -463,7 +462,7 @@ describe('OIDC auth e2e', function () { await verifyUser(shell, 'testuser-at', 'testuser-at-group'); // Actual test: ID token data is used when --oidcIdTokenAsAccessToken is set - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer3.connectionString(), '--authenticationMechanism=MONGODB-OIDC', @@ -479,7 +478,7 @@ describe('OIDC auth e2e', function () { }); it('can print tokens as debug information if requested', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString(), '--authenticationMechanism=MONGODB-OIDC', @@ -499,7 +498,7 @@ describe('OIDC auth e2e', function () { shell.assertContainsOutput('"lastServerIdPInfo":'); shell.assertNotContainsOutput(/"refreshToken": "(?!debugid:)/); - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString(), '--authenticationMechanism=MONGODB-OIDC', diff --git a/packages/e2e-tests/test/e2e-proxy.spec.ts b/packages/e2e-tests/test/e2e-proxy.spec.ts index 98c94ad330..b53a942ea5 100644 --- a/packages/e2e-tests/test/e2e-proxy.spec.ts +++ b/packages/e2e-tests/test/e2e-proxy.spec.ts @@ -11,7 +11,6 @@ import { startSharedTestServer, startTestServer, } from '../../../testing/integration-testing-hooks'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; import type { Server as HTTPSServer } from 'https'; import { createServer as createHTTPSServer } from 'https'; import { @@ -40,8 +39,6 @@ describe('e2e proxy support', function () { skipIfApiStrict(); skipIfEnvServerVersion('< 7.0'); - cleanTestShellsAfterEach(); - const tmpdir = useTmpdir(); const testServer = startSharedTestServer(); @@ -132,7 +129,7 @@ describe('e2e proxy support', function () { }); it('can connect using an HTTP proxy', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString()], env: { ...process.env, @@ -152,7 +149,7 @@ describe('e2e proxy support', function () { }); it('can connect using an HTTP proxy with auth', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString()], env: { ...process.env, @@ -174,7 +171,7 @@ describe('e2e proxy support', function () { }); it('can connect using an HTTP proxy specified via ALL_PROXY', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString()], env: { ...process.env, @@ -196,7 +193,7 @@ describe('e2e proxy support', function () { }); it('can connect using an HTTPS proxy (explicit CA on command line)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString(), '--tlsCAFile', CA_CERT], env: { ...process.env, @@ -216,7 +213,7 @@ describe('e2e proxy support', function () { }); it('can connect using an HTTPS proxy (CA in connection string)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString({ tlsCAFile: CA_CERT })], env: { ...process.env, @@ -243,7 +240,7 @@ describe('e2e proxy support', function () { path.join(tmpdir.path, 'certs', 'somefilename.crt') ); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString()], env: { ...process.env, @@ -264,7 +261,7 @@ describe('e2e proxy support', function () { }); it('fails to connect using HTTPS proxy (no CA)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString({ connectTimeoutMS: '2000' })], env: { ...process.env, @@ -292,7 +289,7 @@ describe('e2e proxy support', function () { }); it('can connect using an HTTP proxy', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(tlsServer, { tlsCAFile: CA_CERT, @@ -317,7 +314,7 @@ describe('e2e proxy support', function () { }); it('can connect using an HTTPS proxy', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(tlsServer, { tlsCAFile: CA_CERT, @@ -343,7 +340,7 @@ describe('e2e proxy support', function () { }); it('will exclude a proxy host specified in NO_PROXY', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [await testServer.connectionString()], env: { ...process.env, @@ -457,7 +454,7 @@ describe('e2e proxy support', function () { }); it('can route all traffic through the proxy', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await oidcTestServer.connectionString({}, { username: 'testuser' }), '--authenticationMechanism=MONGODB-OIDC', @@ -501,7 +498,7 @@ describe('e2e proxy support', function () { }); it('can route only http traffic through the proxy', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await oidcTestServer.connectionString({}, { username: 'testuser' }), '--authenticationMechanism=MONGODB-OIDC', @@ -560,7 +557,7 @@ describe('e2e proxy support', function () { })().catch(console.error); } ); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await oidcTestServer.connectionString({}, { username: 'testuser' }), '--authenticationMechanism=MONGODB-OIDC', @@ -605,7 +602,7 @@ describe('e2e proxy support', function () { }); it('can route all traffic through the proxy (https, incomplete without CA)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await oidcTestServer.connectionString( {}, diff --git a/packages/e2e-tests/test/e2e-snapshot.spec.ts b/packages/e2e-tests/test/e2e-snapshot.spec.ts index 0454d88a98..ef690582d1 100644 --- a/packages/e2e-tests/test/e2e-snapshot.spec.ts +++ b/packages/e2e-tests/test/e2e-snapshot.spec.ts @@ -2,7 +2,7 @@ import { skipIfApiStrict, startSharedTestServer, } from '../../../testing/integration-testing-hooks'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; +import { TestShell } from './test-shell'; import { expect } from 'chai'; const setDifference = (a: T[], b: T[]) => a.filter((e) => !b.includes(e)); @@ -18,8 +18,6 @@ const commonPrefix = (a: string, b: string): string => describe('e2e snapshot support', function () { skipIfApiStrict(); - cleanTestShellsAfterEach(); - const testServer = startSharedTestServer(); context('modules included in snapshots', function () { diff --git a/packages/e2e-tests/test/e2e-snippet.spec.ts b/packages/e2e-tests/test/e2e-snippet.spec.ts index e4ec8fc5c6..70b3c1914f 100644 --- a/packages/e2e-tests/test/e2e-snippet.spec.ts +++ b/packages/e2e-tests/test/e2e-snippet.spec.ts @@ -1,13 +1,11 @@ import { promises as fs } from 'fs'; import path from 'path'; import { expect } from 'chai'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; +import type { TestShell } from './test-shell'; import { useTmpdir } from './repl-helpers'; import { eventually } from '../../../testing/eventually'; describe('snippet integration tests', function () { - cleanTestShellsAfterEach(); - this.timeout(120_000); const tmpdir = useTmpdir(); @@ -21,7 +19,7 @@ describe('snippet integration tests', function () { } makeTestShell = () => - TestShell.start({ + this.startTestShell({ args: ['--nodb'], cwd: tmpdir.path, env: { diff --git a/packages/e2e-tests/test/e2e-tls.spec.ts b/packages/e2e-tests/test/e2e-tls.spec.ts index 5794435a3e..b0e8950357 100644 --- a/packages/e2e-tests/test/e2e-tls.spec.ts +++ b/packages/e2e-tests/test/e2e-tls.spec.ts @@ -9,7 +9,6 @@ import { getCertPath, connectionStringWithLocalhost, } from './repl-helpers'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; const CA_CERT = getCertPath('ca.crt'); const NON_CA_CERT = getCertPath('non-ca.crt'); @@ -30,8 +29,6 @@ const CRL_INCLUDING_SERVER = getCertPath('ca-server.crl'); * and compliance with user-specified behavior that is specific to TLS connectivity. */ describe('e2e TLS', function () { - cleanTestShellsAfterEach(); - let homedir: string; let env: Record; let logBasePath: string; @@ -75,7 +72,7 @@ describe('e2e TLS', function () { // and subsequently can't connect to the server to find out if it's up, // then thinks it isn't and doesn't shut it down cleanly. We shut it down // here to work around that. - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server), '--tls', @@ -102,7 +99,7 @@ describe('e2e TLS', function () { }); it('works with matching CA (args)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server), '--tls', @@ -115,7 +112,7 @@ describe('e2e TLS', function () { }); it('works with matching CA (connection string)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { tls: 'true', @@ -128,7 +125,7 @@ describe('e2e TLS', function () { }); it('fails when not using --tls (args)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -141,7 +138,7 @@ describe('e2e TLS', function () { }); it('fails when not using --tls (connection string)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -155,7 +152,7 @@ describe('e2e TLS', function () { }); it('fails with invalid CA (args)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -173,7 +170,7 @@ describe('e2e TLS', function () { }); it('fails with invalid CA (connection string)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -190,7 +187,7 @@ describe('e2e TLS', function () { }); it('fails when providing a CRL including the servers cert', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -216,7 +213,7 @@ describe('e2e TLS', function () { CA_CERT, path.join(tmpdir.path, 'certs', 'somefilename.crt') ); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -254,7 +251,7 @@ describe('e2e TLS', function () { CA_CERT, path.join(tmpdir.path, 'certs', 'somefilename.crt') ); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -287,7 +284,7 @@ describe('e2e TLS', function () { if (process.platform !== 'darwin' && process.platform !== 'win32') { return this.skip(); } - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -309,7 +306,7 @@ describe('e2e TLS', function () { context('connecting with client cert to server with valid cert', function () { after(async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server), '--tls', @@ -343,7 +340,7 @@ describe('e2e TLS', function () { return this.skip(); // createUser is unversioned } /* connect with cert to create user */ - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -367,7 +364,7 @@ describe('e2e TLS', function () { }); it('works with valid cert (args)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -398,7 +395,7 @@ describe('e2e TLS', function () { }); it('works with valid cert (args, encrypted)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -436,7 +433,7 @@ describe('e2e TLS', function () { }); it('works with valid cert (connection string)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -464,7 +461,7 @@ describe('e2e TLS', function () { }); it('works with valid cert (connection string, encrypted)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -498,7 +495,7 @@ describe('e2e TLS', function () { }); it('fails with invalid cert (args)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -518,7 +515,7 @@ describe('e2e TLS', function () { }); it('fails with invalid cert (connection string)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -549,7 +546,7 @@ describe('e2e TLS', function () { }); ` ); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -574,7 +571,7 @@ describe('e2e TLS', function () { }); it('fails with an invalid tlsCertificateSelector', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server, { serverSelectionTimeoutMS: '1500', @@ -612,7 +609,7 @@ describe('e2e TLS', function () { // and subsequently can't connect to the server to find out if it's up, // then thinks it isn't and doesn't shut it down cleanly. We shut it down // here to work around that. - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server), '--tls', @@ -638,7 +635,7 @@ describe('e2e TLS', function () { }); it('works with allowInvalidCertificates', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server), '--tls', @@ -652,7 +649,7 @@ describe('e2e TLS', function () { }); it('works with allowInvalidHostnames', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server), '--tls', @@ -666,7 +663,7 @@ describe('e2e TLS', function () { }); it('fails when no additional args are provided', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await connectionStringWithLocalhost(server), '--tls', diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts index 96f534d54d..a6fb727eff 100644 --- a/packages/e2e-tests/test/e2e.spec.ts +++ b/packages/e2e-tests/test/e2e.spec.ts @@ -2,8 +2,9 @@ import { expect } from 'chai'; import type { Db } from 'mongodb'; import { MongoClient } from 'mongodb'; + import { eventually } from '../../../testing/eventually'; -import { cleanTestShellsAfterEach, TestShell } from './test-shell'; +import type { TestShell } from './test-shell'; import { skipIfServerVersion, startSharedTestServer, @@ -29,11 +30,9 @@ const jsContextFlagCombinations: `--jsContext=${'plain-vm' | 'repl'}`[][] = [ describe('e2e', function () { const testServer = startSharedTestServer(); - cleanTestShellsAfterEach(); - describe('--version', function () { it('shows version', async function () { - const shell = TestShell.start({ args: ['--version'] }); + const shell = this.startTestShell({ args: ['--version'] }); await shell.waitForExit(); shell.assertNoErrors(); @@ -43,7 +42,7 @@ describe('e2e', function () { describe('--build-info', function () { it('shows build info in JSON format', async function () { - const shell = TestShell.start({ args: ['--build-info'] }); + const shell = this.startTestShell({ args: ['--build-info'] }); await shell.waitForExit(); shell.assertNoErrors(); @@ -91,7 +90,7 @@ describe('e2e', function () { let processReport: any; { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ '--quiet', '--nodb', @@ -109,7 +108,7 @@ describe('e2e', function () { }); it('provides build info via the buildInfo() builtin', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ '--quiet', '--eval', @@ -129,7 +128,7 @@ describe('e2e', function () { describe('--nodb', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: ['--nodb'], }); await shell.waitForPrompt(); @@ -156,7 +155,7 @@ describe('e2e', function () { expect(shell.output).to.include('No connected database\n> '); }); it('colorizes syntax errors', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: ['--nodb'], env: { ...process.env, FORCE_COLOR: 'true', TERM: 'xterm-256color' }, forceTerminal: true, @@ -245,7 +244,7 @@ describe('e2e', function () { }); }); it('accepts a --tlsFIPSMode argument', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: ['--nodb', '--tlsFIPSMode'], }); const result = await shell.waitForPromptOrExit(); @@ -261,7 +260,7 @@ describe('e2e', function () { } }); it('prints full output even when that output is buffered', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [ '--nodb', '--quiet', @@ -286,7 +285,7 @@ describe('e2e', function () { }); it('handles custom prompt() function in conjunction with line-by-line input well', async function () { // https://jira.mongodb.org/browse/MONGOSH-1617 - shell = TestShell.start({ + shell = this.startTestShell({ args: [ '--nodb', '--shell', @@ -328,7 +327,7 @@ describe('e2e', function () { describe('via host:port/test', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [`${await testServer.hostport()}/${dbname}`], }); await shell.waitForPrompt(); @@ -342,7 +341,7 @@ describe('e2e', function () { describe('via mongodb://uri', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [`mongodb://${await testServer.hostport()}/${dbnameUri}`], }); await shell.waitForPrompt(); @@ -357,7 +356,7 @@ describe('e2e', function () { let shell: TestShell; beforeEach(async function () { const port = await testServer.port(); - shell = TestShell.start({ args: [dbname, `--port=${port}`] }); + shell = this.startTestShell({ args: [dbname, `--port=${port}`] }); await shell.waitForPrompt(); shell.assertNoErrors(); }); @@ -369,7 +368,7 @@ describe('e2e', function () { describe('via use() method', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [`mongodb://${await testServer.hostport()}/`], }); await shell.waitForPrompt(); @@ -390,7 +389,7 @@ describe('e2e', function () { context('with default appName', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [`mongodb://${await testServer.hostport()}/`], }); await shell.waitForPrompt(); @@ -411,7 +410,7 @@ describe('e2e', function () { context('with custom appName', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [`mongodb://${await testServer.hostport()}/?appName=Felicia`], }); await shell.waitForPrompt(); @@ -438,7 +437,7 @@ describe('e2e', function () { beforeEach(async function () { const connectionString = await testServer.connectionString(); dbName = `test-${Date.now()}`; - shell = TestShell.start({ args: [connectionString] }); + shell = this.startTestShell({ args: [connectionString] }); client = await MongoClient.connect(connectionString, {}); @@ -882,7 +881,7 @@ describe('e2e', function () { describe('with --host', function () { let shell: TestShell; it('allows invalid hostnames with _', async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: ['--host', 'xx_invalid_domain_xx'], env: { ...process.env, FORCE_COLOR: 'true', TERM: 'xterm-256color' }, forceTerminal: true, @@ -921,7 +920,7 @@ describe('e2e', function () { 'load', 'long-sleep.js' ); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', ...jsContextFlags, filename], removeSigintListeners: true, forceTerminal: true, @@ -949,7 +948,7 @@ describe('e2e', function () { describe('interactive', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: ['--nodb'], removeSigintListeners: true, }); @@ -1007,7 +1006,7 @@ describe('e2e', function () { describe('printing', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ args: ['--nodb'] }); + shell = this.startTestShell({ args: ['--nodb'] }); await shell.waitForPrompt(); shell.assertNoErrors(); }); @@ -1026,7 +1025,9 @@ describe('e2e', function () { describe('pipe from stdin', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ args: [await testServer.connectionString()] }); + shell = this.startTestShell({ + args: [await testServer.connectionString()], + }); }); it('reads and runs code from stdin, with .write()', async function () { @@ -1080,7 +1081,7 @@ describe('e2e', function () { it('works fine with custom prompts', async function () { // https://jira.mongodb.org/browse/MONGOSH-1617 - shell = TestShell.start({ + shell = this.startTestShell({ args: [ await testServer.connectionString(), '--eval', @@ -1101,7 +1102,7 @@ describe('e2e', function () { describe('Node.js builtin APIs in the shell', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: ['--nodb'], cwd: path.resolve(__dirname, 'fixtures', 'require-base'), env: { @@ -1140,7 +1141,7 @@ describe('e2e', function () { )})`, function () { context('file from disk', function () { it('loads a file from the command line as requested', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', ...jsContextFlags, './hello1.js'], cwd: path.resolve( __dirname, @@ -1165,7 +1166,7 @@ describe('e2e', function () { if (!jsContextFlags.includes('--jsContext=plain-vm')) { it('drops into shell if --shell is used', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', ...jsContextFlags, '--shell', './hello1.js'], cwd: path.resolve( __dirname, @@ -1184,7 +1185,7 @@ describe('e2e', function () { }); it('fails with the error if the loaded script throws', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', ...jsContextFlags, '--shell', './throw.js'], cwd: path.resolve( __dirname, @@ -1207,7 +1208,7 @@ describe('e2e', function () { context('--eval', function () { const script = 'const a = "hello", b = " one"; a + b'; it('loads a script from the command line as requested', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', ...jsContextFlags, '--eval', script], }); await eventually(() => { @@ -1219,7 +1220,7 @@ describe('e2e', function () { if (!jsContextFlags.includes('--jsContext=plain-vm')) { it('drops into shell if --shell is used', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', ...jsContextFlags, '--eval', script, '--shell'], }); await shell.waitForPrompt(); @@ -1230,7 +1231,7 @@ describe('e2e', function () { } it('fails with the error if the loaded script throws synchronously', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ '--nodb', ...jsContextFlags, @@ -1245,7 +1246,7 @@ describe('e2e', function () { }); it('fails with the error if the loaded script throws asynchronously (setImmediate)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ '--nodb', ...jsContextFlags, @@ -1262,7 +1263,7 @@ describe('e2e', function () { }); it('fails with the error if the loaded script throws asynchronously (Promise)', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ '--nodb', ...jsContextFlags, @@ -1286,7 +1287,7 @@ describe('e2e', function () { executionAsyncId: async_hooks.executionAsyncId() }; })()`; - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ '--nodb', ...jsContextFlags, @@ -1366,7 +1367,7 @@ describe('e2e', function () { EJSON.parse(await fs.readFile(configPath, 'utf8')); readLogfile = async () => readReplLogfile(logPath); startTestShell = async (...extraArgs: string[]) => { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['--nodb', ...extraArgs], env: env, forceTerminal: true, @@ -1378,7 +1379,6 @@ describe('e2e', function () { }); afterEach(async function () { - await TestShell.killAll(); try { await fs.rm(homedir, { recursive: true, force: true }); } catch (err: any) { @@ -1417,7 +1417,7 @@ describe('e2e', function () { globalConfig, 'mongosh:\n redactHistory: remove-redact' ); - shell = TestShell.start({ + shell = this.startTestShell({ args: ['--nodb'], env: { ...env, @@ -1709,7 +1709,7 @@ describe('e2e', function () { skipIfServerVersion(testServer, '> 4.4'); it('errors if an API version is specified', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await testServer.connectionString({}, { pathname: `/${dbName}` }), '--apiVersion', @@ -1727,7 +1727,7 @@ describe('e2e', function () { skipIfServerVersion(testServer, '<= 4.4'); it('can specify an API version', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await testServer.connectionString({}, { pathname: `/${dbName}` }), '--apiVersion', @@ -1743,7 +1743,7 @@ describe('e2e', function () { }); it('can specify an API version and strict mode', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await testServer.connectionString({}, { pathname: `/${dbName}` }), '--apiVersion', @@ -1762,7 +1762,7 @@ describe('e2e', function () { it('can iterate cursors', async function () { // Make sure SERVER-55593 doesn't happen to us. - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ await testServer.connectionString({}, { pathname: `/${dbName}` }), '--apiVersion', @@ -1784,7 +1784,7 @@ describe('e2e', function () { describe('fail-fast connections', function () { it('fails fast for ENOTFOUND errors', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ 'mongodb://' + 'verymuchnonexistentdomainname'.repeat(4) + @@ -1798,7 +1798,7 @@ describe('e2e', function () { it('fails fast for ENOTFOUND/EINVAL errors', async function () { // Very long & nonexistent domain can result in EINVAL in Node.js >= 20.11 // In lower versions, it would be ENOTFOUND - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ 'mongodb://' + 'verymuchnonexistentdomainname'.repeat(10) + @@ -1810,7 +1810,7 @@ describe('e2e', function () { }); it('fails fast for ECONNREFUSED errors to a single host', async function () { - const shell = TestShell.start({ args: ['--port', '1'] }); + const shell = this.startTestShell({ args: ['--port', '1'] }); const result = await shell.waitForPromptOrExit(); expect(result).to.deep.equal({ state: 'exit', exitCode: 1 }); }); @@ -1822,7 +1822,7 @@ describe('e2e', function () { // isn't a shell-specific issue. return this.skip(); } - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [ 'mongodb://127.0.0.1:1,127.0.0.2:1,127.0.0.3:1/?replicaSet=foo&readPreference=secondary', ], @@ -1836,7 +1836,9 @@ describe('e2e', function () { let shell: TestShell; beforeEach(async function () { - shell = TestShell.start({ args: [await testServer.connectionString()] }); + shell = this.startTestShell({ + args: [await testServer.connectionString()], + }); await shell.waitForPrompt(); shell.assertNoErrors(); }); @@ -1874,7 +1876,7 @@ describe('e2e', function () { let shell: TestShell; beforeEach(function () { - shell = TestShell.start({ + shell = this.startTestShell({ args: [], env: { ...process.env, MONGOSH_FORCE_CONNECTION_STRING_PROMPT: '1' }, forceTerminal: true, @@ -1901,7 +1903,7 @@ describe('e2e', function () { describe('with incomplete loadBalanced connectivity', function () { it('prints a warning at startup', async function () { - const shell = TestShell.start({ + const shell = this.startTestShell({ args: ['mongodb://localhost:1/?loadBalanced=true'], }); await shell.waitForPrompt(); @@ -1923,7 +1925,7 @@ describe('e2e', function () { 'fixtures', 'simple-console-log.js' ); - const shell = TestShell.start({ + const shell = this.startTestShell({ args: [filename], env: { ...process.env, MONGOSH_RUN_NODE_SCRIPT: '1' }, }); From 37b6a3fcecf8fbec915b8eec379a316c95ca1103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 09:33:20 +0200 Subject: [PATCH 08/14] Export and test ensureTestShellAfterHook --- packages/e2e-tests/test/test-shell.spec.ts | 22 ++++++++++++++++++++-- packages/e2e-tests/test/test-shell.ts | 16 +++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/packages/e2e-tests/test/test-shell.spec.ts b/packages/e2e-tests/test/test-shell.spec.ts index 919dfbe649..b94f118782 100644 --- a/packages/e2e-tests/test/test-shell.spec.ts +++ b/packages/e2e-tests/test/test-shell.spec.ts @@ -1,7 +1,7 @@ -import { TestShell } from './test-shell'; +import { ensureTestShellAfterHook, TestShell } from './test-shell'; describe('TestShell', function () { - context('sub-suite', function () { + context('hooks and tests', function () { before(async function () { const shell = this.startTestShell({ args: ['--nodb'] }); await shell.waitForPrompt(); @@ -28,6 +28,24 @@ describe('TestShell', function () { }); }); + context('adding an after each running after cleanup', function () { + beforeEach(async function () { + const shell = this.startTestShell({ args: ['--nodb'] }); + await shell.waitForPrompt(); + }); + + // Calling this before the "afterEach" below, ensures the cleanup hook gets added before it + ensureTestShellAfterHook('afterEach', this); + + afterEach(function () { + TestShell.assertNoOpenShells(); + }); + + it('works', function () { + /* */ + }); + }); + after(function () { TestShell.assertNoOpenShells(); }); diff --git a/packages/e2e-tests/test/test-shell.ts b/packages/e2e-tests/test/test-shell.ts index 07e0b2cc8b..739f55abb0 100644 --- a/packages/e2e-tests/test/test-shell.ts +++ b/packages/e2e-tests/test/test-shell.ts @@ -359,17 +359,19 @@ type AfterEachInjectedSuite = { }; /** - * Registers an after (all or each) hook to kill test shells started during the hooks or tests + * Registers an after (all or each) hook to kill test shells started during the hooks or tests. + * You don't have to call this from tests, but you can if you want to register an after (each) hook + * which runs after the shells have been killed. */ -function ensureAfterHook( +export function ensureTestShellAfterHook( hookName: 'afterEach', suite: Mocha.Suite ): asserts suite is AfterEachInjectedSuite & Mocha.Suite; -function ensureAfterHook( +export function ensureTestShellAfterHook( hookName: 'afterAll', suite: Mocha.Suite ): asserts suite is AfterAllInjectedSuite & Mocha.Suite; -function ensureAfterHook( +export function ensureTestShellAfterHook( hookName: 'afterEach' | 'afterAll', suite: Partial & Mocha.Suite ): void { @@ -409,19 +411,19 @@ Mocha.Context.prototype.startTestShell = function ( runnable.originalTitle === '"before each" hook' || runnable.originalTitle === '"after each" hook' ) { - ensureAfterHook('afterEach', parent); + ensureTestShellAfterHook('afterEach', parent); parent[TEST_SHELLS_AFTER_EACH].add(shell); } else if ( runnable.originalTitle === '"before all" hook' || runnable.originalTitle === '"after all" hook' ) { - ensureAfterHook('afterAll', parent); + ensureTestShellAfterHook('afterAll', parent); parent[TEST_SHELLS_AFTER_ALL].add(shell); } else { throw new Error(`Unexpected ${runnable.originalTitle || runnable.title}`); } } else if (runnable instanceof Mocha.Test) { - ensureAfterHook('afterEach', parent); + ensureTestShellAfterHook('afterEach', parent); parent[TEST_SHELLS_AFTER_EACH].add(shell); } else { throw new Error('Unexpected Runnable: Expected a Hook or a Test'); From eb249e2f42bdb64e98feaeeb0036b3c7b413ec89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 09:33:48 +0200 Subject: [PATCH 09/14] Use ensureTestShellAfterHook to ensure order of hooks --- packages/e2e-tests/test/e2e.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts index a6fb727eff..a3236fd990 100644 --- a/packages/e2e-tests/test/e2e.spec.ts +++ b/packages/e2e-tests/test/e2e.spec.ts @@ -4,7 +4,7 @@ import type { Db } from 'mongodb'; import { MongoClient } from 'mongodb'; import { eventually } from '../../../testing/eventually'; -import type { TestShell } from './test-shell'; +import { ensureTestShellAfterHook, TestShell } from './test-shell'; import { skipIfServerVersion, startSharedTestServer, @@ -1378,7 +1378,11 @@ describe('e2e', function () { }; }); + // Ensure the afterEach below runs after shells are killed + ensureTestShellAfterHook('afterEach', this); + afterEach(async function () { + TestShell.assertNoOpenShells(); try { await fs.rm(homedir, { recursive: true, force: true }); } catch (err: any) { From 8f3b45e6493121cd5efdb12929c31594a1d8c0ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 09:49:25 +0200 Subject: [PATCH 10/14] Adding back the after block in e2e-direct tests --- packages/e2e-tests/test/e2e-direct.spec.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/e2e-tests/test/e2e-direct.spec.ts b/packages/e2e-tests/test/e2e-direct.spec.ts index e4f5e0aec4..913cdfc7c1 100644 --- a/packages/e2e-tests/test/e2e-direct.spec.ts +++ b/packages/e2e-tests/test/e2e-direct.spec.ts @@ -82,6 +82,13 @@ describe('e2e direct connection', function () { await shell.executeLine('db.testcollection.insertOne({})'); shell.writeInputLine('exit'); }); + after(async function () { + const shell = this.startTestShell({ + args: [await rs0.connectionString()], + }); + await shell.executeLine(`db.getSiblingDB("${dbname}").dropDatabase()`); + shell.writeInputLine('exit'); + }); context('connecting to secondary members directly', function () { it('works when specifying a connection string', async function () { From 06018088c6de50ebd1ac3a2a18d256872cb508ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 11:35:11 +0200 Subject: [PATCH 11/14] Split test-shell.ts to separate concerns --- packages/e2e-tests/package.json | 2 +- packages/e2e-tests/test/e2e.spec.ts | 3 +- ...ell.spec.ts => test-shell-context.spec.ts} | 5 +- packages/e2e-tests/test/test-shell-context.ts | 96 ++++++++++++++++++ packages/e2e-tests/test/test-shell.ts | 99 +------------------ 5 files changed, 104 insertions(+), 101 deletions(-) rename packages/e2e-tests/test/{test-shell.spec.ts => test-shell-context.spec.ts} (89%) create mode 100644 packages/e2e-tests/test/test-shell-context.ts diff --git a/packages/e2e-tests/package.json b/packages/e2e-tests/package.json index c35fbbb7e5..156ee4019b 100644 --- a/packages/e2e-tests/package.json +++ b/packages/e2e-tests/package.json @@ -11,7 +11,7 @@ "url": "git://github.com/mongodb-js/mongosh.git" }, "scripts": { - "test": "mocha -r ts-node/register -r \"../../scripts/import-expansions.js\" -r \"./test/test-shell.ts\" --timeout 15000 --colors \"./test/*.spec.ts\"", + "test": "mocha -r ts-node/register -r \"../../scripts/import-expansions.js\" -r \"./test/test-shell-context.ts\" --timeout 15000 --colors \"./test/*.spec.ts\"", "test-ci": "node ../../scripts/run-if-package-requested.js npm test", "test-coverage": "nyc --no-clean --cwd ../.. --reporter=none npm run test", "test-ci-coverage": "nyc --no-clean --cwd ../.. --reporter=none npm run test-ci", diff --git a/packages/e2e-tests/test/e2e.spec.ts b/packages/e2e-tests/test/e2e.spec.ts index a3236fd990..dc58693b95 100644 --- a/packages/e2e-tests/test/e2e.spec.ts +++ b/packages/e2e-tests/test/e2e.spec.ts @@ -4,7 +4,8 @@ import type { Db } from 'mongodb'; import { MongoClient } from 'mongodb'; import { eventually } from '../../../testing/eventually'; -import { ensureTestShellAfterHook, TestShell } from './test-shell'; +import { TestShell } from './test-shell'; +import { ensureTestShellAfterHook } from './test-shell-context'; import { skipIfServerVersion, startSharedTestServer, diff --git a/packages/e2e-tests/test/test-shell.spec.ts b/packages/e2e-tests/test/test-shell-context.spec.ts similarity index 89% rename from packages/e2e-tests/test/test-shell.spec.ts rename to packages/e2e-tests/test/test-shell-context.spec.ts index b94f118782..01f144e573 100644 --- a/packages/e2e-tests/test/test-shell.spec.ts +++ b/packages/e2e-tests/test/test-shell-context.spec.ts @@ -1,6 +1,7 @@ -import { ensureTestShellAfterHook, TestShell } from './test-shell'; +import { TestShell } from './test-shell'; +import { ensureTestShellAfterHook } from './test-shell-context'; -describe('TestShell', function () { +describe('TestShell context', function () { context('hooks and tests', function () { before(async function () { const shell = this.startTestShell({ args: ['--nodb'] }); diff --git a/packages/e2e-tests/test/test-shell-context.ts b/packages/e2e-tests/test/test-shell-context.ts new file mode 100644 index 0000000000..f4efc83693 --- /dev/null +++ b/packages/e2e-tests/test/test-shell-context.ts @@ -0,0 +1,96 @@ +import assert from 'assert'; +import Mocha from 'mocha'; +import { TestShell, type TestShellOptions } from './test-shell'; + +declare module 'mocha' { + interface Context { + /** + * Starts a test shell and registers a hook to kill it after the test + */ + startTestShell(options?: TestShellOptions): TestShell; + } +} + +const TEST_SHELLS_AFTER_ALL = Symbol('test-shells-after-all'); +const TEST_SHELLS_AFTER_EACH = Symbol('test-shells-after-each'); + +type AfterAllInjectedSuite = { + [TEST_SHELLS_AFTER_ALL]: Set; +}; + +type AfterEachInjectedSuite = { + [TEST_SHELLS_AFTER_EACH]: Set; +}; + +/** + * Registers an after (all or each) hook to kill test shells started during the hooks or tests. + * You don't have to call this from tests, but you can if you want to register an after (each) hook + * which runs after the shells have been killed. + */ +export function ensureTestShellAfterHook( + hookName: 'afterEach', + suite: Mocha.Suite +): asserts suite is AfterEachInjectedSuite & Mocha.Suite; +export function ensureTestShellAfterHook( + hookName: 'afterAll', + suite: Mocha.Suite +): asserts suite is AfterAllInjectedSuite & Mocha.Suite; +export function ensureTestShellAfterHook( + hookName: 'afterEach' | 'afterAll', + suite: Partial & Mocha.Suite +): void { + const symbol = + hookName === 'afterAll' ? TEST_SHELLS_AFTER_ALL : TEST_SHELLS_AFTER_EACH; + if (!suite[symbol]) { + // Store the set of shells to kill afterwards + const shells = new Set(); + suite[symbol] = shells; + suite[hookName](async () => { + const shellsToKill = [...shells]; + shells.clear(); + await Promise.all( + shellsToKill.map((shell) => { + // TODO: Consider if it's okay to kill those that are already killed? + shell.kill(); + return shell.waitForExit(); + }) + ); + }); + } +} + +Mocha.Context.prototype.startTestShell = function ( + this: Mocha.Context, + options: TestShellOptions +) { + const { test: runnable } = this; + assert(runnable, 'Expected a runnable / test'); + const { parent } = runnable; + assert(parent, 'Expected runnable to have a parent'); + // Start the shell + const shell = TestShell.start(options); + // Register a hook to kill the shell + if (runnable instanceof Mocha.Hook) { + if ( + runnable.originalTitle === '"before each" hook' || + runnable.originalTitle === '"after each" hook' + ) { + ensureTestShellAfterHook('afterEach', parent); + parent[TEST_SHELLS_AFTER_EACH].add(shell); + } else if ( + runnable.originalTitle === '"before all" hook' || + runnable.originalTitle === '"after all" hook' + ) { + ensureTestShellAfterHook('afterAll', parent); + parent[TEST_SHELLS_AFTER_ALL].add(shell); + } else { + throw new Error(`Unexpected ${runnable.originalTitle || runnable.title}`); + } + } else if (runnable instanceof Mocha.Test) { + ensureTestShellAfterHook('afterEach', parent); + parent[TEST_SHELLS_AFTER_EACH].add(shell); + } else { + throw new Error('Unexpected Runnable: Expected a Hook or a Test'); + } + return shell; +}; diff --git a/packages/e2e-tests/test/test-shell.ts b/packages/e2e-tests/test/test-shell.ts index 739f55abb0..267f7b3e93 100644 --- a/packages/e2e-tests/test/test-shell.ts +++ b/packages/e2e-tests/test/test-shell.ts @@ -1,4 +1,3 @@ -import Mocha from 'mocha'; import assert from 'assert'; import type { ChildProcess, @@ -49,7 +48,8 @@ export class TestShell { private static _openShells: Set = new Set(); /** - * @deprecated Use the {@link Mocha.Context.startTestShell} hook instead + * Starts a test shell. + * @private Use the {@link Mocha.Context.startTestShell} hook instead */ static start(options: TestShellOptions = { args: [] }): TestShell { let shellProcess: ChildProcessWithoutNullStreams; @@ -335,98 +335,3 @@ export class TestShell { return match.groups!.logId; } } - -// Context extension to manage TestShell lifetime - -declare module 'mocha' { - interface Context { - /** - * Starts a test shell and registers a hook to kill it after the test - */ - startTestShell(options?: TestShellOptions): TestShell; - } -} - -const TEST_SHELLS_AFTER_ALL = Symbol('test-shells-after-all'); -const TEST_SHELLS_AFTER_EACH = Symbol('test-shells-after-each'); - -type AfterAllInjectedSuite = { - [TEST_SHELLS_AFTER_ALL]: Set; -}; - -type AfterEachInjectedSuite = { - [TEST_SHELLS_AFTER_EACH]: Set; -}; - -/** - * Registers an after (all or each) hook to kill test shells started during the hooks or tests. - * You don't have to call this from tests, but you can if you want to register an after (each) hook - * which runs after the shells have been killed. - */ -export function ensureTestShellAfterHook( - hookName: 'afterEach', - suite: Mocha.Suite -): asserts suite is AfterEachInjectedSuite & Mocha.Suite; -export function ensureTestShellAfterHook( - hookName: 'afterAll', - suite: Mocha.Suite -): asserts suite is AfterAllInjectedSuite & Mocha.Suite; -export function ensureTestShellAfterHook( - hookName: 'afterEach' | 'afterAll', - suite: Partial & Mocha.Suite -): void { - const symbol = - hookName === 'afterAll' ? TEST_SHELLS_AFTER_ALL : TEST_SHELLS_AFTER_EACH; - if (!suite[symbol]) { - // Store the set of shells to kill afterwards - const shells = new Set(); - suite[symbol] = shells; - suite[hookName](async () => { - const shellsToKill = [...shells]; - shells.clear(); - await Promise.all( - shellsToKill.map((shell) => { - // TODO: Consider if it's okay to kill those that are already killed? - shell.kill(); - return shell.waitForExit(); - }) - ); - }); - } -} - -Mocha.Context.prototype.startTestShell = function ( - this: Mocha.Context, - options: TestShellOptions -) { - const { test: runnable } = this; - assert(runnable, 'Expected a runnable / test'); - const { parent } = runnable; - assert(parent, 'Expected runnable to have a parent'); - // Start the shell - const shell = TestShell.start(options); - // Register a hook to kill the shell - if (runnable instanceof Mocha.Hook) { - if ( - runnable.originalTitle === '"before each" hook' || - runnable.originalTitle === '"after each" hook' - ) { - ensureTestShellAfterHook('afterEach', parent); - parent[TEST_SHELLS_AFTER_EACH].add(shell); - } else if ( - runnable.originalTitle === '"before all" hook' || - runnable.originalTitle === '"after all" hook' - ) { - ensureTestShellAfterHook('afterAll', parent); - parent[TEST_SHELLS_AFTER_ALL].add(shell); - } else { - throw new Error(`Unexpected ${runnable.originalTitle || runnable.title}`); - } - } else if (runnable instanceof Mocha.Test) { - ensureTestShellAfterHook('afterEach', parent); - parent[TEST_SHELLS_AFTER_EACH].add(shell); - } else { - throw new Error('Unexpected Runnable: Expected a Hook or a Test'); - } - return shell; -}; From dad6c958ae0981a29805cf7d12a5dbba31079689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 12:05:22 +0200 Subject: [PATCH 12/14] Add assertNoOpenShells to e2e-editor --- packages/e2e-tests/test/e2e-editor.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test/e2e-editor.spec.ts b/packages/e2e-tests/test/e2e-editor.spec.ts index a7cbdeefd7..ac6abb2fb9 100644 --- a/packages/e2e-tests/test/e2e-editor.spec.ts +++ b/packages/e2e-tests/test/e2e-editor.spec.ts @@ -2,7 +2,8 @@ import { expect } from 'chai'; import path from 'path'; import { promises as fs } from 'fs'; import { eventually } from '../../../testing/eventually'; -import type { TestShell } from './test-shell'; +import { TestShell } from './test-shell'; +import { ensureTestShellAfterHook } from './test-shell-context'; import { useTmpdir, fakeExternalEditor, @@ -41,7 +42,10 @@ describe('external editor e2e', function () { ); }); + ensureTestShellAfterHook('afterEach', this); + afterEach(async function () { + TestShell.assertNoOpenShells(); try { await fs.rm(homedir, { recursive: true, force: true }); } catch (err: any) { From 0f92ba5677fb4d2db1b391077ee6de5818d445c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 12:10:18 +0200 Subject: [PATCH 13/14] Using a doc comment instead of a @deprecated tag --- packages/e2e-tests/test/test-shell.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/test/test-shell.ts b/packages/e2e-tests/test/test-shell.ts index 267f7b3e93..43a031b385 100644 --- a/packages/e2e-tests/test/test-shell.ts +++ b/packages/e2e-tests/test/test-shell.ts @@ -49,7 +49,11 @@ export class TestShell { /** * Starts a test shell. - * @private Use the {@link Mocha.Context.startTestShell} hook instead + * + * Beware that the caller is responsible for calling {@link TestShell.kill} (and potentially {@link TestShell.waitForExit}). + * + * Consider calling the `startTestShell` function on a {@link Mocha.Context} instead, as that manages the lifetime the shell + * and ensures it gets killed eventually. */ static start(options: TestShellOptions = { args: [] }): TestShell { let shellProcess: ChildProcessWithoutNullStreams; From bb88c9eb0579e1d6b89039d0fb7423fe523963e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Thu, 26 Sep 2024 13:33:11 +0200 Subject: [PATCH 14/14] Refactored "runAndGetOutputWithoutErrors" into "waitForCleanOutout" --- packages/e2e-tests/test/e2e-snapshot.spec.ts | 3 +-- packages/e2e-tests/test/test-shell.ts | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/e2e-tests/test/e2e-snapshot.spec.ts b/packages/e2e-tests/test/e2e-snapshot.spec.ts index ef690582d1..f887fae94b 100644 --- a/packages/e2e-tests/test/e2e-snapshot.spec.ts +++ b/packages/e2e-tests/test/e2e-snapshot.spec.ts @@ -2,7 +2,6 @@ import { skipIfApiStrict, startSharedTestServer, } from '../../../testing/integration-testing-hooks'; -import { TestShell } from './test-shell'; import { expect } from 'chai'; const setDifference = (a: T[], b: T[]) => a.filter((e) => !b.includes(e)); @@ -57,7 +56,7 @@ describe('e2e snapshot support', function () { ] = ( await Promise.all( argLists.map((args) => - TestShell.runAndGetOutputWithoutErrors({ args }) + this.startTestShell({ args }).waitForCleanOutput() ) ) ).map((output) => diff --git a/packages/e2e-tests/test/test-shell.ts b/packages/e2e-tests/test/test-shell.ts index 43a031b385..66565a1a14 100644 --- a/packages/e2e-tests/test/test-shell.ts +++ b/packages/e2e-tests/test/test-shell.ts @@ -50,7 +50,7 @@ export class TestShell { /** * Starts a test shell. * - * Beware that the caller is responsible for calling {@link TestShell.kill} (and potentially {@link TestShell.waitForExit}). + * Beware that the caller is responsible for calling {@link kill} (and potentially {@link waitForExit}). * * Consider calling the `startTestShell` function on a {@link Mocha.Context} instead, as that manages the lifetime the shell * and ensures it gets killed eventually. @@ -108,15 +108,6 @@ export class TestShell { return shell; } - static async runAndGetOutputWithoutErrors( - options: TestShellOptions - ): Promise { - const shell = this.start(options); - await shell.waitForExit(); - shell.assertNoErrors(); - return shell.output; - } - debugInformation() { return { pid: this.process.pid, @@ -232,6 +223,15 @@ export class TestShell { return this._onClose; } + /** + * Waits for the shell to exit, asserts no errors and returns the output. + */ + async waitForCleanOutput(): Promise { + await this.waitForExit(); + this.assertNoErrors(); + return this.output; + } + async waitForPromptOrExit(): Promise { return Promise.race([ this.waitForPrompt().then(() => ({ state: 'prompt' } as const)),