From 6f8a0db1302567305c3309ac0d5233e6207aa242 Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Thu, 3 Sep 2026 13:43:25 -0500 Subject: [PATCH] test(integration): clean up background-removal test-harness leftovers (#281) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit backgroundRemoval.integration.test.ts exported seedSubmission for no reason — nothing imports it, since draftingBackgroundRemoval.integration.test.ts and adminItemDrafts.integration.test.ts each wrote their own seeding helpers. Dropped the export, kept the function for local use. All three of these suites create a temporary uploads directory with mkdtemp and point UPLOADS_DIR at it, but none of them removed the directory afterward or restored the previous UPLOADS_DIR value — checked and the leak existed in all three, not just the one the review flagged. Each afterEach now removes its temp directory with fs.rm and restores (or deletes) UPLOADS_DIR to what it held before the test touched it, so this suite no longer leaves rubbish in the OS temp directory or a stale environment variable for whatever runs after it in the same process. This is test scaffolding cleanup, not a feature change — no runtime path in the application deletes anything. Co-Authored-By: Claude Opus 5 --- .../adminItemDrafts.integration.test.ts | 9 +++++++++ .../backgroundRemoval.integration.test.ts | 15 +++++++++++++-- .../draftingBackgroundRemoval.integration.test.ts | 9 +++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/backend/tests/integration/adminItemDrafts.integration.test.ts b/backend/tests/integration/adminItemDrafts.integration.test.ts index 4e4897a..6ebfca8 100644 --- a/backend/tests/integration/adminItemDrafts.integration.test.ts +++ b/backend/tests/integration/adminItemDrafts.integration.test.ts @@ -317,10 +317,12 @@ describe('the review queue’s background-removal control', () => { let uploads = ''; let stub: http.Server | null = null; + let previousUploadsDir: string | undefined; /** A stub sidecar on an ephemeral port, and a temporary uploads directory. */ async function startStub(status: number, body: Buffer | string): Promise { uploads = await fsp.mkdtemp(path.join(os.tmpdir(), 'adminbg-')); + previousUploadsDir = process.env.UPLOADS_DIR; process.env.UPLOADS_DIR = uploads; stub = http.createServer((req, res) => { @@ -340,6 +342,13 @@ describe('the review queue’s background-removal control', () => { await new Promise((resolve) => stub!.close(() => resolve())); stub = null; } + if (uploads !== '') { + await fsp.rm(uploads, { recursive: true, force: true }); + uploads = ''; + } + if (previousUploadsDir === undefined) delete process.env.UPLOADS_DIR; + else process.env.UPLOADS_DIR = previousUploadsDir; + previousUploadsDir = undefined; }); /** A ready draft with one photo, on disk, named to match the assertions. */ diff --git a/backend/tests/integration/backgroundRemoval.integration.test.ts b/backend/tests/integration/backgroundRemoval.integration.test.ts index 0832617..a0eba9c 100644 --- a/backend/tests/integration/backgroundRemoval.integration.test.ts +++ b/backend/tests/integration/backgroundRemoval.integration.test.ts @@ -17,7 +17,7 @@ afterAll(async () => { }); /** An item with a draft row and one image, which is what a submission leaves. */ -export async function seedSubmission( +async function seedSubmission( imagePath = '/uploads/photo.jpg' ): Promise<{ itemId: number; imageId: number }> { const item = await pool.query<{ id: number }>( @@ -69,13 +69,16 @@ interface ImagePaths { let uploads = ''; let stub: http.Server | null = null; +let previousUploadsDir: string | undefined; /** * A real uploads directory and a stub sidecar. * * A temporary directory rather than the configured one, because these tests * write files and a suite that leaves rubbish in a developer's uploads volume - * is a suite people stop running. + * is a suite people stop running. `afterEach` removes it and puts back + * whatever UPLOADS_DIR held before, so this suite does not leak either the + * directory or the environment variable into whatever runs after it. * * No test here contacts the real sidecar. It takes forty seconds to start, and * a suite that depends on that is broken by construction. @@ -84,6 +87,7 @@ async function startStub( handler: (req: http.IncomingMessage, res: http.ServerResponse) => void ): Promise { uploads = await fsp.mkdtemp(path.join(os.tmpdir(), 'bgremoval-')); + previousUploadsDir = process.env.UPLOADS_DIR; process.env.UPLOADS_DIR = uploads; stub = http.createServer((req, res) => { @@ -107,6 +111,13 @@ afterEach(async () => { await new Promise((resolve) => stub!.close(() => resolve())); stub = null; } + if (uploads !== '') { + await fsp.rm(uploads, { recursive: true, force: true }); + uploads = ''; + } + if (previousUploadsDir === undefined) delete process.env.UPLOADS_DIR; + else process.env.UPLOADS_DIR = previousUploadsDir; + previousUploadsDir = undefined; }); async function seedWithFile(): Promise<{ itemId: number; imageId: number; name: string }> { diff --git a/backend/tests/integration/draftingBackgroundRemoval.integration.test.ts b/backend/tests/integration/draftingBackgroundRemoval.integration.test.ts index 3180f6e..f3fbd2a 100644 --- a/backend/tests/integration/draftingBackgroundRemoval.integration.test.ts +++ b/backend/tests/integration/draftingBackgroundRemoval.integration.test.ts @@ -27,6 +27,7 @@ const PNG_BYTES = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0 let uploads = ''; let stub: http.Server | null = null; +let previousUploadsDir: string | undefined; beforeEach(async () => { await resetDb(); @@ -51,6 +52,7 @@ beforeEach(async () => { resetAnthropicClient(); uploads = await fsp.mkdtemp(path.join(os.tmpdir(), 'workerbg-')); + previousUploadsDir = process.env.UPLOADS_DIR; process.env.UPLOADS_DIR = uploads; stub = http.createServer((req, res) => { @@ -72,6 +74,13 @@ afterEach(async () => { await new Promise((resolve) => stub!.close(() => resolve())); stub = null; } + if (uploads !== '') { + await fsp.rm(uploads, { recursive: true, force: true }); + uploads = ''; + } + if (previousUploadsDir === undefined) delete process.env.UPLOADS_DIR; + else process.env.UPLOADS_DIR = previousUploadsDir; + previousUploadsDir = undefined; }); afterAll(async () => {