[v3] feat: concurrent test execution via concurrency - #67
Conversation
session.consoleLog.clear() wiped the whole shared buffer at the start of every test. Fine when only one test runs at a time, but it means two tests running concurrently would race to wipe each other's messages out from under them. ServerWrapper now captures its own startIndex at construction (and can resetCursor() to "now"), and the toHaveReceivedMessage matcher defaults to reading from that instead of index 0 when since isn't given. Same observable behavior for a solo test, but the log itself is never destroyed, so nothing racing to read it can lose messages.
test(name, { concurrency: N }, fn) and describe.serial(name, { concurrency: N }, fn)
fan out into N independent instances running at once, each with its
own bot leased from the account pool. One failing instance fails the
whole result — this is for races between real players, not a
pass-rate to average.
- Whole-session preflight: every spec file is loaded (imported once,
registrations snapshotted) before any test runs, and every
concurrency value is checked against the account pool's capacity()
up front. A misconfigured concurrency aborts immediately instead of
the Nth lease() hanging mid-run. An environment with no pool
(LocalMode mints a throwaway account per bot) has nothing to check.
- createBotScope.close() used to call session.disconnectAllBots()
with no arguments, tearing down every bot in the session rather than
just its own scope's. Harmless when nothing ran concurrently; with
concurrency it would mean one finishing instance kicking every
still-running sibling's bot. Fixed to keep every bot outside its own
scope.
- The report still has one row per test/test-in-block. Its durationMs
is the slowest instance, and a new instances array carries every
instance's own outcome (bot username, pass/fail, duration) so a
failure names which bot lost the race. Wired into the console
summary and the JSON report; JUnit keeps the single aggregated
pass/fail, no per-instance breakdown.
- A concurrent describe.serial block runs N full copies of the block;
instances can diverge mid-block (one loses its race and stops early
while another keeps going), so a test position only counts as
skipped if every instance skipped it.
- Console log lines (Test:, Serial block:, PASSED/FAILED, bot
creation) are tagged with [i/N] — N instances logging the same test
name at the same time is unreadable without it.
Covers both shapes: a plain test with concurrency, and a concurrent describe.serial block. Each instance checks its own marker against the shared server log and confirms it's still connected afterward — the two bugs concurrency exposed (destructive log clear, scope-wide bot teardown) would show up here as flaky or crashed instances.
Writing Tests gets a new section covering test()/describe.serial with concurrency: what it's for, what you get back, how many instances you can ask for, and why the server log stays shared and unfiltered across instances. Reports gets the aggregated JSON shape (instances array, botUsername) and a note that JUnit only ever sees the one aggregate.
…ut isn't full expect(server).toHaveReceivedMessage() needs consoleOutput: 'full'. The stand environment's RCON console only offers 'responses', same limitation simple-ts.spec.ts's 'server logs command execution' already declares — this test needed the same requires and didn't have it, so it failed test-example-plugin-stand in CI instead of skipping.
Two additions, both surfacing data the aggregate result already had: - TestInstanceResult gets index (1-based, matching the [i/N] console log tag for that same run) — was missing from both the console detail breakdown and the JSON report. - The summary table's own row for a concurrent test now tags itself with [passed/total] next to the duration, instead of that count only showing up in the Failed Tests detail section below. A passing concurrent test previously gave no indication in the table that it was even concurrent.
|
Nice work on the concurrency support, the overall approach looks solid. Found a couple of race conditions around bot teardown during review and a small ux issue 1. Bot connection leak on join failure
2. Race condition in The method awaits disconnection of the unwanted bots, then filters 3. Inner If someone sets 4. Load order changed by preflight check The Let me know if any of this needs more context, happy to help in any way |
disconnectAllBots recomputed "remaining" from this.bots after its await, using a keepSet fixed at call start. A bot connected by a concurrent test while the await was in flight was neither in keepSet nor among the ones just disconnected, so the post-await reset silently dropped it from tracking without ever disconnecting it. Now it snapshots the bots to disconnect before awaiting and removes exactly those afterward, leaving anything added mid-await untouched.
createBotScope.close() tracked "own" bots as connected.map(p => p.bot) — populated only after a successful join(). A bot whose join() failed never landed in connected, so close() treated it as belonging to another scope and left it connected, leaking the connection. Track every bot the scope creates in ownBots instead, regardless of join outcome, and tear down from that.
A serial block always runs its tests sequentially on one player, so a per-test concurrency set inside one was silently ignored by the block runner — nothing warned, the test just ran once. Now it throws at registration time, same as an invalid concurrency value does.
validateConcurrency needed every file loaded (imported) up front to check concurrency before any test ran, which meant main/suite spec files were now imported before preflight ran — importing against whatever state preflight was about to set up, rather than what it left behind. Preflight files are now loaded, validated, and run on their own first; main and suite files are loaded and validated afterward, restoring the original import order while keeping the fail-fast concurrency check.
5a59f1e to
37fc104
Compare
The previous fix tracked "own" bots in ownBots, snapshotted once per connect() call. player.rejoin() swaps a player's .bot for a freshly created connection under the same username but never touches ownBots, so close() no longer recognized the new bot as its own and left it connected — a permanent "already playing" kick for every later test that leased the same account. close() now also reads connected.map(p => p.bot) fresh, which reflects whatever rejoin() last swapped in.
|
Thanks for the review, all four caught real bugs. Pushed fixes: 1. Bot leak on join failure ( 2. Race in 3. Silent nested concurrency ( 4. Load order ( One more I found while testing against the stand suite: my first pass at the Ran the full stand suite against this — clean except for one |
2 participants
Closes #64.
test()anddescribe.serial()had no way to put several bots on the same feature at once — everything runs as one instance, sequentially,awaited one at a time. Races between real players (two bots claiming the same chest, buying the last item in stock) never got exercised.{ concurrency: N }N independent instances run at once, each with its own bot leased from the pool. One instance failing fails the whole result — this is for finding a race, not averaging a pass rate.
describe.serialblocks take the same option and run N full copies of the ordered chain.Concurrency is validated against
AccountPool.capacity()before any test in the session runs (every spec file is loaded up front for this — seerunner.ts), soconcurrency: 10against a 4-account pool fails immediately instead of the 5th bot hanging on a lease.LocalModehas no pool and nothing to check; its ceiling ismax-players.Two bugs concurrency exposed
session.consoleLog.clear()at the top of every test would have raced two concurrent tests wiping each other's messages. Replaced with a non-destructive cursor:ServerWrappercapturessession.consoleLog.lengthat construction, andtoHaveReceivedMessagedefaults to reading from there instead of index 0.createBotScope.close()calledsession.disconnectAllBots()with no arguments, tearing down every bot in the session — harmless sequentially, but the first concurrent instance to finish would have kicked every still-running sibling. Fixed to keep every bot outside its own scope.Report shape
Still one row per test/test-in-block —
concurrencymultiplies bots, not report rows.durationMsis the slowest instance; a newinstancesarray carries every instance's own outcome (bot username, pass/fail, duration), so a failure names which bot lost. Wired into the console summary and the JSON report; JUnit keeps the single aggregate.Console log labeling
Test:,Serial block:,PASSED/FAILED, and bot-creation lines get an[i/N]tag — N instances logging the same test name at the same time was unreadable without it:Docs
Writing Tests gets a new section on
concurrency; Reports gets the aggregated JSON shape.Checked
tsc --noEmitclean.plugwrightTestLocal: 54/54 pass, run three times across the changes.