【第945期】代码审查应该关注什么：测试

前言 

本文分享的虽然不是javascript，但关注点应该是通用的。看到写测试代码就心虚，哈哈。今日早读文章由@唐先僧翻译分享。

正文从这开始～

昨天其一，今日其二

在【第944期】代码审查应该关注什么之一中我们讨论了你在代码审查中应该关注的各种内容。现在我们关注一个领域：在测试代码中关注什么？

本文假设：

• 你的团队已经为代码编写了自动化测试。

• 这些测试有规律的运行在持续集成（CI）环境。

• 被审查的代码已经通过了自动化测试。

在本文中我们将讨论代码审查者在审查测试代码时应该考虑的东西。

新的/修改后的代码有测试代码吗？

不管是 bug 修复还是新功能添加，很少出现不需要新增测试代码或者修改已有测试代码的情况。即使是因为性能这样的“非功能性”原因修改，常常也可以通过测试代码来验证。如果在代码审查中没有包含测试代码，作为审查者你需要问的第一个问题是“为什么没有？”。

测试代码有没有覆盖令人费解的或者复杂的代码区？

在简单的回答了“是否有测试代码”后，下一个问题就是“重要的代码是否至少有一个测试用例覆盖”？

检查代码覆盖率当然可以自动化完成。但是我们能做的不仅仅是检查具体的覆盖率，我们可以使用覆盖工具确保正确区域的代码被覆盖。

一起来看一下这个这例子：

你可以通过覆盖报告来确认它新的代码已经被充分覆盖（这很容易确认，尤其是如果使用了像 Upsource 这样的工具）。

在上面的这个例子中，审查者可以要求作者添加一个测试用例保证第 303 行中的 if条件为 true 被执行，因为覆盖工具已经用红色标明第 304-306 行没有被测试到。

对几乎所有的团队来说100%的覆盖率都是不现实的，因此来自你覆盖工具的数字的价值，也许不如仔细检查那些特殊区域的覆盖情况有意义。

特别指出，你需要检查所有的逻辑分支都已经被覆盖，并且复杂区域也要覆盖到。

我能理解这些测试吗？

提供充分覆盖率的测试用例是一方面，但是，作为开发者如果我不能理解这些测试，它们的作用有限--当它们出问题时要怎么办？我不知道怎么去修复它们。

看一下这个例子：

这个一个很简单的测试，但是我不能完全确定它在测试什么。它是在测试 save 方法？还是 getMyLongId？为什么它需要把同样的事情做两遍？

修改成这样，这个测试背后的意图就会清晰很多：

澄清测试意图所需要的具体步骤取决于你使用的语言、库、团队以及个人喜好。这个例子说明通过选择清晰的命名、内联常量甚至添加注释等方法，作者可以使得测试代码更加容易被其他开发者理解，而不仅仅是他/她一个人。

这些测试和需求匹配吗？

这里真的是需要人的经验的领域。被审查的代码是否和需求匹配，这被编码在正式的文档中，一张 user story 的卡片上或者是用户提交的bug中，代码审查应该和最初的需求关联起来。

代码审查者应该找出原始的需求，并检查是否满足：

1、测试，不管是单元测试还是端到端的，或者其他，要和需求匹配。例如，如果需求是“应该允许 ‘#’，'!'，和‘&’这几个特殊字符出现在密码字段”，就应该有在密码字段中使用这些特殊字符的测试。如果测试代码使用了其他的特殊字符，那就不能说明代码和和需求匹配。

2、测试要覆盖了所有涉及到的标准。我们举的特殊字符的例子，需求可能还说了“。。。如果使用了其他特殊字符，就会给用户错误信息”。那么在这里审查者应该检查是否有输入无效字符的测试。

我能想到已有的测试用例没有覆盖到的场景吗？

通常我们的需求不是很明确。在这种情况下，审查者应该考虑原始 bug/issue/story 没有覆盖到的一些边界场景。

