review-testing

你正在审查与功能实现或错误修复一同编写的测试代码。确保测试设计良好、覆盖全面且易于维护，而不只是确保它们能够通过。仅仅照搬实现细节的测试会造成虚假的信心，并在重构时成为维护负担。

审查范围

确定审查内容：

1. 查找当前分支上变更的测试文件（相对于基准分支）。
2. 查找这些测试覆盖的生产代码——追踪导入、函数调用和文件命名约定，确定测试对应的目标。
3. 查找同一模块或函数的相关现有测试，尤其是未变更的测试——它们可能需要更新，也可能暴露出覆盖缺口。

先阅读测试文件，再阅读生产代码。如果仅凭测试就能推断出功能需求和边界情况，说明测试编写得很好。如果必须阅读实现才能理解测试在做什么，这本身就是值得报告的问题。

有价值的测试具备哪些特点

从以下四个方面评估每项测试：

回归保护——这项测试真的能捕获错误吗？测试平凡代码或跳过复杂分支，对防止问题毫无帮助。请检查：测试涉及的是业务关键逻辑，还是只验证了简单 getter 的正常路径？

重构韧性——如果有人重组代码但不改变行为，这项测试会不会失败？与内部方法名、调用顺序或私有状态紧密耦合的测试，会让每次代码清理都引发误报，削弱人们对测试套件的信任。

快速反馈——单元测试应在毫秒内运行。如果测试不必要地访问文件系统、网络或数据库，就是设计问题。但不要把速度与价值混为一谈——验证真实数据库查询的集成测试，胜过快速但模拟一切、毫无验证价值的单元测试。

可维护性——不熟悉这段代码的人能否读懂测试，并理解它验证了什么以及原因？设置过程冗长、名称晦涩或模拟层层嵌套的测试不符合这一标准。

审查要点

1. 断言完整性

自动生成的测试最常见的问题是：只断言显而易见的输出，却忽略状态变更的完整“影响范围”。

当测试触发某个操作时，请思考还有哪些内容发生了变化。如果测试向购物车添加商品，它是否只检查了商品数量？还是也验证了价格计算、小计更新，以及其他商品未受影响？

以下情况应标记：

• 测试断言了返回值，却忽略副作用
• 测试检查操作是否成功，却未检查操作是否产生了正确结果
• 测试验证了正常路径，却跳过边界条件（空输入、null、最大值、差一错误）
• 错误路径测试只检查是否抛出错误，却不检查错误消息、类型或清理行为

2. 测试结构

每项测试都应恰好包含一个 Arrange-Act-Assert（准备-执行-断言）循环。如果测试按顺序多次执行操作并进行断言，说明它测试了多种行为，应拆分。

以下情况应标记：

• 测试包含多个执行阶段（测试的是工作流，而非单一行为）
• 准备阶段过于庞大，掩盖了实际要测试的行为
• 断言出现在设置辅助函数或实用函数中（隐藏了验证内容）
• 测试逻辑包含条件语句或循环——测试代码应按直线流程编写

3. Fixture 和状态管理

测试数据的创建方式决定了测试套件能否随着规模增长而保持可维护。内联设置（在测试主体中创建数据）适用于简单测试，但当许多测试中的构造函数签名发生变化时，就会很难维护。隐式设置（共享的 beforeEach/setUp 代码块）可以消除重复，但会掩盖每项测试实际依赖的内容。委托设置（显式调用工厂函数或构建器）既能让测试保持可读，又能集中管理构造逻辑。

以下情况应标记：

• 共享设置代码块创建了大多数测试都不会使用的对象
• 测试依赖外部文件、数据库状态或在其他地方定义的全局变量（“神秘访客”——测试所需的一切都应该能在测试主体中看到，或通过一次函数调用获取）
• 测试假设资源已存在，却没有创建这些资源（文件、数据库记录、环境变量）
• 测试修改共享状态却没有清理，导致测试结果依赖执行顺序

4. 模拟边界

模拟对于隔离测试至关重要，但过度使用会让测试变成实现的镜像。

**核心原则：**在架构边界处进行模拟，而不是模拟每次函数调用。对于查询类依赖，使用存根（预置数据）；仅对具有副作用的命令（例如发送电子邮件或向外部系统写入数据）使用模拟对象（验证交互）。

遵循代码库现有的模拟惯例。应标记项目内部的不一致，而不是偏离某条普遍规则。

