diff --git a/.claude/skills/testing/SKILL.md b/.claude/skills/testing/SKILL.md index fd923817..6da0df69 100644 --- a/.claude/skills/testing/SKILL.md +++ b/.claude/skills/testing/SKILL.md @@ -259,8 +259,12 @@ renders. The rule stands on consistency, not on timer safety. the helper's shared slack** — `HEADER_ANIM_MS + RAF_SLACK_MS`, both imported, never a literal: the first term tracks the component and the second tracks how busy the machine is, and only the second should move when the machine gets - busier (#2323). Do **not** also use `vi.useFakeTimers()` in that test - (the auto-settle no-ops under fake timers), and if the test unmounts the tree + busier (#2323). Do **not** also use `vi.useFakeTimers()` in that test: + the auto-settle awaits a real `setTimeout`, so under fake timers it **throws** + with a message telling you to call `vi.useRealTimers()` first — it does not + silently skip. That is deliberate (a deadlock would otherwise hang until the + project's `hookTimeout`), but it means the combination fails the test rather + than degrading. If the test unmounts the tree itself use the `unmount()` the helper returns. The mechanism is documented at length on the helper — read there before changing it. diff --git a/scripts/verify-test-timeouts.mjs b/scripts/verify-test-timeouts.mjs index c3ca4201..f00b83ed 100644 --- a/scripts/verify-test-timeouts.mjs +++ b/scripts/verify-test-timeouts.mjs @@ -318,8 +318,12 @@ export function checkAsyncUtilSource( // is something else entirely (Copilot). Requiring all of them to agree is // stricter than checking the last and gives a clearer message than "the // effective value is X" would. + // ⚠️ `(? { `vi.mock("x", () => ({ retry: 2 }));`, `it("has retry: 2 in the name", () => {});`, `const o = { noRetry: 2 };`, + // Shorthand for a DIFFERENT property must not match the shorthand form. + `it("x", { retryCount }, () => {});`, ]) { assert.deepEqual(findTestLevelRetries(src), [], src); } @@ -411,3 +413,34 @@ configure({ testIdAttribute: "data-testid" }); configure({ asyncUtilTimeout: 1000 });`; assert.deepEqual(checkAsyncUtilSource(source, "storybook/test"), []); }); + +test("the shorthand property form is caught", () => { + // `{ retry }` is the same declaration as `{ retry: 2 }` with the value bound + // above, and a colon-requiring pattern misses it entirely (Copilot). + assert.deepEqual( + findTestLevelRetries('const retry = 2; it("x", { retry }, () => {});'), + ["retry (shorthand)"], + ); + assert.deepEqual( + findTestLevelRetries('it("x", { timeout: 100, retry }, () => {});'), + ["retry (shorthand)"], + ); +}); + +test("a member call is not mistaken for the imported binding", () => { + // `\\b` succeeds straight after a dot, so `other.configure(…)` satisfied the + // check while the imported binding was never called at all (Copilot). + const source = `import { configure } from "storybook/test"; +other.configure({ asyncUtilTimeout: 1000 });`; + assert.deepEqual(checkAsyncUtilSource(source, "storybook/test"), [ + "does not call configure()", + ]); +}); + +test("a longer identifier ending in the binding name is not a call to it", () => { + const source = `import { configure } from "storybook/test"; +reconfigure({ asyncUtilTimeout: 1000 });`; + assert.deepEqual(checkAsyncUtilSource(source, "storybook/test"), [ + "does not call configure()", + ]); +});