なぜ優秀な開発者はダメな単体テストを書いてしまうのか
原文は Michael Lynch により に公開されました。 このブログを購読する
おめでとうございます！ あなたはついに大量のコードを書き上げ、ビーチハウスを買えるほどのお金を手に入れました。あなたは超高層ビルで世界的に有名な建築家、ピーター・キーティングを雇います。彼はあなたの海辺の土地に素晴らしい計画があると請け合います。
数ヶ月後、あなたは盛大なお披露目にやってきます。目の前に現れた新しい家は、鉄とコンクリートと反射ガラスでできた、威圧的な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行も必要です。このレベルになると、定型的なコードが多すぎて、テストしたい挙動から注意が逸れてしまいます。
つまらないコードをすべてテスト用のヘルパーメソッドに委譲したくなるのが自然な発想かもしれませんが、まずもっと重要な問いを自問すべきです。なぜこのシステムはテストがこんなにも困難なのか、と。
過剰なボイラープレートコードは、しばしば貧弱なアーキテクチャの症状です。例えば、上記のテストはいくつかの設計上の異臭（design smells）を露わにしています。
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()メソッドで何かをやらかしたことを教えてくれますが、公開メソッドが1つしかないクラスではそれは無意味です。失敗を診断するには、テストの実装を読まなければなりません。
では、代わりに次のような表示が出たらどうでしょうか。
[ 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
コメント
ログインしてコメントする