Metabase Cypress E2E 测试评审方法论：从评审 Skill 到 Lint 规则与 e2e/单测分层决策

Metabase Cypress E2E 测试评审方法论：从评审 Skill 到 Lint 规则与 e2e/单测分层决策

【免费下载链接】metabaseThe easy-to-use open source Business Intelligence and Embedded Analytics tool that lets everyone work with data :bar_chart:项目地址: https://gitcode.com/GitHub_Trending/me/metabase

本文以 Metabase 仓库中 .claude/skills/e2e-test-review/SKILL.md 及其引用的共享约定 cypress-conventions.md 为主体，系统讲解 Metabase 是如何对e2e/test/scenarios/下的 Cypress spec 文件做代码评审的：完整评审流程、分领域审查清单、反模式速查表、Lint 规则的源码级实现、基于 CI 耗时数据的性能审查，以及评审报告末尾必选的"e2e vs 单元测试"诚实分层判定。读完本文，你可以按同一套标准去审查任何一份 Metabase 的 E2E spec，并理解每条规则背后的稳定性（flakiness）与性能依据。

1. 评审 Skill 的定位与适用前提

Metabase 将"Cypress E2E 测试评审"沉淀为一个可被 AI Agent 直接执行的 Skill，其 frontmatter 声明的触发条件非常明确：

Review Cypress E2E spec files for Metabase conventions, common gotchas, and flakiness/performance issues. Use when reviewing pull requests or diffs containing Cypress spec files ine2e/test/scenarios/.

