From cdd721f85bf4adcdd350c8f23d2e929298d77623 Mon Sep 17 00:00:00 2001 From: Monikon Date: Mon, 24 Aug 2026 15:13:29 +0300 Subject: [PATCH 01/10] fix(npm): don't cut git/URL package specs at the last @ npmPackageNameOf assumed the last @ was always the version separator. git+ssh://git@host/repo and https://user:pass@host/pkg carry @ of their own, so two different URL specs got truncated to the same wrong key and collided in the map. Reported by Drownek on PR #50. --- .../kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt index 2948ef7..aecce6f 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt @@ -298,8 +298,11 @@ class PlugwrightCorePlugin : Plugin { } /** `@scope/name@^1.0.0` → `@scope/name`; the version separator is the last `@`, which for - * a scoped package is never the leading one. */ + * a scoped package is never the leading one. A git/URL spec (`git+ssh://git@host/repo`, + * `https://user:pass@registry/pkg`) carries its own `@`s that aren't a version separator + * at all, so it is returned as-is instead of being cut at the last one. */ private fun npmPackageNameOf(spec: String): String { + if (spec.contains("://") || spec.startsWith("git+")) return spec val separator = spec.lastIndexOf('@') return if (separator > 0) spec.substring(0, separator) else spec } From 1ff9119a2e9638d6e10fae901e2a3de5910cfd27 Mon Sep 17 00:00:00 2001 From: Monikon Date: Mon, 24 Aug 2026 15:13:36 +0300 Subject: [PATCH 02/10] fix(npm): insert call after cmd /c to stop it eating outer quotes cmd.exe strips the outer pair of quotes from the whole command line when the line starts with a quote and holds more than two quotes total. quoteForCmd now adds quotes to protect ^ in version ranges, so a spaced npm.cmd path (already quoted by Java) plus one quoted argument hits that case and cmd mangles the path. call makes the line start with a letter instead of a quote, which cmd doesn't touch. Reported by Drownek on PR #50. --- .../main/kotlin/me/drownek/plugwright/AbstractNodeTask.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/AbstractNodeTask.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/AbstractNodeTask.kt index 9170071..c9f09e8 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/AbstractNodeTask.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/AbstractNodeTask.kt @@ -49,7 +49,11 @@ abstract class AbstractNodeTask : DefaultTask() { val isWindows = System.getProperty("os.name").lowercase().contains("win") val cmdName = File(command[0]).nameWithoutExtension.lowercase() val cmd = if (isWindows && (cmdName == "npm" || cmdName == "node")) { - listOf("cmd", "/c") + command.map { quoteForCmd(it) } + // "call" after /c so the line handed to cmd starts with a letter, not a quote: + // when it starts with a quote and holds more than two quotes total (guaranteed + // once quoteForCmd wraps an argument), cmd strips the outer pair itself, mangling + // a spaced npm.cmd path Java already quoted. + listOf("cmd", "/c", "call") + command.map { quoteForCmd(it) } } else { command.toList() } From f457e44004cac1b0fdbc096e53b321b2e034e422 Mon Sep 17 00:00:00 2001 From: Monikon Date: Sun, 16 Aug 2026 13:21:48 +0300 Subject: [PATCH 03/10] feat(runner): reuse a connected player across test boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an opt-in mechanism to hand a test a bot that survived from an earlier test instead of reconnecting for every one. Matching goes by ability labels (op, gamemode:*, and anything a plugin marks by hand) rather than resetting server state, since the runner has no way to undo what a command changed. - lib/player.ts: abilities set, mark()/unmark(); makeOp/deOp/ setGameMode label automatically. makeOp also now recognizes an already-op player on a stdio/full console, which reuse can hand it a second time. - lib/player-registry.ts: PlayerRegistry — resolves a request against free entries, evicts LRU at maxPlayers, transparently rejoins a dead connection before handing it out, invalidates on failure. - lib/session.ts: owns the registry; disconnectAllBots takes a keep list so a per-test sweep leaves other live registry entries alone. - lib/test-registry.ts, lib/types.ts, lib/plugin.ts: reuse option on test()/describe(), TestContext.invalidatePlayer, TestResult.reuse, PlugwrightPlugin.onPlayerReuse, PluginTestRef.reuse. - lib/test-runner.ts, runner.ts: resolves the primary player and any createPlayer() call through the registry when reuse applies; computes the default maxPlayers from account pool capacity; reports a per-test reuse summary line. - lib/config.ts: tests.reuse config, PLUGWRIGHT_REUSE env override. - gradle-plugin: reuse { enabled, maxPlayers } extension block, threaded through to every environment's tests.reuse. - auth-authme-package: preflight declares reuse: false — its whole point is proving the login flow runs, which a reused, already authenticated player would skip. - docs: reuse coverage across writing-tests, configuration, test-filtering, plugins, external-servers, custom-modes, reports. Verified against example_plugin's local suite both ways (PLUGWRIGHT_REUSE=1 and unset): 47/47 either way. Three specs needed reuse: false / excludeAbilities to keep their isolation assumptions once reuse was on, annotated with why. --- auth-authme-package/index.ts | 4 +- docs/configuration.mdx | 17 ++ docs/custom-modes.mdx | 5 +- docs/external-servers.mdx | 6 + docs/plugins.mdx | 21 ++- docs/reports.mdx | 8 +- docs/test-filtering.mdx | 4 + docs/writing-tests.mdx | 48 ++++- .../src/test/e2e/tests/events.spec.ts | 4 +- .../src/test/e2e/tests/kits.spec.ts | 4 +- .../src/test/e2e/tests/player-wrapper.spec.ts | 4 +- .../src/test/e2e/tests/shop.spec.ts | 4 +- .../plugwright/PlugwrightCorePlugin.kt | 6 + .../drownek/plugwright/PlugwrightExtension.kt | 8 + .../plugwright/PlugwrightMatrixTask.kt | 4 + .../drownek/plugwright/PlugwrightTestTask.kt | 13 ++ .../kotlin/me/drownek/plugwright/ReuseSpec.kt | 24 +++ .../me/drownek/plugwright/RunnerLauncher.kt | 10 + runner-package/lib/account.ts | 7 + runner-package/lib/config.ts | 37 +++- runner-package/lib/environment.ts | 3 + runner-package/lib/player-registry.ts | 168 +++++++++++++++++ runner-package/lib/player.ts | 65 ++++++- runner-package/lib/plugin-host.ts | 10 +- runner-package/lib/plugin.ts | 8 + runner-package/lib/reporter.ts | 1 + runner-package/lib/session.ts | 24 ++- runner-package/lib/test-registry.ts | 21 ++- runner-package/lib/test-runner.ts | 171 +++++++++++++----- runner-package/lib/types.ts | 8 +- runner-package/runner.ts | 56 +++++- 31 files changed, 695 insertions(+), 78 deletions(-) create mode 100644 gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/ReuseSpec.kt create mode 100644 runner-package/lib/player-registry.ts diff --git a/auth-authme-package/index.ts b/auth-authme-package/index.ts index 76ff2ed..122ff6e 100644 --- a/auth-authme-package/index.ts +++ b/auth-authme-package/index.ts @@ -59,7 +59,9 @@ let resolved: Required> & { password?: strin export default definePlugin({ name: 'authme', apiVersion: 1, - tests: [{ file: join(__dirname, 'auth.spec.js'), mode: 'preflight' }], + // Preflight exists to prove the login/register flow actually runs — a reused, already + // authenticated player would skip straight past what this test checks. + tests: [{ file: join(__dirname, 'auth.spec.js'), mode: 'preflight', reuse: false }], setup({ options }) { resolved = { ...DEFAULTS, ...options }; diff --git a/docs/configuration.mdx b/docs/configuration.mdx index 83b5014..91d89a0 100644 --- a/docs/configuration.mdx +++ b/docs/configuration.mdx @@ -273,6 +273,19 @@ matrix { } ``` + + Settings for reusing a connected bot across test boundaries instead of reconnecting for every test. `enabled` is `false` by default — an existing suite that depends on a fresh player per test keeps working unchanged until it opts in. `maxPlayers` caps live registry entries; unset falls back to 4, or the environment's account pool capacity minus one when it has a pool. + + +```kotlin +reuse { + enabled.set(true) + maxPlayers.set(4) +} +``` + +`PLUGWRIGHT_REUSE=1` / `PLUGWRIGHT_REUSE=0` overrides `reuse.enabled` from the environment, for trying it in a dev loop without editing a committed build script. See [Writing Tests](/writing-tests) for the test-side API. + Per-environment, inside `create(...) { }`: @@ -309,4 +322,8 @@ plugins { Set `PLUGWRIGHT_DEBUG=1` in your environment to enable verbose debug logging during test execution. This is particularly useful for troubleshooting GUI flows and inspecting window open/close events from the bot. + + Overrides `reuse.enabled` for this run: `1`/`true` turns it on, `0`/`false` turns it off. Unset, or any other value, leaves the build script's own setting alone. + + diff --git a/docs/custom-modes.mdx b/docs/custom-modes.mdx index 6a43ecb..37e336a 100644 --- a/docs/custom-modes.mdx +++ b/docs/custom-modes.mdx @@ -152,6 +152,9 @@ class VelocityEnvironment implements Environment { arbitraryUsernames: true, lifecycle: true, cleanupStrategy: 'compensating', + // Absent means "allowed". Set this to false if a bot sitting connected between + // tests would break the environment (an idle-kick timeout, a per-test world reset). + playerReuse: true, }; async setup(session: Session): Promise { /* connect, probe, warm up */ } @@ -163,7 +166,7 @@ class VelocityEnvironment implements Environment { } ``` -Capabilities are a promise the runner holds you to. Tests declaring `requires: ['op']` are skipped when you report `op: false`, so report what is true after `setup()` rather than what the build script hoped for. `consoleOutput` is three-valued (`full`, `responses`, `none`) because a console that answers its own commands still cannot show a test the server log. +Capabilities are a promise the runner holds you to. Tests declaring `requires: ['op']` are skipped when you report `op: false`, so report what is true after `setup()` rather than what the build script hoped for. `consoleOutput` is three-valued (`full`, `responses`, `none`) because a console that answers its own commands still cannot show a test the server log. `playerReuse: false` overrides `tests.reuse.enabled` for the whole run against this environment — set it when a long-lived bot would break something the environment can't tell tests about any other way. `accounts()` and `beforeJoin()` are optional. Returning no pool means every bot gets a throwaway `Test_` username, which is what `local` does. diff --git a/docs/external-servers.mdx b/docs/external-servers.mdx index f8dc16e..4e211e6 100644 --- a/docs/external-servers.mdx +++ b/docs/external-servers.mdx @@ -94,6 +94,12 @@ That is a request for a specific identity, not for whatever is free — so nothi A leased account comes back with the previous test's inventory, balance and op status. Nothing resets it for you. Reset what you can in a plugin's `beforeEach`, exclude what you can't, and treat `capabilities.freshState = false` as the honest description it is. +## Reuse and the account pool + +With `tests.reuse` on ([Configuration](/configuration)), a registry entry holds its leased account for as long as the entry lives, not just for one test — an account checked out by a long-lived player doesn't return to the pool until that player is evicted, invalidated, or the run ends. Size `accounts { }` accordingly: `maxPlayers` defaults to the pool's capacity minus one so a test's own `createPlayer()` still has a spare slot, but a pool exactly as big as `maxPlayers` leaves nothing free for it. + +An environment that can't tolerate a bot sitting connected between tests — an idle-kick timeout, a per-test world reset — should report `capabilities.playerReuse = false` after `setup()` rather than let reuse quietly misbehave. See [Writing a Mode](/custom-modes). + ## Checking the stand before you test ```bash diff --git a/docs/plugins.mdx b/docs/plugins.mdx index a9dc070..f09dfb9 100644 --- a/docs/plugins.mdx +++ b/docs/plugins.mdx @@ -32,11 +32,12 @@ export interface PlugwrightPlugin { apiVersion?: number; setup?(ctx: { session, env, options: O }): Promise | void; onPlayerCreate?(player, ctx: { account, env }): Promise | void; + onPlayerReuse?(player, ctx: { account, env }): Promise | void; beforeEach?(ctx: TestContext): Promise | void; afterEach?(ctx: TestContext): Promise | void; extendContext?(ctx: TestContext): Record | void; matchers?: Record; - tests?: Array<{ file: string; mode: 'preflight' | 'suite' }>; + tests?: Array<{ file: string; mode: 'preflight' | 'suite'; reuse?: false }>; cleanup?(ctx: { session, scope: 'session' | 'manual' }): Promise | void; teardown?(): Promise | void; } @@ -71,6 +72,24 @@ export default definePlugin({ `onPlayerCreate` fires on every connection: the first bot of a test, a second bot from `createPlayer()`, every `player.rejoin()`, and the admin-bot console channel. A "log in first" test fires once, in whatever order the spec files happen to load, and leaves every other connection unauthenticated. If you want the visible reassurance of a login test in the report, ship one as a `preflight` test alongside the hook. +## Reuse + +```ts +export interface PlugwrightPlugin { + onPlayerReuse?(player, ctx: { account, env }): Promise | void; +} +``` + +Fires before a reused player is handed to the next test — never on the first connection, where `onPlayerCreate` already runs. By the time it fires, the core has already done its own safe minimum (closing a leftover open window); anything beyond that — clearing a hotbar, resetting a scoreboard value your plugin tracks — is yours to do here. See [Writing Tests](/writing-tests) for the test-side `reuse` option and ability labels. + +An auth plugin's preflight exists to prove the login flow runs, so a reused, already-authenticated player would defeat the point: + +```ts +tests: [{ file: join(__dirname, 'auth.spec.js'), mode: 'preflight', reuse: false }] +``` + +`PluginTestRef.reuse: false` forces every test in that file onto a fresh connection, regardless of the run's own `reuse` setting. + ## Inherited tests ```ts diff --git a/docs/reports.mdx b/docs/reports.mdx index a125849..2b3432e 100644 --- a/docs/reports.mdx +++ b/docs/reports.mdx @@ -25,7 +25,8 @@ build/reports/plugwright/.log per-environment output, matrix runs o "durationMs": 63, "error": null, "skipReason": null, - "plugin": null + "plugin": null, + "reuse": { "key": "auto:[]!()", "reused": true, "abilities": [] } }, { "file": "…/dist/simple-ts.spec.js", @@ -34,7 +35,8 @@ build/reports/plugwright/.log per-environment output, matrix runs o "durationMs": 0, "error": null, "skipReason": "requires capability [consoleOutput:full], unavailable on \"staging\"", - "plugin": null + "plugin": null, + "reuse": null } ] } @@ -42,6 +44,8 @@ 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. +`reuse` is `null` when `tests.reuse` is off for the run. Otherwise it's always present, even for a test that opted out with `reuse: false` (`reused: false`, `key: "none"`). `reused: true` means the player came from an earlier test instead of a fresh connection; `abilities` is the label set it was matched against. See [Writing Tests](/writing-tests). + Every skip carries its reason: excluded by name, wrong environment, or a capability the environment doesn't have. A skipped test that doesn't say why is worse than a failing one, because it reads as coverage. ## JUnit XML diff --git a/docs/test-filtering.mdx b/docs/test-filtering.mdx index f3ebc12..2e61c88 100644 --- a/docs/test-filtering.mdx +++ b/docs/test-filtering.mdx @@ -77,6 +77,10 @@ test('the /debug dev command', { environments: ['local'] }, async ({ player }) = }); ``` +## Reuse is a different axis + +`requires` and `environments` decide whether a test runs at all. `reuse` (see [Writing Tests](/writing-tests)) decides which bot it gets once it's already running — a filter never skips a test because of its `reuse` option. The two do interact on one environment setting: `capabilities.playerReuse === false` disables reuse for the whole environment without affecting anything a test declared through `requires`. + ## Skips are reported Every skipped test lands in the report with its reason: diff --git a/docs/writing-tests.mdx b/docs/writing-tests.mdx index 8d59a41..c2f5972 100644 --- a/docs/writing-tests.mdx +++ b/docs/writing-tests.mdx @@ -123,9 +123,55 @@ test('advanced waiting', async ({ player }) => { }); ``` +## Player Reuse + +By default, every test gets a fresh bot and disconnects it when the test ends. When `tests.reuse` is on ([Configuration](/configuration)), a test can ask for a bot that survived from an earlier test instead of reconnecting: + +```typescript +test('shows prices', async ({ player }) => { + // same long-lived player as any other default-reuse test, if one is free +}); + +opTest('admin can edit', async ({ player }) => { + // matched to a player already carrying the `op` label — makeOp() only runs if none does +}); + +test('regular player cannot edit', { reuse: { excludeAbilities: ['op'] } }, async ({ player }) => { + // explicitly asks for a player that is NOT op, even if an op'd one is sitting free +}); + +test('first join flow', { reuse: false }, async ({ player }) => { + // always a brand-new connection, regardless of the run's reuse setting +}); +``` + +Matching goes by **ability labels**, not by resetting server state — the runner has no way to undo what a command changed, so it doesn't pretend to. `player.makeOp()`, `player.deOp()` and `player.setGameMode()` label the player automatically (`op`, `gamemode:creative`, …); anything else needs an explicit `player.mark('kit:starter')` / `player.unmark(...)`. Read the current set with `player.abilities`. + +```typescript +export interface ReuseOptions { + key?: string; // explicit identity — same bot every time, e.g. the second player in a multiplayer test + abilities?: string[]; // player must carry all of these + excludeAbilities?: string[]; // player must carry none of these + strict?: boolean; // player's labels must equal `abilities` exactly, no extras +} +``` + +`reuse` on `test()` accepts `false`, a string (shorthand for `{ key }`), or a `ReuseOptions` object. `ctx.createPlayer({ reuse: … })` takes the same shape for any secondary player a test creates. + +A test that cares about a clean nick, the absence of a label, or a first-registration flow declares `reuse: false` (or the right `excludeAbilities`) explicitly — reuse never guesses on a test's behalf. + +```typescript +test('cleans up on failure', async ({ player, invalidatePlayer }) => { + // ... + if (somethingLeftThePlayerInABadState) invalidatePlayer(player); +}); +``` + +`invalidatePlayer` marks a player unfit for the next test: it disconnects instead of being handed out again. The runner does this automatically for a test that fails or times out — one bad test shouldn't hand its mess to the next one. + ## Best Practices -1. **Keep tests isolated** - Each test gets a fresh bot +1. **Keep tests isolated** - Each test gets a fresh bot, unless reuse is on 2. **Use descriptive names** - Make test failures easy to understand 3. **Wait for conditions** - Use assertions that auto-retry 4. **Test one thing** - Each test should verify one behavior diff --git a/example_plugin/src/test/e2e/tests/events.spec.ts b/example_plugin/src/test/e2e/tests/events.spec.ts index a91ffbc..cb5bad9 100644 --- a/example_plugin/src/test/e2e/tests/events.spec.ts +++ b/example_plugin/src/test/e2e/tests/events.spec.ts @@ -1,6 +1,8 @@ import { test, expect } from '@drownek/plugwright'; -test('player receives item on first join', async ({ player }) => { +// Depends on the join itself, not just on a player's current state, so it always needs a +// brand-new connection — reuse would hand it a player who already joined once before. +test('player receives item on first join', { reuse: false }, async ({ player }) => { await expect(player).toHaveReceivedMessage('Welcome'); await expect(player).toContainItem('wooden_sword'); }); diff --git a/example_plugin/src/test/e2e/tests/kits.spec.ts b/example_plugin/src/test/e2e/tests/kits.spec.ts index 9874194..9a79156 100644 --- a/example_plugin/src/test/e2e/tests/kits.spec.ts +++ b/example_plugin/src/test/e2e/tests/kits.spec.ts @@ -14,7 +14,9 @@ test('kit has cooldown', async ({ player }) => { await expect(player).toHaveReceivedMessage('cooldown'); }); -test('VIP kit requires permission', async ({ player }) => { +// Op bypasses permission checks in Bukkit by default, so this only proves anything against a +// player that isn't one. +test('VIP kit requires permission', { reuse: { excludeAbilities: ['op'] } }, async ({ player }) => { player.chat('/kit vip'); await expect(player).toHaveReceivedMessage('no permission'); }); diff --git a/example_plugin/src/test/e2e/tests/player-wrapper.spec.ts b/example_plugin/src/test/e2e/tests/player-wrapper.spec.ts index b43aec9..d354ce6 100644 --- a/example_plugin/src/test/e2e/tests/player-wrapper.spec.ts +++ b/example_plugin/src/test/e2e/tests/player-wrapper.spec.ts @@ -4,7 +4,9 @@ import { expect, test } from '@drownek/plugwright'; -test('makeOp', async ({ player }) => { +// Needs a player that isn't already op, or the server never sends the "Made ... a server +// operator" confirmation this test checks for. +test('makeOp', { reuse: { excludeAbilities: ['op'] } }, async ({ player }) => { // This executes op server command, and we wait for response from server // so when await completes, we are sure player is op. await player.makeOp(); diff --git a/example_plugin/src/test/e2e/tests/shop.spec.ts b/example_plugin/src/test/e2e/tests/shop.spec.ts index 0b0ef34..5e8de4d 100644 --- a/example_plugin/src/test/e2e/tests/shop.spec.ts +++ b/example_plugin/src/test/e2e/tests/shop.spec.ts @@ -19,7 +19,9 @@ test('purchase item from shop', async ({ player }) => { await expect(player).toContainItem('diamond'); }); -test('cannot buy without money', async ({ player }) => { +// Depends on starting with no currency, which a reused player carried over from an earlier +// test can't promise — a fresh connection is the only way to guarantee it. +test('cannot buy without money', { reuse: false }, async ({ player }) => { player.chat('/shop'); const gui = await player.gui({ title: 'Shop' }); await gui.locator(item => item.name === 'diamond').click(); diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt index aecce6f..2b3aa20 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt @@ -210,6 +210,10 @@ class PlugwrightCorePlugin : Plugin { runtimePackage.set(ref.name) ref.export?.let { runtimeExport.set(it) } } + if (extension.reuse.enabled.get()) { + reuseEnabled.set(true) + extension.reuse.maxPlayers.orNull?.let { reuseMaxPlayers.set(it) } + } if (project.hasProperty("testFiles")) testFiles.set(project.property("testFiles") as String) if (project.hasProperty("testNames")) testNames.set(project.property("testNames") as String) @@ -261,6 +265,8 @@ class PlugwrightCorePlugin : Plugin { journalFile = journalFilePath, runtimePackage = runtimeRef?.name, runtimeExport = runtimeRef?.export, + reuseEnabled = extension.reuse.enabled.get().takeIf { it }, + reuseMaxPlayers = extension.reuse.maxPlayers.orNull, ) ctx.prepareTaskRef?.let { matrixPrepareTasks += it } } diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightExtension.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightExtension.kt index b6adc04..a7fdd41 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightExtension.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightExtension.kt @@ -59,6 +59,14 @@ abstract class PlugwrightExtension(project: Project) : LegacyEnvironmentProperti matrix.action() } + /** Settings for reusing a connected bot across test boundaries. See [reuse]. */ + val reuse: ReuseSpec = project.objects.newInstance(ReuseSpec::class.java) + + /** Configures reuse: `reuse { enabled.set(true); maxPlayers.set(4) }`. */ + fun reuse(action: ReuseSpec.() -> Unit) { + reuse.action() + } + /** Registries the workspace installs from, and the credentials for them. See [npm]. */ val npm: NpmSpec = NpmSpec() diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightMatrixTask.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightMatrixTask.kt index ad3e9fa..9e7c4f0 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightMatrixTask.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightMatrixTask.kt @@ -30,6 +30,8 @@ internal data class MatrixEnvironmentInput( val journalFile: File?, val runtimePackage: String? = null, val runtimeExport: String? = null, + val reuseEnabled: Boolean? = null, + val reuseMaxPlayers: Int? = null, ) private data class EnvironmentSummary(val total: Int, val passed: Int, val failed: Int, val skipped: Int, val durationMs: Long) @@ -128,6 +130,8 @@ abstract class PlugwrightMatrixTask : AbstractNodeTask() { journalFile = env.journalFile, runtimePackage = env.runtimePackage, runtimeExport = env.runtimeExport, + reuseEnabled = env.reuseEnabled, + reuseMaxPlayers = env.reuseMaxPlayers, ) RunnerLauncher.writeConfig(entry) val cliJs = RunnerLauncher.resolveCliJs(env.workspaceDir) diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightTestTask.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightTestTask.kt index 0578217..5fa44ba 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightTestTask.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightTestTask.kt @@ -49,6 +49,17 @@ abstract class PlugwrightTestTask : AbstractNodeTask() { @get:Optional abstract val excludeTests: ListProperty + /** From `plugwright.reuse.enabled`. Unset means "reuse off", matching a config with no + * `tests.reuse` key at all. */ + @get:Input + @get:Optional + abstract val reuseEnabled: Property + + /** From `plugwright.reuse.maxPlayers`. Unset means "runner default". */ + @get:Input + @get:Optional + abstract val reuseMaxPlayers: Property + /** * The mode-specific part of the runner config (`environment.config`). Set by the plugin * from either [me.drownek.plugwright.api.PlugwrightMode.serialize] or the mode's own @@ -130,6 +141,8 @@ abstract class PlugwrightTestTask : AbstractNodeTask() { journalFile = journalFile.orNull?.asFile, runtimePackage = runtimePackage.orNull, runtimeExport = runtimeExport.orNull, + reuseEnabled = reuseEnabled.orNull, + reuseMaxPlayers = reuseMaxPlayers.orNull, ) RunnerLauncher.writeConfig(entry) logger.lifecycle("Runner config: ${configDestination.absolutePath}") diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/ReuseSpec.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/ReuseSpec.kt new file mode 100644 index 0000000..b3c40fc --- /dev/null +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/ReuseSpec.kt @@ -0,0 +1,24 @@ +package me.drownek.plugwright + +import org.gradle.api.provider.Property + +/** + * Settings for reusing a connected bot across test boundaries instead of reconnecting for + * every test. Written into every environment's `tests.reuse`, the same way `MatrixSpec` + * configures the matrix run rather than any one environment. + */ +abstract class ReuseSpec { + + /** Off by default: a project turns this on deliberately, so an existing suite that + * depends on a fresh player per test (a unique nick, no leftover op) keeps working + * unchanged until it opts in. */ + abstract val enabled: Property + + /** Live registry entries allowed at once. Unset means "runner default" — 4, or the + * environment's account pool capacity minus one when it has a pool. */ + abstract val maxPlayers: Property + + init { + enabled.convention(false) + } +} diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/RunnerLauncher.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/RunnerLauncher.kt index 31abd16..ef16307 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/RunnerLauncher.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/RunnerLauncher.kt @@ -38,6 +38,10 @@ object RunnerLauncher { val runtimeExport: String? = null, /** Crash-recovery journal path for `Session.journal`; null disables on-disk persistence. */ val journalFile: File? = null, + /** null means "reuse off", matching a config with no `tests.reuse` key at all. */ + val reuseEnabled: Boolean? = null, + /** null means "runner default" — only meaningful when [reuseEnabled] is true. */ + val reuseMaxPlayers: Int? = null, ) fun writeConfig(entry: Entry) { @@ -66,6 +70,12 @@ object RunnerLauncher { if (entry.excludeTests.isNotEmpty()) putStrings("exclude", entry.excludeTests) else putNull("exclude") // null means "runner default", which TEST_TIMEOUT can still override. putNull("timeoutMs") + if (entry.reuseEnabled != null) { + obj("reuse") { + put("enabled", entry.reuseEnabled) + entry.reuseMaxPlayers?.let { put("maxPlayers", it) } + } + } } if (entry.jsonReportFile != null || entry.junitReportFile != null) { obj("reports") { diff --git a/runner-package/lib/account.ts b/runner-package/lib/account.ts index 7bb7a36..0e39f3b 100644 --- a/runner-package/lib/account.ts +++ b/runner-package/lib/account.ts @@ -83,6 +83,13 @@ export class AccountPool { : null; } + /** Total configured slots (pool + microsoft + autoRegister's max), not the number + * currently free. Used to size a fixed-slot consumer (e.g. reuse's `maxPlayers` + * default) before anything has been leased. */ + capacity(): number { + return this.queue.length + (this.autoRegister?.max ?? 0); + } + async lease(): Promise { const entry = this.queue.shift(); if (entry) { diff --git a/runner-package/lib/config.ts b/runner-package/lib/config.ts index d129e84..9193a26 100644 --- a/runner-package/lib/config.ts +++ b/runner-package/lib/config.ts @@ -31,6 +31,17 @@ export interface EnvironmentConfig { config: Record; } +/** Settings for reusing a connected bot across test boundaries instead of reconnecting for + * every test. Absent, or `enabled: false`, is the pre-reuse behavior: connect → test → + * disconnect, every time. */ +export interface ReuseConfig { + enabled: boolean; + /** Live registry entries allowed at once. Defaults to 4, or `AccountPool` capacity minus + * one when the environment has a pool — one slot is kept free for a test's own + * `createPlayer()` call. */ + maxPlayers?: number | null; +} + export interface TestsConfig { /** Directory scanned for compiled spec files. Defaults to the working directory. */ dir?: string | null; @@ -42,6 +53,7 @@ export interface TestsConfig { names?: string[] | null; /** Per-test timeout; falls back to TEST_TIMEOUT and then to 30s. */ timeoutMs?: number | null; + reuse?: ReuseConfig | null; } export interface ReportsConfig { @@ -188,10 +200,15 @@ function configFromEnvironment(): RunnerConfig { */ export function loadRunnerConfig(argv: string[] = process.argv.slice(2)): RunnerConfig { const flagPath = readConfigFlag(argv); - if (flagPath) { - return readConfigFile(isAbsolute(flagPath) ? flagPath : resolve(process.cwd(), flagPath)); - } + const config = flagPath + ? readConfigFile(isAbsolute(flagPath) ? flagPath : resolve(process.cwd(), flagPath)) + : loadDefaultOrLegacyConfig(); + + applyReuseEnvOverride(config); + return config; +} +function loadDefaultOrLegacyConfig(): RunnerConfig { const defaultPath = resolve(process.cwd(), DEFAULT_CONFIG_FILENAME); try { readFileSync(defaultPath); @@ -201,6 +218,20 @@ export function loadRunnerConfig(argv: string[] = process.argv.slice(2)): Runner } } +/** `PLUGWRIGHT_REUSE` overrides `tests.reuse.enabled` from a committed config file — the + * toggle for a dev's own edit loop, so reuse never has to live in a checked-in build script + * just to be tried locally. `1`/`true` enables it, `0`/`false` disables it; anything else, or + * unset, leaves the config's own value alone. */ +function applyReuseEnvOverride(config: RunnerConfig): void { + const raw = process.env.PLUGWRIGHT_REUSE; + if (raw === undefined) return; + const enabled = raw === '1' || raw.toLowerCase() === 'true'; + const disabled = raw === '0' || raw.toLowerCase() === 'false'; + if (!enabled && !disabled) return; + + config.tests.reuse = { ...config.tests.reuse, enabled }; +} + /** True when [value] is a secret pointer rather than a plain value. */ export function isSecretRef(value: unknown): value is SecretRef { return typeof value === 'object' && value !== null && typeof (value as SecretRef).from === 'string'; diff --git a/runner-package/lib/environment.ts b/runner-package/lib/environment.ts index 61e43c6..bef610f 100644 --- a/runner-package/lib/environment.ts +++ b/runner-package/lib/environment.ts @@ -12,6 +12,9 @@ export interface EnvironmentCapabilities { arbitraryUsernames: boolean; lifecycle: boolean; cleanupStrategy: 'wipe' | 'compensating' | 'none'; + /** Absent means "allowed". An environment that breaks under a bot that stays connected + * across tests (an idle-kick timeout, a world reset between tests) sets this to `false`. */ + playerReuse?: boolean; } export interface BotConnectionOptions { diff --git a/runner-package/lib/player-registry.ts b/runner-package/lib/player-registry.ts new file mode 100644 index 0000000..4345ece --- /dev/null +++ b/runner-package/lib/player-registry.ts @@ -0,0 +1,168 @@ +import pc from 'picocolors'; +import type { Bot } from 'mineflayer'; +import type { PlayerWrapper } from './player.js'; +import type { Account, AccountPool } from './account.js'; +import type { Session } from './session.js'; + +/** What a test asks for when it wants a long-lived player instead of a fresh connection. + * `key` names an identity directly; without it, matching goes by ability labels. */ +export interface ReuseOptions { + /** Explicit identity. Needed when a test cares that it gets the *same* bot back — + * the second player in a multiplayer test, for instance. */ + key?: string; + /** Labels the player must carry. */ + abilities?: string[]; + /** Labels the player must not carry. */ + excludeAbilities?: string[]; + /** The player's label set must equal `abilities` exactly — no extras allowed. */ + strict?: boolean; +} + +export interface ConnectedPlayer { + player: PlayerWrapper; + account: Account; + pool: AccountPool | null; +} + +export interface ResolveResult { + player: PlayerWrapper; + key: string; + reused: boolean; +} + +interface RegistryEntry { + key: string; + player: PlayerWrapper; + account: Account; + pool: AccountPool | null; + /** Held by the current test: not handed out a second time, not evicted by LRU. */ + checkedOut: boolean; + lastUsedAt: number; +} + +/** Implicit key for a request with no explicit `key`: the normalized requirement set, so two + * requests asking for the same shape of player land on the same entry. */ +function derivedKey(options: ReuseOptions): string { + const abilities = [...(options.abilities ?? [])].sort().join(','); + const exclude = [...(options.excludeAbilities ?? [])].sort().join(','); + return `auto:[${abilities}]!(${exclude})${options.strict ? ':strict' : ''}`; +} + +function matches(entry: RegistryEntry, options: ReuseOptions): boolean { + const abilities = entry.player.abilities; + if ((options.abilities ?? []).some(a => !abilities.has(a))) return false; + if ((options.excludeAbilities ?? []).some(a => abilities.has(a))) return false; + if (options.strict && abilities.size !== (options.abilities?.length ?? 0)) return false; + return true; +} + +/** + * Long-lived bots that survive test boundaries within one run. Entries are matched by the + * ability labels a player carries (see `PlayerWrapper.abilities`) rather than by resetting + * server state back to a known baseline — the core has no way to undo what a plugin's own + * commands changed, so it doesn't pretend to. + * + * `resolve()` never connects a bot itself; it calls the `connect` callback it's given, so the + * caller keeps ownership of connection options, throttling and account leasing. + */ +export class PlayerRegistry { + private readonly entries: RegistryEntry[] = []; + + constructor( + private readonly session: Session, + private readonly maxPlayers: number, + ) {} + + /** Every bot this registry currently owns — checked out by the running test or sitting + * free for the next one. Callers pass this to `Session.disconnectAllBots` as the "keep" + * list, so a per-test sweep doesn't take down an entry no test happened to touch this + * time. */ + ownedBots(): Bot[] { + return this.entries.map(e => e.player.bot); + } + + async resolve(options: ReuseOptions, connect: () => Promise): Promise { + if (options.key) { + const existing = this.entries.find(e => e.key === options.key); + if (existing) { + if (matches(existing, options)) return this.checkout(existing, connect); + await this.drop(existing, `abilities don't match a new request for key "${options.key}"`); + } + return this.createEntry(options.key, connect); + } + + const free = this.entries.find(e => !e.checkedOut && matches(e, options)); + if (free) return this.checkout(free, connect); + + if (this.entries.length >= this.maxPlayers) { + const victim = this.entries + .filter(e => !e.checkedOut) + .sort((a, b) => a.lastUsedAt - b.lastUsedAt)[0]; + if (!victim) { + throw new Error( + `PlayerRegistry: maxPlayers=${this.maxPlayers} reached and every entry is checked out ` + + 'by the current test. Request fewer simultaneous players, or raise tests.reuse.maxPlayers.' + ); + } + await this.drop(victim, `evicted: maxPlayers=${this.maxPlayers} reached`); + } + + return this.createEntry(derivedKey(options), connect); + } + + /** Returns a checked-out entry to the free pool. No-op for a player this registry doesn't own. */ + release(player: PlayerWrapper): void { + const entry = this.entries.find(e => e.player === player); + if (!entry) return; + entry.checkedOut = false; + entry.lastUsedAt = Date.now(); + } + + /** Drops a broken or disqualified entry: disconnects it, returns its account, forgets it. + * No-op for a player this registry doesn't own. */ + async invalidate(player: PlayerWrapper, reason: string = 'invalidated'): Promise { + const entry = this.entries.find(e => e.player === player); + if (entry) await this.drop(entry, reason); + } + + /** Disconnects and forgets every entry — end-of-run teardown. */ + async disconnectAll(): Promise { + for (const entry of [...this.entries]) await this.drop(entry, 'session teardown'); + } + + /** A dead connection is transparently rejoined before it's handed out — a bot picked back + * up by the registry is otherwise indistinguishable from one that's still live, and the + * test has no reason to expect it might not be. A failed rejoin falls back to a fresh + * entry under the same key, same as a first-time miss. */ + private async checkout(entry: RegistryEntry, connect: () => Promise): Promise { + if ((entry.player.bot as any)._client?.ended) { + try { + await entry.player.rejoin(); + } catch (error) { + await this.drop(entry, `dead connection, rejoin failed: ${(error as Error).message}`); + return this.createEntry(entry.key, connect); + } + } + + entry.checkedOut = true; + const labels = [...entry.player.abilities].join(', ') || '-'; + console.log(pc.dim(`[Reuse] ${entry.player.username} from registry (key "${entry.key}", abilities: ${labels})`)); + return { player: entry.player, key: entry.key, reused: true }; + } + + private async createEntry(key: string, connect: () => Promise): Promise { + const { player, account, pool } = await connect(); + this.entries.push({ key, player, account, pool, checkedOut: true, lastUsedAt: Date.now() }); + console.log(pc.dim(`[Reuse] ${player.username} new player (key "${key}")`)); + return { player, key, reused: false }; + } + + private async drop(entry: RegistryEntry, reason: string): Promise { + const idx = this.entries.indexOf(entry); + if (idx !== -1) this.entries.splice(idx, 1); + console.log(pc.dim(`[Reuse] ${entry.player.username} discarded (${reason})`)); + await this.session.disconnectBot(entry.player.bot, entry.player.username); + this.session.removeBot(entry.player.bot); + entry.pool?.release(entry.account); + } +} diff --git a/runner-package/lib/player.ts b/runner-package/lib/player.ts index 2b9bdfb..c38ed81 100644 --- a/runner-package/lib/player.ts +++ b/runner-package/lib/player.ts @@ -45,7 +45,13 @@ export class PlayerWrapper { private _botOptions?: BotConnectionOptions; private _spawnPromise: Promise | null = null; private _listenersBot: Bot | null = null; - private account?: Account; + private _account?: Account; + /** Labels describing server state this player is known to carry — set automatically by + * `makeOp`/`deOp`/`setGameMode`, and by hand via `mark`/`unmark` for anything else. Survives + * `rejoin()`: it describes server state, which a reconnect doesn't touch. Used by + * `PlayerRegistry` to match a reused player against a test's requirements; the core never + * parses or verifies a label's meaning. */ + private readonly _abilities = new Set(); constructor(bot: Bot, session: Session) { this.bot = bot; @@ -119,12 +125,12 @@ export class PlayerWrapper { // is a prompt no authentication plugin can answer. this._registerPersistentListeners(); - if (this.account) { + if (this._account) { // Authentication has to happen while the server still holds the player: AuthMe and // friends keep an unauthenticated bot out of the world entirely, so waiting for the // spawn first would wait for something login is the precondition of. await Promise.race([this._spawnPromise, this._waitForLogin(timeout)]); - await this.session.onPlayerCreate?.(this, { account: this.account, env: this.session.env }); + await this.session.onPlayerCreate?.(this, { account: this._account, env: this.session.env }); } await this._spawnPromise; @@ -154,7 +160,13 @@ export class PlayerWrapper { /** @internal */ _setAccount(account: Account): void { - this.account = account; + this._account = account; + } + + /** The account this player connected with. Set for every player the runner creates + * (`createPlayer` always calls `_setAccount`); undefined only if constructed by hand. */ + get account(): Account | undefined { + return this._account; } private _registerPersistentListeners(): void { @@ -192,6 +204,29 @@ export class PlayerWrapper { this.serverWrapper = server; } + /** Read-only snapshot of this player's ability labels. */ + get abilities(): ReadonlySet { + return this._abilities; + } + + /** Records that this player carries `ability`. A statement, not a check — nothing here + * verifies it against real server state. */ + mark(ability: string): void { + this._abilities.add(ability); + } + + /** Removes `ability`. No-op if the player never carried it. */ + unmark(ability: string): void { + this._abilities.delete(ability); + } + + private markGameMode(mode: string): void { + for (const ability of this._abilities) { + if (ability.startsWith('gamemode:')) this._abilities.delete(ability); + } + this._abilities.add(`gamemode:${mode}`); + } + getCurrentGui(): GuiWrapper | null { let currentWindow = this.bot.currentWindow; return currentWindow ? new GuiWrapper(this.bot, currentWindow as Window) : null; @@ -259,24 +294,39 @@ export class PlayerWrapper { const response = await this.serverWrapper!.executeAndWait(command); // "Made X a server operator" on success, "Nothing changed. The player already is // an operator" when it was already granted — both mean the player is op now. - if (/operator/i.test(response)) return; + if (/operator/i.test(response)) { + this.mark('op'); + return; + } throw new Error(`Player ${this.username} was not opped: ${response.trim() || 'no response from the console'}`); } + const messagesSince = this.messageBuffer.length; + const consoleSince = this.session.consoleLog.length; this.serverWrapper!.execute(command); + // "Made X a server operator" reaches the player's own chat. "Nothing changed. The + // player already is an operator" — the case a reused, already-op player hits on a + // second `makeOp()` — never does; it only ever shows up in the server's own log. await poll( - () => this.messageBuffer.find(m => m.includes(`Made ${this.username} a server operator`)), + () => + this.messageBuffer.slice(messagesSince).find(m => m.includes(`Made ${this.username} a server operator`)) ?? + this.session.consoleLog.slice(consoleSince).find(m => /operator/i.test(m)), { message: `Player ${this.username} was not opped` } ); + this.mark('op'); } async deOp(): Promise { await this.executeAndSync(`minecraft:deop ${this.username}`); + this.unmark('op'); } async setGameMode(mode: 'survival' | 'creative' | 'adventure' | 'spectator'): Promise { - if (this.bot.game.gameMode === mode) return; + if (this.bot.game.gameMode === mode) { + this.markGameMode(mode); + return; + } this.requireServer(); this.serverWrapper!.execute(`minecraft:gamemode ${mode} ${this.username}`); @@ -284,6 +334,7 @@ export class PlayerWrapper { () => this.bot.game.gameMode === mode ? true : undefined, { message: `Game mode did not change to "${mode}"` } ); + this.markGameMode(mode); } async teleport(x: number, y: number, z: number): Promise { diff --git a/runner-package/lib/plugin-host.ts b/runner-package/lib/plugin-host.ts index 177a18f..974169d 100644 --- a/runner-package/lib/plugin-host.ts +++ b/runner-package/lib/plugin-host.ts @@ -76,6 +76,12 @@ export class PluginHost { } } + async onPlayerReuse(player: PlayerWrapper, ctx: { account: Account; env: Environment }): Promise { + for (const { plugin } of this.plugins) { + await plugin.onPlayerReuse?.(player, ctx); + } + } + async beforeEach(ctx: TestContext): Promise { for (const { plugin } of this.plugins) { await plugin.beforeEach?.(ctx); @@ -105,13 +111,13 @@ export class PluginHost { /** Inherited test files for the given mode, across every plugin with `inheritTests` * enabled. `findSpecFiles` never sees these — it skips `node_modules` — so this is the * only way a plugin's own tests run. */ - testFiles(mode: PluginTestRef['mode']): { file: string; pluginName: string }[] { + testFiles(mode: PluginTestRef['mode']): { file: string; pluginName: string; reuse?: false }[] { return this.plugins .filter(p => p.inheritTests) .flatMap(({ plugin }) => (plugin.tests ?? []) .filter(t => t.mode === mode) - .map(t => ({ file: t.file, pluginName: plugin.name })) + .map(t => ({ file: t.file, pluginName: plugin.name, reuse: t.reuse })) ); } diff --git a/runner-package/lib/plugin.ts b/runner-package/lib/plugin.ts index 352666c..3f1aec4 100644 --- a/runner-package/lib/plugin.ts +++ b/runner-package/lib/plugin.ts @@ -30,6 +30,10 @@ export interface PluginTestRef { * `suite` runs alongside user specs as regular tests, tagged with the plugin's name * in reports. */ mode: 'preflight' | 'suite'; + /** How this file relates to reuse. Absent means "follow the run's general rule". `false` + * forces every test in the file onto a fresh connection — the shape a preflight auth + * check needs, since it exists to prove the login flow, not to skip it. */ + reuse?: false; } export type MatcherFn = (this: any, ...args: any[]) => unknown; @@ -46,6 +50,10 @@ export interface PlugwrightPlugin { * the first. A one-shot "first test" can't cover a second bot or a rejoin, which is * why this is a hook rather than a `preflight` test. */ onPlayerCreate?(player: PlayerWrapper, ctx: { account: Account; env: Environment }): Promise | void; + /** Fired before a reused player is handed to the next test — never on the first connection, + * where `onPlayerCreate` already runs. The core has already done its own safe minimum + * (closing a leftover open window); anything beyond that is the plugin's call. */ + onPlayerReuse?(player: PlayerWrapper, ctx: { account: Account; env: Environment }): Promise | void; beforeEach?(ctx: TestContext): Promise | void; afterEach?(ctx: TestContext): Promise | void; extendContext?(ctx: TestContext): Record | void; diff --git a/runner-package/lib/reporter.ts b/runner-package/lib/reporter.ts index 8211386..16bad53 100644 --- a/runner-package/lib/reporter.ts +++ b/runner-package/lib/reporter.ts @@ -126,6 +126,7 @@ export function writeJsonReport(path: string, environmentName: string, testResul error: r.error ? r.error.message : null, skipReason: r.skipReason ?? null, plugin: r.plugin ?? null, + reuse: r.reuse ?? null, })), }; diff --git a/runner-package/lib/session.ts b/runner-package/lib/session.ts index 93f201c..5c6374f 100644 --- a/runner-package/lib/session.ts +++ b/runner-package/lib/session.ts @@ -1,6 +1,7 @@ import mineflayer, { Bot } from 'mineflayer'; import pc from 'picocolors'; import { CleanupJournal } from './journal.js'; +import { PlayerRegistry } from './player-registry.js'; import type { Environment, BotConnectionOptions } from './environment.js'; import type { ServerConsole } from './console.js'; import type { PlayerWrapper } from './player.js'; @@ -55,15 +56,19 @@ export class Session { readonly bots: Bot[] = []; readonly consoleLog = new MessageBuffer(); readonly journal: CleanupJournal; + /** Players that survive a test boundary instead of disconnecting in `finally`. Always + * present; unused unless a test actually asks for reuse (`tests.reuse.enabled`). */ + readonly players: PlayerRegistry; /** Set once by the runner after loading plugins. Fired by `PlayerWrapper.join()` on * every connection (initial join and every `rejoin()`), not called directly by * `Session` itself. */ onPlayerCreate: ((player: PlayerWrapper, ctx: { account: Account; env: Environment }) => Promise | void) | null = null; - constructor(env: Environment, journalPath: string | null = null) { + constructor(env: Environment, journalPath: string | null = null, reuseMaxPlayers: number = 4) { this.env = env; this.journal = new CleanupJournal(journalPath); + this.players = new PlayerRegistry(this, reuseMaxPlayers); } /** Pulls the console channel from the environment. Called once `env.setup()` has produced one. */ @@ -163,12 +168,25 @@ export class Session { }); } - async disconnectAllBots(): Promise { + /** Disconnects every bot except those in `keep` (registry-owned bots a test released + * rather than dropped, typically). Called with no argument, this is a full teardown — + * the shape every caller before player reuse existed relied on. + * + * 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. */ + async disconnectAllBots(keep: Bot[] = []): Promise { + const keepSet = new Set(keep); + await Promise.all( - this.bots.map((b, i) => this.disconnectBot(b, b.username ?? `bot-${i}`, 2000)) + this.bots + .filter(b => !keepSet.has(b)) + .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); } /** 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 5e4705e..1245aec 100644 --- a/runner-package/lib/test-registry.ts +++ b/runner-package/lib/test-registry.ts @@ -1,4 +1,5 @@ import type { TestContext } from './types.js'; +import type { ReuseOptions } from './player-registry.js'; export type Hook = (context: TestContext) => Promise | void; type TestFn = (context: TestContext) => Promise; @@ -14,6 +15,11 @@ type TestFn = (context: TestContext) => Promise; export interface TestOptions { requires?: string[]; environments?: string[]; + /** How this test wants its player resolved when `tests.reuse` is on. `false` forces a + * fresh connection regardless of the run's reuse setting — for a test that depends on a + * brand-new nick or the absence of a label another test might have left behind. A string + * is shorthand for `{ key }`. Omitted means "match by ability labels", the default. */ + reuse?: false | string | ReuseOptions; } interface DescribeScope { @@ -32,6 +38,7 @@ export interface TestCase { afterHooks: Hook[]; requires: string[]; environments: string[] | null; + reuse?: false | string | ReuseOptions; } export const testRegistry: TestCase[] = []; @@ -57,6 +64,7 @@ function registerTest(name: string, options: TestOptions, fn: TestFn): void { afterHooks: [...scopeStack].reverse().flatMap(s => s.afterHooks), requires: options.requires ?? [], environments: options.environments ?? null, + reuse: options.reuse, }); } @@ -70,13 +78,22 @@ export function test(name: string, fnOrOptions: TestFn | TestOptions, maybeFn?: } } +/** Appends `abilities: ['op']` to whatever `reuse` the test declared (or the implicit `{}`), + * so a reused player is matched by op status same as a fresh one gets opped. */ +function withOpAbility(reuse: TestOptions['reuse']): ReuseOptions { + const base: ReuseOptions = reuse === false ? {} : reuse === undefined ? {} : typeof reuse === 'string' ? { key: reuse } : reuse; + return { ...base, abilities: [...(base.abilities ?? []), 'op'] }; +} + export function opTest(name: string, fn: TestFn): void; export function opTest(name: string, options: TestOptions, fn: TestFn): void; export function opTest(name: string, fnOrOptions: TestFn | TestOptions, maybeFn?: TestFn): void { const options = typeof fnOrOptions === 'function' ? {} : fnOrOptions; const fn = typeof fnOrOptions === 'function' ? fnOrOptions : maybeFn!; - registerTest(name, options, async (context: TestContext) => { - await context.player.makeOp(); + registerTest(name, { ...options, reuse: options.reuse === false ? false : withOpAbility(options.reuse) }, async (context: TestContext) => { + // Only when the resolved player doesn't already carry it: resolution above already + // matched on `op`, so a reused player skips straight to the test body. + if (!context.player.abilities.has('op')) await context.player.makeOp(); await fn(context); }); } diff --git a/runner-package/lib/test-runner.ts b/runner-package/lib/test-runner.ts index 6088d0b..76c892f 100644 --- a/runner-package/lib/test-runner.ts +++ b/runner-package/lib/test-runner.ts @@ -10,6 +10,7 @@ import type { PluginHost } from './plugin-host.js'; import type { BotConnectionOptions } from './environment.js'; import type { TestCase } from './test-registry.js'; import type { TestContext, TestResult } from './types.js'; +import type { ConnectedPlayer, ReuseOptions } from './player-registry.js'; export interface RunTestCaseParams { file: string; @@ -20,17 +21,34 @@ export interface RunTestCaseParams { timeoutMs: number; /** Set when this test came from a plugin's inherited `tests`, for report labeling. */ pluginName?: string | null; + /** Whole-run setting: `tests.reuse.enabled` narrowed by the environment's + * `capabilities.playerReuse`. `false` reproduces the pre-reuse behavior exactly, down to + * the absence of `TestResult.reuse`. */ + reuseEnabled?: boolean; + /** Set for a plugin test file declaring `PluginTestRef.reuse === false` — forces a fresh + * connection for every test in the file regardless of `reuseEnabled` or the test's own + * `reuse` option. */ + forceReuseOff?: boolean; +} + +/** Normalizes a `TestOptions.reuse` value to what `PlayerRegistry.resolve` takes. */ +function normalizeReuse(reuse: false | string | ReuseOptions | undefined): false | ReuseOptions { + if (reuse === false) return false; + if (reuse === undefined) return {}; + if (typeof reuse === 'string') return { key: reuse }; + return reuse; } /** - * Runs one test case end to end: creates the primary bot (firing `onPlayerCreate`), - * builds `TestContext`, and sequences hooks in order — plugin beforeEach → spec beforeEach - * → body → cleanup finalizers → spec afterEach → plugin afterEach. Finalizer errors are - * logged but never flip the test result; spec afterEach errors do, matching the runner's - * pre-plugin-host behavior. + * Runs one test case end to end: resolves the primary bot (a fresh connection, or — when + * reuse applies — a registry lookup that may hand back a player from an earlier test), builds + * `TestContext`, and sequences hooks in order — plugin beforeEach → spec beforeEach → body → + * cleanup finalizers → spec afterEach → plugin afterEach. Finalizer errors are logged but + * never flip the test result; spec afterEach errors do, matching the runner's pre-plugin-host + * behavior. */ 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, reuseEnabled = false, forceReuseOff = false } = params; console.log(` ${pc.bold(`Test: ${testCase.name}`)}`); session.consoleLog.clear(); @@ -38,50 +56,100 @@ export async function runTestCase(params: RunTestCaseParams): Promise void | Promise> = []; - // Accounts leased from `session.env.accounts()` for this test, returned in the `finally` - // below regardless of how the test ends. - const leasedAccounts: Array<{ account: Account; pool: AccountPool }> = []; - - const createPlayer = async (options?: { username?: string }): Promise => { - // An explicit username always bypasses the pool: it names a specific bot identity - // the test wants, not "give me whatever account is free". + // Accounts leased outside the registry (a bypass `createPlayer({ username })`, or any + // player created while reuse doesn't apply to this test) — returned in `finally` below, + // same as before player reuse existed. + const adhocAccounts: Array<{ account: Account; pool: AccountPool }> = []; + // Players this test drew from the registry, so `finally` knows what to release or drop. + const registryPlayers: PlayerWrapper[] = []; + const invalidated = new Set(); + const reuseEffective = reuseEnabled && !forceReuseOff; + let primaryReuse: { key: string; reused: boolean } | null = null; + + // The actual connect: leases an account (or generates a throwaway identity), joins the + // server, and returns the wrapper. Used directly for a fresh connection, and passed to + // the registry as the "nothing free matched" fallback. + const connectNewPlayer = async (options?: { username?: string }): Promise => { const pool = options?.username ? null : session.env.accounts?.() ?? null; - let account: Account; - if (pool) { - account = await pool.lease(); - leasedAccounts.push({ account, pool }); - } else { - const uniqueId = randomUUID().split('-')[0]; - account = syntheticAccount(options?.username || `Test_${uniqueId}`); + const account: Account = pool + ? await pool.lease() + : syntheticAccount(options?.username || `Test_${randomUUID().split('-')[0]}`); + + try { + const botUsername = account.username; + console.log(`${pc.cyan('[Bot]')} Creating bot: ${pc.bold(botUsername)}`); + + await session.env.beforeJoin?.(); + + const botOptions: BotConnectionOptions = { + ...connOpts, + auth: account.auth, + profilesFolder: account.microsoftCacheDir, + }; + const bot = session.createBot({ ...botOptions, username: botUsername }); + const player = new PlayerWrapper(bot, session); + player._captureSpawnPromise(); + player.setServerWrapper(server); + player._setBotOptions(botOptions); + player._setAccount(account); + + await player.join(); + return { player, account, pool }; + } catch (error) { + if (pool) pool.release(account); + throw error; } - const botUsername = account.username; - console.log(`${pc.cyan('[Bot]')} Creating bot: ${pc.bold(botUsername)}`); + }; - await session.env.beforeJoin?.(); + /** `ctx.player` and `ctx.createPlayer` both funnel through here. `usernameOverride` is + * `createPlayer({ username })` — a specific identity, which always bypasses both the + * account pool and the registry, same as before reuse existed. */ + const resolvePlayer = async ( + usernameOverride: string | undefined, + reuseRequest: false | string | ReuseOptions | undefined, + ): Promise<{ player: PlayerWrapper; key: string; reused: boolean } | { player: PlayerWrapper; key: null; reused: false }> => { + if (usernameOverride) { + const { player } = await connectNewPlayer({ username: usernameOverride }); + return { player, key: null, reused: false }; + } - const botOptions: BotConnectionOptions = { - ...connOpts, - auth: account.auth, - profilesFolder: account.microsoftCacheDir, - }; - const bot = session.createBot({ ...botOptions, username: botUsername }); - const player = new PlayerWrapper(bot, session); - player._captureSpawnPromise(); - player.setServerWrapper(server); - player._setBotOptions(botOptions); - player._setAccount(account); - - await player.join(); + const normalized = normalizeReuse(reuseRequest); + if (!reuseEffective || normalized === false) { + const { player, account, pool } = await connectNewPlayer(); + if (pool) adhocAccounts.push({ account, pool }); + return { player, key: null, reused: false }; + } + + const result = await session.players.resolve(normalized, () => connectNewPlayer()); + registryPlayers.push(result.player); + + if (result.reused) { + // Core's own safe minimum for a player coming back from a previous test — anything + // beyond this is the plugin's domain via onPlayerReuse. + const openWindow = result.player.bot.currentWindow; + if (openWindow) { + try { result.player.bot.closeWindow(openWindow); } catch { /* best effort */ } + } + await plugins.onPlayerReuse(result.player, { account: result.player.account!, env: session.env }); + } + + return { player: result.player, key: result.key, reused: result.reused }; + }; + + const createPlayer = async (options?: { username?: string; reuse?: false | string | ReuseOptions }): Promise => { + const { player } = await resolvePlayer(options?.username, options?.reuse); return player; }; - const player = await createPlayer(); + const { player, key: primaryKey, reused: primaryReused } = await resolvePlayer(undefined, testCase.reuse); + if (primaryKey !== null) primaryReuse = { key: primaryKey, reused: primaryReused }; const abortController = new AbortController(); const ctx: TestContext = { player, server, createPlayer, + invalidatePlayer: (p: PlayerWrapper) => { invalidated.add(p); }, signal: abortController.signal, cleanup: (fn: () => void | Promise) => { finalizers.push(fn); }, }; @@ -89,6 +157,7 @@ export async function runTestCase(params: RunTestCaseParams): Promise; @@ -133,16 +202,36 @@ export async function runTestCase(params: RunTestCaseParams): Promise clearTimeout(timeoutHandle)), timeoutPromise]); + testPassed = true; const durationMs = Date.now() - testStartTime; console.log(` ${pc.green(pc.bold('PASSED'))} ${pc.dim(`(${formatDuration(durationMs)})`)}\n`); - return { file, testName: testCase.name, passed: true, durationMs, plugin: pluginName }; + return { file, testName: testCase.name, passed: true, durationMs, plugin: pluginName, reuse: reportedReuse() }; } catch (error) { const durationMs = Date.now() - testStartTime; const errorMsg = (error as Error).message; console.log(` ${pc.red(pc.bold('FAILED'))} ${pc.dim(`(${formatDuration(durationMs)})`)}: ${pc.red(errorMsg)}\n`); - return { file, testName: testCase.name, passed: false, durationMs, error: error as Error, plugin: pluginName }; + return { file, testName: testCase.name, passed: false, durationMs, error: error as Error, plugin: pluginName, reuse: reportedReuse() }; } finally { - await session.disconnectAllBots(); - for (const { account, pool } of leasedAccounts) pool.release(account); + // A failed or timed-out test hands nothing forward: one bad test turning into a + // cascade of unrelated failures would put the real cause somewhere other than the + // report points at. + for (const p of registryPlayers) { + const dead = !!(p.bot as any)._client?.ended; + if (!testPassed || dead || invalidated.has(p)) { + await session.players.invalidate(p, !testPassed ? 'test failed' : dead ? 'connection dead' : 'invalidated by test'); + } else { + session.players.release(p); + } + } + // Keep every bot the registry owns, not just the ones this test happened to touch — + // a free entry another test will pick up later is not this test's to disconnect. + await session.disconnectAllBots(session.players.ownedBots()); + for (const { account, pool } of adhocAccounts) pool.release(account); + } + + function reportedReuse(): TestResult['reuse'] { + if (!reuseEnabled) return undefined; + if (!primaryReuse) return { key: 'none', reused: false, abilities: [] }; + return { key: primaryReuse.key, reused: primaryReuse.reused, abilities: [...player.abilities] }; } } diff --git a/runner-package/lib/types.ts b/runner-package/lib/types.ts index 8909f91..573b971 100644 --- a/runner-package/lib/types.ts +++ b/runner-package/lib/types.ts @@ -1,10 +1,14 @@ import type { PlayerWrapper } from './player.js'; import type { ServerWrapper } from './server.js'; +import type { ReuseOptions } from './player-registry.js'; export interface TestContext { player: PlayerWrapper; server: ServerWrapper; - createPlayer: (options?: { username?: string }) => Promise; + createPlayer: (options?: { username?: string; reuse?: false | string | ReuseOptions }) => Promise; + /** Marks a player unfit for the next test: it disconnects instead of being handed out + * again. No-op for a player reuse never picked up (a plain fresh connection). */ + invalidatePlayer: (player: PlayerWrapper) => void; signal: AbortSignal; /** Registers a LIFO finalizer that always runs after the test body, before afterEach. * Errors are logged but never override the test result. */ @@ -23,4 +27,6 @@ export interface TestResult { skipReason?: string; /** Name of the plugin this test was inherited from, or null for a user spec. */ plugin?: string | null; + /** How the primary player was obtained. Absent when reuse is off for this run. */ + reuse?: { key: string; reused: boolean; abilities: string[] }; } \ No newline at end of file diff --git a/runner-package/runner.ts b/runner-package/runner.ts index 458b4d0..b8e1054 100644 --- a/runner-package/runner.ts +++ b/runner-package/runner.ts @@ -32,8 +32,10 @@ export { test, opTest, describe, beforeEach, afterEach } from './lib/test-regist export type { TestOptions, TestCase } from './lib/test-registry.js'; export { expect } from './lib/matchers.js'; export { loadRunnerConfig, resolveSecret, isSecretRef } from './lib/config.js'; -export type { RunnerConfig, EnvironmentConfig, TestsConfig, LocalEnvironmentConfig, SecretRef, PluginConfig } from './lib/config.js'; -export type { TestContext } from './lib/types.js'; +export type { RunnerConfig, EnvironmentConfig, TestsConfig, LocalEnvironmentConfig, SecretRef, PluginConfig, ReuseConfig } from './lib/config.js'; +export type { TestContext, TestResult } from './lib/types.js'; +export { PlayerRegistry } from './lib/player-registry.js'; +export type { ReuseOptions } from './lib/player-registry.js'; export type { Environment, EnvironmentCapabilities, BotConnectionOptions } from './lib/environment.js'; export type { ServerConsole } from './lib/console.js'; export { Session } from './lib/session.js'; @@ -109,6 +111,25 @@ async function findSpecFiles(dir: string): Promise { return results; } +/** `tests.reuse.enabled`, narrowed by the environment's own `capabilities.playerReuse`. An + * environment that can't tolerate a long-lived bot always wins over the config. */ +function resolveReuse(config: RunnerConfig, env: Environment): { enabled: boolean; maxPlayers: number } { + const requested = config.tests.reuse?.enabled ?? false; + if (!requested) return { enabled: false, maxPlayers: 4 }; + + if (env.capabilities.playerReuse === false) { + console.log(pc.yellow( + `[Reuse] tests.reuse.enabled is true, but environment "${config.environment.name}" declares ` + + 'capabilities.playerReuse = false — running with reuse off for this environment.' + )); + return { enabled: false, maxPlayers: 4 }; + } + + const maxPlayers = config.tests.reuse?.maxPlayers + ?? Math.max(1, (env.accounts?.()?.capacity() ?? 5) - 1); + return { enabled: true, maxPlayers }; +} + export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): Promise { const testFileFilters = config.tests.include ?? null; const testNameFilters = config.tests.names ?? null; @@ -118,7 +139,11 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): const testResults: TestResult[] = []; const env = await resolveEnvironment(config.environment); - const session = new Session(env, config.journal ?? null); + const reuse = resolveReuse(config, env); + const session = new Session(env, config.journal ?? null, reuse.maxPlayers); + if (reuse.enabled) { + console.log(pc.dim(`[Reuse] enabled, maxPlayers=${reuse.maxPlayers}`)); + } const plugins = new PluginHost(); await plugins.load(config.plugins ?? []); // Must happen before the first spec file is imported — see PluginHost.registerMatchers. @@ -160,7 +185,7 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): /** 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 { + async function runFile(file: string, pluginName: string | null, forceReuseOff: boolean = false): Promise { resetRegistry(); await import(pathToFileURL(file).href); @@ -172,17 +197,20 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): continue; } - const result = await runTestCase({ file, testCase, session, plugins, connOpts, timeoutMs, pluginName }); + const result = await runTestCase({ + file, testCase, session, plugins, connOpts, timeoutMs, pluginName, + reuseEnabled: reuse.enabled, forceReuseOff, + }); testResults.push(result); } } // Preflight: plugin auth/setup tests, run before anything else. A failure aborts the // whole session. - for (const { file, pluginName } of plugins.testFiles('preflight')) { + for (const { file, pluginName, reuse: fileReuse } of plugins.testFiles('preflight')) { console.log(`\n${pc.blue(pc.bold(`Running preflight tests from: ${file} ${pc.dim(`(plugin ${pluginName})`)}`))}`); const before = testResults.length; - await runFile(file, pluginName); + await runFile(file, pluginName, fileReuse === false); 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'}`); @@ -211,17 +239,27 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): } // Suite: plugin tests that run alongside user specs, tagged with the plugin's name. - for (const { file, pluginName } of plugins.testFiles('suite')) { + for (const { file, pluginName, reuse: fileReuse } of plugins.testFiles('suite')) { console.log(`\n${pc.blue(pc.bold(`Running tests from: ${file} ${pc.dim(`(plugin ${pluginName})`)}`))}`); - await runFile(file, pluginName); + await runFile(file, pluginName, fileReuse === false); } } finally { await plugins.runCleanup(session, 'session'); await plugins.teardown(); + // Registry-owned bots first: it disconnects and forgets each entry, so the plain + // sweep after it only has to deal with whatever was never handed to the registry. + await session.players.disconnectAll(); await session.disconnectAllBots(); await env.teardown(); + if (reuse.enabled) { + const reusedCount = testResults.filter(r => r.reuse?.reused).length; + if (reusedCount > 0) { + console.log(pc.dim(`[Reuse] ${reusedCount} test(s) reused an existing connection instead of reconnecting`)); + } + } + if (config.reports?.json) { writeJsonReport(config.reports.json, config.environment.name, testResults); console.log(pc.dim(`JSON report: ${config.reports.json}`)); From 44110b5b5ac2a52fb0234a2a86c4a5192bb932c3 Mon Sep 17 00:00:00 2001 From: Monikon Date: Sun, 16 Aug 2026 22:45:02 +0300 Subject: [PATCH 04/10] feat(reuse): let a reused player leave the server between tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reuse held a bot connected from the test that created it until the run ended, which is not something a server that kicks an idle player allows. The only way out was to switch reuse off entirely, and with it the account, the nick and the ability labels — none of which have anything to do with idling. `stay` splits the two apart. Left alone it is `true` and nothing changes. `false` parks the registry entry instead of holding its connection: the bot leaves at the end of every test, the entry keeps the identity, and the next test matched to it gets a `rejoin()` first. What carries over is the identity, not the connection. It is settable for the run (`tests.reuse.stay`, `reuse { stay }`, PLUGWRIGHT_REUSE_STAY) and for one test (`reuse: { stay }`). An environment forces it with `capabilities.playerReuse = 'rejoin'`, a middle value between the `true` and `false` that field already had, for a server that objects to an idle bot but not to a reused one. Neither the config nor a test overrides that, the same way `false` already outranks `tests.reuse.enabled`. The registry now calls `env.beforeJoin()` before a rejoin, which it never did. A rejoin skipping an external server's join throttle was easy to miss while it happened once in a run on a dead connection; under `stay: false` it happens on every test. --- docs/configuration.mdx | 11 ++++- docs/custom-modes.mdx | 7 +-- docs/external-servers.mdx | 2 +- docs/reports.mdx | 4 +- docs/test-filtering.mdx | 2 +- docs/writing-tests.mdx | 11 +++++ .../plugwright/PlugwrightCorePlugin.kt | 2 + .../drownek/plugwright/PlugwrightExtension.kt | 2 +- .../plugwright/PlugwrightMatrixTask.kt | 2 + .../drownek/plugwright/PlugwrightTestTask.kt | 7 +++ .../kotlin/me/drownek/plugwright/ReuseSpec.kt | 8 +++ .../me/drownek/plugwright/RunnerLauncher.kt | 4 ++ runner-package/lib/config.ts | 36 ++++++++++---- runner-package/lib/environment.ts | 10 ++-- runner-package/lib/player-registry.ts | 49 +++++++++++++++---- runner-package/lib/test-registry.ts | 4 +- runner-package/lib/test-runner.ts | 36 ++++++++++---- runner-package/lib/types.ts | 5 +- runner-package/runner.ts | 37 ++++++++++---- 19 files changed, 184 insertions(+), 55 deletions(-) diff --git a/docs/configuration.mdx b/docs/configuration.mdx index 91d89a0..f892a59 100644 --- a/docs/configuration.mdx +++ b/docs/configuration.mdx @@ -274,17 +274,20 @@ matrix { ``` - Settings for reusing a connected bot across test boundaries instead of reconnecting for every test. `enabled` is `false` by default — an existing suite that depends on a fresh player per test keeps working unchanged until it opts in. `maxPlayers` caps live registry entries; unset falls back to 4, or the environment's account pool capacity minus one when it has a pool. + Settings for reusing a connected bot across test boundaries instead of reconnecting for every test. `enabled` is `false` by default — an existing suite that depends on a fresh player per test keeps working unchanged until it opts in. `maxPlayers` caps live registry entries; unset falls back to 4, or the environment's account pool capacity minus one when it has a pool. `stay` decides whether a reused bot keeps its connection between the tests that borrow it; `true` by default. ```kotlin reuse { enabled.set(true) maxPlayers.set(4) + stay.set(true) } ``` -`PLUGWRIGHT_REUSE=1` / `PLUGWRIGHT_REUSE=0` overrides `reuse.enabled` from the environment, for trying it in a dev loop without editing a committed build script. See [Writing Tests](/writing-tests) for the test-side API. +`stay.set(false)` keeps the reuse but drops the parking: at the end of every test the bot leaves the server, and the entry it came from — same account, same nick, same ability labels — waits offline until a later test takes it and rejoins under that identity. What carries over is the identity, not the connection. That's the only form of reuse a server which kicks idle players allows, and it's what an environment declaring `capabilities.playerReuse = 'rejoin'` forces regardless of this setting. A single test can override it with `reuse: { stay }` — see [Writing Tests](/writing-tests). + +`PLUGWRIGHT_REUSE=1` / `PLUGWRIGHT_REUSE=0` overrides `reuse.enabled` from the environment, and `PLUGWRIGHT_REUSE_STAY` does the same for `reuse.stay`, for trying either in a dev loop without editing a committed build script. Per-environment, inside `create(...) { }`: @@ -326,4 +329,8 @@ plugins { Overrides `reuse.enabled` for this run: `1`/`true` turns it on, `0`/`false` turns it off. Unset, or any other value, leaves the build script's own setting alone. + + Overrides `reuse.stay` for this run, same `1`/`true` and `0`/`false` spelling. `0` keeps reuse on but sends each bot off the server at the end of every test, rejoining it when a later test takes its entry. Ignored when reuse itself is off. + + diff --git a/docs/custom-modes.mdx b/docs/custom-modes.mdx index 37e336a..d871026 100644 --- a/docs/custom-modes.mdx +++ b/docs/custom-modes.mdx @@ -152,8 +152,9 @@ class VelocityEnvironment implements Environment { arbitraryUsernames: true, lifecycle: true, cleanupStrategy: 'compensating', - // Absent means "allowed". Set this to false if a bot sitting connected between - // tests would break the environment (an idle-kick timeout, a per-test world reset). + // Absent means "allowed". `false` if a bot surviving a test boundary at all would + // break the environment (a per-test world reset); 'rejoin' if only a bot *sitting* + // there is the problem (an idle-kick timeout, an AFK check). playerReuse: true, }; @@ -166,7 +167,7 @@ class VelocityEnvironment implements Environment { } ``` -Capabilities are a promise the runner holds you to. Tests declaring `requires: ['op']` are skipped when you report `op: false`, so report what is true after `setup()` rather than what the build script hoped for. `consoleOutput` is three-valued (`full`, `responses`, `none`) because a console that answers its own commands still cannot show a test the server log. `playerReuse: false` overrides `tests.reuse.enabled` for the whole run against this environment — set it when a long-lived bot would break something the environment can't tell tests about any other way. +Capabilities are a promise the runner holds you to. Tests declaring `requires: ['op']` are skipped when you report `op: false`, so report what is true after `setup()` rather than what the build script hoped for. `consoleOutput` is three-valued (`full`, `responses`, `none`) because a console that answers its own commands still cannot show a test the server log. `playerReuse: false` overrides `tests.reuse.enabled` for the whole run against this environment — set it when a long-lived bot would break something the environment can't tell tests about any other way. `playerReuse: 'rejoin'` is the softer form for a server that only objects to an *idle* bot: reuse stays on, but every entry leaves at the end of its test and rejoins when a later one takes it, and no `tests.reuse.stay` or per-test `reuse: { stay: true }` can talk it out of that. `accounts()` and `beforeJoin()` are optional. Returning no pool means every bot gets a throwaway `Test_` username, which is what `local` does. diff --git a/docs/external-servers.mdx b/docs/external-servers.mdx index 4e211e6..ae078f0 100644 --- a/docs/external-servers.mdx +++ b/docs/external-servers.mdx @@ -98,7 +98,7 @@ A leased account comes back with the previous test's inventory, balance and op s With `tests.reuse` on ([Configuration](/configuration)), a registry entry holds its leased account for as long as the entry lives, not just for one test — an account checked out by a long-lived player doesn't return to the pool until that player is evicted, invalidated, or the run ends. Size `accounts { }` accordingly: `maxPlayers` defaults to the pool's capacity minus one so a test's own `createPlayer()` still has a spare slot, but a pool exactly as big as `maxPlayers` leaves nothing free for it. -An environment that can't tolerate a bot sitting connected between tests — an idle-kick timeout, a per-test world reset — should report `capabilities.playerReuse = false` after `setup()` rather than let reuse quietly misbehave. See [Writing a Mode](/custom-modes). +An environment that can't tolerate a bot surviving a test boundary at all — a per-test world reset — should report `capabilities.playerReuse = false` after `setup()` rather than let reuse quietly misbehave. When the problem is narrower than that, and it usually is on a public server, `capabilities.playerReuse = 'rejoin'` keeps the reuse and drops the idling: the bot leaves at the end of every test and rejoins under the same account when a later test takes its entry. `tests.reuse.stay = false` asks for the same thing from the config side. Either way the account stays leased while the entry is parked, so pool sizing doesn't change. See [Writing a Mode](/custom-modes). ## Checking the stand before you test diff --git a/docs/reports.mdx b/docs/reports.mdx index 2b3432e..710ef2e 100644 --- a/docs/reports.mdx +++ b/docs/reports.mdx @@ -26,7 +26,7 @@ build/reports/plugwright/.log per-environment output, matrix runs o "error": null, "skipReason": null, "plugin": null, - "reuse": { "key": "auto:[]!()", "reused": true, "abilities": [] } + "reuse": { "key": "auto:[]!()", "reused": true, "stay": true, "abilities": [] } }, { "file": "…/dist/simple-ts.spec.js", @@ -44,7 +44,7 @@ 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. -`reuse` is `null` when `tests.reuse` is off for the run. Otherwise it's always present, even for a test that opted out with `reuse: false` (`reused: false`, `key: "none"`). `reused: true` means the player came from an earlier test instead of a fresh connection; `abilities` is the label set it was matched against. See [Writing Tests](/writing-tests). +`reuse` is `null` when `tests.reuse` is off for the run. Otherwise it's always present, even for a test that opted out with `reuse: false` (`reused: false`, `key: "none"`). `reused: true` means the player came from an earlier test instead of a fresh connection; `abilities` is the label set it was matched against; `stay` is whether it kept its connection after this test or was parked offline until a later test rejoins it. See [Writing Tests](/writing-tests). Every skip carries its reason: excluded by name, wrong environment, or a capability the environment doesn't have. A skipped test that doesn't say why is worse than a failing one, because it reads as coverage. diff --git a/docs/test-filtering.mdx b/docs/test-filtering.mdx index 2e61c88..817eb33 100644 --- a/docs/test-filtering.mdx +++ b/docs/test-filtering.mdx @@ -79,7 +79,7 @@ test('the /debug dev command', { environments: ['local'] }, async ({ player }) = ## Reuse is a different axis -`requires` and `environments` decide whether a test runs at all. `reuse` (see [Writing Tests](/writing-tests)) decides which bot it gets once it's already running — a filter never skips a test because of its `reuse` option. The two do interact on one environment setting: `capabilities.playerReuse === false` disables reuse for the whole environment without affecting anything a test declared through `requires`. +`requires` and `environments` decide whether a test runs at all. `reuse` (see [Writing Tests](/writing-tests)) decides which bot it gets once it's already running — a filter never skips a test because of its `reuse` option. The two do interact on one environment setting: `capabilities.playerReuse` disables reuse for the whole environment when it's `false`, or forces every player off the server between tests when it's `'rejoin'` — neither affects anything a test declared through `requires`. ## Skips are reported diff --git a/docs/writing-tests.mdx b/docs/writing-tests.mdx index c2f5972..c3cdcbe 100644 --- a/docs/writing-tests.mdx +++ b/docs/writing-tests.mdx @@ -153,11 +153,22 @@ export interface ReuseOptions { abilities?: string[]; // player must carry all of these excludeAbilities?: string[]; // player must carry none of these strict?: boolean; // player's labels must equal `abilities` exactly, no extras + stay?: boolean; // keep the connection after this test, or park the entry offline } ``` `reuse` on `test()` accepts `false`, a string (shorthand for `{ key }`), or a `ReuseOptions` object. `ctx.createPlayer({ reuse: … })` takes the same shape for any secondary player a test creates. +`stay` is the one option that describes what happens *after* the test rather than which player it gets. Left alone it follows the run's `tests.reuse.stay` (`true` by default): the bot stays on the server, and the next test that matches it skips connecting entirely. `stay: false` sends it off at the end of this test and keeps only the entry — the account, the nick and the labels — so a later test gets the same identity back through a rejoin: + +```typescript +test('slow inventory walk', { reuse: { stay: false } }, async ({ player }) => { + // the bot leaves when this test ends; the next test to want this player rejoins it +}); +``` + +Reach for it when a parked bot is the problem — an idle-kick timeout, an AFK check, a server that counts online players. Everything is disconnected at the end of the run either way. An environment can force it for every test by declaring `capabilities.playerReuse = 'rejoin'`, and then `stay: true` here doesn't lift it. + A test that cares about a clean nick, the absence of a label, or a first-registration flow declares `reuse: false` (or the right `excludeAbilities`) explicitly — reuse never guesses on a test's behalf. ```typescript diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt index 2b3aa20..4f3b717 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightCorePlugin.kt @@ -213,6 +213,7 @@ class PlugwrightCorePlugin : Plugin { if (extension.reuse.enabled.get()) { reuseEnabled.set(true) extension.reuse.maxPlayers.orNull?.let { reuseMaxPlayers.set(it) } + reuseStay.set(extension.reuse.stay) } if (project.hasProperty("testFiles")) testFiles.set(project.property("testFiles") as String) @@ -267,6 +268,7 @@ class PlugwrightCorePlugin : Plugin { runtimeExport = runtimeRef?.export, reuseEnabled = extension.reuse.enabled.get().takeIf { it }, reuseMaxPlayers = extension.reuse.maxPlayers.orNull, + reuseStay = extension.reuse.stay.orNull, ) ctx.prepareTaskRef?.let { matrixPrepareTasks += it } } diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightExtension.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightExtension.kt index a7fdd41..4dc1c4e 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightExtension.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightExtension.kt @@ -62,7 +62,7 @@ abstract class PlugwrightExtension(project: Project) : LegacyEnvironmentProperti /** Settings for reusing a connected bot across test boundaries. See [reuse]. */ val reuse: ReuseSpec = project.objects.newInstance(ReuseSpec::class.java) - /** Configures reuse: `reuse { enabled.set(true); maxPlayers.set(4) }`. */ + /** Configures reuse: `reuse { enabled.set(true); maxPlayers.set(4); stay.set(true) }`. */ fun reuse(action: ReuseSpec.() -> Unit) { reuse.action() } diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightMatrixTask.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightMatrixTask.kt index 9e7c4f0..fc4f107 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightMatrixTask.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightMatrixTask.kt @@ -32,6 +32,7 @@ internal data class MatrixEnvironmentInput( val runtimeExport: String? = null, val reuseEnabled: Boolean? = null, val reuseMaxPlayers: Int? = null, + val reuseStay: Boolean? = null, ) private data class EnvironmentSummary(val total: Int, val passed: Int, val failed: Int, val skipped: Int, val durationMs: Long) @@ -132,6 +133,7 @@ abstract class PlugwrightMatrixTask : AbstractNodeTask() { runtimeExport = env.runtimeExport, reuseEnabled = env.reuseEnabled, reuseMaxPlayers = env.reuseMaxPlayers, + reuseStay = env.reuseStay, ) RunnerLauncher.writeConfig(entry) val cliJs = RunnerLauncher.resolveCliJs(env.workspaceDir) diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightTestTask.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightTestTask.kt index 5fa44ba..06d81ac 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightTestTask.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/PlugwrightTestTask.kt @@ -60,6 +60,12 @@ abstract class PlugwrightTestTask : AbstractNodeTask() { @get:Optional abstract val reuseMaxPlayers: Property + /** From `plugwright.reuse.stay`. Unset means "runner default" — the bot stays connected + * between the tests that borrow it. */ + @get:Input + @get:Optional + abstract val reuseStay: Property + /** * The mode-specific part of the runner config (`environment.config`). Set by the plugin * from either [me.drownek.plugwright.api.PlugwrightMode.serialize] or the mode's own @@ -143,6 +149,7 @@ abstract class PlugwrightTestTask : AbstractNodeTask() { runtimeExport = runtimeExport.orNull, reuseEnabled = reuseEnabled.orNull, reuseMaxPlayers = reuseMaxPlayers.orNull, + reuseStay = reuseStay.orNull, ) RunnerLauncher.writeConfig(entry) logger.lifecycle("Runner config: ${configDestination.absolutePath}") diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/ReuseSpec.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/ReuseSpec.kt index b3c40fc..ba075f3 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/ReuseSpec.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/ReuseSpec.kt @@ -18,7 +18,15 @@ abstract class ReuseSpec { * environment's account pool capacity minus one when it has a pool. */ abstract val maxPlayers: Property + /** Whether a reused bot keeps its connection between the tests that borrow it. On by + * default, which is what reuse meant before this existed. `false` keeps the identity — + * account, nick, ability labels — but drops the connection at the end of every test and + * rejoins when a later one takes it: the only form of reuse a server that kicks idle bots + * allows. A single test can still override this with `reuse: { stay }`. */ + abstract val stay: Property + init { enabled.convention(false) + stay.convention(true) } } diff --git a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/RunnerLauncher.kt b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/RunnerLauncher.kt index ef16307..e699fe8 100644 --- a/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/RunnerLauncher.kt +++ b/gradle-plugin/plugwright-core/src/main/kotlin/me/drownek/plugwright/RunnerLauncher.kt @@ -42,6 +42,9 @@ object RunnerLauncher { val reuseEnabled: Boolean? = null, /** null means "runner default" — only meaningful when [reuseEnabled] is true. */ val reuseMaxPlayers: Int? = null, + /** Whether a reused bot stays connected between tests; null means "runner default" + * (true). Only meaningful when [reuseEnabled] is true. */ + val reuseStay: Boolean? = null, ) fun writeConfig(entry: Entry) { @@ -74,6 +77,7 @@ object RunnerLauncher { obj("reuse") { put("enabled", entry.reuseEnabled) entry.reuseMaxPlayers?.let { put("maxPlayers", it) } + entry.reuseStay?.let { put("stay", it) } } } } diff --git a/runner-package/lib/config.ts b/runner-package/lib/config.ts index 9193a26..57e8c4b 100644 --- a/runner-package/lib/config.ts +++ b/runner-package/lib/config.ts @@ -40,6 +40,13 @@ export interface ReuseConfig { * one when the environment has a pool — one slot is kept free for a test's own * `createPlayer()` call. */ maxPlayers?: number | null; + /** Whether a reused bot keeps its connection between the tests that borrow it. Defaults to + * `true`, which is what reuse meant before this setting existed. `false` parks each entry + * instead — the identity (account, nick, ability labels) is what carries over, and the bot + * rejoins when a later test takes it. A single test can override this through + * `reuse: { stay }`; an environment declaring `playerReuse: 'rejoin'` can't be overridden + * upwards by either. */ + stay?: boolean | null; } export interface TestsConfig { @@ -218,18 +225,27 @@ function loadDefaultOrLegacyConfig(): RunnerConfig { } } -/** `PLUGWRIGHT_REUSE` overrides `tests.reuse.enabled` from a committed config file — the - * toggle for a dev's own edit loop, so reuse never has to live in a checked-in build script - * just to be tried locally. `1`/`true` enables it, `0`/`false` disables it; anything else, or - * unset, leaves the config's own value alone. */ +/** `1`/`true` and `0`/`false`; anything else, including unset, reads as "not specified". */ +function booleanEnv(raw: string | undefined): boolean | null { + if (raw === undefined) return null; + if (raw === '1' || raw.toLowerCase() === 'true') return true; + if (raw === '0' || raw.toLowerCase() === 'false') return false; + return null; +} + +/** `PLUGWRIGHT_REUSE` and `PLUGWRIGHT_REUSE_STAY` override `tests.reuse` from a committed config + * file — the toggle for a dev's own edit loop, so neither reuse nor the choice between a parked + * and a rejoining bot has to live in a checked-in build script just to be tried locally. */ function applyReuseEnvOverride(config: RunnerConfig): void { - const raw = process.env.PLUGWRIGHT_REUSE; - if (raw === undefined) return; - const enabled = raw === '1' || raw.toLowerCase() === 'true'; - const disabled = raw === '0' || raw.toLowerCase() === 'false'; - if (!enabled && !disabled) return; + const enabled = booleanEnv(process.env.PLUGWRIGHT_REUSE); + const stay = booleanEnv(process.env.PLUGWRIGHT_REUSE_STAY); + if (enabled === null && stay === null) return; - config.tests.reuse = { ...config.tests.reuse, enabled }; + config.tests.reuse = { + ...config.tests.reuse, + enabled: enabled ?? config.tests.reuse?.enabled ?? false, + ...(stay === null ? {} : { stay }), + }; } /** True when [value] is a secret pointer rather than a plain value. */ diff --git a/runner-package/lib/environment.ts b/runner-package/lib/environment.ts index bef610f..a578c76 100644 --- a/runner-package/lib/environment.ts +++ b/runner-package/lib/environment.ts @@ -12,9 +12,13 @@ export interface EnvironmentCapabilities { arbitraryUsernames: boolean; lifecycle: boolean; cleanupStrategy: 'wipe' | 'compensating' | 'none'; - /** Absent means "allowed". An environment that breaks under a bot that stays connected - * across tests (an idle-kick timeout, a world reset between tests) sets this to `false`. */ - playerReuse?: boolean; + /** Absent or `true` means "allowed". `false` turns reuse off here entirely — an environment + * that breaks under a bot surviving a test boundary at all (a world reset between tests). + * `'rejoin'` is the middle ground for a server that only objects to the bot *sitting* there: + * entries are reused, but each one leaves at the end of its test and rejoins when a later + * test takes it. It caps `tests.reuse.stay` — a test asking for `stay: true` still gets a + * rejoin, the same way `false` outranks the config today. */ + playerReuse?: boolean | 'rejoin'; } export interface BotConnectionOptions { diff --git a/runner-package/lib/player-registry.ts b/runner-package/lib/player-registry.ts index 4345ece..255f25a 100644 --- a/runner-package/lib/player-registry.ts +++ b/runner-package/lib/player-registry.ts @@ -16,6 +16,11 @@ export interface ReuseOptions { excludeAbilities?: string[]; /** The player's label set must equal `abilities` exactly — no extras allowed. */ strict?: boolean; + /** Whether the bot keeps its connection once the test that borrowed it finishes. `false` + * parks the entry instead: the account, the nick and the labels survive, the connection + * doesn't, and a later test that takes the entry gets a `rejoin()` first. That's the shape + * a server which kicks an idle bot needs. Defaults to the run's `tests.reuse.stay`. */ + stay?: boolean; } export interface ConnectedPlayer { @@ -37,6 +42,9 @@ interface RegistryEntry { pool: AccountPool | null; /** Held by the current test: not handed out a second time, not evicted by LRU. */ checkedOut: boolean; + /** Released with `stay: false`, so the bot left the server but the entry stayed. Checking + * it out again rejoins first. */ + parked: boolean; lastUsedAt: number; } @@ -64,6 +72,11 @@ function matches(entry: RegistryEntry, options: ReuseOptions): boolean { * * `resolve()` never connects a bot itself; it calls the `connect` callback it's given, so the * caller keeps ownership of connection options, throttling and account leasing. + * + * An entry surviving a test boundary does not have to stay *connected* across it: released with + * `stay: false`, it parks — the bot leaves, the identity (account, nick, labels) stays, and the + * next test to take the entry gets a rejoin. What's reused there is the identity, not the + * connection, which is the only form of reuse a server that kicks idle bots allows. */ export class PlayerRegistry { private readonly entries: RegistryEntry[] = []; @@ -110,12 +123,20 @@ export class PlayerRegistry { return this.createEntry(derivedKey(options), connect); } - /** Returns a checked-out entry to the free pool. No-op for a player this registry doesn't own. */ - release(player: PlayerWrapper): void { + /** Returns a checked-out entry to the free pool. `stay: false` parks it on the way out — + * the connection goes, the entry stays, and the next checkout rejoins it. No-op for a + * player this registry doesn't own. */ + async release(player: PlayerWrapper, stay: boolean = true): Promise { const entry = this.entries.find(e => e.player === player); if (!entry) return; entry.checkedOut = false; entry.lastUsedAt = Date.now(); + + if (stay || entry.parked) return; + entry.parked = true; + console.log(pc.dim(`[Reuse] ${entry.player.username} parked (stay: false — rejoins when a later test takes it)`)); + await this.session.disconnectBot(entry.player.bot, entry.player.username); + this.session.removeBot(entry.player.bot); } /** Drops a broken or disqualified entry: disconnects it, returns its account, forgets it. @@ -130,29 +151,37 @@ export class PlayerRegistry { for (const entry of [...this.entries]) await this.drop(entry, 'session teardown'); } - /** A dead connection is transparently rejoined before it's handed out — a bot picked back - * up by the registry is otherwise indistinguishable from one that's still live, and the - * test has no reason to expect it might not be. A failed rejoin falls back to a fresh - * entry under the same key, same as a first-time miss. */ + /** An entry that isn't connected — parked on purpose, or dropped by the server — is + * transparently rejoined before it's handed out: a bot picked back up by the registry is + * otherwise indistinguishable from one that's still live, and the test has no reason to + * expect it might not be. A failed rejoin falls back to a fresh entry under the same key, + * same as a first-time miss. */ private async checkout(entry: RegistryEntry, connect: () => Promise): Promise { - if ((entry.player.bot as any)._client?.ended) { + const parked = entry.parked; + if (parked || (entry.player.bot as any)._client?.ended) { try { + // The same gate a first connection passes through: a rejoin is just another + // login as far as a shared server's join throttle is concerned, and `stay: false` + // turns every single test into one. + await this.session.env.beforeJoin?.(); await entry.player.rejoin(); } catch (error) { - await this.drop(entry, `dead connection, rejoin failed: ${(error as Error).message}`); + await this.drop(entry, `${parked ? 'parked' : 'dead connection'}, rejoin failed: ${(error as Error).message}`); return this.createEntry(entry.key, connect); } + entry.parked = false; } entry.checkedOut = true; const labels = [...entry.player.abilities].join(', ') || '-'; - console.log(pc.dim(`[Reuse] ${entry.player.username} from registry (key "${entry.key}", abilities: ${labels})`)); + const how = parked ? 'from registry, rejoined' : 'from registry'; + console.log(pc.dim(`[Reuse] ${entry.player.username} ${how} (key "${entry.key}", abilities: ${labels})`)); return { player: entry.player, key: entry.key, reused: true }; } private async createEntry(key: string, connect: () => Promise): Promise { const { player, account, pool } = await connect(); - this.entries.push({ key, player, account, pool, checkedOut: true, lastUsedAt: Date.now() }); + this.entries.push({ key, player, account, pool, checkedOut: true, parked: false, lastUsedAt: Date.now() }); console.log(pc.dim(`[Reuse] ${player.username} new player (key "${key}")`)); return { player, key, reused: false }; } diff --git a/runner-package/lib/test-registry.ts b/runner-package/lib/test-registry.ts index 1245aec..9f66934 100644 --- a/runner-package/lib/test-registry.ts +++ b/runner-package/lib/test-registry.ts @@ -18,7 +18,9 @@ export interface TestOptions { /** How this test wants its player resolved when `tests.reuse` is on. `false` forces a * fresh connection regardless of the run's reuse setting — for a test that depends on a * brand-new nick or the absence of a label another test might have left behind. A string - * is shorthand for `{ key }`. Omitted means "match by ability labels", the default. */ + * is shorthand for `{ key }`. Omitted means "match by ability labels", the default. + * `{ stay }` decides whether the player this test used keeps its connection afterwards, + * overriding the run's `tests.reuse.stay` for this test alone. */ reuse?: false | string | ReuseOptions; } diff --git a/runner-package/lib/test-runner.ts b/runner-package/lib/test-runner.ts index 76c892f..7018c38 100644 --- a/runner-package/lib/test-runner.ts +++ b/runner-package/lib/test-runner.ts @@ -25,6 +25,11 @@ export interface RunTestCaseParams { * `capabilities.playerReuse`. `false` reproduces the pre-reuse behavior exactly, down to * the absence of `TestResult.reuse`. */ reuseEnabled?: boolean; + /** Whole-run default for `ReuseOptions.stay`: does a registry player keep its connection + * once this test is done, or does it park until a later test rejoins it. `'rejoin'` is the + * environment's own `capabilities.playerReuse` saying it can't hold an idle bot at all — + * a test asking for `stay: true` doesn't get to lift that. */ + reuseStay?: boolean | 'rejoin'; /** Set for a plugin test file declaring `PluginTestRef.reuse === false` — forces a fresh * connection for every test in the file regardless of `reuseEnabled` or the test's own * `reuse` option. */ @@ -48,7 +53,7 @@ function normalizeReuse(reuse: false | string | ReuseOptions | undefined): false * behavior. */ export async function runTestCase(params: RunTestCaseParams): Promise { - const { file, testCase, session, plugins, connOpts, timeoutMs, pluginName = null, reuseEnabled = false, forceReuseOff = false } = params; + const { file, testCase, session, plugins, connOpts, timeoutMs, pluginName = null, reuseEnabled = false, reuseStay = true, forceReuseOff = false } = params; console.log(` ${pc.bold(`Test: ${testCase.name}`)}`); session.consoleLog.clear(); @@ -60,11 +65,17 @@ export async function runTestCase(params: RunTestCaseParams): Promise = []; - // Players this test drew from the registry, so `finally` knows what to release or drop. - const registryPlayers: PlayerWrapper[] = []; + // Players this test drew from the registry, with the `stay` each was taken under, so + // `finally` knows what to release, what to park and what to drop. + const registryPlayers: Array<{ player: PlayerWrapper; stay: boolean }> = []; const invalidated = new Set(); const reuseEffective = reuseEnabled && !forceReuseOff; - let primaryReuse: { key: string; reused: boolean } | null = null; + let primaryReuse: { key: string; reused: boolean; stay: boolean } | null = null; + + /** The run's `stay` unless the request overrides it — except under `'rejoin'`, where the + * environment has said an idle bot doesn't survive and no test gets to disagree. */ + const stayFor = (options: ReuseOptions): boolean => + reuseStay === 'rejoin' ? false : options.stay ?? reuseStay; // The actual connect: leases an account (or generates a throwaway identity), joins the // server, and returns the wrapper. Used directly for a fresh connection, and passed to @@ -121,7 +132,7 @@ export async function runTestCase(params: RunTestCaseParams): Promise connectNewPlayer()); - registryPlayers.push(result.player); + registryPlayers.push({ player: result.player, stay: stayFor(normalized) }); if (result.reused) { // Core's own safe minimum for a player coming back from a previous test — anything @@ -142,7 +153,10 @@ export async function runTestCase(params: RunTestCaseParams): Promise { return results; } -/** `tests.reuse.enabled`, narrowed by the environment's own `capabilities.playerReuse`. An - * environment that can't tolerate a long-lived bot always wins over the config. */ -function resolveReuse(config: RunnerConfig, env: Environment): { enabled: boolean; maxPlayers: number } { +/** `tests.reuse`, narrowed by the environment's own `capabilities.playerReuse`. An environment + * that can't tolerate a long-lived bot always wins over the config — outright when it declares + * `false`, and down to a rejoin per test when it declares `'rejoin'`. */ +function resolveReuse(config: RunnerConfig, env: Environment): { enabled: boolean; maxPlayers: number; stay: boolean | 'rejoin' } { const requested = config.tests.reuse?.enabled ?? false; - if (!requested) return { enabled: false, maxPlayers: 4 }; + if (!requested) return { enabled: false, maxPlayers: 4, stay: true }; if (env.capabilities.playerReuse === false) { console.log(pc.yellow( `[Reuse] tests.reuse.enabled is true, but environment "${config.environment.name}" declares ` + 'capabilities.playerReuse = false — running with reuse off for this environment.' )); - return { enabled: false, maxPlayers: 4 }; + return { enabled: false, maxPlayers: 4, stay: true }; } const maxPlayers = config.tests.reuse?.maxPlayers ?? Math.max(1, (env.accounts?.()?.capacity() ?? 5) - 1); - return { enabled: true, maxPlayers }; + + if (env.capabilities.playerReuse === 'rejoin') { + if (config.tests.reuse?.stay ?? true) { + console.log(pc.yellow( + `[Reuse] environment "${config.environment.name}" declares capabilities.playerReuse = 'rejoin' — ` + + 'players leave at the end of every test and rejoin when a later one takes them, whatever tests.reuse.stay says.' + )); + } + return { enabled: true, maxPlayers, stay: 'rejoin' }; + } + + return { enabled: true, maxPlayers, stay: config.tests.reuse?.stay ?? true }; } export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): Promise { @@ -142,7 +154,8 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): const reuse = resolveReuse(config, env); const session = new Session(env, config.journal ?? null, reuse.maxPlayers); if (reuse.enabled) { - console.log(pc.dim(`[Reuse] enabled, maxPlayers=${reuse.maxPlayers}`)); + const stayLabel = reuse.stay === 'rejoin' ? "stay=false (environment's own 'rejoin')" : `stay=${reuse.stay}`; + console.log(pc.dim(`[Reuse] enabled, maxPlayers=${reuse.maxPlayers}, ${stayLabel}`)); } const plugins = new PluginHost(); await plugins.load(config.plugins ?? []); @@ -199,7 +212,7 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): const result = await runTestCase({ file, testCase, session, plugins, connOpts, timeoutMs, pluginName, - reuseEnabled: reuse.enabled, forceReuseOff, + reuseEnabled: reuse.enabled, reuseStay: reuse.stay, forceReuseOff, }); testResults.push(result); } @@ -256,7 +269,13 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): if (reuse.enabled) { const reusedCount = testResults.filter(r => r.reuse?.reused).length; if (reusedCount > 0) { - console.log(pc.dim(`[Reuse] ${reusedCount} test(s) reused an existing connection instead of reconnecting`)); + // Under `stay: false` the connection is not what carried over — the identity is, + // and the bot rejoined under it — so the count is worth reporting either way. + const rejoined = testResults.filter(r => r.reuse?.reused && !r.reuse.stay).length; + const how = rejoined === reusedCount ? 'a registry player, rejoined' + : rejoined > 0 ? `a registry player (${rejoined} of them rejoined)` + : 'an existing connection instead of reconnecting'; + console.log(pc.dim(`[Reuse] ${reusedCount} test(s) reused ${how}`)); } } From c8cead61039541fa12a55ab480e08bd2f69c1cac Mon Sep 17 00:00:00 2001 From: Monikon Date: Mon, 17 Aug 2026 01:11:26 +0300 Subject: [PATCH 05/10] feat(reuse): add reuseTest pool-initialization hooks Registers a one-time setup step per reuse pool (the same string used as reuse: 'pool' / { key: 'pool' }). PlayerRegistry.resolve/createEntry now take an onFreshEntry callback, fired only when an entry is actually (re)built - first-ever request for a key, or a rebuild after a drop - never on a plain checkout of an already-live entry. A rejected onFreshEntry discards the half-built entry so the next attempt retries initialization instead of handing out a broken player. --- runner-package/lib/player-registry.ts | 33 ++++++++++++++++++++++---- runner-package/lib/test-registry.ts | 34 ++++++++++++++++++++++++++- 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/runner-package/lib/player-registry.ts b/runner-package/lib/player-registry.ts index 255f25a..cfb4d37 100644 --- a/runner-package/lib/player-registry.ts +++ b/runner-package/lib/player-registry.ts @@ -94,14 +94,22 @@ export class PlayerRegistry { return this.entries.map(e => e.player.bot); } - async resolve(options: ReuseOptions, connect: () => Promise): Promise { + /** `onFreshEntry` fires exactly when this call ends up creating a brand-new entry — + * first-ever request for a key, or a rebuild after a drop — never on a plain checkout of + * an entry that's already live. A rejected `onFreshEntry` discards the entry it just built, + * same as a broken connection would, and the rejection propagates to the caller. */ + async resolve( + options: ReuseOptions, + connect: () => Promise, + onFreshEntry?: (key: string, player: PlayerWrapper) => Promise, + ): Promise { if (options.key) { const existing = this.entries.find(e => e.key === options.key); if (existing) { if (matches(existing, options)) return this.checkout(existing, connect); await this.drop(existing, `abilities don't match a new request for key "${options.key}"`); } - return this.createEntry(options.key, connect); + return this.createEntry(options.key, connect, onFreshEntry); } const free = this.entries.find(e => !e.checkedOut && matches(e, options)); @@ -120,7 +128,7 @@ export class PlayerRegistry { await this.drop(victim, `evicted: maxPlayers=${this.maxPlayers} reached`); } - return this.createEntry(derivedKey(options), connect); + return this.createEntry(derivedKey(options), connect, onFreshEntry); } /** Returns a checked-out entry to the free pool. `stay: false` parks it on the way out — @@ -179,10 +187,25 @@ export class PlayerRegistry { return { player: entry.player, key: entry.key, reused: true }; } - private async createEntry(key: string, connect: () => Promise): Promise { + private async createEntry( + key: string, + connect: () => Promise, + onFreshEntry?: (key: string, player: PlayerWrapper) => Promise, + ): Promise { const { player, account, pool } = await connect(); - this.entries.push({ key, player, account, pool, checkedOut: true, parked: false, lastUsedAt: Date.now() }); + const entry: RegistryEntry = { key, player, account, pool, checkedOut: true, parked: false, lastUsedAt: Date.now() }; + this.entries.push(entry); console.log(pc.dim(`[Reuse] ${player.username} new player (key "${key}")`)); + + if (onFreshEntry) { + try { + await onFreshEntry(key, player); + } catch (error) { + await this.drop(entry, `reuseTest failed: ${(error as Error).message}`); + throw error; + } + } + return { player, key, reused: false }; } diff --git a/runner-package/lib/test-registry.ts b/runner-package/lib/test-registry.ts index 9f66934..5b471f0 100644 --- a/runner-package/lib/test-registry.ts +++ b/runner-package/lib/test-registry.ts @@ -46,9 +46,22 @@ export interface TestCase { export const testRegistry: TestCase[] = []; export const scopeStack: DescribeScope[] = [{ label: '', beforeHooks: [], afterHooks: [] }]; +/** A reuseTest's body, keyed by the reuse `key` ("pool") it initializes. */ +export interface ReuseTestCase { + pool: string; + name: string; + fn: TestFn; +} + +/** One reuseTest per pool, kept for the whole run rather than reset per spec file — + * `PlayerRegistry` entries live for the whole run too, so a pool declared in one file must + * still be found when a later file is the first to actually create that entry. */ +export const reuseTestRegistry = new Map(); + /** Discards whatever a previously-imported spec file registered, ready for the next one. * `testRegistry`/`scopeStack` stay module-level with this per-file reset — correct only - * as long as one process runs one environment and files run sequentially. */ + * as long as one process runs one environment and files run sequentially. `reuseTestRegistry` + * is deliberately NOT cleared here — see its own comment. */ export function resetRegistry(): void { testRegistry.length = 0; scopeStack.length = 0; @@ -100,6 +113,25 @@ export function opTest(name: string, fnOrOptions: TestFn | TestOptions, maybeFn? }); } +/** + * Registers a one-time initializer for a reuse pool. `pool` is the same string a test passes + * as `reuse: 'poolName'` (or `reuse: { key: 'poolName' }`) — `reuseTest` runs `fn` against that + * pool's player right when `PlayerRegistry` (re)creates its entry: the very first time any test + * asks for `poolName`, or later if that entry was dropped (rejoin failed, abilities stopped + * matching) and needs to be built again. It does NOT run on an ordinary checkout of an + * already-live entry — that's every other call, which is the common case. + * + * Runs as its own reported test, right before whichever test triggered the (re)creation. If + * `fn` throws, that test fails as a dependency failure and the entry is discarded, so the next + * attempt runs `reuseTest` again instead of handing out a half-initialized player. + */ +export function reuseTest(pool: string, fn: TestFn): void { + if (reuseTestRegistry.has(pool)) { + throw new Error(`reuseTest: pool "${pool}" is already registered (reuseTest can only be declared once per pool)`); + } + reuseTestRegistry.set(pool, { pool, name: `reuse:${pool}`, fn }); +} + export function describe(label: string, fn: () => void): void { scopeStack.push({ label, beforeHooks: [], afterHooks: [] }); try { From 702d9ee4f57080a2bd61ef4eb3836cd6341a8072 Mon Sep 17 00:00:00 2001 From: Monikon Date: Mon, 17 Aug 2026 01:11:31 +0300 Subject: [PATCH 06/10] feat(reuse): run reuseTest as a reported dependency of its trigger test Wires PlayerRegistry's onFreshEntry into test-runner.ts: when a fresh registry entry is being built for a key with a registered reuseTest, run it with its own timeout/finalizers/plugin fixtures, report it as its own test result via onExtraResult (appended ahead of the test that triggered creation), and propagate failure so the dependent test fails too instead of the whole run crashing on an uncaught rejection. Exports reuseTest from the package's public API alongside test/opTest. --- runner-package/lib/test-runner.ts | 98 ++++++++++++++++++++++++++++--- runner-package/runner.ts | 3 +- 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/runner-package/lib/test-runner.ts b/runner-package/lib/test-runner.ts index 7018c38..04624c1 100644 --- a/runner-package/lib/test-runner.ts +++ b/runner-package/lib/test-runner.ts @@ -8,6 +8,7 @@ import type { Account, AccountPool } from './account.js'; import type { Session } from './session.js'; import type { PluginHost } from './plugin-host.js'; import type { BotConnectionOptions } from './environment.js'; +import { reuseTestRegistry } from './test-registry.js'; import type { TestCase } from './test-registry.js'; import type { TestContext, TestResult } from './types.js'; import type { ConnectedPlayer, ReuseOptions } from './player-registry.js'; @@ -34,6 +35,10 @@ export interface RunTestCaseParams { * connection for every test in the file regardless of `reuseEnabled` or the test's own * `reuse` option. */ forceReuseOff?: boolean; + /** Reports a `reuseTest`'s own result, whenever one runs as a dependency of this test case. + * Called before `runTestCase` resolves, so the caller can append it to the report ahead of + * the test case's own result. */ + onExtraResult?: (result: TestResult) => void; } /** Normalizes a `TestOptions.reuse` value to what `PlayerRegistry.resolve` takes. */ @@ -53,7 +58,7 @@ function normalizeReuse(reuse: false | string | ReuseOptions | undefined): false * behavior. */ export async function runTestCase(params: RunTestCaseParams): Promise { - const { file, testCase, session, plugins, connOpts, timeoutMs, pluginName = null, reuseEnabled = false, reuseStay = true, forceReuseOff = false } = params; + const { file, testCase, session, plugins, connOpts, timeoutMs, pluginName = null, reuseEnabled = false, reuseStay = true, forceReuseOff = false, onExtraResult } = params; console.log(` ${pc.bold(`Test: ${testCase.name}`)}`); session.consoleLog.clear(); @@ -112,6 +117,65 @@ export async function runTestCase(params: RunTestCaseParams): Promise => { + const reuseCase = reuseTestRegistry.get(poolKey); + if (!reuseCase) return; + + console.log(` ${pc.bold(`Test: ${reuseCase.name}`)}`); + const reuseAbort = new AbortController(); + const reuseFinalizers: Array<() => void | Promise> = []; + const reuseCtx: TestContext = { + player: reusePlayer, + server, + createPlayer, + invalidatePlayer: (p: PlayerWrapper) => { invalidated.add(p); }, + signal: reuseAbort.signal, + cleanup: (fn: () => void | Promise) => { reuseFinalizers.push(fn); }, + }; + plugins.extendContext(reuseCtx); + + const start = Date.now(); + let timeoutHandle: ReturnType; + const timeoutPromise = new Promise((_, reject) => { + timeoutHandle = setTimeout(() => { + reuseAbort.abort(); + reject(new Error(`reuseTest "${poolKey}" timed out after ${timeoutMs}ms`)); + }, timeoutMs); + }); + + const body = (async (): Promise => { + try { + await reuseCase.fn(reuseCtx); + } finally { + for (const finalizer of [...reuseFinalizers].reverse()) { + try { + await finalizer(); + } catch (e) { + console.error(pc.red(`[cleanup] reuseTest "${poolKey}" finalizer error: ${(e as Error).message}`)); + } + } + } + })().finally(() => clearTimeout(timeoutHandle)); + + try { + await Promise.race([body, timeoutPromise]); + const durationMs = Date.now() - start; + console.log(` ${pc.green(pc.bold('PASSED'))} ${pc.dim(`(${formatDuration(durationMs)})`)}\n`); + onExtraResult?.({ file, testName: reuseCase.name, passed: true, durationMs, plugin: pluginName }); + } catch (error) { + const durationMs = Date.now() - start; + const errorMsg = (error as Error).message; + console.log(` ${pc.red(pc.bold('FAILED'))} ${pc.dim(`(${formatDuration(durationMs)})`)}: ${pc.red(errorMsg)}\n`); + onExtraResult?.({ file, testName: reuseCase.name, passed: false, durationMs, error: error as Error, plugin: pluginName }); + throw error; + } + }; + /** `ctx.player` and `ctx.createPlayer` both funnel through here. `usernameOverride` is * `createPlayer({ username })` — a specific identity, which always bypasses both the * account pool and the registry, same as before reuse existed. */ @@ -131,7 +195,11 @@ export async function runTestCase(params: RunTestCaseParams): Promise connectNewPlayer()); + const result = await session.players.resolve( + normalized, + () => connectNewPlayer(), + normalized.key ? (key, p) => runReuseTestCase(key, p) : undefined, + ); registryPlayers.push({ player: result.player, stay: stayFor(normalized) }); if (result.reused) { @@ -152,10 +220,27 @@ export async function runTestCase(params: RunTestCaseParams): Promise testResults.push(r), }); testResults.push(result); } From 6950497f73c91b8356390fca49555a5950aeb9c2 Mon Sep 17 00:00:00 2001 From: Monikon Date: Mon, 17 Aug 2026 01:11:35 +0300 Subject: [PATCH 07/10] docs(writing-tests): document reuseTest --- docs/writing-tests.mdx | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/writing-tests.mdx b/docs/writing-tests.mdx index c3cdcbe..8306c9a 100644 --- a/docs/writing-tests.mdx +++ b/docs/writing-tests.mdx @@ -180,6 +180,23 @@ test('cleans up on failure', async ({ player, invalidatePlayer }) => { `invalidatePlayer` marks a player unfit for the next test: it disconnects instead of being handed out again. The runner does this automatically for a test that fails or times out — one bad test shouldn't hand its mess to the next one. +### Initializing a reuse pool with `reuseTest` + +`reuseTest(pool, fn)` registers a one-time setup step for a named reuse pool (the same string used as `reuse: 'poolName'` or `reuse: { key: 'poolName' }`). It runs **only** when the pool's registry entry is actually (re)built — the first time any test asks for `'poolName'`, or later if that entry was dropped (a rejoin failed, abilities stopped matching) and needs to be created again. An ordinary checkout of an already-live entry never runs it: + +```typescript +reuseTest('shopkeeper', async ({ player }) => { + await player.chat('/vip add'); + player.mark('vip'); +}); + +test('shop shows vip discount', { reuse: 'shopkeeper' }, async ({ player }) => { + // guaranteed to run after 'shopkeeper' has been initialized at least once +}); +``` + +`reuseTest` is reported as its own test, listed right before whichever test triggered the (re)creation. If its body throws, that test fails too — the entry is discarded so the next attempt runs `reuseTest` again instead of handing out a half-initialized player. + ## Best Practices 1. **Keep tests isolated** - Each test gets a fresh bot, unless reuse is on From 9a095c4f4409bd167f91f7c77f60b7ac61c39e49 Mon Sep 17 00:00:00 2001 From: Monikon Date: Mon, 17 Aug 2026 01:25:48 +0300 Subject: [PATCH 08/10] feat(reuse): give reuseTest the full TestOptions scope of a regular test reuseTest now takes (pool, fn) or (pool, options, fn) with the same requires/environments TestOptions carries, plus describe nesting for its name and beforeEach/afterEach hooks - same scope as test/opTest, minus reuse itself (a reuseTest initializes a pool, it doesn't resolve into one). requires/environments skip it exactly like a regular test: reported skipped, fn never runs, no error - a player handed out under a pool whose reuseTest doesn't apply on this environment still connects, just never gets initialized. Extracted the environments+requires skip check (previously inline in runner.ts's skipReasonFor) into lib/skip-reason.ts so runFile and reuseTest execution share one implementation instead of two copies drifting apart. runner.ts now threads environmentName through to test-runner.ts so reuseTest's own skip check has something to compare against. --- runner-package/lib/skip-reason.ts | 39 +++++++++++++++++++++++++++ runner-package/lib/test-registry.ts | 41 +++++++++++++++++++++-------- runner-package/lib/test-runner.ts | 35 +++++++++++++++++++++++- runner-package/runner.ts | 29 +++----------------- 4 files changed, 106 insertions(+), 38 deletions(-) create mode 100644 runner-package/lib/skip-reason.ts diff --git a/runner-package/lib/skip-reason.ts b/runner-package/lib/skip-reason.ts new file mode 100644 index 0000000..077d91c --- /dev/null +++ b/runner-package/lib/skip-reason.ts @@ -0,0 +1,39 @@ +import type { Environment } from './environment.js'; + +/** Capability keys from a `requires` list that `env` does not actually satisfy. A value of + * `false`, `'none'`, or an absent key all count as unmet. + * + * `'key:value'` demands one specific value instead — `'consoleOutput:full'` for a test that + * reads the server log, which a console answering only its own commands cannot provide even + * though it satisfies plain `'console'`. */ +export function missingCapabilities(env: Environment, required: string[]): string[] { + const capabilities = env.capabilities as unknown as Record; + return required.filter(key => { + const separator = key.indexOf(':'); + if (separator !== -1) { + return String(capabilities[key.slice(0, separator)]) !== key.slice(separator + 1); + } + const value = capabilities[key]; + return value === false || value === 'none' || value === undefined; + }); +} + +/** Shared by `runFile`'s `skipReasonFor` (for a normal `TestCase`) and `reuseTest` execution + * (for a `ReuseTestCase`) — both check the same two `TestOptions` fields, `environments` and + * `requires`, against the same running environment. Name filters (`tests.names`/`exclude`) stay + * local to `runFile`: they're a run-level concern, not part of what a test itself declares. */ +export function skipReasonForOptions( + env: Environment, + environmentName: string, + requires: string[], + environments: string[] | null, +): string | null { + if (environments && !environments.includes(environmentName)) { + return `requires environment in [${environments.join(', ')}], running "${environmentName}"`; + } + const missing = missingCapabilities(env, requires); + if (missing.length > 0) { + return `requires capability [${missing.join(', ')}], unavailable on "${environmentName}"`; + } + return null; +} diff --git a/runner-package/lib/test-registry.ts b/runner-package/lib/test-registry.ts index 5b471f0..612bd43 100644 --- a/runner-package/lib/test-registry.ts +++ b/runner-package/lib/test-registry.ts @@ -46,11 +46,17 @@ export interface TestCase { export const testRegistry: TestCase[] = []; export const scopeStack: DescribeScope[] = [{ label: '', beforeHooks: [], afterHooks: [] }]; -/** A reuseTest's body, keyed by the reuse `key` ("pool") it initializes. */ +/** A reuseTest's body, keyed by the reuse `key` ("pool") it initializes. Carries the same + * `describe`-scoped hooks and `requires`/`environments` filters a regular `TestCase` does — + * a reuseTest is a real test in every way but how it gets triggered. */ export interface ReuseTestCase { pool: string; name: string; fn: TestFn; + beforeHooks: Hook[]; + afterHooks: Hook[]; + requires: string[]; + environments: string[] | null; } /** One reuseTest per pool, kept for the whole run rather than reset per spec file — @@ -68,19 +74,21 @@ export function resetRegistry(): void { scopeStack.push({ label: '', beforeHooks: [], afterHooks: [] }); } -function registerTest(name: string, options: TestOptions, fn: TestFn): void { +/** Everything a registered test needs from the current `describe` scope, shared by `test`/ + * `opTest` (pushed into `testRegistry`) and `reuseTest` (kept in `reuseTestRegistry` instead). */ +function scopedEntry(name: string, options: TestOptions | Omit) { const labels = scopeStack.map(s => s.label).filter(l => l); - const fullName = [...labels, name].join(' > '); - - testRegistry.push({ - name: fullName, - fn, + return { + name: [...labels, name].join(' > '), beforeHooks: scopeStack.flatMap(s => s.beforeHooks), afterHooks: [...scopeStack].reverse().flatMap(s => s.afterHooks), requires: options.requires ?? [], environments: options.environments ?? null, - reuse: options.reuse, - }); + }; +} + +function registerTest(name: string, options: TestOptions, fn: TestFn): void { + testRegistry.push({ ...scopedEntry(name, options), fn, reuse: options.reuse }); } export function test(name: string, fn: TestFn): void; @@ -121,15 +129,26 @@ export function opTest(name: string, fnOrOptions: TestFn | TestOptions, maybeFn? * matching) and needs to be built again. It does NOT run on an ordinary checkout of an * already-live entry — that's every other call, which is the common case. * + * Takes the same scope as `test`/`opTest`: `describe` nesting names it and contributes its + * `beforeEach`/`afterEach` hooks, `requires`/`environments` skip it the same way (reported + * `skipped`, not run — same as a regular test would be), and plugin `beforeEach`/`afterEach` + * and fixtures wrap it too. `reuse` isn't accepted — a reuseTest initializes a pool, it doesn't + * resolve into one itself. + * * Runs as its own reported test, right before whichever test triggered the (re)creation. If * `fn` throws, that test fails as a dependency failure and the entry is discarded, so the next * attempt runs `reuseTest` again instead of handing out a half-initialized player. */ -export function reuseTest(pool: string, fn: TestFn): void { +export function reuseTest(pool: string, fn: TestFn): void; +export function reuseTest(pool: string, options: Omit, fn: TestFn): void; +export function reuseTest(pool: string, fnOrOptions: TestFn | Omit, maybeFn?: TestFn): void { + const options = typeof fnOrOptions === 'function' ? {} : fnOrOptions; + const fn = typeof fnOrOptions === 'function' ? fnOrOptions : maybeFn!; + if (reuseTestRegistry.has(pool)) { throw new Error(`reuseTest: pool "${pool}" is already registered (reuseTest can only be declared once per pool)`); } - reuseTestRegistry.set(pool, { pool, name: `reuse:${pool}`, fn }); + reuseTestRegistry.set(pool, { pool, ...scopedEntry(`reuse:${pool}`, options), fn }); } export function describe(label: string, fn: () => void): void { diff --git a/runner-package/lib/test-runner.ts b/runner-package/lib/test-runner.ts index 04624c1..8416b1b 100644 --- a/runner-package/lib/test-runner.ts +++ b/runner-package/lib/test-runner.ts @@ -9,6 +9,7 @@ import type { Session } from './session.js'; import type { PluginHost } from './plugin-host.js'; import type { BotConnectionOptions } from './environment.js'; import { reuseTestRegistry } from './test-registry.js'; +import { skipReasonForOptions } from './skip-reason.js'; import type { TestCase } from './test-registry.js'; import type { TestContext, TestResult } from './types.js'; import type { ConnectedPlayer, ReuseOptions } from './player-registry.js'; @@ -20,6 +21,10 @@ export interface RunTestCaseParams { plugins: PluginHost; connOpts: BotConnectionOptions; timeoutMs: number; + /** The running environment's configured name — `TestOptions.environments` on a `reuseTest` + * is checked against this, same as `runFile`'s own `skipReasonFor` checks it for a + * regular `TestCase`. */ + environmentName: string; /** Set when this test came from a plugin's inherited `tests`, for report labeling. */ pluginName?: string | null; /** Whole-run setting: `tests.reuse.enabled` narrowed by the environment's @@ -58,7 +63,7 @@ function normalizeReuse(reuse: false | string | ReuseOptions | undefined): false * behavior. */ export async function runTestCase(params: RunTestCaseParams): Promise { - const { file, testCase, session, plugins, connOpts, timeoutMs, pluginName = null, reuseEnabled = false, reuseStay = true, forceReuseOff = false, onExtraResult } = params; + const { file, testCase, session, plugins, connOpts, timeoutMs, environmentName, pluginName = null, reuseEnabled = false, reuseStay = true, forceReuseOff = false, onExtraResult } = params; console.log(` ${pc.bold(`Test: ${testCase.name}`)}`); session.consoleLog.clear(); @@ -126,6 +131,16 @@ export async function runTestCase(params: RunTestCaseParams): Promise void | Promise> = []; @@ -148,9 +163,17 @@ export async function runTestCase(params: RunTestCaseParams): Promise => { + await plugins.beforeEach(reuseCtx); + for (const hook of reuseCase.beforeHooks) await hook(reuseCtx); + + let testError: unknown; try { await reuseCase.fn(reuseCtx); + } catch (e) { + testError = e; } finally { for (const finalizer of [...reuseFinalizers].reverse()) { try { @@ -159,7 +182,17 @@ export async function runTestCase(params: RunTestCaseParams): Promise clearTimeout(timeoutHandle)); try { diff --git a/runner-package/runner.ts b/runner-package/runner.ts index 1e723af..351ae2e 100644 --- a/runner-package/runner.ts +++ b/runner-package/runner.ts @@ -8,6 +8,7 @@ import { testRegistry, resetRegistry } from './lib/test-registry.js'; import { Session } from './lib/session.js'; import { PluginHost } from './lib/plugin-host.js'; import { runTestCase } 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'; import { PlayerWrapper } from './lib/player.js'; @@ -81,24 +82,6 @@ async function resolveEnvironment(cfg: EnvironmentConfig): Promise throw new Error(`Environment "${cfg.name}" uses mode "${cfg.mode}", which this runner cannot run yet.`); } -/** Capability keys from `testCase.requires` that `env` does not actually satisfy. A - * value of `false`, `'none'`, or an absent key all count as unmet. - * - * `'key:value'` demands one specific value instead — `'consoleOutput:full'` for a test that - * reads the server log, which a console answering only its own commands cannot provide even - * though it satisfies plain `'console'`. */ -function missingCapabilities(env: Environment, required: string[]): string[] { - const capabilities = env.capabilities as unknown as Record; - return required.filter(key => { - const separator = key.indexOf(':'); - if (separator !== -1) { - return String(capabilities[key.slice(0, separator)]) !== key.slice(separator + 1); - } - const value = capabilities[key]; - return value === false || value === 'none' || value === undefined; - }); -} - async function findSpecFiles(dir: string): Promise { const results: string[] = []; for (const entry of await readdir(dir, { withFileTypes: true })) { @@ -185,14 +168,7 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): if (testNameFilters && !testNameFilters.some(pattern => testCase.name.includes(pattern))) { return `filtered out by tests.names (${testNameFilters.join(',')})`; } - if (testCase.environments && !testCase.environments.includes(config.environment.name)) { - return `requires environment in [${testCase.environments.join(', ')}], running "${config.environment.name}"`; - } - const missing = missingCapabilities(env, testCase.requires); - if (missing.length > 0) { - return `requires capability [${missing.join(', ')}], unavailable on "${config.environment.name}"`; - } - return null; + return skipReasonForOptions(env, config.environment.name, testCase.requires, testCase.environments); } /** Imports one compiled spec file (a fresh `testRegistry`) and runs everything it @@ -212,6 +188,7 @@ export async function runTestSession(config: RunnerConfig = loadRunnerConfig()): const result = await runTestCase({ file, testCase, session, plugins, connOpts, timeoutMs, pluginName, + environmentName: config.environment.name, reuseEnabled: reuse.enabled, reuseStay: reuse.stay, forceReuseOff, onExtraResult: r => testResults.push(r), }); From b02d4bed1002471f9a8573c3ab2c9878fb0f2b48 Mon Sep 17 00:00:00 2001 From: Monikon Date: Mon, 17 Aug 2026 01:25:52 +0300 Subject: [PATCH 09/10] docs(writing-tests): document reuseTest's full TestOptions scope --- docs/writing-tests.mdx | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/writing-tests.mdx b/docs/writing-tests.mdx index 8306c9a..220f403 100644 --- a/docs/writing-tests.mdx +++ b/docs/writing-tests.mdx @@ -197,6 +197,20 @@ test('shop shows vip discount', { reuse: 'shopkeeper' }, async ({ player }) => { `reuseTest` is reported as its own test, listed right before whichever test triggered the (re)creation. If its body throws, that test fails too — the entry is discarded so the next attempt runs `reuseTest` again instead of handing out a half-initialized player. +It takes the same scope as `test`/`opTest` — everything except `reuse` itself, which doesn't apply to a test that's initializing a pool rather than resolving into one: + +```typescript +describe('Shop', () => { + reuseTest('shopkeeper', { requires: ['op'] }, async ({ player }) => { + // named "Shop > reuse:shopkeeper" — describe nesting applies same as any other test + }); +}); +``` + +- `requires` / `environments` skip it exactly like a regular test would be skipped — reported `skipped`, `fn` never runs. A player handed out under a pool whose `reuseTest` doesn't apply on this environment still connects; it's just never initialized, same as if no `reuseTest` had been declared for that pool at all. +- Spec-level `beforeEach`/`afterEach` from the enclosing `describe` wrap it, and so do plugin `beforeEach`/`afterEach` and `extendContext` fixtures — `ctx.holy`, matchers, everything a normal test body gets. +- It does not accept `reuse` — pass `(pool, fn)` or `(pool, options, fn)` where `options` is `requires`/`environments` only. + ## Best Practices 1. **Keep tests isolated** - Each test gets a fresh bot, unless reuse is on From 76ffb24d147370feb6242945a57fbc89d4a8d65d Mon Sep 17 00:00:00 2001 From: Monikon Date: Fri, 21 Aug 2026 02:01:09 +0300 Subject: [PATCH 10/10] fix(reuse): clear a stayed player's chat history on checkout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Message buffers are per player, and `rejoin` empties one on the way back in. A player checked out under `stay` never left, so it never rejoins and never gets that clear — its chat from the previous test stayed in the buffer and could satisfy an assertion in the next one. Nothing caught it before because the buffer was session-wide and every test started by clearing it. Per-player buffers are the right shape, but they moved that clear out from under reuse, so reuse now does it where it belongs: beside the closed window, in core's safe minimum for a returning player. --- runner-package/lib/test-runner.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/runner-package/lib/test-runner.ts b/runner-package/lib/test-runner.ts index 8416b1b..b7cbbd4 100644 --- a/runner-package/lib/test-runner.ts +++ b/runner-package/lib/test-runner.ts @@ -242,6 +242,11 @@ export async function runTestCase(params: RunTestCaseParams): Promise