fix(web-shell,core): clear leaked test-run async under the unhandled-error gate #10655
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| // @vitest-environment jsdom | ||
| /** | ||
| * @license | ||
| * Copyright 2026 Qwen Team | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { act } from 'react'; | ||
| import { createRoot, type Root } from 'react-dom/client'; | ||
| import { useCopiedFlash } from './useCopiedFlash'; | ||
|
|
||
| globalThis.IS_REACT_ACT_ENVIRONMENT = true; | ||
|
|
||
| let root: Root; | ||
| let container: HTMLDivElement; | ||
| let latest: [boolean, () => void] | undefined; | ||
|
|
||
| function Probe({ resetMs }: { resetMs?: number }) { | ||
| latest = useCopiedFlash(resetMs); | ||
| return null; | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| vi.useFakeTimers(); | ||
| container = document.createElement('div'); | ||
| document.body.appendChild(container); | ||
| root = createRoot(container); | ||
| latest = undefined; | ||
| act(() => { | ||
| root.render(<Probe />); | ||
| }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| act(() => { | ||
| root.unmount(); | ||
| }); | ||
| container.remove(); | ||
| vi.useRealTimers(); | ||
| }); | ||
|
|
||
| describe('useCopiedFlash', () => { | ||
| it('flashes and resets after the delay', () => { | ||
| act(() => latest![1]()); | ||
| expect(latest![0]).toBe(true); | ||
| act(() => { | ||
| vi.advanceTimersByTime(2000); | ||
| }); | ||
| expect(latest![0]).toBe(false); | ||
| }); | ||
|
|
||
| it('restarts the reset window on a re-flash', () => { | ||
| act(() => latest![1]()); | ||
| act(() => { | ||
| vi.advanceTimersByTime(1500); | ||
| }); | ||
| act(() => latest![1]()); | ||
| act(() => { | ||
| vi.advanceTimersByTime(1500); | ||
| }); | ||
| // The older reset must not cut the newer feedback short. | ||
| expect(latest![0]).toBe(true); | ||
| act(() => { | ||
| vi.advanceTimersByTime(500); | ||
| }); | ||
| expect(latest![0]).toBe(false); | ||
| }); | ||
|
|
||
| it('clears the pending reset on unmount', () => { | ||
| act(() => latest![1]()); | ||
| act(() => { | ||
| root.unmount(); | ||
| }); | ||
| // A leaked timer would fire after the test environment is gone and | ||
| // fail the run through the unhandled-error gate. | ||
| expect(vi.getTimerCount()).toBe(0); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2026 Qwen Team | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { useCallback, useEffect, useRef, useState } from 'react'; | ||
|
|
||
| /** | ||
| * The transient "copied" feedback every copy button shows: `flash()` turns | ||
| * the flag on and schedules the reset. The pending reset is cleared on | ||
| * unmount — a leaked timer fires after a test file's environment is torn | ||
| * down and, since the unit suites fail on unhandled errors, turns an | ||
| * all-green run red (`window is not defined` out of the reset callback). | ||
| * A re-flash restarts the window instead of letting the older reset cut | ||
| * the newer feedback short. | ||
| */ | ||
| export function useCopiedFlash( | ||
| resetMs = 2000, | ||
| ): [copied: boolean, flash: () => void] { | ||
| const [copied, setCopied] = useState(false); | ||
| const timerRef = useRef<number | undefined>(undefined); | ||
| useEffect(() => () => window.clearTimeout(timerRef.current), []); | ||
| const flash = useCallback(() => { | ||
| setCopied(true); | ||
| window.clearTimeout(timerRef.current); | ||
| timerRef.current = window.setTimeout(() => setCopied(false), resetMs); | ||
| }, [resetMs]); | ||
|
Comment on lines
+24
to
+28
Collaborator
There was a problem hiding this comment. [Suggestion] R1-1: Witness: Suggested fix — track mounted state inside the hook and bail in const mountedRef = useRef(true);
useEffect(() => {
mountedRef.current = true;
return () => {
mountedRef.current = false;
window.clearTimeout(timerRef.current);
};
}, []);
const flash = useCallback(() => {
if (!mountedRef.current) return;
setCopied(true);
...
}, [resetMs]);Note that call sites list 中文说明组件卸载后 建议修复：在 hook 内跟踪挂载状态， — qwen3.8-max via Qwen Code /review (v0.22.3) |
||
| return [copied, flash]; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] R1-4: The hook's
resetMsparameter — used in production byGitLogDialog.tsxasuseCopiedFlash(1500)— is exercised by no test anywhere, even though thisProbealready declares aresetMsprop that no test ever sets. Every test mounts the probe withoutresetMs, so only the 2000ms default path runs: hardcoding2000in the hook'ssetTimeoutkeeps the entire suite green (verified mutant: 27/27 pass), so GitLogDialog's commit-SHA copy feedback would silently stretch from 1.5s to 2s with no failing test to notice — and the unused prop tells the next reader that custom delays are covered when they are not.Witness:
Suggested fix — re-render the probe with a custom delay (the harness
rootis reusable):The new case itself is the acceptance criterion — replacing
resetMswith a hardcoded2000inuseCopiedFlash.tsmust make it fail.中文说明
hook 的
resetMs参数——生产环境里GitLogDialog.tsx以useCopiedFlash(1500)使用——没有任何测试覆盖，尽管这个Probe已经声明了一个从未被任何测试赋值的resetMsprop。所有测试挂载探针时都不传resetMs，因此只有 2000ms 默认路径被执行：把2000硬编码进setTimeout后整个套件依然全绿（已验证的变异体：27/27 通过），Git log 对话框的复制反馈会从 1.5 秒悄悄拉长到 2 秒而无任何测试察觉——未使用的 prop 还会让后来的读者误以为自定义时长已有覆盖。建议修复：给探针传入自定义时长重新渲染（见上方代码，harness 的
root可复用）。新增用例本身就是验收标准——把useCopiedFlash.ts里的resetMs替换成硬编码2000必须让它变红。— qwen3.8-max via Qwen Code /review (v0.22.3)