也就是说，它的适用范围被严格限定为位于e2e/test/scenarios/目录下的 Cypress spec 文件（即 e2e/test/scenarios/ 下的*.cy.spec.ts/*.cy.spec.js）。Skill 声明的可用工具为Read, Grep, Bash, Glob，属于纯静态审查能力，不依赖运行测试。

评审开始前必须先做模式检测（Review mode detection），二选一：

1. PR 评审模式——当mcp__github__create_pending_pull_request_review工具可用时，把发现的问题作为一个连贯的 pending review 一次性提交；
2. 本地评审模式——否则在对话中输出一份编号问题列表。

模式检测之后是六步评审流程（原文第 18–25 行）：

1. 检测模式；
2. 先通读整份被修改的 spec，理解测试意图，不要逐行"冷评审"（don't review line-by-line cold）；
3. （条件触发）只有当读完后某个断言或选择器存在真实的歧义——分不清锚点选得对不对、测试是否真的验证了标题所声称的东西——才去 grep 相关组件中的 test ID / role / 文本。默认不读组件源码，只有 spec 里出现真实的困惑信号才做这一步；
4. 按后文的审查清单 + 模式匹配表逐项扫描；
5. 所有问题按顺序编号，跳过吹毛求疵的小问题——只标记值得修的东西；
6. 每次报告都必须以"诚实的 e2e-vs-unit 分层判定"收尾（见本文第 8 节），它是每份报告的组成部分，而非可选附录。

1.1 何时应该拒绝评审并移交

Skill 明确划出了边界：如果 spec 引用了 issue（形如metabase#NNNNN），而用户想修复flakiness 或评估该测试是否仍能复现原始 bug，这已经超出评审 Skill 的职责范围。此时应把用户指向专门的 flake 修复工作流（它会去拉取 issue 与解决该 issue 的 PR diff）。评审 Skill 的焦点始终是"这个测试是否写得规范、是否合规"，默认不拉取外部 issue 上下文。

2. 审查清单：评审的标准检查项

清单的结构刻意镜像共享约定文件 cypress-conventions.md 的章节顺序，方便写测与审测共用一套标准。标注(lint)的条目同时会被 ESLint 捕获——只有当它们绕过 lint 溜进来时（比如包在 helper 里、被eslint-disable关掉）才需要人工标记。以下逐节继承原文清单：

2.1 文件与命名

• spec 必须位于e2e/test/scenarios/<area>/下；
• 扩展名应为.cy.spec.ts（推荐）或.cy.spec.js——对存量文件不要标记.js；
• describe块在相关时应命名到领域："area > sub-area > feature (#issue-number)"；
• 不允许遗留.only或.skip（pre-commit hook 应当拦住，但一旦漏网要标记出来）。

2.2 辅助函数与常量

• helper 一律通过const { H } = cy;访问——禁止从e2e/support/helpers直接 import(lint)；
• 样例数据库 schema 从 cypress_sample_database.js 导入；
• 实例数据 ID 从 cypress_sample_instance_data.js 导入；
• 任何位置都不允许硬编码数字 ID——包括测试自己创建的实体。要从创建响应中捕获 ID，或给 intercept 起别名（alias）；
• 优先使用已有的导航 helper（H.openOrdersTable、H.visitDashboard(id)等），而不是裸的cy.visit()链。

约定文件给出的标准样板：

const { H } = cy; describe("feature name", () => { beforeEach(() => { H.restore(); cy.signInAsAdmin(); }); });

关于"永不硬编码数字 ID"，约定文件解释了根本原因：自增主键并不稳定，运行中更早的测试或 seed 步骤可能让"下一个 ID"从 10 漂移到 11。正确写法是从创建响应捕获并复用，或用 intercept 别名从响应体取 id：

// 好 —— 捕获并复用 H.createDashboard({ name: "My dashboard" }).then(({ body: dashboard }) => { cy.visit(`/dashboard/${dashboard.id}`); }); // 好 —— 给 intercept 起别名，从响应中取 id cy.intercept("POST", "/api/dashboard").as("createDashboard"); // ...触发表单创建... cy.wait("@createDashboard").its("response.body.id").then((id) => { ... });

// 坏 —— `10` 只是自增恰好落到的值 cy.visit("/dashboard/10");

2.3 选择器

• 优先使用可访问性查询（findByRole、findByLabelText）而非findByText；
• 仅当 a11y 查询不适用时才用findByText；
• data-testid属性一律用findByTestId（绝不裸写cy.get("[data-testid='...']")）；
• 禁用 CSS 类名——尤其是 styled-components / Mantine 生成的.css-1abc2d；
• spec 与新 helper 中禁止 ad-hoc CSS 属性选择器（e2e/support/helpers/e2e-visual-tests-helpers.js 中的可视化 helper 是唯一被有意保留的例外，见第 5 节"不应标记的情形"）；
• 禁用 XPath；
• 位置选择器（.eq()、.first()、.last()、:nth-child）只在"顺序本身就是断言"或"紧邻其前有一个 length 断言做守卫"时才允许。(lint 只捕获.last()与.eq(<负数)，其余靠评审人)；
• 文本选择器必须有作用域——it/before/beforeEach顶层的cy.findByText(...)/cy.contains(...)是禁止的，必须写成cy.contains(selector, text)、cy.someQuery().findByText(...)或someQuery().within(...)。(lint 只捕获顶层情况——不捕获被 helper 包裹的查询；必须人工扫描 helper 函数体)。

约定文件给出的选择器优先级：a11y 查询（cy.findByRole()/cy.findByLabelText()，来自@testing-library/cypress）→cy.findByText()→cy.findByTestId()→ 其他data-*属性兜底。

2.4 状态设置与隔离

• 状态搭建用cy.request/ API helper，不走 UI；
• H.restore()与登录放在beforeEach，而不是before；
• 每个it()必须可独立运行——任何it()都不得依赖前一个it()的状态；
• 当H.restore()与H.resetTestTable()同时出现时，H.restore()必须在前(lint)。

2.5 等待与时序

• 禁止数字型cy.wait(ms)——再小也不行；
• cy.intercept()必须定义在触发请求的动作之前；
• 禁止setTimeout、Cypress.Promise.delay或任何手动睡眠；
• 禁止用长自定义超时（如{ timeout: 30000 }）掩盖竞态；
• DOM 就绪检查用.should("be.visible")，不是.should("exist")。exist仅保留给隐藏输入框 / 屏幕外 / portal 脱离 DOM 等特殊情形。约定文件对这条的解释是：.should("exist")只证明节点在 DOM 里，不构成就绪检查。

cy.intercept("POST", "/api/dataset").as("dataset"); // ...触发操作... cy.wait("@dataset");

2.6 绝不给cy.*命令的返回值赋值

这是清单中单列一节的高危项：

• 禁止const x = cy.someCommand(...)——x是一次性 chainer，不是解析后的值(lint 能捕获简单情形)；
• 查询需要命名时，应包在函数里（const foo = () => cy.findByText("Foo")），而不是赋给const；
• 解析后的值通过.then()访问，或.as()+cy.get("@alias")引用；
• 别名只应在"查找"与"使用"之间存在距离时才用（否则直接链式调用）。

约定文件对const与函数两种命名方式的区别给出了关键解释：const在定义时刻捕获 chainer（已在执行中、不可复用）；函数则推迟查找，每次调用都重新执行查询并保留完整的重试语义——可视化 helper（echartsContainer()、goalLine()等）用的正是后者。

// 坏 —— `button` 是 chainer，不是 DOM 节点 const button = cy.findByRole("button", { name: "Save" }); button.click(); // 好 —— 需要命名查询时用函数形式，每次调用重新入队 const foo = () => cy.findByText("Foo"); foo().click();

2.7 断言

• 断言目标应是用户可见状态（文本、URL、aria），而不是 DOM 结构；
• 负向断言必须与正向断言配对。孤立的should("not.exist")/should("not.be.visible")在页面还没渲染时就会"侥幸通过"——必须先锚定一个正向信号；
• 对同一父节点的多个文本检查要折叠成一条链：.should("contain", ...).and("contain", ...).and("not.contain", ...)，而不是三条独立的findByText().should(...)查询。一次重试预算、对同一 DOM 快照原子生效；
• expect()只允许出现在cy.then/cy.wrap回调内；
• .should("not.exist")与.should("not.be.visible")按意图区分使用；
• 禁止在 cy 链上做 JS 条件判断（.then(el => if (...))）——.then()只执行一次，.should()才有重试。

// 坏 —— 页面为空时（包括渲染前）都会通过 cy.findByText("Editing").should("not.exist"); // 好 —— 先锚定正向信号，再断言缺失 cy.findByText("Saved").should("be.visible"); cy.findByText("Editing").should("not.exist");

2.8cy.within（必须链式调用）

• 每个cy.within(...)都必须从上一个选择器链出来。孤立的cy.within(...)没有作用域，从构造上就是错的——见到即标记；
• 禁止单语句的within回调——回调里只有一条命令时，直接从父查询链下去即可。within保留给"两条以上命令共享作用域"的场景；
• within回调不要命名参数——within((modal) => ...)是冗余的，内部命令自动继承 subject。真的需要 jQuery subject 时用.then($el => ...)；
• .within()回调要正确闭合；回调外的断言不得意外依赖 within 的作用域。

2.9 日志与标注

• 步骤标注用cy.log("...")，不用// 注释（cy.log会出现在命令面板、截图和视频里）；
• cy.log不得是下一条命令的冗余复述——它应该标记阶段或表达非显性的意图。

2.10 性能

在标记"慢"之前，先查真实 CI 耗时。e2e/support/timings.json 保存了最近一次 CI 运行中每个 spec 的 wall time（毫秒），条目键形如../test/scenarios/...。读它，不要为了查耗时去跑 spec——跑要几分钟，读是即时的。把被审 spec 与同目录的兄弟 spec 对比，才能为"这个 spec 很贵"的论断提供依据。注意该文件不含每个it()的粒度。

实测该文件结构为{"durations": [{"spec": "../test/scenarios/actions/actions-on-dashboards.cy.spec.js", "duration": 224349}, ...]}，确实按 spec 文件维度记录毫秒耗时。

性能清单逐项：

• 微型测试——同一流程、同一beforeEach下拆出多个it()是异味。每个it()要付出 5–10 秒的 Cypress runner 开销加上beforeEach的成本，应尽量合并为单一流程；
• 本该是单元测试的微型测试——只断言元素存在/可见（无流程、无真实后端交互、无跨屏遍历）的测试属于 Jest + RTL 单元测试的范畴，不该进 e2e。常见违规者：token 门控的 UI 检查、"渲染了帮助面板"之类的测试；
• 近重复测试——与兄弟测试共享 80–90% 搭建与步骤的新it()应该扩展既有测试，而不是克隆；
• 多次cy.visit()——每次都是冷启动（多秒级）。首次访问后应通过 UI（点链接、面包屑、侧栏）导航，而不是再发第二个cy.visit()；
• 与更廉价层测试冗余——当 spec 名或describe中引用了 issue（如metabase#12345）时，用裸的#NNNNN（不带metabase前缀——后端测试不一定带前缀）grep 代码库。如果 Jest spec 或后端_test.clj已引用同一 issue，这份 e2e 测试就是冗余的，建议删除。原文给出的检索配方：

rg "#12345" -g '*.spec.{ts,tsx,js,jsx}' -g '!*.cy.spec.*' -g '*_test.clj' -g '*_test.cljc'

其中-g '!*.cy.spec.*'的作用是排除正在被评审的 e2e spec 本身（它显然会匹配到自己引用的 issue 号）。

2.11 Cypress 框架反模式

• 禁止对 cy 查询做forEach——需要迭代时用cy.each()；
• 禁止把原生 Promise /async-await混入 cy 链；
• 重渲染后不得依赖过期的元素引用——重新查询；
• 禁止单参数顶层的cy.contains("text")——与findByText同一条作用域规则。

3. 模式匹配表：常见问题的速查

Skill 内置了一张"快速扫描"表，(lint)标记表示该模式的简单形式 ESLint 已经捕获——看到绕过时才标记。下表完整继承原文：

4. Lint 防线：清单中 (lint) 标记的源码级实现

清单中标注 (lint) 的规则在仓库中都有真实实现，评审时"绕过 lint 才标记"的判定标准由此而来。规则源码位于 frontend/lint/eslint-plugin-metabase/rules/，在 e2e 文件上的启用状态见 eslint.config.mjs：

"metabase/no-unscoped-text-selectors": "error", "cypress/no-assigning-return-values": "error", "cypress/no-async-tests": "error", "cypress/no-pause": "error", "metabase/no-direct-helper-import": "error", "metabase/no-unsafe-element-filtering": "warn", "metabase/no-unordered-test-helpers": "error",

注意两个细节：no-unsafe-element-filtering是warn级而非error级；cypress/no-async-tests与cypress/no-pause也在 e2e 配置中以error启用，与清单中"不混原生 Promise/async-await"的条目对应。

no-unscoped-text-selectors的实现机理（no-unscoped-text-selectors.js）解释了为什么清单要特别提醒"helper 包裹的是 lint 盲区"。该规则的判定逻辑是：

1. 识别顶层cy.findByText(...)和单参数cy.contains('text')（两参数但第二参是对象字面量的情形也按单参处理）；
2. 从节点向上找最近的 BlockStatement（findNearestBlockStatement），判断该块是否直接位于it/before/beforeEach之下（兼容.only形式的 callee）。

问题在于：当查询被包进一个 helper 函数体时，最近块就是helper 的函数体而不是测试块，isTestBlock返回 false，规则不报——这正是约定文件中所说的"规则向上走到最近块，而那个块是 helper 体，不是测试块"。因此评审时必须人工扫描 helper 函数体。

no-unsafe-element-filtering的实现（no-unsafe-element-filtering.js）则精确对应清单中"lint 只捕获.last()与.eq(<负数)"的说法：它只把.last()与.eq(负数字面量)（或.eq(动态索引)）视为风险调用，然后沿链向上检查是否已被 length 断言守卫——识别的断言模式包括have.length、have.lengthOf、have.length.at.least、have.length.gte等一组。.first()和非负.eq(N)不在检查范围，因此这部分"同等审慎"落在作者/评审人身上，与清单描述完全一致。

另有 no-direct-helper-import.js（禁止从e2e/support/helpers直接 import）与 no-unordered-test-helpers.js（强制H.restore()先于H.resetTestTable()），分别对应清单中 Helpers 一节与隔离一节的 (lint) 条目。

5. 不应标记的情形（What NOT to flag）

一份好的评审标准同样要定义"不做什么"，否则会产生大量噪音。Skill 明确列出了豁免清单：

• 不对存量文件的.cy.spec.js扩展名开火——.js与.ts都可接受，.ts只是新spec 的偏好；
• 不标记 e2e-visual-tests-helpers.js 内部的 CSS 属性选择器（path[fill='...']、[stroke-dasharray='...']、text[stroke-width='3']等）。原因是 ECharts 渲染的 SVG 没有data-testid、可访问性面极小，该文件是被有意保留的例外。但同样模式出现在 spec 文件或新 helper 中时必须标记——那些应当走既有 helper 或扩展该文件；
• 不标记"顺序即断言"的.first()/.eq(N)/:nth-child(N)（如测试排序顺序），或紧邻其前有.should("have.length", n)的情形；
• 不标记 helper 体内的cy.findByText(...)/cy.contains(...)，前提是该 helper 有文档说明或明显意图是"在调用点的外层within(...)作用域内被调用"（继承的 within 作用域在运行期使其安全）。拿不准就问作者；
• 本地开发评审时不标记.only（用户是有意聚焦）；PR / commit 时评审则必须标记；
• 不发"看起来不错"或祝贺性评论，只发问题；
• 不评论 linter 已处理的格式问题（Prettier/ESLint）；
• 不标记不影响稳定性、速度或正确性的风格偏好。

6. 反馈格式：本地模式与 PR 模式

所有问题从Issue 1开始顺序编号，格式为**Issue N: [Brief title]**。

本地评审模式的输出模板：

## Issues **Issue 1: [Brief title]** File:Line — succinct description Suggested fix **Issue 2: [Brief title]** ...

PR 评审模式走 pending review 工作流：

1. mcp__github__create_pending_pull_request_review—— 开启草稿评审；
2. mcp__github__get_pull_request_diff—— 获取文件路径与行号；
3. 找出全部问题并顺序编号；
4. mcp__github__add_pull_request_review_comment_to_pending_review—— 每个问题作为独立评论提交，并在单条响应内并行发出；
5. mcp__github__submit_pending_pull_request_review，事件用"COMMENT"（不是REQUEST_CHANGES），无 body。

每条评论正文以**Issue N: [Brief title]**开头。选择COMMENT而非REQUEST_CHANGES体现了该评审流程的定位：给出改进意见，不阻塞合入。

7. 评审流程与仓库基础设施的对应关系

把 Skill 的每个环节落到仓库实际设施上，可以看到它并非凭空约定：

• spec 存放位置：e2e/test/scenarios/，与 URL 结构镜像（约定文件"File location and naming"一节），现有文件均为*.cy.spec.ts/*.cy.spec.js；
• helper 访问方式：const { H } = cy;的机制由 e2e/support/helpers/ 下约 140 个 helper 文件支撑（e2e-dashboard-helpers.ts、e2e-collection-helpers.ts、e2e-visual-tests-helpers.js等），约定文件建议用 grep 该目录来发现可用 helper；
• 常量来源：样例库表结构来自 cypress_sample_database.js（如ORDERS、ORDERS_ID、PRODUCTS），实例级 ID 来自 cypress_sample_instance_data.js（如ORDERS_DASHBOARD_ID）；
• CI 耗时数据：e2e/support/timings.json 即性能检查的数据源；
• 提交拦截：Skill 与约定文件均提到.only/.skip"应由 pre-commit hook 拦截"，这与仓库提交钩子机制相衔接；
• ESLint 规则：frontend/lint/eslint-plugin-metabase/与eslint.config.mjs的 e2e 段（见第 4 节），是 (lint) 标记的落地实现。

从源码结构看，这套"Skill 定义评审标准 + 共享约定定义编写标准 + Lint 规则自动化其中一部分"的三层结构，使得写测、审测与机器检查共用同一份事实来源，评审清单与约定文件按同一顺序组织正是为此。

8. 诚实的 e2e-vs-unit 分层判定（每份报告的必选收尾）

这是 Skill 中最具方法论价值的部分。编号问题回答的是"测试写得好不好"，而这一节回答的是"它该不该是 e2e 测试"——两者正交：一份无懈可击的 spec 仍可能在为组件测试级的价值支付 e2e 的价格，这一点值得明说。

判定标准一句话：一个it()只有当它执行了"不假以真身就会掏空测试价值"的层，才配得上 e2e 的位置。在 Metabase 代码库中，这意味着至少满足以下之一：

• 针对种子数据的真实后端查询——断言依赖服务端真的做了过滤/排序/分桶，而非 mock 响应；
• 真实渲染且需要交互——在计算出的坐标上点击 ECharts SVG 元素、拖拽/命中测试、drill-through；
• 跨屏路由——导航、后端随后兑现的 URL 参数往返、浏览器前进/后退；
• 跨屏遍历——列表 → 详情 → 返回，价值在屏幕之间的接缝处。

反之，当测试的真实对象纯属前端时，它属于Jest + RTL（该仓库用于组件/单元覆盖的更廉价层）：

• 纯前端逻辑——分桶/格式化/派生决策（如"单日范围 → 按小时分桶"），无论后端如何都有唯一正确答案；
• 渲染断言——"页面挂载了 N 个带标题的卡片且存在一个 SVG"。mock 数据集响应 + RTL 就能验证标题；ECharts SVG 能渲染是库的行为，不是你的逻辑；
• 状态/接线——tab →aria-selected→ 激活态，或 select → querystring 映射。"URL 拿到参数"这半是组件级的事；只有"后端兑现参数"那半才需要 e2e。

判定报告须写成三个诚实的桶，每个结论都要落在"测试实际触碰了哪些层"上，而不是看测试叫什么名字：

1. 必须是 e2e（保留）——承重的。点名那些只有"浏览器 + 后端"联合才能验证的层；
2. 站得住但可收窄的 e2e——作为集成测试合理，但拆得过碎或冷启动次数超出覆盖所需。说明应合并什么；
3. 应转为单元/组件测试——真正的节省在这里。点名更廉价的层以及它会更精确地断言什么（往往比 e2e 的间接断言更精确）。

最后给一段结论：哪些保留在 e2e，哪些下沉；如果为性能检查拉过e2e/support/timings.json的 CI 耗时，则估算整份 spec 的 wall time 中有多大比例是"被必须-e2e 集合赚取的"、多大比例是"其余部分支付的"。

为保证该判定"诚实而非表演"，Skill 还附了四条交战规则：

• 不要把保留桶吹大以显得平衡。如果 spec 的大部分是"以 e2e 价格支付组件测试价值"，就直说；如果全部测试都承重，也直说——一份 12 个测试全都要 e2e 的 spec 是合法结果，不是没找到候选的失败；
• 要具体。引用it()标题与行号范围。"若干测试可以是单元测试"毫无用处；"测试 1/17/18 只断言 N 个带标题卡片挂载——RTL + mock 数据集即可"才是可执行的；
• 这是给作者的建议，不是阻塞问题。它是建议，独立于编号问题列表，不得重新编号进 issue 列表；
• 立足于现存设施。这里的廉价层是 Jest + RTL 组件/单元测试。不要发明仓库没有的基础设施，也不要因为测试的一部分是前端的就建议删掉依赖后端的覆盖——拆分它，保留集成那一半。

9. 评审前的最后自检（Final check）

Skill 以四步收尾清单结束，可作为每次输出报告前的核对表：

1. 剪掉那些不会对作者产生实质帮助的问题；
2. 确认编号连续无缺号；
3. PR 模式下，确认每个问题都已作为独立评审评论提交；
4. 确认"诚实的 e2e-vs-unit 分层判定"存在——即使答案是"这些全都必须是 e2e"，它也是每份报告的必需部分。

10. 小结

Metabase 的 e2e 测试评审体系由三个文件构成闭环：SKILL.md 定义"怎么审"（流程、清单、模式表、反馈格式、分层判定），cypress-conventions.md 定义"怎么写"（写与审共用同一标准），而 eslint.config.mjs 与 frontend/lint/eslint-plugin-metabase/rules/ 中的自研规则把其中可机器判定的部分（无作用域文本选择器、.last()/负索引.eq()无长度守卫、直接 import helper、restore/resetTestTable顺序、赋值 cy 返回值、async 测试、cy.pause()）固化为 lint 防线。评审人的职责则集中在 lint 的盲区上：helper 包裹的无作用域查询、位置选择器的非 lint 情形、硬编码 ID、微型测试与近重复测试、多次冷启动cy.visit()，以及最关键的——判断哪些测试根本不该存在于 e2e 层。理解这套标准后，无论是人还是 Agent，都能对 Metabase 的 Cypress spec 给出可验证、可复现、且与仓库自身工程设施对齐的评审结论。

【免费下载链接】metabaseThe easy-to-use open source Business Intelligence and Embedded Analytics tool that lets everyone work with data :bar_chart:项目地址: https://gitcode.com/GitHub_Trending/me/metabase