Skip to content

Commit

Permalink
feat(cli): add t.jobs to configure concurrent subtests
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dirkdev98 committed Jun 11, 2023
1 parent d937d17 commit 3d0b9eb
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 77 deletions.
1 change: 0 additions & 1 deletion packages/cli/src/compas/commands/test.js
Original file line number Diff line number Diff line change
@@ -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";
Expand Down
58 changes: 31 additions & 27 deletions packages/cli/src/migrate/migrate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/testing/config.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
};
Expand Down
9 changes: 7 additions & 2 deletions packages/cli/src/testing/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
*/

/**
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down
4 changes: 4 additions & 0 deletions packages/cli/src/testing/runner.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@
*
* @param {import("./config").TestConfig} testConfig
* @param {import("./state").TestState} testState
* @param {Partial<Pick<import("./config").TestConfig, "timeout">>} [configOverrides]
* @returns {Promise<void>}
*/
export function runTestsRecursively(
testConfig: import("./config").TestConfig,
testState: import("./state").TestState,
configOverrides?:
| Partial<Pick<import("./config").TestConfig, "timeout">>
| undefined,
): Promise<void>;
/**
* Register top-level tests. The main entry point of the test runner
Expand Down
47 changes: 33 additions & 14 deletions packages/cli/src/testing/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,19 @@ import { state, testLogger } from "./state.js";
*
* @param {import("./config").TestConfig} testConfig
* @param {import("./state").TestState} testState
* @param {Partial<Pick<import("./config").TestConfig, "timeout">>} [configOverrides]
* @returns {Promise<void>}
*/
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}`);
Expand All @@ -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);
Expand All @@ -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);
}),
]);
}
Expand All @@ -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 (
Expand All @@ -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;
}

/**
Expand Down
24 changes: 24 additions & 0 deletions packages/cli/src/testing/runner.test.js
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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`);
});
});
});
13 changes: 12 additions & 1 deletion packages/cli/types/advanced-types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
60 changes: 30 additions & 30 deletions test/examples.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
});
}
}),
);
});
}
});
});

0 comments on commit 3d0b9eb

Please sign in to comment.