让我们多谈谈代码审查

一则或许对你有用的小广告

欢迎加入小哈的星球 ,你将获得:专属的项目实战 / Java 学习路线 / 一对一提问 / 学习打卡/ 赠书活动

目前,正在 星球 内带小伙伴们做第一个项目:全栈前后端分离博客项目,采用技术栈 Spring Boot + Mybatis Plus + Vue 3.x + Vite 4手把手,前端 + 后端全栈开发,从 0 到 1 讲解每个功能点开发步骤,1v1 答疑,陪伴式直到项目上线,目前已更新了 204 小节,累计 32w+ 字,讲解图:1416 张,还在持续爆肝中,后续还会上新更多项目,目标是将 Java 领域典型的项目都整上,如秒杀系统、在线商城、IM 即时通讯、权限管理等等,已有 870+ 小伙伴加入,欢迎点击围观

在上一篇文章中,我们讨论了您可以在代码审查中寻找的各种各样的东西。现在我们将关注一个领域:在测试代码中寻找什么。

本文假设:

  • 您的团队已经为代码编写了自动化测试。
  • 这些测试定期在持续集成 (CI) 环境中运行。

在这篇文章中,我们将介绍审阅者在查看代码审阅中的测试时可能会考虑的一些事情。

是否有针对此新/修订代码的测试?

很少有新代码(无论是错误修复还是新功能)不需要新的或更新的测试来覆盖它。即使是出于“非功能性”原因(如性能)而进行的更改也经常可以通过测试来证明。如果代码审查中没有包含测试,作为审查者,您应该问的第一个问题是“为什么不呢?”。

测试是否至少涵盖了令人困惑或复杂的代码部分?

超越简单的“是否有测试?”的一步是回答“重要代码是否实际上被至少一次测试覆盖?”这个问题。

检查测试覆盖率当然是我们应该自动化的事情。但是我们不仅可以检查覆盖率中的特定百分比,还可以使用覆盖率工具来确保覆盖了 正确的 代码区域。

例如考虑这个:

您可以检查新代码行的覆盖率报告(这应该很容易识别,特别是如果您使用像 Upsource 这样的工具)以确保它被充分覆盖

在上面的这个例子中,审阅者可能会要求作者添加一个测试来覆盖第 303 行的 if 评估为真的情况——覆盖工具将第 304-306 行标记为红色以表明它们没有被测试。

100% 的测试覆盖率对于几乎任何团队来说都是一个不切实际的目标,因此来自覆盖率工具的数字可能不如对覆盖哪些特定领域的见解有价值。

特别是,您要检查是否覆盖了所有逻辑分支,以及是否覆盖了复杂的代码区域。

我能理解测试吗?

拥有提供足够覆盖率的测试是一回事,但如果我作为一个人无法理解这些测试,那么它们的用途就会受到限制——当它们崩溃时会发生什么?很难知道如何修复它们。

考虑以下:

这是一个相当简单的测试,但我不完全确定它在测试什么。它是在测试 save 方法吗?或者 getMyLongId ?为什么它需要做同样的事情两次?

测试背后的意图可能更清楚:

您为阐明测试目的而采取的具体步骤将取决于您的语言、图书馆、团队和个人偏好。这个例子表明,通过选择更清晰的名称、内联常量甚至添加注释,作者可以使测试对他或她以外的开发人员更具可读性。

测试是否符合要求?

这是一个真正需要人类专业知识的领域。无论被审查的代码满足的需求是编码在一些正式的文档中、用户故事中的一张卡片上,还是包含在用户提出的错误中,被审查的代码都应该与一些初始需求相关。

审阅者应找到原始要求并查看是否:

  1. 测试,无论是单元测试、端到端测试还是其他测试,都符合要求。例如,如果需求是“应该允许特殊字符‘#’、‘!’”和密码字段中的“&””,应该在密码字段中使用这些值进行测试。如果测试使用不同的特殊字符,则不能证明代码符合标准。
  2. 测试涵盖了所有提到的标准。在我们的特殊字符示例中,需求可能会继续说“……如果使用其他特殊字符,则给用户一条错误消息”。在这里,审阅者应该检查是否存在使用无效字符时会发生什么情况的测试。

