リーダブルなテストコードの書き方って記事についての雑感

テストの書き方を人と話していたとき

logmi.jp

の記事を権威付けのために出された。

読んでみてなんかウッとなったところがないでもないので、今後この手の記事を同じようなシチュエーションで出された時の自分の立場を明確にすべくこの機会に言語化しておく。 これは自分の考えとギャップがあるというだけで出典元が間違ってることを言ってるとか主張したいものではない。

ちなみに本記事でのテストはユニットテストを対象としている。

リーダブルとは

出典元にはリーダブルであることの十分条件として脳内メモリを消費しないことを挙げている。 ただ、脳内メモリを消費する場面って色々パターンがあると思っていて、例えばテスト対象の関数の責務やテストデータの妥当性などが例として挙げられる。 そもそもテストデータが作りづらいものやテストケースが長大になるような原因はそもそもテストコードではなく設計に問題があったりもするのでテストコードをリーダブルにしようってのは限界があると考える。そのため個人的にはリーダブルであることと脳内メモリの消費はあまり関係ない。

あとリーダブルは往々にして主観のぶつけ合いなのでもうちょっと定性的な定義をしておいた方がいいと思う。 自分としては以下の条件を満たすと一貫性のある"リーダブル"なテストコードだと感じる。

  • テストデータは宣言的、説明的な名称でつくる
  • テストダブルを使っていいのは原則として副作用をテストする場合のみ
  • real duplication 以外での再利用は行わない
  • 文脈の集まりがテストスイートである
  • expectation で行う検証は1つの事柄のみ
  • 原則として性質をテストする

ほかにもあるかもしれないけどぱっと思いつく重要なものはこんな感じ。

過度なDRYという表現について

「過度な」ものはすべて肯定できない状態のものだと思うので印象操作の感を受けたのであまり読んでいて気持ちよくなかった。 なんでも適度がいいに決まっているだろう。恣意的な表現は避けてほしかった。

ようわからんなら approve しちゃだめについて

完全に同意見。 テストコードはなめられがちなのであまり真剣にレビューされなかったりするのは本当によくない。 テストデータこそレビューする上で重要なはずなのに。

読みやすいコードのポイントについて

logmi.jp

にあるが、ドキュメントのように読めるというのはある程度考慮したほうがいいとは思うが無理があると思う。 出典元の著者は RSpec に明るいようだからこの意見が出るのは自然だなと思うし、うまく context をきることで自然言語としてテストケースを読むことはできるだろう。

しかしそれはツールに依存するもので例えば QuickCheck のようなフレームワークであれば上から下によむようなものではない。そのためポジショントークに見える。

つぎにスカラ値のハードコードというのがあるが、これは性質のテストが困難になるので同意できない。程度や管理頻度の問題もあるかもしれないが、基本的に適切な名前を付けたテストデータを使って「ある文脈」のテストであることを明示すべきだ。

賢くてロジカルなテストコードより誰でも読める愚直なテストコードをとの標語を掲げているが、複数の計算方法の結果が同じという検証方法もあるわけだし、そもそもテストなんてものはニンゲンのために書くのではなくシステムのために書くのであるのだからロジカルなのは当たり前じゃなかろうかとも思う。

誰でも読めることに振った結果品質の低いゴミを生み出していたら世話がないと思う。テストはそもそもそんなに安易なものじゃない。

実例について

さすがに例はそれなりに粗悪なものを出しているのでいいとして、これの修正案について思うところがある。 一番気になるのはハードコードした場合 age の算出に脳内メモリを使わなければならなくなっている。

自分が書くとしたらこのようにする

let(:birth_date) {fuzzer(:date)}  # ランダムに日付を生み出すファクトリを仮定
let(:user) {create(:user, birth_date: birth_date)}
let(:current_age) {fuzzer(:positive_int)}  # ランダムに正の整数を生み出すファクトリを仮定
def setup_current_date(day) # 指定の日数を足す
  passed_year = current_age.year
  travel_to(birth_date + passed_year + day.day)
end
def expect_age_is(user, delta)
  expect(user.age).to eq(current_age + delta)
end
def expect_age_no_change(user)
  expect_age_is(user, 0)
end
def expect_aged(user)
  expect_age_is(user, 1)
end
context "実行時点が誕生日"
  context "以前の場合" do
    before :example do
       setup_current_date(1)  # 定義域としては ∀x ∈Z( 0 < x < 当該年の日数) 
    end
    it "誕生日を過ぎた年齢を返すこと" do
      expect_aged(user)
    end 
  end
  context "当日の場合" do 
    before :example do
       setup_current_date(0)  # エッジケースなので1件のみ
    end
    it "誕生日を過ぎた年齢を返すこと"  # 仕様わからんけど
      expect_aged(user)
    end
  end
  context "以降の場合" do
    before :example do
       setup_current_date(-1)  # 定義域としては ∀x ∈N( -当該年の日数 < x < 0) 
    end
    it "現在の年齢を返すこと"  # real duplication なので shared_example 使える
      expect_age_no_change(user)
    end
  end
end

これが万人にとって脳内メモリを使わないリーダブルなコードだろうがというつもりは毛頭ない。が、ポリシーはある。 性質をランダムテストしているのでマジックナンバーが出てこない。また、expectation を説明的にしつつ事前条件の表明をすることで、文字列で記述しているテストケースの文脈がテストできていることになる。

この程度でも"過度なDRY"になるんだろうか。気になるところだ。

テストコードは仕様書と思いたい気持ちについて

わかるんだけど、ユニットテストレベルでは無理。E2Eテストでやれ。

E2Eテストの考え方について

これは基本的に統合テストになるため出典元の言うとおりテストの値はベタ書きでいいと思う。 テストの値というよりパターンと再現性のほうがよっぽど大事なのでそちらに腐心したい。

結局のところ

とはいえ、こんなものは人の好き好きだ。書き手にポリシーがあって、意図のわかるコードであればいいと思う。 大事なのはなぜそのテストデータで、なぜそのテストケースなのか(あるいはそうではないのか)が、それこそ誰が見ても理解できることではないだろうか。