Fix flaky download.test.js caused by leaked URL mock across test files

[AlanCalvillo]

Summary

• Fixes an intermittent CI failure in src/utils/download.test.js where blob.type assertions randomly received a wrong MIME type (e.g. text/javascript, image/jpeg instead of text/plain/application/json).
• Root cause: img.test.js and eventMapIcons.test.js replaced global.URL.createObjectURL/revokeObjectURL via direct assignment (global.URL.createObjectURL = jest.fn()) instead of jest.spyOn. jest.restoreAllMocks() can only undo spies, not plain assignments, so the mock permanently replaced the real implementation for the rest of that Jest worker process. Because jest-fixed-jsdom binds window.URL to Node's actual process-wide singleton class (not a per-file copy), this leaked mock silently accumulated calls from other test files. When download.test.js spied on the same (already-mocked) function, it inherited those stale calls, so mock.calls[0] sometimes pointed at another test's call instead of its own.
• Fix: both files now use jest.spyOn(...).mockImplementation(...), which jest.restoreAllMocks() (already called in afterEach/added afterAll) can properly undo.

Test plan

• yarn jest src/utils/img.test.js src/utils/eventMapIcons.test.js src/utils/download.test.js — all pass
• Reproduced the original flake locally (~1-in-3 full-suite runs failed before the fix)
• Ran the full suite (yarn test-ci) 6 consecutive times after the fix — all green, no recurrence
• eslint on both changed files — no new issues introduced

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Fixes intermittent Jest CI failures caused by leaking URL.createObjectURL / URL.revokeObjectURL mocks across test files in the same worker process. This aligns with the codebase’s testing approach by ensuring global browser APIs are mocked via spies that jest.restoreAllMocks() can reliably undo.

Changes:

• Replaced direct global.URL.* = jest.fn() assignments with jest.spyOn(...).mockImplementation(...) in affected tests.
• Added a jest.restoreAllMocks() cleanup in img.test.js to ensure the URL spies are restored after the suite completes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/utils/img.test.js	Switches URL mocks to spies and adds teardown to prevent cross-file leakage.
src/utils/eventMapIcons.test.js	Switches URL mocks to spies so jest.restoreAllMocks() can properly restore globals after each test.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[luixlive]

Approved. Probably worth doing what Copilot suggests for scalability.

[AlanCalvillo]

Approved. Probably worth doing what Copilot suggests for scalability.

i chatted it with Claude, and it seems like the pattern is right, but it's not really a concern since internally Image uses its own constructor per test (vs. the global object used in URL).

here Claude's take:

Why URL/Blob leaked but Image doesn't: jest-fixed-jsdom explicitly hoists Blob, URL, fetch, Headers, etc. from Node's real, process-wide singleton classes into each test file's environment (this.global.URL = URL, using the outer Node global). Since that's the same class object reused across every file's environment in a worker, an unrestored mock left sitting on it persists into later files. Image isn't in that hoisted list at all — it's jsdom's own HTMLImageElement-backed constructor, freshly generated per JSDOM() instance, i.e. per test file.

I confirmed this directly: I wrote two throwaway test files run with --maxWorkers=1 (the same single-process, sequential setup that reproduced the real URL leak) — one that permanently replaces global.Image with a class named PoisonedImage and never restores it, and a second file that runs after it and checks global.Image.name. Result: the second file got the pristine native Image, not the poisoned one. No leak.

Also worth noting: even setting that aside, both files reassign global.Image fresh in beforeEach before every test, so there's no cross-test accumulation risk within a file either — unlike URL.createObjectURL, nothing depends on that mock's call history surviving between tests.