作为审稿人,您能想到现有测试未涵盖的案例吗?

我们的要求通常没有明确规定。在这些情况下,审阅者应该考虑原始错误/问题/故事中未涵盖的边缘案例。

例如,如果我们的新功能是“让用户能够登录系统”,审阅者可能会想“如果用户为用户名输入 null 会发生什么?”,或者“如果出现什么类型的错误该用户在系统中不存在?”。如果这些测试存在于被审查的代码中,那么审查者就会增加代码本身处理这些情况的信心。如果不存在针对这些异常情况的测试,那么审阅者必须检查代码以查看它们是否已被处理。

如果代码存在但测试不存在,则由您的团队决定您的策略是什么——您是否让作者添加这些测试?或者您对代码审查证明边缘情况已被涵盖感到满意吗?

是否有测试来记录代码的局限性?

作为审阅者,通常可以看到正在审阅的代码中的局限性。这些限制有时是有意的——例如,一个批处理过程每批最多只能处理 1000 个项目。

记录这些有意限制的一种方法是明确测试它们。在我们上面的例子中,我们可能有一个测试证明如果你的批量大小大于 1000 就会抛出某种异常。

在自动化测试中表达这些限制不是强制性的,但如果作者编写了一个测试来显示他们所实现的限制,那么进行测试意味着这些限制是有意的(并记录在案的)而不仅仅是疏忽。

Code Review 中的测试类型/级别是否正确?

例如,作者是否在单元测试可能就足够的情况下进行昂贵的集成测试?他们是否编写了无法在 CI 环境中有效或以一致方式运行的性能微基准?

理想情况下,您的自动化测试将尽可能快地运行,这意味着可能不需要昂贵的端到端测试来检查所有类型的功能。执行某些数学函数或布尔逻辑检查的方法似乎是方法级单元测试的良好候选者。

是否需要编写安全测试?

安全是代码审查真正受益的领域之一。稍后我们会写一整篇关于安全的文章,但在测试主题上,我们可以为一些常见问题编写测试。例如,如果我们正在编写上面的登录代码,我们可能还想编写一个测试来表明如果不先进行身份验证,我们就无法进入站点的受保护区域(或调用受保护的 API 方法)。

性能测试

在上一篇文章中,我谈到性能是审阅者可能要检查的一个领域。自动化性能测试显然是我可以在本文中探讨的另一种测试类型,但我将在稍后的文章中讨论这些类型的测试,以便在代码审查中专门查看性能方面。

审稿人也可以编写测试

不同的组织有不同的代码审查方法 - 有时很明显作者负责进行所有所需的代码更改,有时它更多地与审查者协作,向代码本身提交建议。

无论您采用哪种方法,作为审阅者,您可能会发现编写一些额外的测试来检查审阅中的代码对于理解该代码非常有价值,就像启动 UI 和使用新功能一样有价值.一些方法和代码审查工具比其他方法和代码审查工具更容易试验代码。在代码审查中尽可能轻松地查看和使用代码符合团队的利益。

提交额外的测试作为评审的一部分可能很有价值,但同样也可能没有必要,例如,如果实验已经给了我,评审员,我的问题得到了满意的答案。

总之

无论您如何处理组织中的流程,执行代码审查都有很多优势。在将代码集成到主代码库 之前, 可以使用代码审查来发现代码的潜在问题,同时修复它的成本仍然很低,而且上下文仍在开发人员的脑海中。

作为代码审阅者,您应该检查原始开发人员是否考虑了他或她的代码的使用方式,在哪些条件下可能会破坏代码,并处理边缘情况,最好是“记录”预期的行为(两者在正常使用和特殊情况下)与自动化测试。这些测试可以让您了解代码应该做什么,如果通过,则证明它确实做到了。

如果审阅者检查测试是否存在并检查测试的正确性,作为一个团队,您可以对代码的工作有很高的信心。此外,如果这些测试在 CI 环境中定期运行,您可以看到代码 继续 工作——它们提供自动回归检查。如果代码审阅者高度重视对他们正在审阅的代码进行高质量测试,那么在审阅者按下“接受”按钮后,此代码审阅的价值将持续很长时间。