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二选一PR 评审模式——当mcp__github__create_pending_pull_request_review工具可用时把发现的问题作为一个连贯的 pending review 一次性提交本地评审模式——否则在对话中输出一份编号问题列表。模式检测之后是六步评审流程原文第 18–25 行检测模式先通读整份被修改的 spec理解测试意图不要逐行冷评审dont review line-by-line cold条件触发只有当读完后某个断言或选择器存在真实的歧义——分不清锚点选得对不对、测试是否真的验证了标题所声称的东西——才去 grep 相关组件中的 test ID / role / 文本。默认不读组件源码只有 spec 里出现真实的困惑信号才做这一步按后文的审查清单 模式匹配表逐项扫描所有问题按顺序编号跳过吹毛求疵的小问题——只标记值得修的东西每次报告都必须以诚实的 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——对存量文件不要标记.jsdescribe块在相关时应命名到领域area sub-area feature (#issue-number)不允许遗留.only或.skippre-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优先使用已有的导航 helperH.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 查询不适用时才用findByTextdata-testid属性一律用findByTestId绝不裸写cy.get([data-testid...])禁用 CSS 类名——尤其是 styled-components / Mantine 生成的.css-1abc2dspec 与新 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不走 UIH.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已在执行中、不可复用函数则推迟查找每次调用都重新执行查询并保留完整的重试语义——可视化 helperechartsContainer()、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 已经捕获——看到绕过时才标记。下表完整继承原文模式问题cy.wait(2000)数字等待——改用 intercept 别名或.should(be.visible)cy.get(.css-1abc2d)生成的 CSS 类——改用 a11y 查询 /findByText/findByTestIdcy.get([data-testidfoo])永远应使用cy.findByTestId(foo)spec 中的cy.get(path[fill#abc])ad-hoc 图表选择器——用e2e-visual-tests-helpers或往该文件新增 helpercy.get(li:nth-child(3))CSS 选择器里的位置依赖——锚定文本或 role无前序 length 断言的.last()/.eq(-1)集合大小变化时有 off-by-one 风险(lint)cy.visit(/dashboard/10)硬编码数字 ID——从创建响应捕获或从cypress_sample_instance_data导入import { restore } from e2e/support/helpers直接 import helper——用const { H } cy;(lint)it()/beforeEach顶部的cy.findByText(Save)无作用域文本选择器——包进cy.contains(selector, text)或从作用域查询链出(lint但漏掉 helper 包裹的)function clickSave() { cy.findByText(Save).click() }helper 包裹的无作用域文本——lint 盲区需人工扫描 helper孤立的cy.within(() ...)cy.within必须链自既有选择器——从构造上就是错的cy.intercept放在触发动作之后intercept 必须先于触发动作cy.get(...).then(el { if (...) })cy 链上的 JS 条件——.then无重试语义用.should()await cy.something()Cypress 链不是真正的 Promiseels.forEach(el cy....)应改用cy.each()单条命令上{ timeout: 30000 }多半在掩盖竞态——找根因const button cy.findByRole(...)给cy.*返回值赋值——button是一次性 chainer不是 DOM 元素(lint 捕获简单情形)const foo () cy.findByText(Foo)这是正确的——函数形式每次调用重新入队查询某步骤中唯一的断言是.should(not.exist)纯负向断言——页面未渲染时会侥幸通过先锚定正向断言H.resetTestTable()出现在H.restore()之前restore必须在前(lint)一个it()内多次cy.visit()每次都是冷启动——屏间用 UI 导航it.only(/describe.only(pre-commit hook 应拦截——漏网则标记与兄弟测试同搭建 80–90% 同步骤的新it()大概率是近重复——扩展既有测试it(..., () { cy.findBy*().should(be.visible) })仅此而已纯静态 UI 测试——强烈信号应改为 Jest 单元测试cy.log(Visit dashboard); H.visitDashboard(id);冗余日志——复述下一条命令// Visit dashboard后跟H.visitDashboard(id);应改用cy.log(...)——截图/视频中可见4. Lint 防线清单中 (lint) 标记的源码级实现清单中标注 (lint) 的规则在仓库中都有真实实现评审时绕过 lint 才标记的判定标准由此而来。规则源码位于 frontend/lint/eslint-plugin-metabase/rules/在 e2e 文件上的启用状态见 eslint.config.mjsmetabase/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 盲区。该规则的判定逻辑是识别顶层cy.findByText(...)和单参数cy.contains(text)两参数但第二参是对象字面量的情形也按单参处理从节点向上找最近的 BlockStatementfindNearestBlockStatement判断该块是否直接位于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-width3]等。原因是 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 工作流mcp__github__create_pending_pull_request_review—— 开启草稿评审mcp__github__get_pull_request_diff—— 获取文件路径与行号找出全部问题并顺序编号mcp__github__add_pull_request_review_comment_to_pending_review—— 每个问题作为独立评论提交并在单条响应内并行发出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.jshelper 访问方式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_IDCI 耗时数据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。判定报告须写成三个诚实的桶每个结论都要落在测试实际触碰了哪些层上而不是看测试叫什么名字必须是 e2e保留——承重的。点名那些只有浏览器 后端联合才能验证的层站得住但可收窄的 e2e——作为集成测试合理但拆得过碎或冷启动次数超出覆盖所需。说明应合并什么应转为单元/组件测试——真正的节省在这里。点名更廉价的层以及它会更精确地断言什么往往比 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 checkSkill 以四步收尾清单结束可作为每次输出报告前的核对表剪掉那些不会对作者产生实质帮助的问题确认编号连续无缺号PR 模式下确认每个问题都已作为独立评审评论提交确认诚实的 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创作声明：本文部分内容由AI辅助生成（AIGC），仅供参考
