From 7b4dd6f27e8fe30be4fe59df99f449dda5fddd15 Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Wed, 5 Aug 2026 12:12:06 +0200 Subject: [PATCH] test(media): inject the failed registry write instead of chmod-ing the temp dir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "logs a refresh it cannot write" arranged its failure with `chmod(tempDir, 0o555)`. That is not a portable way to make a write fail: on Windows the bit lands on a directory attribute that does not stop a file being created inside, so the refresh succeeded, nothing warned, and the case failed on every Windows dev machine. `skipIf(process.getuid?.() === 0)` could not catch it either — `getuid` is undefined there, so the guard read as "not root" and ran anyway. CI only runs the suite on ubuntu-latest, so this stayed green there while being permanently red locally — which is the worst shape a test can have: it teaches you to skim past a red suite on the machine the app is actually developed on. The write is now failed by injection (`vi.spyOn(fs, "writeFile")`), which asserts the same thing on every platform and at any privilege level, and the spy is asserted to have been called so the case cannot pass with the refresh path deleted. Verified by ablation: dropping the `.catch()` in findMediaLinksByFingerprint still turns it red, with the rejection escaping as the unhandled one it exists to prevent. --- electron/media/mediaLinksRegistry.test.ts | 55 +++++++++++++---------- 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/electron/media/mediaLinksRegistry.test.ts b/electron/media/mediaLinksRegistry.test.ts index 4247dfb14..bf50b11cf 100644 --- a/electron/media/mediaLinksRegistry.test.ts +++ b/electron/media/mediaLinksRegistry.test.ts @@ -244,31 +244,38 @@ describe("mediaLinksRegistry", () => { return rejections; } - // Running as root defeats the permission bit this case relies on. Skipped - // out loud rather than passing vacuously. - it.skipIf(process.getuid?.() === 0)( - "logs a refresh it cannot write, and still answers the lookup", - async () => { - const { original, moved } = await registerThenMove(); - // Registry readable, directory unwritable: only the write can fail. - await fs.chmod(tempDir, 0o555); - const warned = vi.spyOn(console, "warn").mockImplementation(() => { - // swallowed: the test asserts on it, the suite output does not need it + // The failure is INJECTED, not arranged with `chmod 0o555`. A read-only + // directory does not stop a file being created inside it on Windows, so the + // refresh wrote fine there and this case failed on every Windows dev machine + // while staying green in Linux CI. `skipIf(getuid() === 0)` could not see it + // either: `process.getuid` is undefined on Windows, so the guard read as + // "not root" and ran the test anyway. Failing the write itself asserts the + // same thing on every platform, root or not. + it("logs a refresh it cannot write, and still answers the lookup", async () => { + const { original, moved } = await registerThenMove(); + const warned = vi.spyOn(console, "warn").mockImplementation(() => { + // swallowed: the test asserts on it, the suite output does not need it + }); + // Registry readable, the tmp-file write refused: only the write can fail. + const write = vi + .spyOn(fs, "writeFile") + .mockRejectedValue(Object.assign(new Error("permission denied"), { code: "EACCES" })); + try { + const rejections = await withoutUnhandledRejections(async () => { + const resolved = await findMediaLinksByFingerprint(tempDir, moved); + // A refresh that failed is not a lookup that failed. + expect(resolved?.webcamVideoPath).toBe(`${original}-cam.webm`); }); - try { - const rejections = await withoutUnhandledRejections(async () => { - const resolved = await findMediaLinksByFingerprint(tempDir, moved); - // A refresh that failed is not a lookup that failed. - expect(resolved?.webcamVideoPath).toBe(`${original}-cam.webm`); - }); - expect(rejections).toEqual([]); - expect(warned).toHaveBeenCalled(); - } finally { - warned.mockRestore(); - await fs.chmod(tempDir, 0o755); - } - }, - ); + expect(rejections).toEqual([]); + expect(warned).toHaveBeenCalled(); + // The refresh was really attempted: without this the case would pass + // just as well with the whole write path deleted. + expect(write).toHaveBeenCalled(); + } finally { + write.mockRestore(); + warned.mockRestore(); + } + }); it("survives the directory disappearing while the refresh is queued", async () => { // The CI shape: a suite's `afterEach` removes its temp dir while a write