代码审查应该关注什么:测试

上一篇文章中我们讨论了你在代码审查中应该关注的各种内容。现在我们关注一个领域:在测试代码中关注什么?

本文假设:

  • 你的团队已经为代码编写了自动化测试。
  • 这些测试有规律的运行在持续集成(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”按钮后,本次代码审查的意义还将长远存在。

本文译自What to look for in a Code Review: Tests

最后编辑于
©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 162,475评论 4 372
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 68,744评论 2 307
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 112,101评论 0 254
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 44,732评论 0 221
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 53,141评论 3 297
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 41,049评论 1 226
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 32,188评论 2 320
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 30,965评论 0 213
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 34,716评论 1 250
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 30,867评论 2 254
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 32,341评论 1 265
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 28,663评论 3 263
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 33,376评论 3 244
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 26,200评论 0 8
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 26,990评论 0 201
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 36,179评论 2 285
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 35,979评论 2 279

推荐阅读更多精彩内容

  • 1.测试与软件模型 软件开发生命周期模型指的是软件开发全过程、活动和任务的结构性框架。软件项目的开发包括:需求、设...
    Mr希灵阅读 21,840评论 7 277
  • 1.测试与软件模型 软件开发生命周期模型指的是软件开发全过程、活动和任务的结构性框架。软件项目的开发包括:需求、设...
    宇文臭臭阅读 6,667评论 5 100
  • Spring Cloud为开发人员提供了快速构建分布式系统中一些常见模式的工具(例如配置管理,服务发现,断路器,智...
    卡卡罗2017阅读 134,112评论 18 139
  • 文章来自:http://blog.csdn.net/mj813/article/details/52451355 ...
    好大一只鹏阅读 9,161评论 2 126
  • 一首歌,可以想起一个人;一碗面,亦能想起一座城。 一个人来到一座城跑到杭州逛了一天,肚子空空如也但没什么胃口。站在...
    寻叶亭阅读 273评论 1 1