Skip to content

Commit eae267e

Browse files
authored
✅ 加固 UI 异步断言与 E2E 保存结果观察 (#1727)
* test: stabilize heavy network rules UI cases * test: isolate UI files and await observable state * ✅ 保留 UI 测试原有隔离配置 * ✅ harden async test observations * 🐛 align E2E save expectations with failure cases * 🔍 tighten test guard binding and toast observation
1 parent 8d6f7eb commit eae267e

24 files changed

Lines changed: 624 additions & 152 deletions

AGENTS.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ duplicating its rules.
1010
| Before you… | Read |
1111
| --- | --- |
1212
| write code | [`docs/develop.md`](docs/develop.md) |
13+
| modify tests, test helpers, or test runner configuration | [`docs/references/develop-testing.md`](docs/references/develop-testing.md) — apply the test-boundary, observation, and harness rules before editing |
1314
| review or report a branch/PR, or create/update a PR or publish its branch | [`docs/develop.md#revision-scope-and-publication-binding`](docs/develop.md#revision-scope-and-publication-binding) + [`docs/pull-request.md`](docs/pull-request.md) |
1415
| change a process/message/service/persistence boundary or add a subsystem | [`docs/architecture.md`](docs/architecture.md) + the relevant `docs/references/architecture-*.md` |
1516
| build or modify a page, dialog, or block | [`docs/design.md`](docs/design.md) — Core Constraints apply to every UI change |
@@ -48,6 +49,13 @@ downstream prose does not override it.
4849
Chinese or English titles. The two narrow, non-blanket exceptions are in
4950
[`docs/references/develop-testing.md`](docs/references/develop-testing.md#when-tdd-doesnt-apply); runner,
5051
mocks, and how to run tests are in [`docs/develop.md`](docs/develop.md).
52+
- **Test changes must follow the test route.** Before changing a test, shared test helper, or runner configuration,
53+
identify the observable contract and test boundary, search existing coverage, capture a baseline or reproduction,
54+
then make the smallest correction and rerun focused and relevant broader checks. A single passing run does not
55+
establish a root-cause fix; report the trigger, evidence, and remaining uncertainty.
56+
- **Shared E2E helpers must model both outcomes.** A helper that drives a save, install, or other mutation must make
57+
the expected success or failure explicit and wait for that operation's matching signal. Negative cases must opt into
58+
the failure contract; never make them pass by accepting an arbitrary toast, an old notification, or a page shell.
5159
- **SOLID, high cohesion, low coupling.** Match existing extension points: persistence uses the small
5260
`Repo<T>` / `DAO<T>` / `OPFSRepo` / custom-repo taxonomy, matching an existing entity with the same needs;
5361
messages use `Group.on(...)`; service constructor shapes differ by context and Agent subsystem; depend on

docs/architecture.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,8 @@ premature abstraction.
288288
test-first principle and [develop-testing.md § When TDD doesn't apply](./references/develop-testing.md#when-tdd-doesnt-apply)
289289
for the narrow exceptions — this section only covers architecture-specific test mechanics, not the policy
290290
itself.
291-
- **E2E (Playwright).** `e2e/*.spec.ts`, one worker, real Chromium. `pnpm run test:e2e` (first run:
291+
- **E2E (Playwright).** `e2e/*.spec.ts`, real Chromium; worker count and retries come from
292+
[`playwright.config.ts`](../playwright.config.ts). `pnpm run test:e2e` (first run:
292293
`pnpm run test:e2e:install`).
293294
- **Before a PR:** lint + the relevant suite — owned by [references/develop-testing.md](./references/develop-testing.md)*Testing*.
294295

docs/develop.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ pnpm run coverage
2121
pnpm run typecheck # tsc --noEmit
2222

2323
pnpm run test:e2e:install # install Playwright Chromium (first run only)
24-
pnpm run test:e2e # Playwright (e2e/*.spec.ts, 1 worker)
24+
pnpm run test:e2e # Playwright (e2e/*.spec.ts; worker count comes from playwright.config.ts)
2525
pnpm run lint # prettier --check + tsc --noEmit + check:i18n + check:issue-templates, then eslint
2626
pnpm run lint-fix # prettier --write + tsc --noEmit + eslint --fix
2727

docs/references/develop-testing.md

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,57 @@ This guide owns how contributors design, write, review, clean up, and run automa
77
merely because it raises coverage: it must protect an observable contract, fail for a relevant regression, and
88
cost less to understand and maintain than the confidence it provides.
99

10+
## Test-change route and evidence
11+
12+
Before modifying a test, shared test helper, or runner configuration, classify the contract and boundary first:
13+
14+
1. State the trigger, observable outcome, and plausible regression.
15+
2. Search nearby unit, component, service, E2E, and lint coverage before adding or deleting a case.
16+
3. Run the narrowest baseline or reproduce the failure under the same runner, reporter, coverage, shard, and worker
17+
conditions that exposed it.
18+
4. Change one cause at a time, then run the focused test and the relevant broader combination.
19+
5. Report exact commands and distinguish a passed assertion from an unobserved channel or unverified negative.
20+
21+
One passing run is evidence for that run only. Do not treat a timeout increase, retry, deleted assertion, or arbitrary
22+
sleep as a root-cause repair.
23+
24+
### Observation rules for asynchronous tests
25+
26+
The test must observe completion of the contract under test. A request being called proves that work started; it does
27+
not prove that state, persistence, rendering, or the user-visible result completed. Use the narrowest primitive that
28+
matches the boundary:
29+
30+
- Use direct assertions for synchronous effects and one `act` for a Promise-driven React update.
31+
- Use `findBy*` for a single element that appears asynchronously. Do not wrap `getBy*` in `waitFor` for a lone
32+
`toBeInTheDocument` assertion.
33+
- Use `waitFor` for genuinely open-ended async state, multiple related assertions, or a non-DOM boundary that has no
34+
dedicated completion signal. Keep the callback observational: do not fire events or call `userEvent` inside it,
35+
because retries repeat the interaction.
36+
- Use a real timer only when elapsed time is the contract or the only bounded closure window proves a negative result
37+
(for example, an observer timeout, a runaway retry check, a browser event-loop yield, or a library timer). Add a
38+
local ESLint disable comment stating that contract. A fixed delay used merely to make a test pass is a defect.
39+
40+
The mechanical guards `scriptcat/no-test-waitfor-interaction`, `scriptcat/no-test-waitfor-query`, and
41+
`scriptcat/no-test-fixed-sleep` cover reliably recognizable forms in committed page tests and E2E specs. They do not
42+
prove mock fidelity, the sufficiency of a negative observation window, or that coverage was not weakened; those remain
43+
semantic review duties. Do not disable a whole directory to silence them.
44+
45+
The interaction and query guards follow actual Testing Library import bindings, including local aliases, and respect
46+
lexical shadowing; a same-named ordinary function or object is outside their contract. The sleep guard covers
47+
Playwright `waitForTimeout` and timer-backed `new Promise` forms, while finite observer timeouts remain valid only with
48+
a line-level disable comment that names the timeout contract. Scratch files remain excluded by the committed E2E
49+
configuration; inspect the effective ESLint configuration when a helper moves between tracks.
50+
51+
### UI and Playwright examples
52+
53+
For a UI mutation, assert the returned state, rendered result, or persisted collaborator result after completion;
54+
`expect(client.update).toHaveBeenCalled()` alone only proves dispatch. For Playwright, a helper that saves an editor
55+
must take an explicit success or failure expectation and wait for the matching, operation-specific signal. A negative
56+
case must request the failure contract; a helper that always waits for success turns a valid rejection into a harness
57+
failure. An arbitrary toast, an existing toast from an earlier action, or a page-shell anchor is not proof that the
58+
save completed. Keep real browser API, cross-context, and permission flows in E2E; do not replace them with mocks just
59+
to avoid waiting.
60+
1061
## Applicability gate — read this first
1162

1263
Not every section below applies to every change. Before designing or reviewing tests, check which of these the
@@ -246,7 +297,9 @@ before/after in one environment with the JSON-report method below.
246297
- Co-locate `*.test.ts`/`*.test.tsx` next to source (or place in `tests`).
247298
- Use `describe.concurrent()` / `it.concurrent()` where independent.
248299
- Single file: `pnpm test -- --run path/to/file.test.ts`.
249-
- Playwright tests are `*.spec.ts` files in `e2e`; they run with one worker and retain failure artifacts. Run targeted tests while iterating, then run `pnpm run lint` plus the relevant full suite before a PR.
300+
- Playwright tests are `*.spec.ts` files in `e2e`; worker count, retries, and artifact settings come from
301+
[`playwright.config.ts`](../../playwright.config.ts) and the CI matrix. Run targeted tests while iterating, then
302+
run `pnpm run lint` plus the relevant full suite before a PR.
250303

251304
## Vitest Performance Hygiene
252305

e2e/README.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ playwright config → fixture (launchPersistentContext, loads dist/ext)
7575

7676
[`utils.ts`](./utils.ts) carries the page openers and script installer used by every track:
7777
`openOptionsPage`, `openPopupPage`, `openEditorPage`, `openAgentChatPage`, `openAgentProviderPage`,
78-
`saveCurrentEditor`, `installScriptByCode`, `runInlineTestScript`, and `autoApprovePermissions`.
78+
`saveCurrentEditor`, `installScriptByCode`, `runInlineTestScript`, and `autoApprovePermissions`. Save helpers
79+
default to the successful outcome; a test that intentionally rejects a script must pass
80+
`{ saveOutcome: "failure" }` so the helper waits for the matching save failure signal rather than accepting an
81+
unrelated notification.
7982

8083
### The two-phase launch
8184

e2e/gm-api.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,8 @@ test.describe("GM API", () => {
664664
const script = document.createElement("script");
665665
script.textContent = `window["${key}"] = true;`;
666666
document.head.appendChild(script);
667+
// 页面脚本执行需要一次真实事件循环让步,才能观察 CSP 阻止后的最终状态。
668+
// eslint-disable-next-line scriptcat/no-test-fixed-sleep -- page event-loop observation contract
667669
await new Promise((resolve) => setTimeout(resolve, 0));
668670
return Boolean((window as Record<string, unknown>)[key]);
669671
});

e2e/options-pages-smoke.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ test.describe("Options 各页加载冒烟", () => {
6161
await expect(route.anchor(page), `${route.name} (${route.path}) 未渲染稳定锚点`).toBeVisible({
6262
timeout: 20_000,
6363
});
64-
// 给页面挂载副作用(数据加载/消息往返)一点时间触发可能的异常。
64+
// 页面冒烟契约包含挂载后副作用的有限观察窗口;没有统一完成事件可等待。
65+
// eslint-disable-next-line scriptcat/no-test-fixed-sleep -- finite post-mount error observation window
6566
await page.waitForTimeout(500);
6667
}
6768

e2e/user-config-yaml.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ test.describe("UserConfig YAML prototype pollution (#1494)", () => {
9999
expect(before).toBeUndefined();
100100

101101
// 尝试安装恶意脚本(parseUserConfig 应抛错并阻止安装)
102-
await installScriptByCode(context, extensionId, evilCfg);
102+
await installScriptByCode(context, extensionId, evilCfg, { saveOutcome: "failure" });
103103

104104
const list = await openOptionsPage(context, extensionId);
105105
const evilInfo = await getScriptInfo(list, "UC Evil E2E");

e2e/utils.ts

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -144,38 +144,47 @@ async function focusMonacoEditor(page: Page): Promise<void> {
144144
await page.locator(".monaco-editor textarea.inputarea").focus();
145145
}
146146

147-
async function waitForSavedScriptInList(context: BrowserContext, extensionId: string): Promise<void> {
148-
const listPage = await openOptionsPage(context, extensionId);
149-
try {
150-
// new-ui 列表页加载完成的稳定信号(桌面工具栏搜索框 / 移动搜索栏)
151-
await listPage
152-
.getByTestId("script-search")
153-
.or(listPage.getByTestId("mobile-search"))
154-
.first()
155-
.waitFor({ state: "visible", timeout: 30_000 });
156-
} finally {
157-
await listPage.close();
158-
}
159-
}
160-
161-
export async function saveCurrentEditor(context: BrowserContext, extensionId: string, page: Page): Promise<void> {
147+
export type SaveOutcome = "success" | "failure";
148+
149+
const saveSuccessMessage =
150+
/Saved successfully|successfully created||||||| |Salvo com sucesso|Novo script criado com sucesso|Успешно сохранено|Создание успешно|Erfolgreich gespeichert|Erstellung erfolgreich|Başarıyla kaydedildi|Yeni betik başarıyla oluşturuldu|Đã lưu thành công|Script mi đưc to thành công/i;
151+
const saveFailureMessage =
152+
/Save Failed|Speichern fehlgeschlagen|| |Falha ao salvar|Ошибка сохранения|Kaydetme Başarısız|Lưu tht bi||/i;
153+
154+
export async function saveCurrentEditor(
155+
_context: BrowserContext,
156+
_extensionId: string,
157+
page: Page,
158+
outcome: SaveOutcome = "success"
159+
): Promise<void> {
162160
await focusMonacoEditor(page);
161+
const saveToast = page
162+
.locator(`[data-sonner-toast][data-type="${outcome === "success" ? "success" : "error"}"]`)
163+
.filter({
164+
hasText: outcome === "success" ? saveSuccessMessage : saveFailureMessage,
165+
});
166+
// 先关闭同类旧通知的观察窗口,避免它在保存期间自动卸载后与本次通知共用计数。
167+
await expect.poll(() => saveToast.count(), { timeout: 5_000 }).toBe(0);
163168
await page.keyboard.press("ControlOrMeta+s");
164169

165-
// new-ui 保存成功为 sonner toast
166-
const toastAppeared = await page
167-
.locator("[data-sonner-toast]")
168-
.first()
169-
.waitFor({ timeout: 10_000 })
170-
.then(() => true)
171-
.catch(() => false);
172-
if (toastAppeared) return;
173-
174-
await waitForSavedScriptInList(context, extensionId);
170+
// 只有保存后新出现且与结果匹配的通知能证明保存完成;任意 toast 和列表页挂载都不能替代它。
171+
await expect
172+
.poll(() => saveToast.count(), {
173+
timeout: 10_000,
174+
intervals: [100, 250, 500, 1_000],
175+
message:
176+
outcome === "success" ? "保存操作未产生成功通知,可能被错误通知或未完成状态掩盖" : "保存操作未产生失败通知",
177+
})
178+
.toBeGreaterThan(0);
175179
}
176180

177181
/** Install a script by injecting code into the Monaco editor and saving */
178-
export async function installScriptByCode(context: BrowserContext, extensionId: string, code: string): Promise<void> {
182+
export async function installScriptByCode(
183+
context: BrowserContext,
184+
extensionId: string,
185+
code: string,
186+
options: { saveOutcome?: SaveOutcome } = {}
187+
): Promise<void> {
179188
const page = await openEditorPage(context, extensionId);
180189
// Wait for Monaco editor DOM and default template content to be ready
181190
await focusMonacoEditor(page);
@@ -190,7 +199,7 @@ export async function installScriptByCode(context: BrowserContext, extensionId:
190199
timeout: 5_000,
191200
});
192201
// Save
193-
await saveCurrentEditor(context, extensionId, page);
202+
await saveCurrentEditor(context, extensionId, page, options.saveOutcome);
194203
await page.close();
195204
}
196205

e2e/vscode-connect.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ function createMockWSServer(): Promise<{
6767
}
6868
},
6969
waitForAction: (action: string, timeout = 10_000) =>
70+
// 有限观察窗口:WebSocket action 可能永远不回传,超时用于清理监听器并传播失败。
71+
// eslint-disable-next-line scriptcat/no-test-fixed-sleep -- WebSocket action observation timeout
7072
new Promise<unknown>((resolveAction, rejectAction) => {
7173
const timer = setTimeout(() => {
7274
const idx = messageListeners.indexOf(handler);

0 commit comments

Comments
 (0)