diff --git a/libs/nestjs/src/lib/admin/backup/scheduled-backup.service.spec.ts b/libs/nestjs/src/lib/admin/backup/scheduled-backup.service.spec.ts index aca5293..7ddb8bb 100644 --- a/libs/nestjs/src/lib/admin/backup/scheduled-backup.service.spec.ts +++ b/libs/nestjs/src/lib/admin/backup/scheduled-backup.service.spec.ts @@ -217,8 +217,13 @@ describe('ScheduledBackupService', () => { const second = svc.runNowOrJoin(); expect(first).toEqual({ started: true }); expect(second).toEqual({ started: false }); - // let the in-flight cycle settle - await new Promise((r) => setTimeout(r, 50)); + // Deterministically JOIN the same in-flight cycle rather than sleeping a + // fixed wall-clock 50ms: runSnapshotCycle returns the live `inFlight` promise + // (never starts a second), so awaiting it waits for the fire-and-forget cycle + // to fully finish writing. The old setTimeout(50) let the cycle outlive the + // wait under CPU load, so afterEach's `fs.rm(dir)` raced the still-writing + // cycle and intermittently threw ENOTEMPTY (rmdir on a non-empty dir). + await svc.runSnapshotCycle('manual'); expect((backup.writeSnapshot as jest.Mock).mock.calls.length).toBe(1); }); diff --git a/libs/nestjs/src/lib/common/middleware/request-id.middleware.spec.ts b/libs/nestjs/src/lib/common/middleware/request-id.middleware.spec.ts index de9c786..4b1e677 100644 --- a/libs/nestjs/src/lib/common/middleware/request-id.middleware.spec.ts +++ b/libs/nestjs/src/lib/common/middleware/request-id.middleware.spec.ts @@ -1,4 +1,6 @@ -import express, { type Express, type Request, type Response } from 'express'; +import type { Server } from 'node:http'; + +import express, { type Request, type Response } from 'express'; import request from 'supertest'; import { RequestIdMiddleware } from './request-id.middleware'; @@ -10,19 +12,32 @@ const UUID_V7 = /^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; describe('RequestIdMiddleware', () => { - let app: Express; + // One express app + one persistent ephemeral server for the whole suite, + // matching every other supertest spec in the lib (they all listen once in + // beforeAll). The middleware is stateless — each request still mints a fresh + // id — so there is no per-test state to reset. Passing a fresh `express()` + // app to supertest per test makes it stand up AND tear down a brand-new + // ephemeral server for every request; under full-suite CPU saturation that + // rapid listen/connect/close churn intermittently surfaced as a client-side + // "socket hang up", the sole source of this suite's flakiness (TEST-0006). + let server: Server; - beforeEach(() => { + beforeAll((done) => { const mw = new RequestIdMiddleware(); - app = express(); + const app = express(); app.use((req: Request, res: Response, next) => mw.use(req, res, next)); app.get('/probe', (req: Request, res: Response) => { res.json({ openbucket: req.openbucket }); }); + server = app.listen(0, done); + }); + + afterAll((done) => { + server.close(done); }); it('case 1: mints a UUIDv7 and sets both response headers', async () => { - const res = await request(app).get('/probe'); + const res = await request(server).get('/probe'); expect(res.status).toBe(200); const id = res.body.openbucket.requestId; @@ -33,21 +48,21 @@ describe('RequestIdMiddleware', () => { it('case 2: reuses a syntactically valid upstream X-Request-Id', async () => { const upstream = '0190d9c1-7f32-7c0c-bea5-1f51d1c0b2c4'; - const res = await request(app).get('/probe').set('X-Request-Id', upstream); + const res = await request(server).get('/probe').set('X-Request-Id', upstream); expect(res.body.openbucket.requestId).toBe(upstream); expect(res.headers['x-request-id']).toBe(upstream); }); it('case 3: discards a malformed upstream X-Request-Id and mints fresh', async () => { - const res = await request(app).get('/probe').set('X-Request-Id', 'not-a-uuid'); + const res = await request(server).get('/probe').set('X-Request-Id', 'not-a-uuid'); expect(res.body.openbucket.requestId).not.toBe('not-a-uuid'); expect(res.body.openbucket.requestId).toMatch(UUID_V7); }); it('case 4: initializes the placeholder context (kind=s3, receivedAt=0)', async () => { - const res = await request(app).get('/probe'); + const res = await request(server).get('/probe'); expect(res.body.openbucket.kind).toBe('s3'); expect(res.body.openbucket.receivedAt).toBe(0); diff --git a/libs/nestjs/src/lib/s3/concurrency.spec.ts b/libs/nestjs/src/lib/s3/concurrency.spec.ts index 0b10365..4d6a522 100644 --- a/libs/nestjs/src/lib/s3/concurrency.spec.ts +++ b/libs/nestjs/src/lib/s3/concurrency.spec.ts @@ -93,20 +93,23 @@ describe('concurrency invariants (TEST-0317)', () => { await fs.rm(dataDir, { recursive: true, force: true }); }); - // QUARANTINED (flaky) — these two cases assert concurrency invariants the write - // path does not yet guarantee for writers racing on the SAME target, so they - // fail intermittently / platform-dependently and destabilise CI: - // • same-partNumber UploadPart: both writers rename(2) onto the same `.part`. - // POSIX overwrites atomically (last-wins), but Windows rejects rename-over- - // existing, so the call rejects on a dev box. - // • concurrent first-time same-key PUT: the writer renames the blob BEFORE it - // commits the row; if the losing writer's row commit conflicts, its rollback - // unlinks the shared final blob and tears the winner's result. - // Re-enable after hardening concurrent same-target writes (e.g. per-(bucket,key) - // serialization in ObjectWriterService + rename-over-existing tolerance in - // BlobStore.atomicRename). The deterministic sequential case below stays active. - // Follow-up: harden concurrent same-target writes (see s3/CONCURRENCY.md §4.8). - it.skip('same-partNumber concurrent UploadPart does not throw EEXIST; the part is one whole writer', async () => { + // These two cases assert concurrency invariants for writers racing on the SAME + // target. Both were quarantined (it.skip) while the write path could tear under + // that race; the hardening they waited on has since landed, so they are now + // active and deterministic: + // • same-partNumber UploadPart: each writer stages to a randomUUID-suffixed + // tmp file (no O_EXCL collision) and rename(2)s onto the shared `.part`. + // On POSIX (Linux CI, macOS dev) rename-over-existing is atomic last-wins, + // so the final part is exactly one whole writer's payload — no tear, no + // EEXIST. (Windows rename-over is the only remaining platform caveat; the + // CI runner is Linux.) + // • concurrent first-time same-key PUT: ObjectWriterService now serializes + // writers of the same (bucket,key) through a keyed async mutex + // (`withKeyLock`, F6, commit c87ef90), so the two PUTs run strictly one + // after the other — the loser's rollback can no longer unlink the winner's + // committed blob. Row, blob bytes, and ETag all agree on one winner. + // See s3/CONCURRENCY.md §4.8. + it('same-partNumber concurrent UploadPart does not throw EEXIST; the part is one whole writer', async () => { const uploadId = 'concurrent-upload'; const a = 'A'.repeat(4096); const b = 'B'.repeat(8192); @@ -136,7 +139,7 @@ describe('concurrency invariants (TEST-0317)', () => { expect((await fs.readFile(blobs.paths.blobPath('b', 'seq'))).toString()).toBe('second-wins'); }); - it.skip('concurrent PUT same key: SQLite serializes the writers; row + blob agree on one winner', async () => { + it('concurrent PUT same key: the per-key write lock serializes the writers; row + blob agree on one winner', async () => { const x = 'X'.repeat(500); const y = 'Y'.repeat(700);