そのテストは本当に保証したいことを保証しているのか?
こんにちは。株式会社プラハCEOの松原です。
昨今は自動テストを書かないとサバンナのライオンにまで怒られるほど自動テストの文化が浸透していますが、時々こんなテストを見かけます:
フロントエンドのテスト
it('allows us to set props', () => {
const wrapper = mount(<Foo bar="baz" />);
expect(wrapper.props().bar).to.equal('baz');
wrapper.setProps({ bar: 'foo' });
expect(wrapper.props().bar).to.equal('foo');
});
(Enzymeの公式から拝借)
バックエンドのテスト
test('drinkEach drinks each drink', () => {
const drink = jest.fn();
drinkEach(drink, ['lemon', 'octopus']);
expect(drink).toHaveBeenCalledTimes(2);
});
(Jestの公式から拝借)
気になったこと
保証「していること」と保証「したい」ことがズレていないか?
例えば上述のフロントエンドのテストの場合「FooのsetPropsに{bar:'foo'}を渡したら、props().barした時に'foo'が返ってくる」ことを保証しているわけですが、それって本当に保証したいことでしょうか?
仮にFooがprops.barに渡された文字列を画面に赤字で描画するコンポーネントだったとします
もし実装ミスで緑色に表示されたり、そもそも文字が表示されなかったとしても、「props().barした時に'foo'が返ってくること」さえ満たしていれば問題なくテストが通過してしまいます
テストはそこに書かれたことを保証するだけなので、書かれていない挙動は一切保証しません。Fooコンポーネントにとって重要なのが「barを渡されたらそれが赤字で描画されること」だとしたら、「props().barしたら文字列が返ってくること」をテストしているだけでは、肝心の挙動が保証されていませんよね
もし「赤字が描画されること」を保証したいのであればFooが描画したDOM要素を取得してcomputedStyleをテストしたり、「文字が描画されること」を保証したいのであればFooが描画したDOM要素にテキストが含まれることをテストする必要があります
- 何をどこまで保証したいのか
- 書いたテストはそれを保証しているのか
この2点の整合性が取れていないと保証していることと保証したいことがズレてしまい、せっかく書いたテストの価値が薄れてしまうので注意が必要だな〜と感じました
(EnzymeのテストはあくまでEnzymeの使い方を示すのが目的なので、このテストが悪いと言いたいわけではない)
バックエンドのテストの場合
「drinkEach関数にdrink関数
と['lemon','octopus']
を引数として渡したら、drink関数
が2回呼ばれること」を保証しています。果たしてこれは本当に保証したいことでしょうか?
もしdrinkEach関数が「第一引数に渡された関数を、第二引数に渡された配列長の回数だけ呼び出す」という役割を持つ関数なのであれば、こちらは保証したいことと保証していることが一致していると言えるかもしれません(drink関数に'lemon'と'octopus'が引数として渡されていることを保証していないのは目を瞑ります)
正しくリファクタリングしたのに落ちるテスト?
Enzymeのテスト例とは少し違いますが、コンポーネントのstateを見て「state.bar=='foo'だったらOK」みたいなテストも時々見かけますが、これだと例えば「barだと意図がわかりづらいよね」と思って変数名を変えたら、Fooの仕様は一切変わらず今まで通り正常に赤字でpropsを描画していたとしても、テストが落ちてしまいます
本当は外部からみた振る舞いが変わらないことを保証したいのに内部仕様を保証するテストをガッツリ書いてしまうと単にリファクタリングしづらくなるだけなので、テストで保証したいこととテストが保証していることがズレていないことは気をつけなければいけません
内部仕様をテストするのが常に悪いわけではない
特にバックエンドの場合「hogeしたらメールが送信される」とか、複合的な機能をテストしたいケースが多々あります。テストの度に実際メールを飛ばしてたら大変なので「sendMailメソッドが呼ばれること」みたいな内部仕様をテストすることが多いですよね
でも仮にリファクタリングの結果sendMailメソッドを呼び出すのではなく「pushToMailQueue」メソッドを呼び出すようにメール送信の仕組みを変えた場合、外部から見た挙動としては変わっていなくても(メールが届いていても)、テストは落ちてしまいます
確かにテストは落ちるのですが、だからと言って「内部仕様をテストするのは常に悪!全部外部から見た挙動をテストしよう!」と考えるとめちゃくちゃ実行時間のかかるe2eテストしか書けなくなってしまいます。それだといざテストが落ちた時に原因を特定しづらいし、テスト自体も遅いし、ちょっと現実的ではありません。
なので最終的には
- 何をどこまで保証したいのか
- 書いたテストはそれを保証しているのか
この整合性に気をつけながら、様々な自動テストを特性に応じてバランスよく書いていくのが大事なんだなぁ。みつを
蛇足:フロントエンドで単体テストが少ない理由
フロントエンドはバックエンドと比較して単体テストが少ない気がします。理由は色々あるのですが、今回の「テストで保証したいことを考える」という文脈に絡めると、フロントエンドはバックエンドと比較して保証したいことが多いのが理由の1つとして挙げられるのではないでしょうか?
例えばバックエンドが「1+1したら2が返る」で済むのに対してフロントエンドは
- 1+1したら2が返る
- 2がブラウザに描画される
- 2が赤字で表示される
- 必要なフォントが読み込めない場合2がNoto Sansで表示される
- 2がポップアップなどに遮られずユーザーに見える形で表示される
- 2がアラビア語圏だと右寄せされてる
- 2がhoge...
- 2がfuga...
など様々なことを保証する必要があり、保証したいことを全部定義するのがしんどすぎるから、単体テストではなく統合テストやリグレッションテストにまとめたくなるのではないかな、と
あと単純にコンポーネント単体として保証したいことをしっかり書いても使われ方によって挙動が変わる可能性が高いから(css上書きされたり)、ちょっと努力の割に報われない気がしちゃいますよね...
Discussion