refactor(OPS46): simplify pós-revisão — release do claim ao fim do cleanup (janela purge segura), gate da hook exige o par de flags, Set→boolean, guard setval/nextval\s*\( e spec de steal invariante-primeiro

This commit is contained in:
Francisco Solla 2026-08-23 14:04:52 -03:00
parent 729ce6fed4
commit a74a2dca9e
5 changed files with 47 additions and 33 deletions

View file

@ -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(' ')

View file

@ -79,7 +79,7 @@ export class CampaignE2EOwnership {
readonly payload: Payload
readonly runID = randomUUID()
private counter = 0
private readonly claimedMunicipalityIndexes = new Set<number>()
private claimedAnyMunicipality = false
private readonly touchedMunicipalities = new Set<number>()
private readonly owned = new Map<OwnedCollection, Set<number>>(
deletionOrder.map((collection) => [collection, new Set<number>()]),
@ -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<void> {
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<void> {

View file

@ -216,7 +216,7 @@ export class CampaignFixtures {
private cleaned = false
private counter = 0
private municipalityCursor = 0
private readonly claimedMunicipalityIndexes = new Set<number>()
private claimedAnyMunicipality = false
private readonly markers = new Set<string>()
private readonly owned = emptyOwnedIDs()
private readonly touchedMunicipalities = new Set<number>()
@ -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<void> {
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
}

View file

@ -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)

View file

@ -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))
}
}