以下情况应标记：

• 测试模拟了自己不拥有的类型（第三方库、框架内部实现）——库更新时，模拟对象仍会基于过时的假设通过测试。建议使用：精简适配器、集成测试中的真实实现，或官方测试工具（MemoryRouter、测试数据库、内存缓存）。
• 模拟配置过多——4 个或更多模拟配置通常表明生产代码承担了过多职责
• 测试验证内部方法的调用次数或参数顺序（这会让测试耦合于代码的实现方式，而非代码的行为）
• 存根被拿来验证（检查存根是否被调用，测试的是实现，而非行为）

5. 测试异味

异味	表现形式	影响
断言轮盘	多个断言都没有失败消息	不调试就无法判断哪个断言失败
贪心测试	一个测试执行多个互不相关的方法	失败原因不明确——究竟是哪项行为出错了？
懒惰测试	多个测试使用相同输入调用同一个方法	增加冗余维护成本，却没有增加覆盖率
睡眠测试	硬编码的 sleep()/Sys.sleep()/setTimeout()	在 CI 中不稳定，到处都很慢。请使用轮询或显式等待。
腐坏的绿色	在 try/tryCatch 或条件分支中进行断言	断言永远不会执行，因此测试总是通过
脆弱的相等性	对 toString()/print() 的输出进行断言	格式变化就会导致失败；应改为断言结构属性
打印语句	使用 print()/console.log()，而不是断言	调试遗留代码，无法验证任何内容
滥用快照	用快照代替行为断言	任何变化都会触发失败，开发者只能盲目更新快照。适用场景：对一个 HTML 组件的结构做一次快照（而不是对每种属性组合都做快照）、错误消息文本、CLI 输出。不适用场景：对整个对象或每种渲染变体都做快照。
实现镜像	通过与生产代码相同的逻辑计算预期值	即使测试代码和生产代码都错了，它们也总会一致。应从已知正确的来源硬编码预期值。

6. 命名与可读性

测试名称应描述行为，而不是实现。命名良好的测试套件读起来就像一份功能规格说明。

标记以下情况：

• 测试名称引用内部方法名（例如，test_processData_returns_true）——重命名时会失效
• 测试名称过于笼统（test1、test_it_works、test_basic）
• 名称没有说明场景或预期结果

优先使用描述行为的名称：test_expired_subscription_blocks_access、delivery_with_past_date_is_invalid、empty_cart_shows_zero_total。

7. 覆盖缺口

对照测试套件检查生产代码变更：

• 是否存在没有测试覆盖的分支或条件？
• 是否测试了错误路径？（不只是“是否抛出错误”，还要检查“是否抛出了正确的错误并正确清理”）
• 是否覆盖了边界情况？（空集合、null/NA/None/undefined、边界值，以及适用时的并发访问）
• 如果生产代码改变了现有行为，是否更新了现有测试？

逐步检查实现，并记录每一个决策点——每个 if、match、switch、错误处理程序或提前返回。检查测试套件是否覆盖了这些决策的两种结果。

审查使用 testthat 的 R 测试时，请检查是否有可用的 r-testthat 技能，并调用它来了解 R 特有的约定和模式。

回复格式

## Summary
[Overall assessment: How well do these tests protect the codebase?]

## Critical Issues (Blocking)
[Tests that provide false confidence or will cause real problems.]

## Required Changes
[Design problems that weaken the test suite.]

## Strong Suggestions
[Improvements to test quality and maintainability.]

## Noted
[Minor style or convention issues. Mention once, then move on.]

## Verdict
Request Changes | Needs Discussion | Approve

## Next Steps
[Options for proceeding]

每条发现都使用 file:line 引用。引用能够证明问题的具体测试代码，并展示更好的代码应该是什么样。

后续步骤

在审查结束时，向用户提供以下选项：

讨论并处理发现的问题： 使用 AskUserQuestion 工具逐项讨论问题。按严重程度或主题分组，提供解决方案选项，并标记推荐选项。

修复问题： 提议按优先级直接应用修复——先处理阻塞问题，再处理必须修改的内容，最后处理建议。每组修复后都要确认，然后再继续。

添加到拉取请求： 在审查 PR 时，提议将审查意见作为 PR 评论发布。注明："Review assisted by the review-testing skill."

如果作为子 Agent 运行，请跳过后续步骤，仅输出审查发现。