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] 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 c35fbbb7e..156ee4019 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 a3236fd99..dc58693b9 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 b94f11878..01f144e57 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 000000000..f4efc8369 --- /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 739f55abb..267f7b3e9 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; -};