From a4b879b19dde4b21402d0c2849b3b5e79ac50fa9 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Mon, 17 Aug 2026 11:48:45 +0200 Subject: [PATCH] bucket_catch: 45 eslint errors -> 5, no suppressions These were unmasked, not caused, last session: `lint` is `tsc --noEmit && eslint src`, and a broken tsc had been short-circuiting the && so eslint had never run on this package. The binding constraint is the 100% coverage gate, not the linter. Most idiomatic fixes for no-non-null-assertion add a branch that no test can reach, which trades a lint error for a coverage failure - and the no-suppressions rule forbids escaping either way. So every fix here is one that adds no branch: - no-non-null-assertion 18 -> 2: a waitFor already proves result.current is non-null, so capture it into a local instead of asserting on each read. App.tsx's files[0]! was pure noise (the same position also reported no-unnecessary-type-assertion). usePuzzleGameLoop's shift()! became index-then-shift, since the while condition already indexes [0] unasserted. - non-nullable-type-assertion-style 9 -> 0: the rule-swap trap - eslint asks for `!`, which the other rule forbids. Narrowed with instanceof. - unbound-method 10 -> 1: keep the MockInstance vi.spyOn already returns rather than detaching the method. This surfaced a latent bug: one test calls restoreAllMocks() mid-test, so the beforeEach handle was stale. - no-unnecessary-type-assertion 5 -> 1, require-await 1 -> 0. Coverage held at 100%: 584/217/98/535 -> 585/217/98/536. The +1 statement and +1 line are the index-then-shift split; branches unchanged at 217 is the number that proves no fix added an untested path. 145 tests, tsc 0. The 5 remaining are reported, not silenced. App.tsx's fileResult! needs the phase state to carry the result ({ kind: "done"; result }) - three smaller structures all failed the same way. The rest are documented in the evidence file. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011w2Pj4Ek3ZNMQVYcFHgXDY --- bucket_catch/packages/frontend/src/App.tsx | 2 +- .../frontend/src/components/DropZone.test.tsx | 74 ++++++++++++++----- .../src/components/PuzzleResult.test.tsx | 7 +- .../src/components/ScoreScreen.test.tsx | 2 +- .../src/hooks/useBasketControl.test.ts | 5 +- .../frontend/src/hooks/useGameLoop.test.ts | 35 ++++++--- .../frontend/src/hooks/usePuzzleGameLoop.ts | 6 +- .../src/hooks/usePuzzleGameLoopPlay.test.ts | 24 ++++-- .../frontend/src/lib/sliceImage.test.ts | 13 +++- .../frontend/src/lib/zipDownload.test.ts | 49 ++++++++---- .../frontend/src/test/puzzleLoopFixtures.ts | 5 +- 11 files changed, 155 insertions(+), 67 deletions(-) diff --git a/bucket_catch/packages/frontend/src/App.tsx b/bucket_catch/packages/frontend/src/App.tsx index cf6c327..0e3413a 100644 --- a/bucket_catch/packages/frontend/src/App.tsx +++ b/bucket_catch/packages/frontend/src/App.tsx @@ -65,7 +65,7 @@ export default function App(): React.ReactElement { if (mode === "puzzle") { return ( diff --git a/bucket_catch/packages/frontend/src/components/DropZone.test.tsx b/bucket_catch/packages/frontend/src/components/DropZone.test.tsx index c09ff0d..2ea613f 100644 --- a/bucket_catch/packages/frontend/src/components/DropZone.test.tsx +++ b/bucket_catch/packages/frontend/src/components/DropZone.test.tsx @@ -5,6 +5,20 @@ import { DropZone } from "./DropZone"; const makeFile = (name: string, type = "text/plain"): File => new File(["x"], name, { type }); +/** + * Query one element and narrow it, failing the test if it is absent. + * + * Both `as HTMLElement` and `!` are rejected here - the former by + * non-nullable-type-assertion-style, the latter by no-non-null-assertion - so + * the narrowing is done with a real check that also gives a useful message. + */ +const must = (el: Element | null, what: string): HTMLElement => { + if (!(el instanceof HTMLElement)) { + throw new Error(`expected to find ${what}`); + } + return el; +}; + describe("DropZone", () => { it("renders title and subtitle", () => { render( undefined} onPuzzle={() => undefined} />); @@ -23,7 +37,10 @@ describe("DropZone", () => { const { container } = render( undefined} onPuzzle={() => undefined} />, ); - const zone = container.firstChild as HTMLElement; + const zone = must( + container.firstChild as Element | null, + "the drop zone", + ); expect(zone.className).not.toContain("dragging"); fireEvent.dragOver(zone); expect(zone.className).toContain("dragging"); @@ -36,7 +53,10 @@ describe("DropZone", () => { const { container } = render( undefined} />, ); - const zone = container.firstChild as HTMLElement; + const zone = must( + container.firstChild as Element | null, + "the drop zone", + ); const f1 = makeFile("a.txt"); const f2 = makeFile("b.txt"); fireEvent.drop(zone, { dataTransfer: { files: [f1, f2] } }); @@ -48,7 +68,10 @@ describe("DropZone", () => { const { container } = render( undefined} onPuzzle={() => undefined} />, ); - const zone = container.firstChild as HTMLElement; + const zone = must( + container.firstChild as Element | null, + "the drop zone", + ); fireEvent.drop(zone, { dataTransfer: { files: [makeFile("a.txt")] } }); expect(screen.getByText("1 file ready")).toBeInTheDocument(); }); @@ -58,7 +81,10 @@ describe("DropZone", () => { const { container } = render( undefined} />, ); - const zone = container.firstChild as HTMLElement; + const zone = must( + container.firstChild as Element | null, + "the drop zone", + ); fireEvent.drop(zone, { dataTransfer: { files: [] } }); expect(onFiles).not.toHaveBeenCalled(); }); @@ -68,9 +94,10 @@ describe("DropZone", () => { const { container } = render( undefined} />, ); - const fileInput = container.querySelector( + const fileInput = must( + container.querySelector("input[type='file'][multiple]"), "input[type='file'][multiple]", - ) as HTMLInputElement; + ); const file = makeFile("c.txt"); Object.defineProperty(fileInput, "files", { value: [file], @@ -85,9 +112,10 @@ describe("DropZone", () => { const { container } = render( undefined} />, ); - const fileInput = container.querySelector( + const fileInput = must( + container.querySelector("input[type='file'][multiple]"), "input[type='file'][multiple]", - ) as HTMLInputElement; + ); Object.defineProperty(fileInput, "files", { value: [], configurable: true, @@ -101,9 +129,10 @@ describe("DropZone", () => { const { container } = render( undefined} />, ); - const fileInput = container.querySelector( + const fileInput = must( + container.querySelector("input[type='file'][multiple]"), "input[type='file'][multiple]", - ) as HTMLInputElement; + ); Object.defineProperty(fileInput, "files", { value: null, configurable: true, @@ -117,9 +146,10 @@ describe("DropZone", () => { const { container } = render( undefined} onPuzzle={onPuzzle} />, ); - const puzzleInput = container.querySelector( + const puzzleInput = must( + container.querySelector("input[accept='image/*']"), "input[accept='image/*']", - ) as HTMLInputElement; + ); const imgFile = makeFile("photo.png", "image/png"); Object.defineProperty(puzzleInput, "files", { value: [imgFile], @@ -134,9 +164,10 @@ describe("DropZone", () => { const { container } = render( undefined} onPuzzle={onPuzzle} />, ); - const puzzleInput = container.querySelector( + const puzzleInput = must( + container.querySelector("input[accept='image/*']"), "input[accept='image/*']", - ) as HTMLInputElement; + ); Object.defineProperty(puzzleInput, "files", { value: null, configurable: true, @@ -150,9 +181,10 @@ describe("DropZone", () => { const { container } = render( undefined} onPuzzle={onPuzzle} />, ); - const puzzleInput = container.querySelector( + const puzzleInput = must( + container.querySelector("input[accept='image/*']"), "input[accept='image/*']", - ) as HTMLInputElement; + ); Object.defineProperty(puzzleInput, "files", { value: [], configurable: true, @@ -165,9 +197,10 @@ describe("DropZone", () => { const { container } = render( undefined} onPuzzle={() => undefined} />, ); - const puzzleInput = container.querySelector( + const puzzleInput = must( + container.querySelector("input[accept='image/*']"), "input[accept='image/*']", - ) as HTMLInputElement; + ); const clickSpy = vi.spyOn(puzzleInput, "click").mockImplementation( () => undefined, ); @@ -208,9 +241,10 @@ describe("DropZone", () => { undefined} onPuzzle={onPuzzle} />, ); fireEvent.click(screen.getByText("3×3")); // change gridSize to 3 - const puzzleInput = container.querySelector( + const puzzleInput = must( + container.querySelector("input[accept='image/*']"), "input[accept='image/*']", - ) as HTMLInputElement; + ); const imgFile = makeFile("img.png", "image/png"); Object.defineProperty(puzzleInput, "files", { value: [imgFile], diff --git a/bucket_catch/packages/frontend/src/components/PuzzleResult.test.tsx b/bucket_catch/packages/frontend/src/components/PuzzleResult.test.tsx index 58aafbe..7b8a44f 100644 --- a/bucket_catch/packages/frontend/src/components/PuzzleResult.test.tsx +++ b/bucket_catch/packages/frontend/src/components/PuzzleResult.test.tsx @@ -129,9 +129,10 @@ describe("PuzzleResult", () => { const imgs = container.querySelectorAll("img"); expect(imgs).toHaveLength(2); // The grid div has inline style with gridTemplateColumns; its non-img children are holes - const gridEl = container.querySelector( - '[style*="repeat"]', - ) as HTMLElement; + const gridEl = container.querySelector('[style*="repeat"]'); + if (!(gridEl instanceof HTMLElement)) { + throw new Error("expected to find the puzzle grid element"); + } const holeDivs = Array.from(gridEl.children).filter( (c) => c.tagName === "DIV", ); diff --git a/bucket_catch/packages/frontend/src/components/ScoreScreen.test.tsx b/bucket_catch/packages/frontend/src/components/ScoreScreen.test.tsx index 6d39c40..fcc22a4 100644 --- a/bucket_catch/packages/frontend/src/components/ScoreScreen.test.tsx +++ b/bucket_catch/packages/frontend/src/components/ScoreScreen.test.tsx @@ -39,7 +39,7 @@ describe("ScoreScreen", () => { expect(screen.getByText("Missed")).toBeInTheDocument(); }); - it("shows download button for download mode with multiple files", async () => { + it("shows download button for download mode with multiple files", () => { render( undefined} />, ); diff --git a/bucket_catch/packages/frontend/src/hooks/useBasketControl.test.ts b/bucket_catch/packages/frontend/src/hooks/useBasketControl.test.ts index 917943c..8803f79 100644 --- a/bucket_catch/packages/frontend/src/hooks/useBasketControl.test.ts +++ b/bucket_catch/packages/frontend/src/hooks/useBasketControl.test.ts @@ -4,8 +4,9 @@ import { fireEvent } from "@testing-library/react"; import type React from "react"; import { useBasketControl } from "./useBasketControl"; -const makeRef = (val: T): React.RefObject => - ({ current: val }) as React.RefObject; +const makeRef = (val: T): React.RefObject => ({ + current: val, +}); describe("useBasketControl", () => { it("initialises basket x to half of window.innerWidth", () => { diff --git a/bucket_catch/packages/frontend/src/hooks/useGameLoop.test.ts b/bucket_catch/packages/frontend/src/hooks/useGameLoop.test.ts index 2c07b3f..5bdc477 100644 --- a/bucket_catch/packages/frontend/src/hooks/useGameLoop.test.ts +++ b/bucket_catch/packages/frontend/src/hooks/useGameLoop.test.ts @@ -3,8 +3,7 @@ import { renderHook, act, waitFor } from "@testing-library/react"; import type React from "react"; import { useGameLoop } from "./useGameLoop"; -const makeRef = (val: T): React.RefObject => - ({ current: val }) as React.RefObject; +const makeRef = (val: T): React.RefObject => ({ current: val }); describe("useGameLoop", () => { let rafCallbacks: FrameRequestCallback[]; @@ -88,8 +87,12 @@ describe("useGameLoop", () => { flushTick(); // allDone=true on first frame → setResult await waitFor(() => { expect(result.current).not.toBeNull(); }); - expect(result.current!.caught).toHaveLength(0); - expect(result.current!.missed).toHaveLength(0); + const settled = result.current; + if (settled === null) { + throw new Error('result never settled'); + } + expect(settled.caught).toHaveLength(0); + expect(settled.missed).toHaveLength(0); }); it("catches file and resolves result (caught branch + caught-draw branch)", async () => { @@ -114,8 +117,12 @@ describe("useGameLoop", () => { flushTick(); // tick 1: draw caught + allDone=true → setResult await waitFor(() => { expect(result.current).not.toBeNull(); }); - expect(result.current!.caught).toHaveLength(1); - expect(result.current!.missed).toHaveLength(0); + const settled = result.current; + if (settled === null) { + throw new Error('result never settled'); + } + expect(settled.caught).toHaveLength(1); + expect(settled.missed).toHaveLength(0); }); it("misses file (still-falling + missed branches)", async () => { @@ -142,8 +149,12 @@ describe("useGameLoop", () => { } await waitFor(() => { expect(result.current).not.toBeNull(); }); - expect(result.current!.caught).toHaveLength(0); - expect(result.current!.missed).toHaveLength(1); + const settled = result.current; + if (settled === null) { + throw new Error('result never settled'); + } + expect(settled.caught).toHaveLength(0); + expect(settled.missed).toHaveLength(1); }); it("does not start RAF when canvas.getContext returns null", () => { @@ -187,7 +198,11 @@ describe("useGameLoop", () => { } await waitFor(() => { expect(result.current).not.toBeNull(); }); - expect(result.current!.caught).toHaveLength(2); - expect(result.current!.missed).toHaveLength(0); + const settled = result.current; + if (settled === null) { + throw new Error('result never settled'); + } + expect(settled.caught).toHaveLength(2); + expect(settled.missed).toHaveLength(0); }); }); diff --git a/bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoop.ts b/bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoop.ts index 6696e20..b6f2197 100644 --- a/bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoop.ts +++ b/bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoop.ts @@ -85,8 +85,10 @@ export function usePuzzleGameLoop( scheduleRef.current.length > 0 && frame >= scheduleRef.current[0].spawnFrame ) { - // while condition guarantees length > 0, so shift() is always defined - const s = scheduleRef.current.shift()!; + // The while condition already indexes [0] unasserted, so this is + // the same non-null value; shift() only advances the queue. + const s = scheduleRef.current[0]; + scheduleRef.current.shift(); activeItemsRef.current.push({ kind: "puzzle", id: `puzzle-${s.piece.row}-${s.piece.col}`, diff --git a/bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoopPlay.test.ts b/bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoopPlay.test.ts index 057471c..80457ad 100644 --- a/bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoopPlay.test.ts +++ b/bucket_catch/packages/frontend/src/hooks/usePuzzleGameLoopPlay.test.ts @@ -51,8 +51,12 @@ describe("usePuzzleGameLoop play", () => { flushTick(); // tick 0: spawn, move, caught, flash, allDone → setResult await waitFor(() => { expect(result.current).not.toBeNull(); }); - expect(result.current!.caughtPieces).toHaveLength(1); - expect(result.current!.missedPieces).toHaveLength(0); + const settled = result.current; + if (settled === null) { + throw new Error('result never settled'); + } + expect(settled.caughtPieces).toHaveLength(1); + expect(settled.missedPieces).toHaveLength(0); }); it("misses piece (still-falling + missed branches)", async () => { @@ -77,8 +81,12 @@ describe("usePuzzleGameLoop play", () => { } await waitFor(() => { expect(result.current).not.toBeNull(); }); - expect(result.current!.caughtPieces).toHaveLength(0); - expect(result.current!.missedPieces).toHaveLength(1); + const settled = result.current; + if (settled === null) { + throw new Error('result never settled'); + } + expect(settled.caughtPieces).toHaveLength(0); + expect(settled.missedPieces).toHaveLength(1); }); it("tests flash-during and flash-expired branches with 2 pieces", async () => { @@ -114,8 +122,12 @@ describe("usePuzzleGameLoop play", () => { } await waitFor(() => { expect(result.current).not.toBeNull(); }); - expect(result.current!.caughtPieces).toHaveLength(1); - expect(result.current!.missedPieces).toHaveLength(1); + const settled = result.current; + if (settled === null) { + throw new Error('result never settled'); + } + expect(settled.caughtPieces).toHaveLength(1); + expect(settled.missedPieces).toHaveLength(1); }); it("uses drawImage when piece image is loaded (img.complete=true branch)", async () => { diff --git a/bucket_catch/packages/frontend/src/lib/sliceImage.test.ts b/bucket_catch/packages/frontend/src/lib/sliceImage.test.ts index 295af36..87e0fe4 100644 --- a/bucket_catch/packages/frontend/src/lib/sliceImage.test.ts +++ b/bucket_catch/packages/frontend/src/lib/sliceImage.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type { MockInstance } from "vitest"; import { sliceImage } from "./sliceImage"; // Captures the last Image instance created so tests can trigger onload/onerror. @@ -22,10 +23,14 @@ class FakeImage { } describe("sliceImage", () => { + let revokeUrlSpy: MockInstance; + beforeEach(() => { vi.stubGlobal("Image", FakeImage); vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:mock"); - vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => undefined); + revokeUrlSpy = vi + .spyOn(URL, "revokeObjectURL") + .mockImplementation(() => undefined); }); afterEach(() => { @@ -44,7 +49,7 @@ describe("sliceImage", () => { expect(pieces).toHaveLength(4); // 2×2 grid expect(pieces[0]).toMatchObject({ row: 0, col: 0, gridSize: 2 }); expect(pieces[3]).toMatchObject({ row: 1, col: 1, gridSize: 2 }); - expect(URL.revokeObjectURL).toHaveBeenCalledWith("blob:mock"); + expect(revokeUrlSpy).toHaveBeenCalledWith("blob:mock"); }); it("includes pieceWidth and pieceHeight from image dimensions", async () => { @@ -74,7 +79,7 @@ describe("sliceImage", () => { const promise = sliceImage(file, 1); capturedImg.onload?.(); await expect(promise).rejects.toThrow("Canvas 2D context not available"); - expect(URL.revokeObjectURL).toHaveBeenCalledWith("blob:mock"); + expect(revokeUrlSpy).toHaveBeenCalledWith("blob:mock"); // Restore so subsequent tests in this file have the mock context HTMLCanvasElement.prototype.getContext = savedImpl; @@ -85,6 +90,6 @@ describe("sliceImage", () => { const promise = sliceImage(file, 2); capturedImg.onerror?.(); await expect(promise).rejects.toThrow("Failed to load image for slicing"); - expect(URL.revokeObjectURL).toHaveBeenCalledWith("blob:mock"); + expect(revokeUrlSpy).toHaveBeenCalledWith("blob:mock"); }); }); diff --git a/bucket_catch/packages/frontend/src/lib/zipDownload.test.ts b/bucket_catch/packages/frontend/src/lib/zipDownload.test.ts index 23a8881..9d2568b 100644 --- a/bucket_catch/packages/frontend/src/lib/zipDownload.test.ts +++ b/bucket_catch/packages/frontend/src/lib/zipDownload.test.ts @@ -1,13 +1,22 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type { MockInstance } from "vitest"; import { zipDownload } from "./zipDownload"; describe("zipDownload", () => { + let createUrlSpy: MockInstance; + let revokeUrlSpy: MockInstance; + let clickSpy: MockInstance; + beforeEach(() => { - vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:mock-url"); - vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => undefined); - vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation( - () => undefined, - ); + createUrlSpy = vi + .spyOn(URL, "createObjectURL") + .mockReturnValue("blob:mock-url"); + revokeUrlSpy = vi + .spyOn(URL, "revokeObjectURL") + .mockImplementation(() => undefined); + clickSpy = vi + .spyOn(HTMLAnchorElement.prototype, "click") + .mockImplementation(() => undefined); }); afterEach(() => { @@ -21,30 +30,38 @@ describe("zipDownload", () => { const file = new File(["hello"], "hello.txt"); await zipDownload([file]); - expect(URL.createObjectURL).toHaveBeenCalled(); - expect(HTMLAnchorElement.prototype.click).toHaveBeenCalled(); - expect(URL.revokeObjectURL).toHaveBeenCalledWith("blob:mock-url"); + expect(createUrlSpy).toHaveBeenCalled(); + expect(clickSpy).toHaveBeenCalled(); + expect(revokeUrlSpy).toHaveBeenCalledWith("blob:mock-url"); expect(appendSpy).toHaveBeenCalled(); expect(removeSpy).toHaveBeenCalled(); }); it("sets download attribute and href on the anchor", async () => { const anchors: HTMLAnchorElement[] = []; + // Bound before the spy replaces it, so the implementation below can build + // a real element without recursing into its own mock. + const realCreateElement = document.createElement.bind(document); vi.spyOn(document, "createElement").mockImplementation((tag: string) => { - const el = document.createElement.bind(document)(tag) as HTMLElement; - if (tag === "a") anchors.push(el as HTMLAnchorElement); + const el = realCreateElement(tag); + if (el instanceof HTMLAnchorElement) anchors.push(el); return el; }); - // Restore after spy above since we used bind trick + // Restore after spy above since we used bind trick. That discards the + // handles from beforeEach, so re-spy and keep the new ones. vi.restoreAllMocks(); vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:mock-url"); - vi.spyOn(URL, "revokeObjectURL").mockImplementation(() => undefined); - vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation(() => undefined); + const localRevokeSpy = vi + .spyOn(URL, "revokeObjectURL") + .mockImplementation(() => undefined); + vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation( + () => undefined, + ); const file = new File(["x"], "data.bin"); await zipDownload([file]); - expect(URL.revokeObjectURL).toHaveBeenCalledWith("blob:mock-url"); + expect(localRevokeSpy).toHaveBeenCalledWith("blob:mock-url"); }); it("works with multiple files", async () => { @@ -54,7 +71,7 @@ describe("zipDownload", () => { new File(["c"], "c.png", { type: "image/png" }), ]; await expect(zipDownload(files)).resolves.toBeUndefined(); - expect(URL.createObjectURL).toHaveBeenCalledTimes(1); - expect(URL.revokeObjectURL).toHaveBeenCalledTimes(1); + expect(createUrlSpy).toHaveBeenCalledTimes(1); + expect(revokeUrlSpy).toHaveBeenCalledTimes(1); }); }); diff --git a/bucket_catch/packages/frontend/src/test/puzzleLoopFixtures.ts b/bucket_catch/packages/frontend/src/test/puzzleLoopFixtures.ts index 98e8746..d58b76b 100644 --- a/bucket_catch/packages/frontend/src/test/puzzleLoopFixtures.ts +++ b/bucket_catch/packages/frontend/src/test/puzzleLoopFixtures.ts @@ -10,8 +10,9 @@ import type { PuzzlePiece } from "../types"; * the 100% gate. */ -export const makeRef = (val: T): React.RefObject => - ({ current: val }) as React.RefObject; +export const makeRef = (val: T): React.RefObject => ({ + current: val, +}); export const makePiece = (row: number, col: number): PuzzlePiece => ({ row,