Skip to content

[v3] Replace player reuse with environment-scoped identity and describe.serial() #53

Description

@monikon22

Follow-up from the discussion on #46, after @Drownek's review of #52. Recorded here so the
work has a place to land once v3-dev has PR 6 in it — this is a rework of already-merged
code, not a change to an open PR.

1. Remove player reuse

Agreed: the risk (a bot handed to the next test with state it shouldn't have — an open GUI, a
stale permission) outweighs what it saves. Removing:

  • player-registry.ts in full — ability-label matching and LRU eviction
  • stay / rejoin and the parked-connection lifecycle
  • TestOptions.reuse, reuseTest / reuseTestRegistry
  • capabilities.playerReuse, tests.reuse.*, ReuseSpec.kt, and the reuse block in
    PlugwrightExtension
  • TestResult.reuse in the reporter

Roughly 800 of the ~1100 lines #52 added. The rejoin bug (onFreshEntry not reaching
checkout) disappears with it — nothing to patch there separately.

2. Identity becomes an environment concern

AccountPool (account.ts, from #48) stays — this was never the reuse problem, #52 only
touched it for 7 lines. What changes is where a name comes from:

  • LocalMode: keep autoRegister, switch the counter (pw_%04d) for a random suffix
    (pw_a8f2). Every test gets a fresh, fully-provisioned player with no other test's state on
    it, without the fixed-slot behavior that forced excludeTests around balance/shop/kit state.
  • ExternalMode: keep the declared pool = [...]. A stand's accounts are pre-configured —
    LuckPerms group, whitelist entry, starting balance, whatever rows the plugin under test keeps
    — so there's no "just make up a name" option here.

3. describe.serial() for tests that share state on purpose

Cross-test reuse as a performance optimization goes away, but "claim a kit, assert it's on
cooldown, wait, assert it's claimable again" is a real scenario that three independent random
bots can't express, no matter how they're scheduled. Same split Playwright makes:

describe.serial('kit lifecycle', () => {
  test('claim kit',          async ({ player }) => { ... });
  test('kit is on cooldown', async ({ player }) => { ... });
  test('cooldown expires',   async ({ player }) => { ... });
});

One lease() for the block, one connection, release() on the way out, tests run in the order
they're declared. A failure reports the rest of the block as skipped rather than failed.
Optional second argument for a stand that needs one specific account:
describe.serial('...', { account: 'pw_0001' }).

The block is the unit of scheduling, not a blocker to it — independent blocks and standalone
tests still run concurrently. describe and the scope stack already exist in
test-registry.ts, so this is additive rather than a rewrite of it.

4. stand-reset.ts covers more of the reset

The excludeTests list on external stands was working around an incomplete reset, not a reuse
problem. stand-reset.ts is already the right mechanism (a plugin beforeEach scoped to the
external environment) — it currently only handles deop and inventory clear. Teaching it to
restore balance and clear kit cooldowns should let those exclusions come off the list.

Scope

Runner, Gradle plugin (ReuseSpec.kt, PlugwrightExtension), reporter, docs
(writing-tests.mdx reuse sections), example plugin's kit/shop/balance tests and their
excludeTests entries.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions