Skip to content

⚡️ 降低测试固定等待与渲染开销 - #1737

Open
cyfung1031 wants to merge 10 commits into
mainfrom
codex/fast-network-rules-tests
Open

⚡️ 降低测试固定等待与渲染开销#1737
cyfung1031 wants to merge 10 commits into
mainfrom
codex/fast-network-rules-tests

Conversation

@cyfung1031

@cyfung1031 cyfung1031 commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Checklist / 检查清单

  • Fixes mentioned issues / 修复已提及的问题
  • Code reviewed by human / 代码通过人工检查
  • Changes tested / 已完成测试

Code reviewed by human remains unchecked because no human review was performed in this task.

Description / 描述

背景

CI timing reports identified avoidable wall-clock cost in Network Rules UI tests and several Promise/timer-driven
tests. The original failures were sensitive to the UI project's 850 ms per-test budget and to worker contention;
the goal is to make tests observe their real completion signals without weakening the behavior they protect.

本次改动

  • Optimized seven Network Rules test files: ActionColors, BulkActions, Feedback, RowRenderCost,
    UxRefinements, the NetworkRules page, and the Tools summary section. Page tests use controlled act microtask
    settling, direct selectors where identity is already known, focused RuleSheet rendering for drawer-only behavior,
    and cheaper dnd-kit rectangle measurement. Existing request, confirmation, pagination, feedback, and row-render
    assertions remain. Page-level create/edit bridge tests now assert exact mutation payloads, retry tests assert the
    applied UI state, and bulk actions assert the toolbar's accessible name.
  • Optimized four Agent tests: dom_cdp, tool_loop_orchestrator, MessageToolbar, and ModelFormDialog. Fixed
    production stability/retry delays are advanced with fake timers; Promise-driven React updates use a single
    completion step followed by direct assertions.
  • Optimized src/pages/batchupdate/hooks.test.ts by advancing the production success-hold and row-exit timers in
    the two tests that intentionally observe those transitions.
  • Updated AGENTS.md and docs/references/develop-testing.md to make the test-writing rule durable: choose the
    wait primitive from the contract, use fake timers for production timer behavior, and do not raise timeouts,
    add arbitrary sleeps, or weaken assertions to hide contention. The detailed async mechanics remain owned by
    docs/references/develop-testing.md.

No production runtime code or user-visible behavior changed. The final diff contains 12 test files and 2 agent/
testing-document files.

实现考虑

The changes preserve the original test boundaries and observable assertions. findBy* remains appropriate when an
element's appearance is the asynchronous boundary; direct assertions are used only after the test-owned Promise or
completion signal has settled. Fake timers advance the configured production durations rather than shortening them.
The 20-row render-cost boundary and its exact read-count assertions remain intact.

已知限制

Test wall-clock timings still vary with worker contention and local hardware. Full local execution emits existing
happy-dom chrome-extension:// iframe and Node local-storage warnings, but all tests pass. GitHub checks for the
latest head may still be running after this metadata update.

建议审查重点

  • Confirm the final diff remains test-only plus the two agent/testing-document updates; no production source changed.
  • Confirm Network Rules page boundaries still exercise client mutation payloads, rejection feedback, confirmation
    gates, drag ordering, pagination, and row-render counts.
  • Confirm Agent and batch-update fake-timer tests still assert the terminal state and cleanup behavior, not merely
    that a timer callback ran.
  • Confirm the new agent guidance agrees with the detailed async observation and performance rules in
    docs/references/develop-testing.md.

验证

Final local head: c2c11fc23c88a0a97130a121799cb2ddc76d7825; PR base: dd280349617c8cd5b29710a2c016ce8194dc4280.

  • pnpm exec vitest run --project ui --no-coverage --reporter=default --silent=passed-only src/pages/options/routes/Tools/NetworkRules/index.test.tsx src/pages/options/routes/Tools/NetworkRules/BulkActions.test.tsx src/pages/options/routes/Tools/sections/NetworkRulesSection.test.tsx — 3 files / 38 tests passed.
  • pnpm exec vitest run --no-coverage --reporter=default --silent=passed-only — 368 files / 4,711 tests passed in 27.10 s.
  • pnpm run lint — passed: Prettier, TypeScript, i18n, issue-template validation, and ESLint.
  • Commit hooks on the timing commits — passed typecheck, Prettier, and issue-template validation.
  • Documentation checks — git diff --check, the new Markdown link target/anchor, and the targeted policy-consistency
    scan passed.

Screenshots / 截图

N/A — test and agent-document changes only; no visual behavior changed.

@cyfung1031 cyfung1031 changed the title ⚡️ 降低网络规则 UI 测试等待开销 ⚡️ 降低测试固定等待与渲染开销 Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up rework completed and pushed at b3dff7d58dbe1cc5b524695d1eb6bb3ecea0d80e. The PR title/body were not changed; this comment records the additional correction commits and evidence.

Corrections:

Independent scoped audits found these as concrete test-observation regressions; no production-runtime defect was identified. The final diff remains test/documentation-only.

Local evidence at the published head:

  • pnpm exec vitest run --reporter=default --silent=passed-only --slowTestThreshold=300: 368 files / 4,711 tests passed.
  • pnpm run lint: passed (Prettier, TypeScript, i18n, issue-template validation, and ESLint).
  • Focused Network Rules UI: 7 files / 58 tests passed.
  • Batch-update group: 3 files / 103 tests passed.
  • Both correction commit hooks passed typecheck, Prettier, and issue-template validation.

