谈一次单元测试驱动代码重构

目前团队并没有QA岗，而且在很长一段时间内，可能也不会设立QA岗，所以我们需要RD保证代码的质量。而鉴于人类天生的“惰性”，很多时候质量完全依赖于作者的能力以及职业素质。于是我在团队内推动单元测试，并要求提升测试覆盖率。虽然单元测试不能“根治”bug，但是它可以驱使代码结构简洁可测，为提升测试代码覆盖率奠定基础，从而可以有效降低bug率。（转载请指明出于breaksoftware的csdn博客）

以下我将以工作中一个实际例子讲解如何将一个不可测代码变成更加合理且可测代码。

class CheckLinkRequest:def execute(self):try:db = Db()app_links = db.query(AppLinks).filter(AppLinks.valid == True).all()LOG_DEBUG('app links data is {0}'.format(app_links))data_list = []if app_links:for _ in app_links:LOG_DEBUG('app links data is {0}'.format(_))user = db.query(AccountUser.email).filter(AccountUser.valid == True, AccountUser.id == _.user_id).all()LOG_DEBUG('user email is {0}'.format(user))data_list.append({"source": simplejson.dumps({'url': _.app_link, 'id': _.id, 'email': user[0][0]})})LOG_DEBUG('data list is {0}'.format(data_list))except Exception as e:LOG_ERROR('app link error {0}'.format(e))return JsonFuncResponser({'data': data_list})else:return JsonFuncResponser({'data': data_list})

这段代码大致意思是：

