diff --git a/playwright.config.ts b/playwright.config.ts index 911b0bc6..41446ad1 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -1,5 +1,4 @@ import { register } from 'node:module' -import path from 'node:path' import { fileURLToPath } from 'node:url' import { defineConfig, devices } from '@playwright/test' @@ -11,20 +10,22 @@ import { assertTestDatabase } from './tests/helpers/assertTestDatabase' * OPS46 — a bare `playwright test` (VS Code extension, `pnpm exec playwright * test --list`, ad-hoc runs) must collect like `pnpm test:e2e` does. That * script carries the needed flags in NODE_OPTIONS (`--conditions=react-server - * --import=tsx/esm`); this block re-applies the same behavior when they are - * absent: + * --import=tsx/esm`); this block re-applies the same behavior whenever that + * PAIR is not complete (a lone `--conditions=react-server` from build/migrate + * still leaves `next/cache` unresolvable): * - register() installs the resolution hook (tests/helpers/e2eEsmResolve.mjs) * in THIS process — test collection happens here after config load, which * is why a bare `--list` needs it in-process; * - the NODE_OPTIONS mutation hands the same hook to the worker processes * Playwright spawns for actual runs (they inherit the env at spawn). - * When the flags are already present (the scripted path) nothing changes. + * When the scripted pair is already present nothing changes. */ -const bareInvocation = !process.env.NODE_OPTIONS?.includes('--conditions=react-server') -if (bareInvocation) { +const scriptedNodeOptions = + process.env.NODE_OPTIONS?.includes('--conditions=react-server') && + process.env.NODE_OPTIONS?.includes('--import=tsx/esm') +if (!scriptedNodeOptions) { register(new URL('./tests/helpers/e2eEsmResolve.mjs', import.meta.url)) - const configDir = path.dirname(fileURLToPath(import.meta.url)) - const loaderEntry = path.join(configDir, 'tests', 'helpers', 'e2eEsmLoader.mjs') + const loaderEntry = fileURLToPath(new URL('./tests/helpers/e2eEsmLoader.mjs', import.meta.url)) process.env.NODE_OPTIONS = [process.env.NODE_OPTIONS, `--import=${loaderEntry}`] .filter(Boolean) .join(' ') diff --git a/tests/e2e/fixtures/campaignE2EFixtures.ts b/tests/e2e/fixtures/campaignE2EFixtures.ts index eeb4b4be..6f6d4d3d 100644 --- a/tests/e2e/fixtures/campaignE2EFixtures.ts +++ b/tests/e2e/fixtures/campaignE2EFixtures.ts @@ -79,7 +79,7 @@ export class CampaignE2EOwnership { readonly payload: Payload readonly runID = randomUUID() private counter = 0 - private readonly claimedMunicipalityIndexes = new Set() + private claimedAnyMunicipality = false private readonly touchedMunicipalities = new Set() private readonly owned = new Map>( deletionOrder.map((collection) => [collection, new Set()]), @@ -155,7 +155,7 @@ export class CampaignE2EOwnership { municipalityCatalog.length, this.runID, ) - this.claimedMunicipalityIndexes.add(index) + this.claimedAnyMunicipality = true const slug = municipalityCatalog[index]!.slug const found = await this.rootPayload.find({ collection: 'municipality', @@ -423,9 +423,6 @@ export class CampaignE2EOwnership { } async cleanup(): Promise { - if (this.claimedMunicipalityIndexes.size > 0) { - await releaseMunicipalityClaims(this.rootPayload, this.runID) - } await this.discoverOwnedRows() await withPayloadTransaction(this.rootPayload, async ({ req, transactionID }) => { const transaction = this.rootPayload.db.sessions?.[String(transactionID)]?.db as @@ -499,6 +496,13 @@ export class CampaignE2EOwnership { }) } }) + // Release the allocator claim only AFTER the run's rows are purged: a + // concurrent run that claims this slot in the meantime would purge-on-claim + // the rows still live here (the release→purge window is the corruption the + // registry exists to prevent). + if (this.claimedAnyMunicipality) { + await releaseMunicipalityClaims(this.rootPayload, this.runID) + } } async expectNoOwnedRows(): Promise { diff --git a/tests/helpers/campaignFixtures.ts b/tests/helpers/campaignFixtures.ts index 920e028b..55ad9731 100644 --- a/tests/helpers/campaignFixtures.ts +++ b/tests/helpers/campaignFixtures.ts @@ -216,7 +216,7 @@ export class CampaignFixtures { private cleaned = false private counter = 0 private municipalityCursor = 0 - private readonly claimedMunicipalityIndexes = new Set() + private claimedAnyMunicipality = false private readonly markers = new Set() private readonly owned = emptyOwnedIDs() private readonly touchedMunicipalities = new Set() @@ -363,7 +363,7 @@ export class CampaignFixtures { municipalityCatalog.length, this.runID, ) - this.claimedMunicipalityIndexes.add(index) + this.claimedAnyMunicipality = true requestedSlug = municipalityCatalog[index]!.slug this.municipalityCursor += 1 } @@ -1016,9 +1016,6 @@ export class CampaignFixtures { async cleanup(): Promise { if (this.cleaned) return - if (this.claimedMunicipalityIndexes.size > 0) { - await releaseMunicipalityClaims(this.rootPayload, this.runID) - } await this.discoverDependents() await withPayloadTransaction(this.rootPayload, async ({ req }) => { for (const collection of [ @@ -1072,6 +1069,13 @@ export class CampaignFixtures { ) } }) + // Release the allocator claim only AFTER the run's rows are purged: a + // concurrent run that claims this slot in the meantime would purge-on-claim + // the rows still live here (the release→purge window is the corruption the + // registry exists to prevent). + if (this.claimedAnyMunicipality) { + await releaseMunicipalityClaims(this.rootPayload, this.runID) + } this.cleaned = true } diff --git a/tests/int/campaignMunicipalityAllocator.int.spec.ts b/tests/int/campaignMunicipalityAllocator.int.spec.ts index b778b4c1..40ad8843 100644 --- a/tests/int/campaignMunicipalityAllocator.int.spec.ts +++ b/tests/int/campaignMunicipalityAllocator.int.spec.ts @@ -63,11 +63,13 @@ describe('campaign municipality allocator registry', () => { const runID = freshRunID() const first = await claimMunicipalityIndex(payload, catalogSize, runID) const second = await claimMunicipalityIndex(payload, catalogSize, runID) - expect(second).not.toBe(first) - expect([first, second]).not.toContain(anchorIndex) + expect(second, 'claims of the same run must be distinct').not.toBe(first) + expect([first, second], 'a live run must never receive the anchor slot').not.toContain( + anchorIndex, + ) await releaseMunicipalityClaims(payload, runID) } - expect(await claimCount(anchor)).toBe(1) + expect(await claimCount(anchor), 'the anchor claim must survive every wrap').toBe(1) }) it('fails loudly with an exhaustion error instead of colliding, then recovers on release', async () => { @@ -106,7 +108,7 @@ describe('campaign municipality allocator registry', () => { const crashed = freshRunID() const survivor = freshRunID() - const crashedIndex = await claimMunicipalityIndex(payload, catalogSize, crashed) + await claimMunicipalityIndex(payload, catalogSize, crashed) // Simulate a run killed before cleanup: age its claim past the 2h window. await payload.db.drizzle.execute(sql` UPDATE "campaign_fixture_municipality_claims" @@ -114,17 +116,20 @@ describe('campaign municipality allocator registry', () => { WHERE "run_id" = ${crashed} `) - // The stale slot is reclaimable: keep claiming until the survivor lands - // on the crashed run's index (the sequence advances one slot per claim, - // so a bounded loop covers every slot of the residue space). - let reclaimed = false - for (let attempt = 0; attempt < catalogSize * 3 && !reclaimed; attempt += 1) { - if ((await claimMunicipalityIndex(payload, catalogSize, survivor)) === crashedIndex) { - reclaimed = true - } + // A stale claim must not block the space forever: keep claiming until + // the crashed run's claim is gone. Under the PARALLEL suite another spec + // may steal the aged slot first (shared cursor), so the invariant is + // "the stale claim disappears while the survivor claims succeed" — not + // "the survivor lands on that exact slot". + let stolen = false + for (let attempt = 0; attempt < catalogSize * 3 && !stolen; attempt += 1) { + await claimMunicipalityIndex(payload, catalogSize, survivor) + if ((await claimCount(crashed)) === 0) stolen = true } - expect(reclaimed, 'stale claim must be stealable').toBe(true) - expect(await claimCount(crashed)).toBe(0) + expect(stolen, 'a stale claim must be stolen (or the slot reclaimed) within the loop').toBe( + true, + ) + expect(await claimCount(crashed), 'crashed run must hold no claim after the steal').toBe(0) // The survivor may hold up to catalogSize slots at this point; release // inline so the later tests keep free space (the afterAll is the net). await releaseMunicipalityClaims(payload, survivor) diff --git a/tests/unit/testMunicipalityAllocatorConventions.unit.spec.ts b/tests/unit/testMunicipalityAllocatorConventions.unit.spec.ts index 8d281445..59d6bf20 100644 --- a/tests/unit/testMunicipalityAllocatorConventions.unit.spec.ts +++ b/tests/unit/testMunicipalityAllocatorConventions.unit.spec.ts @@ -112,7 +112,7 @@ describe('municipality allocator discipline (miss #73)', () => { for (const file of scanned) { const source = readFileSync(file, 'utf8') - if (/nextval\(|campaign_fixture_municipality_alloc/.test(source)) { + if (/\bnextval\s*\(|\bsetval\s*\(|campaign_fixture_municipality_alloc/.test(source)) { offenders.push(repoPath(file)) } }