例如，如果我们的新功能是“给用户在系统登录的能力”，审查者应该考虑“如果用户在用户名输入无效会发生什么事情？”或者“如果系统中不存在该用户会发生哪一类错误？”。如果被审查的代码中有这些测试用例，那么审查者会更加确信代码本身可以处理这些异常。如果没有针对这些异常场景的测试用例，审查者必须浏览代码确认它们是否有处理这些异常。

如果存在处理异常的代码，但是没有编写测试代码，那么需要由你的团队来确定你们的策略--你们要让作者添加这些测试用例吗？还是你们觉得代码审查能证明边界情况被覆盖就可以了？

有测试代码记录代码的限制吗？

作为审查者，常常会看到被审查的代码中存在种种限制。这些限制有时是有目的性的--例如：一个批处理流程一次最多只能处理1000个项目。

记录这些内部限制的一种方法就是明确测试它们。在上面的这个例子中，我们可以写一个大于1000的批处理证明它会抛出某一类的异常。

在自动化测试中表述这些限制并不强制，但是如果作者通过测试代码展示了他们内部实现的限制，测试代码暗示了这些限制是有意的而并非疏忽，那么就是强制的了。

测试代码是正确的类型/级别？

例如，作者是不是将单元测试就能完成的事情放到了集成测试中？作者是不是写了性能很差无法有效运行的测试，或者无法在 CI 环境持续运行的测试？

理想情况下自动测试的运行应该越快越好，这意味着不需要付出代价进行端对端测试以检查所有类型的功能。对于算术运算、或者布尔值逻辑检查的方法更适合函数级别的单元测试。

有安全方面的测试吗？

安全真的是通过代码审查获得好处的一个领域。后续我们会有一篇文章专门讨论安全问题，但是在测试这个主题里，我么需要为一些常见问题写测试代码。例如：像前面我们在编写登录的代码，我们应该有测试代码证明：在没有第一次授权之前我们是不能进入网站的受保护区域的（或者调用受保护的 API 方法）。

性能测试

在上一篇博客中我讲到性能是审查者需要检查的一个区域。我在本文中讨论过的自动化性能测试属于另一种类型测试，但是我将在后续文章中专门就性能方面进行测试类型的讨论。

审查者也可以写测试代码

不同的团队有不同的代码审查方法--有的团队很明确，作者负责所有的代码修改，有的他们也会同审查者协作来提交建议。

不管你使用何种方法，作为审查者，你会发现在审查中添加一些额外的测试对理解代码是很有意义的，同样如果打开 UI 界面把玩一下新的功能也是很有价值的。有的代码审查工具和方法比其他的更容易检测代码。团队的兴趣所在也就是在代码审查中怎么尽可能容易查看、把玩代码。

将提交额外的测试代码作为代码审查的一部分是可能是有价值的，但是同样也有可能是不必要的，例如：如果实验对审查者的问题提供满意的答案。

结论

进行代码审查有很多好处，不管你的团队采取什么方法来执行这个过程。应该尽可能的在代码合入代码库之前进行代码审查发现潜在问题，而这个时候相关背景还在开发者的头脑中，修复成本还不是很高。

作为代码审查者，你应该检查原始开发者已经通过自动化测试提供的他/她的代码的使用方法，出错的条件，边界条件的处理，尽可能的“记录”这些预期行为（正常使用情况和异常情况）。

如果代码审查者会去寻找测试代码的存在并检查其正确性，那么作为一个团队对代码正常运行应该有很高的信心。此外，如果这些测试常常在 CI 环境中正常运行，你可以看到代码在持续运行--它们提供自动化回归检查。如果代码审查者对正在审查的代码是否有高质量的测试代码高度重视，在审查者按下“Accept”按钮后，本次代码审查的意义还将长远存在。

关于本文

译者：@唐先僧

译文：http://www.jianshu.com/p/6f31657fe02c

作者：@Trisha Gee

原文：https://blog.jetbrains.com/upsource/2015/08/03/what-to-look-for-in-a-code-review-tests/