なぜ優秀な開発者はダメな単体テストを書いてしまうのか
おめでとうございます！あなたはついに大量のコードを書き上げ、海辺に家を買えるほどの成功を収めました。そこであなたは、超高層ビルで世界的に有名な建築家、ピーター・キーティング（Peter Keating）に依頼します。彼は海辺の土地にぴったりの素晴らしいプランがあると請け合います。
数か月後、いよいよお披露目の日を迎えます。目の前に現れた新居は、鉄とコンクリートと反射ガラスでできた、威圧感のある5階建ての巨大建築物です。回転ドアを通って中に入ると、砂まみれの足跡が高級な大理石の床に残ります。中には受付カウンターがあり、その奥にはエレベーターホールが並んでいます。2階に上がると、主寝室と3つのゲストルームは、ただ4つ並んだオフィスの区画に過ぎませんでした。
建築の大家であるピーター・キーティングは、あなたががっかりしている理由が理解できません。「私はすべてのベストプラクティスを守りました」と彼は弁解するように言います。構造の堅牢性は何よりも重要なので、壁の厚さは3フィート（約90センチ）もあります。だからあなたの家は、風通しがよく光に満ちた隣家よりも優れているというのです。海を望む大きな窓はないかもしれませんが、キーティングによれば、そんな窓はベストプラクティスではないそうです。エネルギー効率を下げ、オフィスで働く人の集中を妨げるからだと言います。
ソフトウェア開発者が単体テストに取り組むとき、同じように誤った考え方をしてしまうことがあまりにもよくあります。プロダクションコードで学んだ「ルール」を、それがテストに本当に適しているかを吟味することなく機械的に当てはめてしまうのです。その結果、海辺に超高層ビルを建ててしまうのです。
テストコードは他のコードとは違う
プロダクションのアプリケーションは、通常、数千から数百万行ものコードで構成されています。人間が一度に全体を把握するには大きすぎます。その複雑さに対処するために、言語設計者は関数やクラス階層といった、抽象化して考えられる仕組みを用意してきました。
良いプロダクションコードはカプセル化を実現しています。読む人が大規模なシステムの中を自在に行き来できるようにし、必要に応じて詳細に潜ったり、より高い抽象度へ上がったりできるのです。
しかしテストコードは別物です。良い単体テストは、開発者がそのロジック全体を一度に頭の中で捉えられるくらい小さなものであることがほとんどです。テストコードに抽象化の層を重ねることは、むしろ複雑さを増してしまいます。テストは診断のための道具なのですから、できる限りシンプルで一目でわかるものであるべきです。
定規を思い浮かべてみてください。何百年も同じ形のままであり続けているのは、シンプルで解釈が容易だからです。仮に私が「抽象定規単位」で測る新しい定規を発明したとしましょう。「定規単位」をインチやセンチに変換するには、別の換算表が必要になります。
そんな定規を大工に渡したら、顔をその定規でひっぱたかれるでしょう。明確で曖昧さのない情報を与えてくれる道具に、わざわざ抽象化の層を加えるのは馬鹿げているからです。
良いテストコードも同じです。読む人に何重もの間接参照を強いることなく、明確な結果を示すべきです。開発者がこの点を見失いがちなのは、プロダクションコードの書き方として学んできたことと異なるからです。
優秀な開発者が書くダメなテスト
それ以外では優秀な開発者が、次のようなテストを書いているのをよく見かけます。
def test_initial_score(self):
initial_score = self.account_manager.get_score(username='joe123')
self.assertEqual(150.0, initial_score)このテストは何をしているのでしょうか。joe123という名前のユーザーの「スコア」を取得し、それが150であることを検証しています。ここで、次のような疑問が浮かぶはずです。
- joe123というアカウントはどこから来たのか。
- なぜjoe123のスコアが150であるはずだと考えられるのか。
答えはsetUpメソッドにあるのかもしれません。これはテストフレームワークが各テスト関数の実行前に呼び出すメソッドです。
def setUp(self):
database = MockDatabase()
database.add_row({
'username': 'joe123',
'score': 150.0
})
self.account_manager = AccountManager(database)なるほど、setUpメソッドでスコア150のjoe123ユーザーが作られていたから、test_initial_scoreがその値を期待していたのですね。これで万事解決でしょうか。
いいえ、これはダメなテストです。
読む人をテスト関数の中に留める
テストを書くときは、次にそのテストの失敗を目にする開発者のことを考えてみてください。その人はあなたのテストスイート全体を読みたいとは思っていませんし、ましてやテストユーティリティの継承ツリーを全部追いたいなどとはまったく思っていません。
テストが失敗したとき、読む人はテスト関数を上から下へ一直線に読むだけで問題を診断できるべきです。テストの外に飛び出して補助的なコードを読まなければならないとしたら、そのテストは役割を果たせていません。
これを踏まえて、前節のテストを書き直してみます。
def test_initial_score(self):
database = MockDatabase()
database.add_row({
'username': 'joe123',
'score': 150.0
})
account_manager = AccountManager(database)
initial_score = account_manager.get_score(username='joe123')
self.assertEqual(150.0, initial_score)やったことはsetUpメソッドのコードをインライン化しただけですが、これで大きな違いが生まれます。読む人が必要とする情報が、すべてテストの中に収まりました。また、この書き方はarrange、act、assertという構造にも沿っており、テストの各段階が明確でわかりやすくなっています。
あえてDRYに違反する
セットアップのコードをインライン化するのは、テストが一つだけならそれでよいのですが、テストがたくさんある場合はどうなるでしょうか。そのたびに同じコードを重複させることにならないでしょうか。心の準備をしてください。これから私はコピー＆ペーストプログラミングを推奨します。
同じクラスに対する別のテストがこちらです。
def test_increase_score(self):
database = MockDatabase() # <
database.add_row({ # <
'username': 'joe123', # <--- Copy/pasted from
'score': 150.0 # <--- previous test
}) # <
account_manager = AccountManager(database) # <
account_manager.adjust_score(username='joe123',
adjustment=25.0)
self.assertEqual(175.0,
account_manager.get_score(username='joe123'))DRY原則（「繰り返すな」）の厳格な信奉者にとって、上記のコードは恐ろしいものに映るでしょう。私はあからさまに自分を繰り返しています。前のテストから6行をそのままコピーしたのです。さらに悪いことに、重複を排除したテストよりも、このDRY違反のテストの方が優れていると主張しているのです。一体どういうことでしょうか。
重複なしで明確なテストを実現できるなら、それが理想です。しかし、重複のないコードは目的ではなく手段であることを忘れないでください。最終的なゴールは、明確でシンプルなテストなのです。
テストにDRYをむやみに適用する前に、テストが失敗したときに何が問題を明白にするかを考えてみてください。リファクタリングは重複を減らせるかもしれませんが、複雑さを増し、失敗したときに情報を覆い隠してしまう可能性もあるのです。
ヘルパーメソッドを追加する前によく考える
各テストで6行をコピー＆ペーストするくらいなら我慢できるかもしれませんが、もしAccountManagerがもっと多くのセットアップコードを必要とするとしたらどうでしょうか。
def test_increase_score(self):
# vvvvvvvvvvvvvvvvvvvvv Beginning of boilerplate code vvvvvvvvvvvvvvvvvvvvv
user_database = MockDatabase()
user_database.add_row({
'username': 'joe123',
'score': 150.0
})
privilege_database = MockDatabase()
privilege_database.add_row({
'privilege': 'upvote',
'minimum_score': 200.0
})
privilege_manager = PrivilegeManager(privilege_database)
url_downloader = UrlDownloader()
account_manager = AccountManager(user_database,
privilege_manager,
url_downloader)
# ^^^^^^^^^^^^^^^^^^^^^ End of boilerplate code ^^^^^^^^^^^^^^^^^^^^^^^^^^^
account_manager.adjust_score(username='joe123',
adjustment=25.0)
self.assertEqual(175.0,
account_manager.get_score(username='joe123'))AccountManagerのインスタンスを取得してテストを始めるだけで15行も必要です。これだけ定型コードが多いと、テストしたい振る舞いから注意が逸れてしまいます。
面白みのないコードをすべてテスト用のヘルパーメソッドに押しやりたくなるのが自然な発想かもしれませんが、まずはより本質的な問いを立てるべきです。なぜこのシステムはこんなにテストしづらいのか、と。
過剰な定型コードは、しばしば脆弱なアーキテクチャの症状です。例えば、上記のテストからは、いくつかの設計の異臭が漂ってきます。
account_manager = AccountManager(user_database,
privilege_manager,
url_downloader)AccountManagerはuser_databaseに直接アクセスしているのに、次のパラメータはprivilege_databaseのラッパーであるprivilege_managerです。なぜ2つの異なる抽象度で動作しているのでしょうか。そして「URLダウンローダー」を何に使っているのでしょうか。これは他の2つのパラメータとは概念的にかなりかけ離れているように見えます。
このケースでは、AccountManagerをリファクタリングすることが根本的な解決になります。ヘルパーメソッドを追加しても、症状を覆い隠すだけだからです。
ヘルパーメソッドが必要なら、責任を持って書く
テストしやすくするためにプロダクションクラスを大胆に解体する自由が常にあるわけではありません。ヘルパーメソッドしか選択肢がないこともあります。だからこそ、必要なときは上手に書きましょう。
効果的なヘルパーメソッドとは、「読む人をテスト関数の中に留める」という原則を支えるものです。読む人のテストへの理解を損なわない限り、定型コードをヘルパー関数に抽出しても構いません。
具体的には、ヘルパーメソッドは次のことをしてはいけません。
- 重要な値を隠す
- テスト対象のオブジェクトを操作する
これらのガイドラインに違反するヘルパーメソッドの例がこちらです。
def add_dummy_account(self): # <- Helper method
dummy_account = Account(username='joe123',
name='Joe Bloggs',
email='[email protected]',
score=150.0)
# BAD: Helper method hides a call to the object under test
self.account_manager.add_account(dummy_account)
def test_increase_score(self):
self.account_manager = AccountManager()
self.add_dummy_account()
account_manager.adjust_score(username='joe123',
adjustment=25.0)
self.assertEqual(175.0, # BAD: Relies on value set in helper method
account_manager.get_score(username='joe123'))読む人は、ヘルパーメソッドの中に隠された150を探し出さない限り、なぜ最終的なスコアが175になるはずなのか理解できません。また、このヘルパーはadd_accountの呼び出しを隠してしまっているため、すべてのやり取りをテスト関数の中に留めておくという原則に反し、account_managerの振る舞いを不明瞭にしています。
これらの問題を解消した書き直しがこちらです。
def make_dummy_account(self, username, score):
return Account(username=username,
name='Dummy User', # <- OK: Buries values but they're
email='[email protected]', # <- irrelevant to the test
score=score)
def test_increase_score(self):
account_manager = AccountManager()
account_manager.add_account(
make_dummy_account(
username='joe123', # <- GOOD: Relevant values stay
score=150.0)) # <- in the test
account_manager.adjust_score(username='joe123',
adjustment=25.0)
self.assertEqual(175.0,
account_manager.get_score(username='joe123'))この書き方でもヘルパーメソッドの中に値は隠れていますが、それらはテストにとって重要ではない値です。また、add_accountの呼び出しをテストの中に戻したことで、読む人はaccount_managerに何が起こるかを簡単に追えるようになっています。
テスト名は思い切り長くする
次のうち、プロダクションコードではどちらの関数名を見たいと思うでしょうか。
userExistsAndTheirAccountIsInGoodStandingWithAllBillsPaidisAccountActive
前者はより多くの情報を伝えますが、57文字もの名前に伴う負担を強います。ほとんどの開発者は、isAccountActiveのような簡潔でほぼ同等に良い名前のために、多少の正確さを犠牲にすることをいとわないでしょう（Java開発者を除けば、彼らにとってはどちらの名前も侮辱的なほど簡潔すぎます）。
テスト関数については、この構図を変える決定的な要因があります。テスト関数を呼び出すコードを書くことは決してないということです。開発者がテスト名を打ち込むのは、関数シグネチャの中の一度きりです。そう考えると、簡潔さは依然として重要ですが、プロダクションコードほど重要ではありません。
テストが失敗したとき、最初に目にするのはテスト名です。だからこそ、できる限り多くのことを伝えるべきです。例えば、次のようなプロダクションクラスを考えてみましょう。
class Tokenizer {
public:
Tokenizer(std::unique_ptr<TextStream> stream);
std::unique_ptr<Token> NextToken();
private:
std::unique_ptr<TextStream> stream_;
};テストスイートを実行したところ、出力に次のような一行が現れたとします。
[ FAILED ] TokenizerTests.TestNextToken (6 ms)何が原因でテストが失敗したかわかるでしょうか。おそらくわからないでしょう。
TestNextTokenの失敗はNextToken()メソッドで何かをやらかしたことを教えてくれますが、公開メソッドが一つしかないクラスでは、それは無意味な情報です。失敗を診断するには、テストの実装を読まなければなりません。
では、代わりにこんな表示が出たらどうでしょうか。
[ FAILED ] TokenizerTests.ReturnsNullptrWhenStreamIsEmpty (6 ms)ReturnsNullptrWhenStreamIsEmptyという関数は、他の文脈では冗長すぎると感じられるでしょうが、テスト名としては優れています。もしこれが失敗したのを見れば、クラスが空のデータストリームを適切に扱えていないことがすぐにわかります。テストの実装を読むことなくバグを修正できるかもしれません。それこそが良いテスト名の証です。
マジックナンバーを受け入れる
「マジックナンバーを使うな」
これはプログラミングの世界における「知らない人についていってはいけません」のようなものです。多くの優秀な開発者がこの教えをあまりにも深く内面化しているため、マジックナンバーがコードを良くする場合があることを考えもしません。
おさらいですが、「マジックナンバー」とは、それが何を表しているかの情報なしにコード中に現れる数値や文字列のことです。例えば次のようなものです。
calculate_pay(80) # <-- Magic numberプログラマーは、プロダクションコードにおけるマジックナンバーは非常に良くないものだという点で一致しており、次のように名前付き定数に置き換えます。
HOURS_PER_WEEK = 40
WEEKS_PER_PAY_PERIOD = 2
calculate_pay(hours=HOURS_PER_WEEK * WEEKS_PER_PAY_PERIOD)残念ながら、マジックナンバーはテストコードにおいても害になるという誤解がありますが、実際はその逆です。
次のテストを考えてみてください。
def test_add_hours(self):
TEST_STARTING_HOURS = 72.0
TEST_HOURS_INCREASE = 8.0
hours_tracker = BillableHoursTracker(initial_hours=TEST_STARTING_HOURS)
hours_tracker.add_hours(TEST_HOURS_INCREASE)
expected_billable_hours = TEST_STARTING_HOURS + TEST_HOURS_INCREASE
self.assertEqual(expected_billable_hours, hours_tracker.billable_hours())マジックナンバーは普遍的に悪だと信じているなら、上記のテストは正しく見えるでしょう。72.0と8.0は名前付き定数になっているので、誰もそのテストをマジックナンバーだと非難できません。
しかし、ほんのひととき、そうした信念を脇に置いて、禁断の果実であるマジックナンバーを味わってみてください。
def test_add_hours(self):
hours_tracker = BillableHoursTracker(initial_hours=72.0)
hours_tracker.add_hours(8.0)
self.assertEqual(80.0, hours_tracker.billable_hours())コードの行数は半分になり、よりシンプルです。そしてより明白でもあります。読む人は関数の中で定数の名前をあちこち追いかける必要がなくなるのです。
開発者がテストコードで定数を定義しているのを見かけるとき、それは大抵、DRYへの誤った固執か、マジックナンバーを使うことへの恐れが原因です。しかし、テストで定数を宣言する必要はめったになく、そうすることはむしろ理解を難しくします。
おわりに
優れたテストを書くために、開発者はエンジニアリング上の判断をテストコードの目的に合わせなければなりません。何よりも、テストは抽象化を最小限に抑えつつ、シンプルさを最大化すべきです。良いテストとは、読む人がテスト関数から離れることなく、意図された振る舞いを理解し、問題を診断できるものなのです。
カバーアート： Loraine Yow