プライベートメソッドではテストコードはかかないの?

はじめに

みなさんはプライベートメソッドに対してテストコードを書いていますか？

僕は実装中に、当たり前のようにプライベートメソッドに対してもテストを書いていたのですが、
以前、レビュワーの方から「このプライベートメソッドのテスト、本当に必要か」といったレビューをいただきました。

最初は「内部ロジックなんだからテストして当然では？」と思ったのですが、調べてみると
「プライベートメソッドは原則テストしない」 という考え方が一般的だと知りました。

知識の定着のために、備忘録として残します。

原則

結論からいきましょう。

プライベートメソッドに対して直接テストは書かない。パブリックメソッド経由で振る舞いをテストする。

これがどうやら原則のようです。
ただ、なぜ書かないのか、これだけだとイメージしづらいと思うので
理由を3つに分けて紹介していきます。

1. プライベートメソッドは「実装の詳細」だから

クラスの外から呼ばれるのはパブリックメソッドだけです。
プライベートメソッドは、あくまでパブリックメソッドの責務を達成するための 内部的な手段 にすぎません。

テストが検証すべきは 「クラスの振る舞い(外から見える挙動)」 であって、内部的な手段ではありません。
プライベートメソッドを直接テストするということは、実装の詳細にテストを縛り付けてしまう行為になります。

2. リファクタリング耐性が下がる

プライベートメソッドに対してテストを書くと、内部実装を変えるたびにテストが壊れます。

例えば、プライベートメソッドをリネームしたり、別のメソッドに分割したり、別クラスに切り出したり
した瞬間にテストが赤くなる...。
これでは 「振る舞いは変わっていないのにテストが壊れる」 状態になり、リファクタリングのブレーキになってしまいます。

良いテストとは、 実装を変えても振る舞いが変わらない限り壊れないテスト です。

3. パブリックメソッド経由で網羅できるはず

そもそもそのプライベートメソッドは、どこかのパブリックメソッドから呼ばれているはずですよね。
つまり、パブリックメソッドのテストを十分に書けば、プライベートメソッドのロジックも自然に通過します。

逆に、「パブリックメソッド経由ではどうしてもカバーできないプライベートメソッド」がある場合
それは そのプライベートメソッドが使われていない(=デッドコード) か、
設計が間違っている サインだったりします。

具体的なコード

さて、実際のコードを見ながら考えていきましょう。
今回は、割引価格を計算する Order クラスを例にします。

class Order
  def initialize(price:, user:)
    @price = price
    @user = user
  end

  def total
    @price - discount
  end

  private

  def discount
    return 0 unless @user.premium?
    @price * 0.1
  end
end

Order クラスは、ユーザーがプレミアム会員かどうかによって、合計金額を計算してくれるクラスです。

Bad: プライベートメソッドを直接テストする

まずは、Badな例から見ていきましょう。

discount を send で直接呼び出してテストする、典型的なNGパターンです。

RSpec.describe Order do
  describe '#discount (private)' do
    it 'returns a 10% discount for a premium user' do
      user = double(premium?: true)
      order = Order.new(price: 1000, user: user)
      expect(order.send(:discount)).to eq 100
    end

    it 'returns 0 for a regular user' do
      user = double(premium?: false)
      order = Order.new(price: 1000, user: user)
      expect(order.send(:discount)).to eq 0
    end
  end
end

一見、ちゃんと動いているテストに見えますが...
このテストには以下のような問題があります。

• discount というメソッド名や計算ロジックの実装にテストが縛られている
• 例えば discount を premium_discount にリネームしただけでテストが壊れる
• Order#total の中で割引計算のフローを変更すると壊れる

リファクタリング耐性が著しく低いテストになってしまっていますね。

Good: パブリックメソッド経由で振る舞いをテストする

さて、Goodな例も見ていきましょう。

外から見えるのは #total だけなので、#total の振る舞いをテストすればOKです。

RSpec.describe Order do
  describe '#total' do
    let(:order) { Order.new(price: 1000, user: user) }

    context 'when the user is a premium member' do
      let(:user) { double(premium?: true) }

      it 'returns the price with a 10% discount applied' do
        expect(order.total).to eq 900
      end
    end

    context 'when the user is a regular member' do
      let(:user) { double(premium?: false) }

      it 'returns the original price without any discount' do
        expect(order.total).to eq 1000
      end
    end
  end
end

このテストの良いところはこんな感じ。

• 「プレミアム会員なら割引される」という振る舞いそのもの をテストしている
• discount というメソッドが存在することにも、その実装にも依存していない
• discount をリネームしようが、計算式をまるごと書き換えようが、振る舞いが同じなら壊れない

実装の詳細から切り離されているので、リファクタリング時にテストが壊れにくくなります！

Better: テストしたくなったらクラスを分ける

「とはいえ、割引ロジックが複雑になってきたら、単体でテストしたくない？」
と思う方もいるかもしれません。

そういう時は、無理に Order クラスの中にとどめず、責務ごとクラスを切り出してしまいましょう。

class DiscountCalculator
  def initialize(price:, user:)
    @price = price
    @user = user
  end

  def call
    return 0 unless @user.premium?
    @price * 0.1
  end
end

class Order
  def initialize(price:, user:)
    @price = price
    @user = user
  end

  def total
    @price - DiscountCalculator.new(price: @price, user: @user).call
  end
end

こうなれば、DiscountCalculator#call はパブリックメソッドなので堂々とテストできます。

RSpec.describe DiscountCalculator do
  describe '#call' do
    context 'when the user is a premium member' do
      it 'returns 10% of the price' do
        user = double(premium?: true)
        expect(DiscountCalculator.new(price: 1000, user: user).call).to eq 100
      end
    end

    context 'when the user is a regular member' do
      it 'returns 0' do
        user = double(premium?: false)
        expect(DiscountCalculator.new(price: 1000, user: user).call).to eq 0
      end
    end
  end
end

Order 側のテストでは「割引が適用されること」だけを確認し、
割引ロジックの細部は DiscountCalculator 側のテストに寄せる、というキレイな分担になります！

つまり、 「テストしたい = パブリックにする価値がある」 と捉えると
設計改善のきっかけにもなります。

終わりに

いかがでしたでしょうか。
レビューでもらった一言から、「テストは振る舞いに対して書くもので、実装の詳細に対して書くものではない」 という 原則がはっきりしました。

簡単にまとめておきます。

• プライベートメソッドは直接テストしないのが原則
• パブリックメソッド経由で振る舞いをテストすれば、内部ロジックも自然と検証される
• どうしても単独でテストしたくなったら、それはクラスを切り出すサイン
• 切り出した先ではパブリックメソッドになるので、堂々とテストできる

「テストが書きづらい」と感じたときは、テストの書き方を工夫するよりも先に
設計を見直す ことが、結果的にメンテナンスしやすいコードにつながりますね🔥

同じようにレビューで指摘を受けた方の参考になれば幸いです。

最後まで読んで頂きありがとうございました！