From 3d0b9eb1c29a7115225d6b1502867bfee80cdd5b Mon Sep 17 00:00:00 2001 From: Dirk de Visser Date: Sun, 11 Jun 2023 16:23:58 +0200 Subject: [PATCH] feat(cli): add `t.jobs` to configure concurrent subtests This is quite a quick and easy implementation. Which picks the first N subtests and waits on them to complete. We may want to optimize this later so a single slow test doesn't hold up running other subtests --- packages/cli/src/compas/commands/test.js | 1 - packages/cli/src/migrate/migrate.test.js | 58 ++++++++++++----------- packages/cli/src/testing/config.d.ts | 6 ++- packages/cli/src/testing/config.js | 9 +++- packages/cli/src/testing/runner.d.ts | 4 ++ packages/cli/src/testing/runner.js | 47 +++++++++++++------ packages/cli/src/testing/runner.test.js | 24 ++++++++++ packages/cli/types/advanced-types.d.ts | 13 ++++- test/examples.test.js | 60 ++++++++++++------------ 9 files changed, 145 insertions(+), 77 deletions(-) diff --git a/packages/cli/src/compas/commands/test.js b/packages/cli/src/compas/commands/test.js index 7bf8747f79..f0f17c2a39 100644 --- a/packages/cli/src/compas/commands/test.js +++ b/packages/cli/src/compas/commands/test.js @@ -1,4 +1,3 @@ -import { url } from "inspector"; import { isMainThread, Worker } from "worker_threads"; import { isNil, spawn } from "@compas/stdlib"; import { testingLoadConfig } from "../../testing/config.js"; diff --git a/packages/cli/src/migrate/migrate.test.js b/packages/cli/src/migrate/migrate.test.js index 00eb1b5c2e..ed044120c3 100644 --- a/packages/cli/src/migrate/migrate.test.js +++ b/packages/cli/src/migrate/migrate.test.js @@ -14,33 +14,37 @@ test("cli/docker/migrate", async (t) => { await mkdir("./packages/cli/tmp", { recursive: true }); await writeFile(filePath, fileContents, "utf-8"); - t.test("no --connection-settings specified", async (t) => { - const { exitCode } = await exec( - `compas migrate info --connection-settings`, - ); - t.equal(exitCode, 1); - }); - - t.test("--connection-settings invalid file", async (t) => { - const { exitCode } = await exec( - `compas migrate info --connection-settings /tmp/${uuid()}.js`, - ); - t.equal(exitCode, 1); - }); - - t.test("--connection-settings are loaded", async (t) => { - const { exitCode, stdout, stderr } = await exec( - `compas migrate info --connection-settings ${filePath}`, - ); - - t.equal(exitCode, 0); - - if (exitCode !== 0) { - t.log.error({ - stdout, - stderr, - }); - } + t.test("tests", (t) => { + t.jobs = 3; + + t.test("no --connection-settings specified", async (t) => { + const { exitCode } = await exec( + `compas migrate info --connection-settings`, + ); + t.equal(exitCode, 1); + }); + + t.test("--connection-settings invalid file", async (t) => { + const { exitCode } = await exec( + `compas migrate info --connection-settings /tmp/${uuid()}.js`, + ); + t.equal(exitCode, 1); + }); + + t.test("--connection-settings are loaded", async (t) => { + const { exitCode, stdout, stderr } = await exec( + `compas migrate info --connection-settings ${filePath}`, + ); + + t.equal(exitCode, 0); + + if (exitCode !== 0) { + t.log.error({ + stdout, + stderr, + }); + } + }); }); t.test("teardown", async (t) => { diff --git a/packages/cli/src/testing/config.d.ts b/packages/cli/src/testing/config.d.ts index f6cd063cc8..612ef5106f 100644 --- a/packages/cli/src/testing/config.d.ts +++ b/packages/cli/src/testing/config.d.ts @@ -14,7 +14,8 @@ * @property {string[]} ignoreDirectories Subdirectories to skip, when looking for all * test files * @property {boolean} coverage Run the test while collecting coverage results. - * @property {boolean} singleFileMode Should be set when only a single test should run via 'mainTestFn' + * @property {boolean} singleFileMode Should be set when only a single test should run + * via 'mainTestFn' */ /** * Load the test config & parse the flags @@ -73,7 +74,8 @@ export type TestConfig = { */ coverage: boolean; /** - * Should be set when only a single test should run via 'mainTestFn' + * Should be set when only a single test should run + * via 'mainTestFn' */ singleFileMode: boolean; }; diff --git a/packages/cli/src/testing/config.js b/packages/cli/src/testing/config.js index 2247d1a3e3..c104431317 100644 --- a/packages/cli/src/testing/config.js +++ b/packages/cli/src/testing/config.js @@ -33,7 +33,8 @@ const configPath = pathJoin(process.cwd(), "test/config.js"); * @property {string[]} ignoreDirectories Subdirectories to skip, when looking for all * test files * @property {boolean} coverage Run the test while collecting coverage results. - * @property {boolean} singleFileMode Should be set when only a single test should run via 'mainTestFn' + * @property {boolean} singleFileMode Should be set when only a single test should run + * via 'mainTestFn' */ /** @@ -105,7 +106,8 @@ export async function testingLoadConfig(logger, flags) { resolvedConfig.randomizeRounds = flags.randomizeRounds; } - // Set env vars so they are read when a worker is starting up, at that point we don't have flags + // Set env vars so they are read when a worker is starting up, at that point we don't + // have flags environment.__COMPAS_TEST_BAIL = String(resolvedConfig.bail); process.env.__COMPAS_TEST_WITH_LOGS = String(resolvedConfig.withLogs); @@ -151,6 +153,7 @@ export async function testingLoadConfig(logger, flags) { setAreTestRunning(true); if (!isMainThread && isNil(logger)) { + // Setup a logger with threadId, so logs of a single thread can be found. const formattedThreadId = String(threadId).padStart( String(resolvedConfig.parallelCount * resolvedConfig.randomizeRounds) .length, @@ -169,6 +172,8 @@ export async function testingLoadConfig(logger, flags) { } if (!resolvedConfig.withLogs) { + // Filter out error logs of all created loggers both in dev & prod modes of running + // the tests. const destination = loggerGetGlobalDestination(); loggerSetGlobalDestination({ write(msg) { diff --git a/packages/cli/src/testing/runner.d.ts b/packages/cli/src/testing/runner.d.ts index 6911e8235c..4db2ad5d8b 100644 --- a/packages/cli/src/testing/runner.d.ts +++ b/packages/cli/src/testing/runner.d.ts @@ -5,11 +5,15 @@ * * @param {import("./config").TestConfig} testConfig * @param {import("./state").TestState} testState + * @param {Partial>} [configOverrides] * @returns {Promise} */ export function runTestsRecursively( testConfig: import("./config").TestConfig, testState: import("./state").TestState, + configOverrides?: + | Partial> + | undefined, ): Promise; /** * Register top-level tests. The main entry point of the test runner diff --git a/packages/cli/src/testing/runner.js b/packages/cli/src/testing/runner.js index 50bd14df39..bb4a3f57a0 100644 --- a/packages/cli/src/testing/runner.js +++ b/packages/cli/src/testing/runner.js @@ -10,12 +10,19 @@ import { state, testLogger } from "./state.js"; * * @param {import("./config").TestConfig} testConfig * @param {import("./state").TestState} testState + * @param {Partial>} [configOverrides] * @returns {Promise} */ -export async function runTestsRecursively(testConfig, testState) { +export async function runTestsRecursively( + testConfig, + testState, + configOverrides, +) { const abortController = new AbortController(); const runner = createRunnerForState(testState, abortController.signal); + const resolvedTimeout = configOverrides?.timeout ?? testConfig.timeout; + if (!isNil(testState.callback)) { if (testState.parent === state) { testLogger.info(`Running: ${testState.name}`); @@ -30,7 +37,7 @@ export async function runTestsRecursively(testConfig, testState) { testLogger.info( `Ignoring timeout for '${testState.name}', detected an active inspector.`, ); - }, testConfig.timeout); + }, resolvedTimeout); await result; clearTimeout(timeoutReminder); @@ -44,15 +51,15 @@ export async function runTestsRecursively(testConfig, testState) { reject( new Error( `Exceeded test timeout of ${ - testConfig.timeout / 1000 + resolvedTimeout / 1000 } seconds. You can increase the timeout by calling 't.timeout = ${ - testConfig.timeout + 1000 + resolvedTimeout + 1000 };' on the parent test function. Or by setting 'export const timeout = ${ - testConfig.timeout + 1000 + resolvedTimeout + 1000 };' in 'test/config.js'.`, ), ); - }, testConfig.timeout); + }, resolvedTimeout); }), ]); } @@ -76,8 +83,15 @@ export async function runTestsRecursively(testConfig, testState) { } } - const originalTimeout = testConfig.timeout; - testConfig.timeout = runner.timeout ?? originalTimeout; + const subTestOverrides = { + timeout: runner.timeout ?? configOverrides?.timeout, + }; + + if (typeof runner.jobs !== "number" || runner.jobs === 0) { + // Remove invalid jobs config + delete runner.jobs; + } + mutateRunnerEnablingWarnings(runner); if ( @@ -100,18 +114,23 @@ export async function runTestsRecursively(testConfig, testState) { } } - for (const child of testState.children) { - await runTestsRecursively(testConfig, child); + const children = [...testState.children]; + while (children.length) { + const partial = children.splice(0, runner.jobs ?? 1); + + await Promise.all( + partial.map((it) => + runTestsRecursively(testConfig, it, subTestOverrides), + ), + ); if (testConfig.bail) { - markTestFailuresRecursively(state); - if (state.hasFailure) { + markTestFailuresRecursively(testState); + if (testState.hasFailure) { return; } } } - - testConfig.timeout = originalTimeout; } /** diff --git a/packages/cli/src/testing/runner.test.js b/packages/cli/src/testing/runner.test.js index 7c92b20244..2c37fcd88b 100644 --- a/packages/cli/src/testing/runner.test.js +++ b/packages/cli/src/testing/runner.test.js @@ -1,4 +1,5 @@ import { match, strictEqual } from "assert"; +import { setTimeout } from "node:timers/promises"; import { mainTestFn, test } from "@compas/cli"; import { AppError } from "@compas/stdlib"; import { runTestsRecursively } from "./runner.js"; @@ -103,4 +104,27 @@ test("cli/testing/runner", (t) => { t.equal(state.caughtException.key, "error.server.notImplemented"); }, ); + + t.test("parallel", (t) => { + const start = new Date(); + + t.test("act", (t) => { + t.jobs = 5; + + for (let i = 0; i < 10; ++i) { + t.test(`${i}`, async (t) => { + await setTimeout(i); + t.pass(); + }); + } + }); + + t.test("assert", (t) => { + const stop = new Date(); + + // total of 9...0 = 45, but should be shorter than that, even with a bit of test + // runner overhead. The minimum time is 13ms -> 4 + 9 + t.ok(stop - start < 25, `Actual time: ${stop - start}ms`); + }); + }); }); diff --git a/packages/cli/types/advanced-types.d.ts b/packages/cli/types/advanced-types.d.ts index f2294362b6..cab4d03793 100644 --- a/packages/cli/types/advanced-types.d.ts +++ b/packages/cli/types/advanced-types.d.ts @@ -13,10 +13,21 @@ export interface TestRunner { name: string; /** - * Configurable timeout used for sub tests + * Configurable timeout in milliseconds used for all subtests (and subtests of these etc.). + * + * This value defaults to 2500 milliseconds. The default can be overwritten globally in the + * `test/config.js` file by setting `export const timeout = 2500;` */ timeout?: number; + /** + * Configure how many subtests run concurrent. Defaults to '1' guaranteeing all subtests to + * run in order that they are added. + * + * This only affects the immediate subtests. + */ + jobs?: number; + /** * Signal to abort when the tests time out */ diff --git a/test/examples.test.js b/test/examples.test.js index 8d690ed8ee..047607aa11 100644 --- a/test/examples.test.js +++ b/test/examples.test.js @@ -31,40 +31,40 @@ test("compas/examples", async (t) => { ) ).filter((it) => !!it); - t.test("run examples", async (t) => { - await Promise.all( - configs.map(async (config) => { + t.test("run examples", (t) => { + t.jobs = configs.length; + + for (const config of configs) { + t.test(config.exampleMetadata.path, (t) => { for (const cmd of config.exampleMetadata.testing) { - const parts = cmd.split(" "); - if (parts[0] === "compas") { - parts[0] = "../../node_modules/.bin/compas"; - } + t.test(cmd, async (t) => { + const parts = cmd.split(" "); + if (parts[0] === "compas") { + parts[0] = "../../node_modules/.bin/compas"; + } - const { exitCode, stdout, stderr } = await exec(parts.join(" "), { - cwd: config.exampleMetadata.path, - env: { - PATH: environment.PATH, - CI: environment.CI, - GITHUB_ACTIONS: environment.GITHUB_ACTIONS, - }, - }); + const { exitCode, stdout, stderr } = await exec(parts.join(" "), { + cwd: config.exampleMetadata.path, + env: { + PATH: environment.PATH, + CI: environment.CI, + GITHUB_ACTIONS: environment.GITHUB_ACTIONS, + }, + }); - t.equal( - exitCode, - 0, - `Path: ${config.exampleMetadata.path}, Command: ${cmd}, Exitcode: ${exitCode}`, - ); + t.equal(exitCode, 0); - if (exitCode !== 0) { - t.log.error({ - config, - cmd, - stdout, - stderr, - }); - } + if (exitCode !== 0) { + t.log.error({ + config, + cmd, + stdout, + stderr, + }); + } + }); } - }), - ); + }); + } }); });