从AppLinks表中检索出所有有效数据（第5行）
遍历1中结果，查询每个信息对应的email(第11,12行）
将1中渠道的link信息和2中渠道的email信息组装成一条记录（第14,15行）

这段代码有好几个问题：

如果异常发生在第7行之前，执行到第19行时由于data_list未声明而被使用，将抛出异常
两处查询数据库可能产生的异常很不方便测试
第8行判断没有必要，而且造成一层嵌套。如果返回的数组，则可以进入异常处理；如果返回空数组，第21行也能正确处理。
第15行想当然的认为user是个二维数组，从而导致抛出异常

我们开始着手对这段代码进行改造。

依据“职责单一原则”，execute方法包含了太多功能，我们需要将其进行拆解重组：

class CheckLinkRequest:def __init__(self):self._db = Nonedef _init_db(self):if not self._db:self._db = Db()def _get_all_valid_applinks(self):self._init_db()return self._db.query_list(AppLinks.app_link, AppLinks.user_id, AppLinks.id).filter(AppLinks.valid == True).all()def _get_email_by_user_id(self, user_id):self._init_db()user = self._db.query_list(AccountUser.email).filter(AccountUser.valid == True, AccountUser.id == user_id).first()return user.emaildef _email_empty(self, user_id):LOG_WARNING("need to set email for user{0}".format(user_id))def _execute_with_exception(self):data_list = []app_links = self._get_all_valid_applinks()LOG_DEBUG('app links data is {0}'.format(app_links))for _ in app_links:LOG_DEBUG('app links data is {0}'.format(_))email = self._get_email_by_user_id(_.user_id)if not email:self._email_empty(_.user_id)LOG_DEBUG('user email is {0}'.format(email))data_list.append({"source": simplejson.dumps({'url': _.app_link, 'id': _.id, 'email': email})})return data_listdef execute(self):data_list = []try:data_list = self._execute_with_exception()except Exception as e:LOG_ERROR('app link error {0}'.format(e))return JsonFuncResponser({'data': data_list})

在原代码中Db对象是可被重用的，而修改后我们需要在不同成员函数中使用到它，所以将其提升成成员变量。

没有在构造函数中直接构造Db对象，是因为希望构造函数足够简单，只是进行一些数值型的构造，而不发生诸如“连接数据库”这类比较重的操作。

这样为了不频繁构建DB对象，我们设计了_init_db方法，同时在使用Db的地方都用其初始化一下。

我们修复了原代码中对user结构的“预设”隐患（直接取用了user[0[0]），同时也给我们暴露出“如果email为空该怎么办？”业务相关的问题。于是我们引入_email_empty方法来处理该业务性问题。

最后我们将execute封装出一个抛出异常的版本和无异常的版本。

经过改造后，代码结构变得清晰，execute函数职责也变得清晰。

分析这段代码，我们可以列出大致的测试点：

_get_all_valid_applinks/_get_email_by_user_id抛出异常
_get_all_valid_applinks/_get_email_by_user_id返回None
_get_all_valid_applinks返回空List
_get_all_valid_applinks返回的不是List

明确好这些测试点，我们开始编写单元测试代码

监测抛出异常

我们使用mock技术，在第9、10和21、22分别让，分别让执行_get_all_valid_applinks、_get_email_by_user_id时抛出异常

class TestCheckLinkRequest():def setup_class(self):passdef teardown_class(self):passdef test_get_all_valid_applinks_raise_exception(self, mocker):mocker.patch('basic_insights.check_link_request.CheckLinkRequest._get_all_valid_applinks', side_effect=Exception)t = CheckLinkRequest()with pytest.raises(Exception):t._execute_with_exception()r = t.execute()assert(r.is_same_data(JsonFuncResponser({'data': []})))def test_get_email_by_user_id_raise_exception(self, mocker):mocker.patch('basic_insights.check_link_request.CheckLinkRequest._get_email_by_user_id', side_effect=Exception)t = CheckLinkRequest()with pytest.raises(Exception):t._execute_with_exception()r = t.execute()assert(r.is_same_data(JsonFuncResponser({'data': []})))

然后在14、15和26、27行监测调用_execute_with_exception时会抛出异常。

最后17和29行执行无抛出异常版本的execute，并在之后判断返回值是否符合预期。

监测返回None

我们先看_get_all_valid_applinks在返回None时的单元测试。

    def test_get_all_valid_applinks_return_none(self, mocker):mocker.patch('basic_insights.check_link_request.CheckLinkRequest._get_all_valid_applinks', return_value=None)t = CheckLinkRequest()with pytest.raises(Exception):t._execute_with_exception()r = t.execute()assert(r.is_same_data(JsonFuncResponser({'data': []})))

我们在2、3行让_get_all_valid_applinks返回None。由于遍历None会抛出异常，所以7、8行将监测异常抛出。其他监测和之前相同。

_get_email_by_user_id返回None的话，它不会抛出异常，所以我们直接调用了_execute_with_exception而不期待其异常。由于email是空，将会触发_email_empty执行，于是我们在第5行mock了一下该对象的该函数，然后在第11行确定该函数被调用了。

    def test_get_email_by_user_id_return_none(self, mocker):mocker.patch('basic_insights.check_link_request.CheckLinkRequest._get_email_by_user_id', return_value=None)t = CheckLinkRequest()mocker_email_empty = mocker.patch.object(t, '_email_empty')t._execute_with_exception()r = t.execute()assert(False == r.is_same_data(JsonFuncResponser({'data': []})))assert(mocker_email_empty.called)

返回空List/dict

_get_all_valid_applinks返回空List或者dict，其返回值结果集也将是空。

    def test_get_all_valid_applinks_return_empty(self, mocker):mocker.patch('basic_insights.check_link_request.CheckLinkRequest._get_all_valid_applinks', return_value=[])t = CheckLinkRequest()t._execute_with_exception()r = t.execute()assert(r.is_same_data(JsonFuncResponser({'data': []})))def test_get_all_valid_applinks_return_obj(self, mocker):mocker.patch('basic_insights.check_link_request.CheckLinkRequest._get_all_valid_applinks', return_value={})t = CheckLinkRequest()t._execute_with_exception()r = t.execute()assert(r.is_same_data(JsonFuncResponser({'data': []})))

最后我们监测一个正常的情况

    def test_result(self, mocker):ret = [{'url': "www.1.com", 'id': 1, 'email': "1@1.com"},{'url': "www.2.com", 'id': 2, 'email': ""}]app_links = []for _ in ret:app_links.append(AppLinks(app_link = _["url"], user_id = _["id"], id = _["id"]))mocker.patch('basic_insights.check_link_request.CheckLinkRequest._get_all_valid_applinks', return_value=app_links)def mocker_get_email_by_user_id(id):emails = {1: "1@1.com"}if id in emails:return emails[id]else:return ""mocker.patch('basic_insights.check_link_request.CheckLinkRequest._get_email_by_user_id', wraps=mocker_get_email_by_user_id)t = CheckLinkRequest()mocker_email_empty = mocker.patch.object(t, '_email_empty')t._execute_with_exception()r = t.execute()r_list = []for _ in r.json()['data']:r_list.append(simplejson.loads(_["source"]))assert(r_list == ret)assert(mocker_email_empty.call_count == 2)

这段代码我们使用mocker_get_email_by_user_id替换了CheckLinkRequest的_get_email_by_user_id，从而我们可以干涉其内部执行。这也是一种非常常用的设计。