diff --git a/auth-authme-package/index.ts b/auth-authme-package/index.ts index c43618e..5a78e4f 100644 --- a/auth-authme-package/index.ts +++ b/auth-authme-package/index.ts @@ -121,7 +121,14 @@ export default definePlugin({ message: `authme: never saw a login/register prompt or session-resume message for "${account.username}"`, }, ); - if (promptResult === 'resumed') return; + if (promptResult === 'resumed') { + const authenticated = new RegExp(resolved.authenticatedPattern, 'i'); + await poll(() => since(joinIndex, authenticated), { + timeout: resolved.timeoutMs, + message: `authme: "${account.username}" session resumed, but never confirmed as authenticated`, + }); + return; + } const isRegistration = promptResult === 'register'; // Everything below only looks at messages newer than the command. A server's greeting diff --git a/docs/reports.mdx b/docs/reports.mdx index 49b7659..54d7b4c 100644 --- a/docs/reports.mdx +++ b/docs/reports.mdx @@ -40,12 +40,36 @@ build/reports/plugwright/.log per-environment output, matrix runs o } ``` -`status` is `pass`, `fail` or `skip`. `plugin` names the plugin a test came from when it was inherited rather than found in your test directory. +`status` is `pass`, `fail` or `skip`. `plugin` names the plugin a test came from when it was inherited rather than found in your test directory. `botUsername` is the bot that ran it, when one connected. Every skip carries its reason: excluded by name, wrong environment, a capability the environment doesn't have, or an earlier test in the same [`describe.serial`](/writing-tests) block that stopped the chain. A skipped test that doesn't say why is worse than a failing one, because it reads as coverage. Tests from a serial block appear as ordinary entries, in the order they ran, under their full `describe` path. +## Concurrent tests + +A test (or block) run with [`concurrency`](/writing-tests) still gets one entry, not N. `durationMs` is the slowest instance, and `instances` carries every instance's own outcome: + +```json +{ + "file": "…/dist/claim.spec.js", + "name": "only one player can claim the chest", + "status": "fail", + "durationMs": 812, + "error": "Expected message matching \"Claimed\" not received", + "skipReason": null, + "plugin": null, + "botUsername": null, + "instances": [ + { "index": 1, "botUsername": "pw_a1", "passed": true, "durationMs": 640, "error": null }, + { "index": 2, "botUsername": "pw_b2", "passed": false, "durationMs": 812, "error": "Expected message matching \"Claimed\" not received" }, + { "index": 3, "botUsername": "pw_c3", "passed": true, "durationMs": 701, "error": null } + ] +} +``` + +`instances` is `null` for an ordinary, non-concurrent test — `botUsername` on the row itself is where its bot lives instead. The JUnit report doesn't carry this breakdown; it only ever sees the one aggregated pass/fail/duration, so read the JSON report when a concurrent test fails. + ## JUnit XML ```xml diff --git a/docs/writing-tests.mdx b/docs/writing-tests.mdx index 9319cc2..53225d8 100644 --- a/docs/writing-tests.mdx +++ b/docs/writing-tests.mdx @@ -199,7 +199,7 @@ describe.serial('vip shop', { account: 'pw_0001' }, async () => { }); ``` -The account must exist in the environment's `accounts { }` pool and be free. If it is already leased, or the environment has no pool at all (`local` invents a name per bot and has none), the block fails with that message rather than quietly running as somebody else. +The account must exist in the environment's `accounts { }` pool and be free. If it is already leased, or the environment has no pool at all (`LocalMode` invents a name per bot and has none), the block fails with that message rather than quietly running as somebody else. ### A second bot for the block @@ -221,6 +221,40 @@ describe.serial('trading', () => { }); ``` +## Racing bots against each other: `concurrency` + +One bot can't produce a race. Two players opening the same chest at once, three players buying the last item in stock — bugs like that only exist when several bots hit the same feature at the same time, on the same server. `concurrency` runs a test as N independent instances at once, each with its own bot: + +```typescript +test('only one player can claim the chest', { concurrency: 3 }, async ({ player }) => { + player.chat('/claim'); + await expect(player).toHaveReceivedMessage(/Claimed|already claimed/); +}); +``` + +Each instance leases its own account and runs the full test body on its own bot. The test only passes if every instance does — one instance losing the race it wasn't supposed to lose is the bug you're trying to catch, not noise to average away. + +`describe.serial` blocks take the same option, running N independent copies of the whole ordered chain at once: + +```typescript +describe.serial('kit lifecycle', { concurrency: 5 }, () => { + test('claims the starter kit', async ({ player }) => { /* ... */ }); + test('is on cooldown right after', async ({ player }) => { /* ... */ }); +}); +``` + +### The report still has one row per test + +`concurrency` multiplies how many bots run a test, not how many rows it produces in the summary. That row's duration is the slowest instance, and it carries an `instances` array with every instance's own outcome — bot username, pass or fail, duration — so a failure tells you which bot lost, not just that somebody did. + +### How many you can ask for + +`concurrency: N` needs N free accounts. On an environment with an account pool, this is checked before any test in the run starts: ask for `concurrency: 10` against a 4-account pool and it fails immediately with a clear error, instead of the 5th bot hanging on a lease nobody's going to release. `LocalMode` has no pool — it mints a throwaway account per bot — so there's nothing to check there; its only real ceiling is the server's own `max-players`. + +### The server log is still one shared log + +`expect(server)` reads the console output the whole session shares, so one instance's commands sit in that log right next to every other instance's. Nothing filters that for you, and that's deliberate — asserting the server log never produced something no bot should have caused (`expect(server).not.toHaveReceivedMessage('NullPointerException')`) is exactly what a concurrency test is for. If you need one bot's output specifically, either put its username in the pattern or check `expect(player)` instead, since a player's own messages never mix with another bot's. + ## Best Practices 1. **Keep tests isolated** - Each test gets a fresh bot unless it is in a `describe.serial` block @@ -232,7 +266,7 @@ describe.serial('trading', () => { ## Tips -- Tests run sequentially, not in parallel +- Tests run sequentially by default — a test that needs bots racing each other can opt into `concurrency` - Server starts fresh for each test run - Bot automatically connects to the server - Server logs are visible in the console output diff --git a/example_plugin/src/test/e2e/tests/concurrency.spec.ts b/example_plugin/src/test/e2e/tests/concurrency.spec.ts new file mode 100644 index 0000000..12776d0 --- /dev/null +++ b/example_plugin/src/test/e2e/tests/concurrency.spec.ts @@ -0,0 +1,42 @@ +/** + * `concurrency` fans a test (or a `describe.serial` block) out into N independent bots running + * at once — the shape a race between real players needs. These also stand in as regression + * coverage for the two bugs that concurrency exposed in the previously-sequential-only runner: + * - `session.consoleLog` used to be wiped by `clear()` at the start of every test; N bots + * writing into it at once would have raced. Each instance now reads from its own + * `ServerWrapper.startIndex` cursor instead, so its own marker is never missed no matter what + * the other instances are doing to the same shared log. + * - `createBotScope.close()` used to disconnect every bot in the session, not just its own — + * the first instance to finish would have kicked every other still-running instance's bot. + */ + +import { describe, expect, test } from '@plugwright/runner'; + +test( + 'concurrent bots each see their own marker and stay connected', + { concurrency: 3, requires: ['consoleOutput:full'] }, + async ({ player, server }) => { + const marker = `concurrency-marker-${player.username}`; + player.chat(marker); + await expect(server).toHaveReceivedMessage(marker, { timeout: 10000 }); + + // Still connected: an earlier-finishing sibling instance's teardown must not have + // disconnected this one. + await player.teleport(50, 100, 50); + await expect(player).toBeNear(50, 100, 50, { tolerance: 2, timeout: 10000 }); + } +); + +describe.serial('concurrent kit lifecycle', { concurrency: 2 }, () => { + test('claims the starter kit', async ({ player }) => { + player.chat('/kit starter'); + + await expect(player).toHaveReceivedMessage('Received starter kit'); + await expect(player).toContainItem('diamond_sword'); + }); + + test('is on cooldown right after', async ({ player }) => { + player.chat('/kit starter'); + await expect(player).toHaveReceivedMessage('cooldown'); + }); +}); diff --git a/runner-package/lib/matchers.ts b/runner-package/lib/matchers.ts index 6bab347..dd5c522 100644 --- a/runner-package/lib/matchers.ts +++ b/runner-package/lib/matchers.ts @@ -87,11 +87,13 @@ export class RunnerMatchers extends Matchers { // A player's messages are its own (see `PlayerWrapper.messageBuffer`) so one bot's chat // never satisfies an assertion made against another; the server log has no such split, - // it's one console shared by the whole session. + // it's one console shared by the whole session — and never cleared, so a test that + // doesn't pass `since` defaults to its own `ServerWrapper.startIndex` instead of 0. const buffer = this.actual instanceof PlayerWrapper ? this.actual.messageBuffer : session.consoleLog; - const view = (): string[] => buffer.slice(since); + const effectiveSince = since ?? (this.actual instanceof PlayerWrapper ? undefined : this.actual.startIndex); + const view = (): string[] => buffer.slice(effectiveSince); await this.pollAssertion( () => view().some(isMatch), diff --git a/runner-package/lib/reporter.ts b/runner-package/lib/reporter.ts index 8211386..dfa8a8c 100644 --- a/runner-package/lib/reporter.ts +++ b/runner-package/lib/reporter.ts @@ -15,6 +15,16 @@ function statusOf(result: TestResult): 'PASS' | 'FAIL' | 'SKIP' { return result.passed ? 'PASS' : 'FAIL'; } +/** min/avg/max duration across a `concurrency > 1` result's instances. */ +function instanceStats(instances: NonNullable): { min: number; avg: number; max: number } { + const durations = instances.map(i => i.durationMs); + return { + min: Math.min(...durations), + avg: Math.round(durations.reduce((sum, d) => sum + d, 0) / durations.length), + max: Math.max(...durations), + }; +} + export function printTestSummary(testResults: TestResult[]): number { console.log(`\n${pc.bold("=".repeat(40))}`); console.log(pc.bold(' Test Summary')); @@ -55,7 +65,12 @@ export function printTestSummary(testResults: TestResult[]): number { ? pc.yellow(pc.bold(statusPadded)) : pc.red(pc.bold(statusPadded)); const duration = formatDuration(result.durationMs); - console.log(` ${coloredStatus} ${result.testName.padEnd(testWidth)} ${pc.dim(duration.padStart(durationWidth))}`); + // A concurrent test/block's row is one aggregate over N instances — say how many + // passed right in the table, not just in the failed-tests detail below. + const instanceTag = result.instances + ? pc.dim(` [${result.instances.filter(i => i.passed).length}/${result.instances.length}]`) + : ''; + console.log(` ${coloredStatus} ${result.testName.padEnd(testWidth)} ${pc.dim(duration.padStart(durationWidth))}${instanceTag}`); } console.log(separator); @@ -83,6 +98,18 @@ export function printTestSummary(testResults: TestResult[]): number { } } + if (result.instances) { + const { min, avg, max } = instanceStats(result.instances); + console.log(` ${pc.dim(`${result.instances.length} instances: min ${formatDuration(min)} / avg ${formatDuration(avg)} / max ${formatDuration(max)}`)}`); + for (const instance of result.instances) { + const tag = `[${instance.index}/${result.instances.length}]`; + const label = instance.botUsername ?? '?'; + const status = instance.passed ? pc.green('OK') : pc.red('FAIL'); + const detail = instance.error ? pc.red(` ${instance.error.message}`) : ''; + console.log(` ${pc.dim(`- ${tag} ${label}:`)} ${status} ${pc.dim(`(${formatDuration(instance.durationMs)})`)}${detail}`); + } + } + console.log(''); } @@ -126,6 +153,18 @@ export function writeJsonReport(path: string, environmentName: string, testResul error: r.error ? r.error.message : null, skipReason: r.skipReason ?? null, plugin: r.plugin ?? null, + botUsername: r.botUsername ?? null, + // Present when this row aggregates a `concurrency > 1` test/block: every instance's + // own outcome, so a failure names which bot lost the race instead of just that one did. + instances: r.instances + ? r.instances.map(i => ({ + index: i.index, + botUsername: i.botUsername ?? null, + passed: i.passed, + durationMs: i.durationMs, + error: i.error ? i.error.message : null, + })) + : null, })), }; diff --git a/runner-package/lib/server.ts b/runner-package/lib/server.ts index 876423a..bfa32f6 100644 --- a/runner-package/lib/server.ts +++ b/runner-package/lib/server.ts @@ -2,9 +2,21 @@ import type { Session } from './session.js'; export class ServerWrapper { readonly session: Session; + /** Default read cursor for `toHaveReceivedMessage` when no `since` is given — the log index + * at construction time, so a fresh test only sees lines from its own start. Non-destructive + * replacement for the old `session.consoleLog.clear()`: the log itself is never wiped, so + * concurrent tests reading it don't race. */ + startIndex: number; constructor(session: Session) { this.session = session; + this.startIndex = session.consoleLog.length; + } + + /** Moves the default read cursor to "now". Used by a `describe.serial` block between its + * tests, which share one `ServerWrapper` — the block-level equivalent of a fresh one. */ + resetCursor(): void { + this.startIndex = this.session.consoleLog.length; } execute(cmd: string): void { diff --git a/runner-package/lib/session.ts b/runner-package/lib/session.ts index 8e1525d..31d2087 100644 --- a/runner-package/lib/session.ts +++ b/runner-package/lib/session.ts @@ -168,19 +168,25 @@ export class Session { * * Each bot goes through `disconnectBot`, which is also what strips its listeners: a kept * bot is still connected and still listening, so tearing the others down must not be a - * second implementation that forgets to. */ + * second implementation that forgets to. + * + * Snapshots which bots to disconnect before the `await`, then removes exactly those from + * `this.bots` afterward — not "whatever isn't in `keep`" recomputed after the fact. A + * concurrent caller can push a new bot onto `this.bots` while this call is awaiting; that + * bot is in neither snapshot, so it survives here untouched instead of being silently + * dropped from tracking without ever being disconnected. */ async disconnectAllBots(keep: Bot[] = []): Promise { const keepSet = new Set(keep); + const toDisconnect = this.bots.filter(b => !keepSet.has(b)); await Promise.all( - this.bots - .filter(b => !keepSet.has(b)) - .map((b, i) => this.disconnectBot(b, b.username ?? `bot-${i}`, 2000)) + toDisconnect.map((b, i) => this.disconnectBot(b, b.username ?? `bot-${i}`, 2000)) ); - const remaining = this.bots.filter(b => keepSet.has(b)); - this.bots.length = 0; - this.bots.push(...remaining); + const disconnectedSet = new Set(toDisconnect); + for (let i = this.bots.length - 1; i >= 0; i--) { + if (disconnectedSet.has(this.bots[i])) this.bots.splice(i, 1); + } } /** Feeds raw environment output (e.g. Minecraft server stdout/stderr) into the console log buffer. */ diff --git a/runner-package/lib/test-registry.ts b/runner-package/lib/test-registry.ts index 6ec04ec..40a4819 100644 --- a/runner-package/lib/test-registry.ts +++ b/runner-package/lib/test-registry.ts @@ -14,6 +14,11 @@ type TestFn = (context: TestContext) => Promise; export interface TestOptions { requires?: string[]; environments?: string[]; + /** Runs this many independent instances of the test concurrently, each with its own bot + * leased from the account pool, to exercise races between players hitting the same feature + * at once. One failing instance fails the whole test. Defaults to 1 (sequential, no pool + * requirement). Validated against the pool's capacity before any test runs. */ + concurrency?: number; } interface DescribeScope { @@ -32,6 +37,7 @@ export interface TestCase { afterHooks: Hook[]; requires: string[]; environments: string[] | null; + concurrency: number; } /** What a `describe.serial` block accepts beyond the usual filters. */ @@ -50,6 +56,7 @@ export interface SerialBlock { tests: TestCase[]; requires: string[]; environments: string[] | null; + concurrency: number; } export type RegistryItem = @@ -82,12 +89,33 @@ function scopedEntry(name: string, options: TestOptions) { afterHooks: [...scopeStack].reverse().flatMap(s => s.afterHooks), requires: options.requires ?? [], environments: options.environments ?? null, + concurrency: normalizeConcurrency(options.concurrency), }; } +/** `concurrency` must be a whole number of at least 1 — anything else can't be turned into a + * bot count. Checked at registration time so a typo fails on import, not mid-run. */ +function normalizeConcurrency(concurrency: number | undefined): number { + if (concurrency === undefined) return 1; + if (!Number.isInteger(concurrency) || concurrency < 1) { + throw new Error(`concurrency must be a whole number >= 1, got ${concurrency}`); + } + return concurrency; +} + function registerTest(name: string, options: TestOptions, fn: TestFn): void { const testCase = { ...scopedEntry(name, options), fn }; if (currentBlock) { + // A serial block always runs its tests one after another on the same player — fanning + // one of them out into concurrent instances makes no sense and would otherwise be + // silently ignored (the block runner never reads a per-test concurrency), leaving + // whoever set it wondering why nothing ran concurrently. + if (testCase.concurrency > 1) { + throw new Error( + `test "${name}": concurrency is not supported on tests inside describe.serial ` + + `(block "${currentBlock.name}") — set concurrency on the describe.serial block itself instead.` + ); + } currentBlock.tests.push(testCase); } else { testRegistry.push({ kind: 'test', testCase }); @@ -156,6 +184,7 @@ function serialImpl(label: string, optionsOrFn: SerialOptions | (() => void), ma tests: [], requires: options.requires ?? [], environments: options.environments ?? null, + concurrency: normalizeConcurrency(options.concurrency), }; currentBlock = block; diff --git a/runner-package/lib/test-runner.ts b/runner-package/lib/test-runner.ts index a492ca2..cb083fb 100644 --- a/runner-package/lib/test-runner.ts +++ b/runner-package/lib/test-runner.ts @@ -1,4 +1,5 @@ import pc from 'picocolors'; +import type { Bot } from 'mineflayer'; import { PlayerWrapper } from './player.js'; import { ServerWrapper } from './server.js'; import { formatDuration } from './reporter.js'; @@ -10,6 +11,17 @@ import type { BotConnectionOptions } from './environment.js'; import type { SerialBlock, TestCase } from './test-registry.js'; import type { TestContext, TestResult } from './types.js'; +/** Which of a `concurrency > 1` run's N instances this is, for console log labeling — without + * it, several instances logging the same test name at the same time is unreadable. */ +export interface InstanceTag { + index: number; + total: number; +} + +function formatInstanceTag(instance?: InstanceTag): string { + return instance ? pc.dim(` [${instance.index}/${instance.total}]`) : ''; +} + export interface RunTestCaseParams { file: string; testCase: TestCase; @@ -19,6 +31,8 @@ export interface RunTestCaseParams { timeoutMs: number; /** Set when this test came from a plugin's inherited `tests`, for report labeling. */ pluginName?: string | null; + /** Set by `runConcurrentTestCase` on each fanned-out instance, for console log labeling. */ + instance?: InstanceTag; } export interface RunSerialBlockParams { @@ -29,6 +43,8 @@ export interface RunSerialBlockParams { connOpts: BotConnectionOptions; timeoutMs: number; pluginName?: string | null; + /** Set by `runConcurrentSerialBlock` on each fanned-out instance, for console log labeling. */ + instance?: InstanceTag; } /** Bots created while one test, or one `describe.serial` block, is running: who leased what, @@ -44,10 +60,14 @@ interface BotScope { close(): Promise; } -function createBotScope(session: Session, server: ServerWrapper, connOpts: BotConnectionOptions): BotScope { +function createBotScope(session: Session, server: ServerWrapper, connOpts: BotConnectionOptions, instance?: InstanceTag): BotScope { const leased: Array<{ account: Account; pool: AccountPool }> = []; const named = new Map(); const connected: PlayerWrapper[] = []; + // Every bot this scope created, whether or not `player.join()` went on to succeed — unlike + // `connected`, which only gains an entry after a successful join. `close()` needs this one: + // a bot whose join failed still opened a real connection and must still be torn down. + const ownBots: Bot[] = []; const connect = async (options?: { username?: string; account?: string }): Promise => { const pool = options?.username ? null : session.env.accounts?.() ?? null; @@ -63,7 +83,7 @@ function createBotScope(session: Session, server: ServerWrapper, connOpts: BotCo try { const botUsername = account.username; - console.log(`${pc.cyan('[Bot]')} Creating bot: ${pc.bold(botUsername)}`); + console.log(`${pc.cyan('[Bot]')} Creating bot: ${pc.bold(botUsername)}${formatInstanceTag(instance)}`); await session.env.beforeJoin?.(); @@ -73,6 +93,7 @@ function createBotScope(session: Session, server: ServerWrapper, connOpts: BotCo profilesFolder: account.microsoftCacheDir, }; const bot = session.createBot({ ...botOptions, username: botUsername }); + ownBots.push(bot); const player = new PlayerWrapper(bot, session); player._captureSpawnPromise(); player.setServerWrapper(server); @@ -103,11 +124,19 @@ function createBotScope(session: Session, server: ServerWrapper, connOpts: BotCo }, players: () => [...connected], async close(): Promise { - await session.disconnectAllBots(); + // Only this scope's own bots — a concurrent sibling instance's bots are still + // running and must not be torn down by this one finishing first. Union of two + // sources: `ownBots` catches a bot whose join() failed and never made it into + // `connected`; reading `p.bot` fresh off `connected` catches the opposite case — + // `player.rejoin()` swaps a player's `.bot` for a new connection under the same + // scope, and that swapped-in bot isn't the one `ownBots` captured at connect time. + const ownBotsSet = new Set([...ownBots, ...connected.map(p => p.bot)]); + await session.disconnectAllBots(session.bots.filter(b => !ownBotsSet.has(b))); for (const { account, pool } of leased) pool.release(account); leased.length = 0; named.clear(); connected.length = 0; + ownBots.length = 0; }, }; } @@ -175,12 +204,12 @@ async function executeTest(params: ExecuteParams): Promise { await Promise.race([body().finally(() => clearTimeout(timeoutHandle)), timeoutPromise]); } -function reportPassed(durationMs: number): void { - console.log(` ${pc.green(pc.bold('PASSED'))} ${pc.dim(`(${formatDuration(durationMs)})`)}\n`); +function reportPassed(durationMs: number, instance?: InstanceTag): void { + console.log(` ${pc.green(pc.bold('PASSED'))}${formatInstanceTag(instance)} ${pc.dim(`(${formatDuration(durationMs)})`)}\n`); } -function reportFailed(durationMs: number, error: Error): void { - console.log(` ${pc.red(pc.bold('FAILED'))} ${pc.dim(`(${formatDuration(durationMs)})`)}: ${pc.red(error.message)}\n`); +function reportFailed(durationMs: number, error: Error, instance?: InstanceTag): void { + console.log(` ${pc.red(pc.bold('FAILED'))}${formatInstanceTag(instance)} ${pc.dim(`(${formatDuration(durationMs)})`)}: ${pc.red(error.message)}\n`); } /** @@ -188,13 +217,12 @@ function reportFailed(durationMs: number, error: Error): void { * body, and disconnects everything it created on the way out. */ export async function runTestCase(params: RunTestCaseParams): Promise { - const { file, testCase, session, plugins, connOpts, timeoutMs, pluginName = null } = params; + const { file, testCase, session, plugins, connOpts, timeoutMs, pluginName = null, instance } = params; - console.log(` ${pc.bold(`Test: ${testCase.name}`)}`); - session.consoleLog.clear(); + console.log(` ${pc.bold(`Test: ${testCase.name}`)}${formatInstanceTag(instance)}`); const server = new ServerWrapper(session); - const bots = createBotScope(session, server, connOpts); + const bots = createBotScope(session, server, connOpts, instance); const finalizers: Array<() => void | Promise> = []; const startedAt = Date.now(); @@ -203,7 +231,7 @@ export async function runTestCase(params: RunTestCaseParams): Promise { + const { concurrency, ...rest } = params; + if (concurrency <= 1) return runTestCase(rest); + + const instanceResults = await Promise.all( + Array.from({ length: concurrency }, (_, i) => runTestCase({ ...rest, instance: { index: i + 1, total: concurrency } })) + ); + return aggregateInstances(instanceResults); +} + /** * Runs a `describe.serial` block: one player, one connection, its tests in declaration order. * @@ -245,13 +288,12 @@ export async function runTestCase(params: RunTestCaseParams): Promise { - const { file, block, session, plugins, connOpts, timeoutMs, pluginName = null } = params; + const { file, block, session, plugins, connOpts, timeoutMs, pluginName = null, instance } = params; - console.log(` ${pc.bold(`Serial block: ${block.name}`)}${block.account ? pc.dim(` (account ${block.account})`) : ''}`); - session.consoleLog.clear(); + console.log(` ${pc.bold(`Serial block: ${block.name}`)}${block.account ? pc.dim(` (account ${block.account})`) : ''}${formatInstanceTag(instance)}`); const server = new ServerWrapper(session); - const bots = createBotScope(session, server, connOpts); + const bots = createBotScope(session, server, connOpts, instance); const results: TestResult[] = []; let player: PlayerWrapper; @@ -260,7 +302,7 @@ export async function runSerialBlock(params: RunSerialBlockParams): Promise index === 0 ? { file, testName: testCase.name, passed: false, durationMs: 0, error: error as Error, plugin: pluginName } @@ -279,7 +321,7 @@ export async function runSerialBlock(params: RunSerialBlockParams): Promise void | Promise> = []; @@ -320,12 +362,12 @@ export async function runSerialBlock(params: RunSerialBlockParams): Promise { + const { concurrency, ...rest } = params; + if (concurrency <= 1) return runSerialBlock(rest); + + const instanceRuns = await Promise.all( + Array.from({ length: concurrency }, (_, i) => runSerialBlock({ ...rest, instance: { index: i + 1, total: concurrency } })) + ); + return instanceRuns[0].map((_, index) => aggregateInstances(instanceRuns.map(run => run[index]))); +} + +/** + * Rolls up N concurrent runs of the same test/test-position into one `TestResult`. `durationMs` + * is the slowest instance (roughly the wall-clock cost of the `Promise.all`). + * + * Only meaningful for a serial block: its instances can diverge mid-block (one instance's race + * loses and it stops early, skipping the rest, while another keeps going) so a position isn't + * uniformly pass/fail/skip the way a plain concurrent test's instances are. An actual failure in + * any instance fails the whole result; short of that, a position only counts as skipped if every + * instance skipped it — one instance actually exercising it is enough to call it run. + */ +function aggregateInstances(results: TestResult[]): TestResult { + const first = results[0]; + const failed = results.find(r => !r.skipped && !r.passed); + const allSkipped = results.every(r => r.skipped); + return { + file: first.file, + testName: first.testName, + plugin: first.plugin, + passed: !failed, + durationMs: Math.max(...results.map(r => r.durationMs)), + error: failed?.error, + skipped: !failed && allSkipped, + skipReason: !failed && allSkipped ? first.skipReason : undefined, + instances: results.map((r, i) => ({ + index: i + 1, + botUsername: r.botUsername, + passed: r.passed, + durationMs: r.durationMs, + error: r.error, + })), + }; +} diff --git a/runner-package/lib/types.ts b/runner-package/lib/types.ts index 6c979f4..896a17a 100644 --- a/runner-package/lib/types.ts +++ b/runner-package/lib/types.ts @@ -18,6 +18,18 @@ export interface TestContext { cleanup: (fn: () => void | Promise) => void; } +/** One concurrent instance's own outcome, rolled up into the `instances` array of the + * aggregate `TestResult` for a `concurrency > 1` test/block. */ +export interface TestInstanceResult { + /** 1-based position among the N concurrent instances — matches the `[i/N]` tag in the + * console log for this same run. */ + index: number; + botUsername?: string; + passed: boolean; + durationMs: number; + error?: Error; +} + export interface TestResult { file: string; testName: string; @@ -30,4 +42,10 @@ export interface TestResult { skipReason?: string; /** Name of the plugin this test was inherited from, or null for a user spec. */ plugin?: string | null; + /** The bot that ran this test, when one connected. Absent for a skip, or a test that failed + * before it got as far as leasing a bot. */ + botUsername?: string; + /** Set when this result aggregates `concurrency > 1` concurrent instances: `passed` is AND + * across all of them, `durationMs` is the slowest, `error` is the first failure. */ + instances?: TestInstanceResult[]; } diff --git a/runner-package/runner.ts b/runner-package/runner.ts index 07f14c7..255c381 100644 --- a/runner-package/runner.ts +++ b/runner-package/runner.ts @@ -7,7 +7,7 @@ import { ItemWrapper, GuiWrapper, LiveGuiHandle, GuiItemLocator } from './lib/wr import { testRegistry, resetRegistry } from './lib/test-registry.js'; import { Session } from './lib/session.js'; import { PluginHost } from './lib/plugin-host.js'; -import { runSerialBlock, runTestCase } from './lib/test-runner.js'; +import { runSerialBlock, runTestCase, runConcurrentSerialBlock, runConcurrentTestCase } from './lib/test-runner.js'; import { skipReasonForOptions } from './lib/skip-reason.js'; import { LocalEnvironment } from './lib/environments/local.js'; import { externalEnvironment } from './lib/environments/external.js'; @@ -19,7 +19,7 @@ import type { Environment } from './lib/environment.js'; import type { EnvironmentConfig, LocalEnvironmentConfig, RunnerConfig } from './lib/config.js'; import type { ExternalEnvironmentConfig } from './lib/environments/external.js'; import type { TestResult } from './lib/types.js'; -import type { SerialBlock, TestCase } from './lib/test-registry.js'; +import type { SerialBlock, TestCase, RegistryItem } from './lib/test-registry.js'; import type { Account, AccountPool } from './lib/account.js'; // Enable source map support for accurate TypeScript stack traces @@ -147,14 +147,54 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): return null; } - /** Imports one compiled spec file (a fresh `testRegistry`) and runs everything it - * registered, appending results to `testResults`. Shared by user specs and every - * plugin-inherited test file. */ - async function runFile(file: string, pluginName: string | null): Promise { + /** One imported spec file's registered tests, snapshotted right after import so its + * concurrency values can be validated before any file's tests run — re-importing later + * to re-check wouldn't work anyway: ESM caches the module, so a second `import()` of + * the same file wouldn't re-run its top-level `test()`/`describe()` calls. */ + interface LoadedFile { + file: string; + pluginName: string | null; + items: RegistryItem[]; + } + + async function loadFile(file: string, pluginName: string | null): Promise { resetRegistry(); await import(pathToFileURL(file).href); + return { file, pluginName, items: [...testRegistry] }; + } - for (const item of testRegistry) { + /** Fails fast, before any test in the session runs, on a `concurrency` the account pool + * here can't satisfy — rather than the test itself blocking on its Nth `pool.lease()`. An + * environment with no pool (e.g. `LocalMode`) mints a synthetic throwaway account per + * connection instead of leasing one, so there's no pool capacity to check against; its + * ceiling is the server's own `max-players`, which is on the operator, not this check. */ + function validateConcurrency(loaded: LoadedFile[]): void { + const pool = env.accounts?.() ?? null; + if (!pool) return; + const capacity = pool.capacity(); + + for (const { file, items } of loaded) { + for (const item of items) { + const [kind, name, concurrency] = item.kind === 'serial' + ? ['describe.serial', item.block.name, item.block.concurrency] as const + : ['test', item.testCase.name, item.testCase.concurrency] as const; + if (concurrency <= 1) continue; + if (concurrency > capacity) { + throw new Error( + `${kind} "${name}" (${file}) declares concurrency: ${concurrency}, exceeding the ` + + `account pool's capacity (${capacity}). Reduce concurrency or grow the pool.` + ); + } + } + } + } + + /** Runs everything one loaded file registered, appending results to `testResults`. + * Shared by user specs and every plugin-inherited test file. */ + async function runLoadedFile(loaded: LoadedFile): Promise { + const { file, pluginName, items } = loaded; + + for (const item of items) { if (item.kind === 'serial') { const { block } = item; const skipReason = blockSkipReason(block); @@ -166,7 +206,10 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): continue; } - testResults.push(...await runSerialBlock({ file, block, session, plugins, connOpts, timeoutMs, pluginName })); + const results = block.concurrency > 1 + ? await runConcurrentSerialBlock({ file, block, session, plugins, connOpts, timeoutMs, pluginName, concurrency: block.concurrency }) + : await runSerialBlock({ file, block, session, plugins, connOpts, timeoutMs, pluginName }); + testResults.push(...results); continue; } @@ -178,20 +221,31 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): continue; } - const result = await runTestCase({ file, testCase, session, plugins, connOpts, timeoutMs, pluginName }); + const result = testCase.concurrency > 1 + ? await runConcurrentTestCase({ file, testCase, session, plugins, connOpts, timeoutMs, pluginName, concurrency: testCase.concurrency }) + : await runTestCase({ file, testCase, session, plugins, connOpts, timeoutMs, pluginName }); testResults.push(result); } } + const preflightEntries = [...plugins.testFiles('preflight')]; + const loadedPreflight: LoadedFile[] = []; + for (const { file, pluginName } of preflightEntries) loadedPreflight.push(await loadFile(file, pluginName)); + + // Preflight files are loaded and validated on their own, before any main/suite spec + // file is imported — importing those here would run their top-level code ahead of + // preflight, against whatever state preflight was going to set up during execution. + validateConcurrency(loadedPreflight); + // Preflight: plugin auth/setup tests, run before anything else. A failure aborts the // whole session. - for (const { file, pluginName } of plugins.testFiles('preflight')) { - console.log(`\n${pc.blue(pc.bold(`Running preflight tests from: ${file} ${pc.dim(`(plugin ${pluginName})`)}`))}`); + for (const loaded of loadedPreflight) { + console.log(`\n${pc.blue(pc.bold(`Running preflight tests from: ${loaded.file} ${pc.dim(`(plugin ${loaded.pluginName})`)}`))}`); const before = testResults.length; - await runFile(file, pluginName); + await runLoadedFile(loaded); const failed = testResults.slice(before).find(r => !r.skipped && !r.passed); if (failed) { - throw new Error(`Preflight test "${failed.testName}" failed (plugin ${pluginName}): ${failed.error?.message ?? 'unknown error'}`); + throw new Error(`Preflight test "${failed.testName}" failed (plugin ${loaded.pluginName}): ${failed.error?.message ?? 'unknown error'}`); } } @@ -208,18 +262,30 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): }) ); } + const loadedMain: LoadedFile[] = []; + for (const file of testFiles) loadedMain.push(await loadFile(file, null)); + + const suiteEntries = [...plugins.testFiles('suite')]; + const loadedSuite: LoadedFile[] = []; + for (const { file, pluginName } of suiteEntries) loadedSuite.push(await loadFile(file, pluginName)); + + // Main and suite files are loaded (imported once, registrations snapshotted) before any + // of them runs, so a misconfigured `concurrency` aborts here instead of after burning + // time on earlier tests. Preflight has already run by this point, so this no longer + // imports them ahead of the state preflight sets up. + validateConcurrency([...loadedMain, ...loadedSuite]); - console.log(`${pc.bold(`Found ${testFiles.length} test file(s)${testFileFilters ? ` matching filter: ${testFileFilters.join(',')}` : ''}`)}\n`); + console.log(`${pc.bold(`Found ${loadedMain.length} test file(s)${testFileFilters ? ` matching filter: ${testFileFilters.join(',')}` : ''}`)}\n`); - for (const file of testFiles) { - console.log(`\n${pc.blue(pc.bold(`Running tests from: ${file}`))}`); - await runFile(file, null); + for (const loaded of loadedMain) { + console.log(`\n${pc.blue(pc.bold(`Running tests from: ${loaded.file}`))}`); + await runLoadedFile(loaded); } // Suite: plugin tests that run alongside user specs, tagged with the plugin's name. - for (const { file, pluginName } of plugins.testFiles('suite')) { - console.log(`\n${pc.blue(pc.bold(`Running tests from: ${file} ${pc.dim(`(plugin ${pluginName})`)}`))}`); - await runFile(file, pluginName); + for (const loaded of loadedSuite) { + console.log(`\n${pc.blue(pc.bold(`Running tests from: ${loaded.file} ${pc.dim(`(plugin ${loaded.pluginName})`)}`))}`); + await runLoadedFile(loaded); } } finally {