GitHub Actions for the new head were pending when this comment was posted; the last remote read showed the PR open and mergeable. Local runs retain existing happy-dom extension-URL warnings, with no test failures.

@CodFrm

CodFrm commented Sep 11, 2026

Copy link
Copy Markdown
Member

基于当前 PR head b3dff7d58dbe1cc5b524695d1eb6bb3ecea0d80e 审查后,建议暂缓合并。GitHub 检查均通过,但还有以下经过变异测试确认的测试覆盖退化:

  1. 页面级保存 payload 的精确覆盖被削弱。

    index.test.tsx 现在由独立 RuleSheet 测试完整 CSP headers,而页面接线测试只检查 action.type;编辑动作切换也只在独立组件测试中验证。我临时让页面创建时清空 CSP headers、编辑时提交旧 action,index.test.tsx 的 20 个测试仍全部通过。原测试可以发现这两类回归。

    建议恢复页面流程对完整 createRule payload 的断言,并在页面内切换动作后精确断言 updateRule 收到最新 { type: "block" }

  2. 重试测试只证明请求发出,没有观察最终状态。

    NetworkRules/index.test.tsx:297NetworkRulesSection.test.tsx:156 最后只断言 retryApply 被调用。我临时让生产代码忽略 retryApply() 返回的新 snapshot,这两个测试仍通过,意味着成功重试后页面即使继续显示“未生效”和失败横幅也不会被发现。

    建议在 Promise 完成后断言失败横幅消失,并且状态恢复为“已生效”。这也符合本 PR 新增的规则:请求被调用只证明工作开始,不证明状态更新完成。

  3. 批量工具栏不再保护可访问名称。

    BulkActions.test.tsx:98 从具名 toolbar 查询改成任意 [role="toolbar"]。临时删除生产代码中的 aria-label 后,全部批量操作测试仍通过。可以保留快速 selector,但应额外断言 aria-label="批量操作" 或 accessible name。

另外有两项文档问题需要同步处理:

  • AGENTS.md:68-72 重复了 docs/references/develop-testing.md 已有的异步观察和性能机制;按文档 owner 规则,建议在 AGENTS.md 只保留原则和链接,把具体机制留在测试指南。
  • PR 描述的验证段仍写最终 head 为 b048e047...、4,710 个测试,已落后于当前 b3dff7d5... 和后续 4,711 个测试的证据,需要重新绑定最终修订。

我在当前 main 与该 head 的临时合并树上复核:12 个受影响测试文件共 128 个测试通过,pnpm run lintgit diff --check 通过;加入上述生产代码变异后,相关 3 个文件共 38 个测试仍全部通过,因此这些是实际存在的测试保护缺口,不是仅凭静态阅读提出的风险。

Copy link
Copy Markdown
Collaborator Author

已按这条审查意见完成修正并推送到 c2c11fc23c88a0a97130a121799cb2ddc76d7825

  • 页面级 CSP 新建现在精确断言完整 createRule payload(包括四个响应头与资源类型)。
  • 页面级编辑流程实际切换到「屏蔽请求」,并精确断言 updateRule 的完整 patch 与 { type: "block" }
  • 列表页与摘要卡的重试测试现在等待已解析 Promise 后断言失败横幅消失、状态变为「已生效」。
  • 批量操作保留宽的 toolbar 缺失检测,同时直接断言 aria-label="批量操作"
  • AGENTS.md 已收回重复的异步机制,只保留原则与测试指南链接;PR 描述已同步到当前 head 和 4,711 个测试。

验证结果:相关 3 个文件 38/38 通过;全量 368 个文件、4,711 个测试通过;pnpm run lint 通过。没有生产运行时代码变更。请重新考虑合并。

@cyfung1031

cyfung1031 commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

已将 UI 性能修正推送到此 PR,当前 HEAD 为 c05006502d1bc99980e242091840f8ac289ed2c3

本次 follow-up 涉及 RuleTable.tsxRuleCards.tsx 和对应的 AGENTS.md 约束:稳定 dnd-kit 的传感器/修饰器/回调与 item identity,在禁用拖拽时跳过 DndContext/SortableContext,并把昂贵的行/卡片内容置于默认浅比较的 memo 边界内;目标是保持现有拖拽语义,同时降低无拖拽场景的渲染开销。

精确工作树上的验证证据:

  • pnpm exec vitest run --no-coverage --reporter=default --silent=passed-only:368 files / 4711 tests passed,24.06s。
  • pnpm run lint:passed;变更相关 focused run:45 tests passed。
  • 同一 4-file benchmark 重复 3 次:process median 3.02s(baseline 3.41s),test median 3.76s(baseline 4.42s);本地趋势约 11–15%,存在运行噪声,不将其表述为保证性百分比。
  • commit hooks 通过 TypeScript、Prettier 和 issue-template 检查。

GitHub Actions run 34584238117 已完成:Lint、E2E 四个分片、测试 shard 2/2 和 License Compliance 成功;测试 shard 1/2 失败,BulkActions.test.tsx:121RowRenderCost.test.tsx:96 在 UI 的 850ms 测试预算内超时,汇总 Run tests 因此失败。该结果与本地完整套件通过不一致,当前仅能确认是 CI 并行负载下的超时,尚未将其归因于代码或认定为基础设施问题;本次没有擅自提高测试预算或重跑 CI。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants