「僕がRSpecでsubjectを使わない理由」に対する反論のようななにか

伊藤淳一(Qiitaアカウント @jnchito)さんのブログで
「僕がRSpecでsubjectを使わない理由」という記事があります
（以降この記事を「対象記事」と呼びます）。

私は積極的にsubjectを使う派なので対象記事に対して思うところがあり、対象記事が執筆されてから1年以上たっていますが、

• 伊藤淳一さんという有名な方が書いた内容である
• 「rspec subjct」でググると1ページ目に対象記事が出てくる

という理由から今からでも対象記事について意見を述べることに一定の価値があるのではないかと思い、筆をとりました。

対象記事の主張に対して一つずつ反論というか意見というか何かしらのコメントをしていきますのでお付き合いいただけると嬉しいです。

（ちなみに対象記事に対して直接コメントを書くこともできますが、なんかHatena Blogのコメント機能が長文を書くのに不向きで、コードも書きにくそうだったのでQiitaの記事という形を取りました。）

以下、コードが出てきますが、
「私が書くとしたら」などと書かれていない「ラベルに「改」」などと書かれていない、引用元が明記されていないものは対象記事からの引用になります。
また、引用する際に説明の都合上こちらで勝手にラベル（「コード1」など）を付けさせていただきました。
ご了承ください。

伊藤淳一さんの主張と私の主張

伊藤淳一さんの主張（と注意点）

対象記事で最初に述べられていますが要約すると以下のようになるかと思います。

• subjectを使うといびつなテストコードができやすい。そのため基本はsubjectなしで書く。明らかにsubjectが有効なときだけsubjectを使う。

ここで伊藤淳一さんは「私はsubjectを滅多に書かない」「subjectを使わないことを推奨する」と言っているだけで「すべてのsubjectは駆逐すべし」などとは全く言っていないことに注意を払う必要があります。
ブログというのは時として著者の意図と反してタイトルや大雑把な主旨だけが（ときにはそれすら歪められて）広まってしまうことがあります。
対象記事を読んで（著者の意志とは無関係に）「すべてのsubjectは駆逐すべし。伊藤淳一もそう言っている。」という輩が現れてもおかしくありません。
当記事（私が今書いているこの文章のこと）ではそのような輩に対する内容だったりします。

私の主張

• subjectは（無理のない範囲で）積極的に書くべき。subjectを書くことには可読性の面でメリットがある。subjectを無理やり書くことで可読性が下がることもあるので、その時にsubjectを使うのはもちろんNG。
  • is_expected というsubjectが省略できる記法はただのrspecのおまけ機能であり、使う/使わないは自由（subjectを書きたい理由は他にありis_expected使いたいからではない）。is_expectedを使うためにsubjectが分かりにくくなるのは当然NG。その場合is_expectedは使ってはいけない。

対象記事に対して

「subjectが向いていないケース = メソッドの副作用をテストするケース」について

対象記事では以下のコードは不自然なコードであり、デメリットが有ると主張しています。

コード1

class Counter
  attr_reader :count

  def initialize
    @count = 0
  end

  def increment
    @count += 1
  end
end

describe 'Counter#increment' do
  let(:counter) { Counter.new }
  subject { ->{ counter.increment } }
  it { is_expected.to change(counter, :count).by(1) }
end

これに対しては全く同意です。「コード1」は不自然なコードあり可読性が良いとは思えません。

続いて以下のコードは何も問題を感じない、言っています。

コード2

describe 'Counter#increment' do
  let(:counter) { Counter.new }
  it 'increments count' do
    expect { counter.increment }.to change(counter, :count).by(1)
  end
end

ですが「コード2」に関してはcounter.incrementをsubjectに変えれば、subjectを使っていてかつ不自然ではないコードになるのではないでしょうか？

コード2改

describe 'Counter#increment' do
  let(:counter) { Counter.new }
  subject { counter.increment }
  it 'increments count' do
    expect { subject }.to change(counter, :count).by(1)
  end
end

ではなぜ「コード1」は不自然なコードだったのでしょう？
それはis_expectedを無理に使っていることに問題があります。is_expectedを使いたいがためにsubjectにラムダを使ってしまったのが問題なのです。

つまりこれは 「subjectを使ったときの問題」ではなく「is_expectedを使ったときの問題」であり、議論がすり替わっている と主張したいです。

ただし、「コード2改」には問題点があります。
それは「counter.incrementを2回実行してcountが2増えることを確認するテストはどうかけばいいの」ということです。

これはsubjectを使わない場合は簡単に書くことができます。

コード3

describe 'Counter#increment' do
  let(:counter) { Counter.new }
  it 'increments count' do
    counter.increment
    counter.increment
    expect(counter.count).to eq 2
  end
end

これをsubjectを使って書くことは難しいです。counter.incrementをsubjectに置換してもテストは通りません。subjectはit内で1回しか実行されないためです。

無理にsubjectを使おうとすると不自然なコードになります。

コード3改（不自然なコード）

describe 'Counter#increment' do
  let(:counter) { Counter.new }
  subject { 2.times { counter.increment } }
  it 'increments count' do
    is_expected.to change(counter, :count).by(2)
  end
end

この場合は素直にsubjectなしで書くべきでしょう。

正直、副作用をテストするケースではsubjectは向いていないです。（これは伊藤淳一さんの主張と私の考えは一致していると思います。）

私が言いたいのは

• 対象記事の内容だけでは「議論のすり替え」起こっておりsubjectを使わないほうが良い理由になっていない
• 「メソッドの副作用をテストするケースではsubjectが向いていないことを主張したかったら「counter.incrementを2回実行したとき」などのケースについて言及すべき

ということです。

「もうひとつの問題：subjectを使うと視線を上下させなければならない」について

以下のコードは「このsubjectっていったい何だっけ？」と思ったときに、視線を上下させなければならなず、可読性が低いと主張されています。

コード4

describe 'Foo#bar' do
  let(:foo) { Foo.new }
  subject { foo.bar }
  context 'when A' do
    before do
      # 何行にも渡ってセットアップが続く
      # .
      # .
      # .
    end
    it { is_expected.to eq 'abc' }
  end
  context 'when B' do
    before do
      # 何行にも渡ってセットアップが続く
      # .
      # .
      # .
    end
    it { is_expected.to eq 'def' }
  end
  context 'when C' do
    before do
      # 何行にも渡ってセットアップが続く
      # .
      # .
      # .
    end
    it { is_expected.to eq 'xyz' }
  end
end

この主張については私には理解できませんでした。
contextを読むときには普通、テスト対象がなんなのかは頭に入っている状態で読むのではないでしょうか。テスト対象がなんなのかが頭に入っていない状態でcontextの中を読む、というシチュエーションがちょっと想像できませんでした。

このサンプルコードではテスト対象が引数を持たないメソッドだったのでなおさらですが、引数があった場合も同様だと思います。メソッド名と引数名くらいは頭の中に入った状態でしかcontextの中を読むことはないと思っています。

私は「describeの直下にsubjectを書いて、describeのテキストとsubjectの内容が一致していることを一瞬で確認できるようにするべき」と思っているので引数があった場合は私なら以下のように書きます。

コード4

describe 'Foo#bar' do
  let(:foo) { Foo.new }
  subject { foo.bar(num1: num1) }
  context 'num1が0のとき' do
    let(:num1) { 0 }

    before do
      # 何行にも渡ってセットアップが続く
      # .
      # .
      # .
    end
    it { is_expected.to eq 'abc' }
  end
  context 'num1が1のとき' do
    let(:num1) { 1 }

    before do
      # 何行にも渡ってセットアップが続く
      # .
      # .
      # .
    end
    it { is_expected.to eq 'def' }
  end
  context 'num1が2以上のとき（例えば5）' do
    let(:num1) { 5 }

    before do
      # 何行にも渡ってセットアップが続く
      # .
      # .
      # .
    end
    it { is_expected.to eq 'xyz' }
  end
end

これならitを読んでいて「このcontextの内容なんだっけ？」となってもcontextブロックの先頭を見れば良いだけなので視線の上下は少なくなります。テキストエディタをスクロールさせないといけないような場合は少ないのではないでしょうか（ないとは言っていない）。

「コード4」を読んで「ちょっと待て、このサンプルはおかしい。Foo#barにオプショナル引数があって、それのあり/なしをテストしたかったらどうすればいいんだ？」と思った方。その方は鋭い。
正直それを言われると現状ぐぅの音も出ません。言い訳がましいサンプルコードを置いてお茶を濁そうと思います。

コード4

describe 'Foo#bar' do
  let(:foo) { Foo.new }

  context 'num2が省略された場合' do
    subject { foo.bar(num1: num1) }

    context 'num1が0のとき'
    context 'num1が1のとき'
    context 'num1が2以上のとき（例えば5）'
  end

  context 'num2が与えられた場合' do
    subject { foo.bar(num1: num1, num2: num2) }

    context 'num1が0のとき' do
      context 'num2が0のとき'
      context 'num2が1のとき'
      context 'num2が2以上のとき（例えば9）'
    end
    context 'num1が1のとき' do
      context 'num2が0のとき'
      context 'num2が1のとき'
      context 'num2が2以上のとき（例えば9）'
    end
    context 'num1が2以上のとき（例えば5）' do
      context 'num2が0のとき'
      context 'num2が1のとき'
      context 'num2が2以上のとき（例えば9）'
    end
  end

「describeの直下にsubjectを書いて、describeのテキストとsubjectの内容が一致していることを一瞬で確認できるようにするべき」とかいう主張はどこいったんだ？って感じですが許してください。

というか、これこそがsubjectを使うデメリットなのかもしれません。私の主張は危うい。

さて、もとの伊藤淳一さんの主張に戻ると。
繰り返しになりますが「「このsubjectっていったい何だっけ？」と思ったとき」というのが私には理解できませんでした。contextを読むときには普通、テスト対象がなんなのかは頭に入っている状態で読むものだと思っているからです。
ただこれは対象記事のサンプルコードを読んで思っただけなので実際の実務のコードを読んだら考えが変わる可能性もあります。私の主張は危うい。

「プルリクエストをレビューするときもsubjectの実体がdiffに出てこない（ことが起こりえる）」について

コード5

   end
+  context 'when C' do
+    before do
+      # 何行にも渡ってセットアップが続く
+      # .
+      # .
+      # .
+    end
+    it { is_expected.to eq 'xyz' }
+  end
 end

コード6

   end
+  context 'when C' do
+    before do
+      # 何行にも渡ってセットアップが続く
+      # .
+      # .
+      # .
+    end
+    it 'returns "xyz"' do
+      expect(foo.bar).to eq 'xyz'
+    end
+  end
 end

「コード5」はdiffにsubjectがないので何の結果を検証しているのかdiffだけではわからないが、「コード6」ではdiffだけで何の結果を検証しているのかわかるのでより良い、と主張されています。

私はこれに一理はあると思いつつ、反論の余地はあると思っています。

そもそもコードレビューするときはdiffだけでなく

• 周辺コード
• 似たような処理を行っている既存のコード
• utilsクラスなどの共通処理

なども読んでレビューするのが当たり前ではないでしょうか？そうしないと例えば「既存コードと同じロジックになっているので共通化するべき」などの指摘ができません。

これはrspecのレビューをするときも同様だと思います。流石にrspecを加筆しただけのPRで「utilsクラスなどの共通処理」みたいのものを見たりはしないと思いますが、周辺コードを読むことは共通ではないでしょうか。

そう考えると「diffだけみて何の結果を検証しているのかわかる」ということにどれだけの意味があるだろう？と思ってしまいます。

「応用：subjectの代わりにrspec-parameterizedを使う」について

これについては特に意見はありません。

問題はそのあとの「rspec-parameterizedすら使わず雑に書く」についてです。

コード6

describe 'Ticket.calc_fee' do
  it 'returns fee by age' do
    # 子どもの場合
    expect(Ticket.calc_fee(10)).to eq 500
    # 大人の場合
    expect(Ticket.calc_fee(20)).to eq 1000
    # 老人の場合
    expect(Ticket.calc_fee(65)).to eq 0
  end
end

こんなテストでも必要十分だったりします、と主張しています。

これはrspecを書くときのスタイルについて問題を抱えていると思っています。

rspecは以下のようにxUnit/minitest的なシンプルなスタイルで書くこともできるし、

シンプルなスタイル

RSpec.describe FoodFactory, '#make_food' do
  it 'makes bananas for monkeys' do
    factory = FoodFactory.new(:monkey)
    expect(factory.make_food).to eq(:banana)
  end

  it 'makes hay for horses' do
    factory = FoodFactory.new(:horse)
    expect(factory.make_food).to eq(:hay)
  end
end

rspecの機能をフルに使ったBDD的な複雑なスタイルで書くこともできます。

複雑なスタイル

RSpec.describe FoodFactory, '#make_food' do
  subject { factory.make_food }
  let(:factory) { FoodFactory.for(animals) }

  context 'for monkeys' do
    let(:animals) { :monkeys }
    it { is_expected.to eq(:banana) }
  end

  context 'for horses' do
    let(:animals) { :horses }
    it { is_expected.to eq(:hay) }
  end
end

（コードは 【翻訳】RSpecのリードメンテナだけど何か質問ある？ から引用）

これはどちらを選んでも自由です。個人開発だったら自分の好きな方で、チーム開発ならチームの好きな方で書けばいいと思います。（ちなみに上記記事でRSpecのリードメンテナの方はシンプルなスタイルを推奨していますね）。

「コード6」はシンプルなスタイル当たると思います。

シンプルなスタイルで書いているプロジェクトでsubjectを使ってないことには私は何も思いません。そうするべきとすら思います。

ただし、複雑なスタイルを選択したならできるだけsubjectを使うべきで、かつ シンプルなスタイルと複雑なスタイルは「混ぜるな危険」 だと思っています。

複雑なスタイルで書いているコードに以下のようなコードが追加されたら理屈抜きで「気持ち悪い」と思うのではないでしょうか（主観ですが）。

コード7

  RSpec.describe FoodFactory, '#make_food' do
    subject { factory.make_food }
    let(:factory) { FoodFactory.for(animals) }

    context 'for monkeys' do
      let(:animals) { :monkeys }
      it { is_expected.to eq(:banana) }
    end

    context 'for horses' do
      let(:animals) { :horses }
      it { is_expected.to eq(:hay) }
    end

+   it 'コアラの場合は笹を返却する' do
+     factory = FoodFactory.new(:koala)
+     expect(factory.make_food).to eq(:bamboo_grass)
+   end
  end

この場合は見てすぐに「明らかに変」と思うので修正を促すことができるかもしれませんが、「トップレベルのdescribeが違えばスタイルが違っても許されるのか」「ファイルが違っていればスタイルが違ってもゆるされるのか」「それとは別の指標があるのか」などと考えると線引は難しそうです。テストを書く度に「このスタイルは許されるのか」を議論するのは面倒ですし、主観が多く含まれるのでチーム全員が納得できる書き方を決めるのは困難そうです。

ならいっそ「シンプルなスタイルを選択したならシンプルなスタイルしか認めない」「複雑なスタイルを選択したなら複雑なスタイルしか認めない」としてしまったほうが書きやすく、レビュー指摘もしやすいのではないでしょうか？

ちなみに伊藤淳一さんはサヨナラBetter Specs!? 雑で気楽なRSpecのススメにて「雑なRSpec」というのを提案しています。内容は「rspec-parameterizedすら使わず雑に書く」と同等のことです。
この中で「重要：「RSpecらしいRSpec」を完全に放棄したわけではない」という章があります。
引用します。

とはいえ、僕も「RSpecらしいRSpec」を完全に放棄したわけではありません。
テストコードの内容によってはRSpecらしく書いた方が、読み書きがしやすかったり、保守性が高かったりするケースもあります。
そういう場合は「RSpecらしいRSpec」を書くようにしています。

ケースバイケースで「RSpecらしいRSpec」と「雑なRSpec」を使い分けている感じですね。

「よくわかんないけど、これが巷で言う守破離の"離"ってやつ？」なんて自分では考えたりしています。（間違ってたらすいません）

この「ケースバイケースで「RSpecらしいRSpec」と「雑なRSpec」を使い分け」るというのはどうやって、なにを基準に行っているのか教えてほしいです。
優れたプログラマーならそれは深く考えなくても自然とできるのでしょう。しかし、それができる人がチームにどれだけいるのでしょうか。「守破離の"離"」に達した人がどれだけいるのでしょうか。「自然とできる人」同士で考え方に齟齬があり議論に時間を取られることはないのでしょうか。

それを考えると私は「シンプルなスタイルを選択したならシンプルなスタイルしか認めない」「複雑なスタイルを選択したなら複雑なスタイルしか認めない」としてしまったほうが良いように感じられるのです。

「参考：RSpecのメンテナから見たsubject」について

参考としていくつか引用しています。

引用元をすべて読んだわけではないのですが、引用されている部分だけを読んだ感想としてはsubjectよりもis_expectedやワンライナー構文について注意を促しているように読めました。

この記事の冒頭で述べさせてもらったように私は「is_expectedは別に使う必要はない、subjectを使いたい理由は他にある。」という立場です。

引用されている文章はsubjectを使わないほうが良い理由としては説得力が低いのではないかと思いました。

以上で対象記事への反論めいたなにかは終わりです。

「で、なんであんたはそこまでしてsubject書きたいの？」という質問に対して

「これだけsubjectを書かないことに対して意見するからにはsubjectを書くことにメリットがあるんだろうなぁ、おらぁ」という声が聞こえてきそうです。

もちろんsubjectを書くことにメリットはあると私は信じています。しかしそれを書くと1つの記事になってしまうので当記事では割愛します。

いつか別記事で書くと思います。気合があれば。きっと。たぶん。おそらく。そのうちには…。

終わりに

一応、対象記事に対して一通り反論めいたことを試みました。
文章にしてみると私の主張は危ういところやごまかしが多く、穴だらけだなぁと思います。

subjectに関して論じると結局最後は宗教論争になって「こっちのほうが絶対的に正しい」とは絶対になりません。そういう結論を出そうとすることは最初から諦めましょう。

対象記事も「まとめ」にて

このあたりは個人の好みで大きく分かれるというか、もはや宗教に近いものを感じることもよくあります。「絶対subject使うマン」から見ると、僕の主張はどれも眉をひそめる話ばかりだったんじゃないかと思います。なので、「subjectを使った方がいい！」と思っている人はそのまま使い続けても構いません。

と述べています。

当記事で私が伝えたかったことは「有名な人が言っているんだから正しいに決まっている」というような考え方の人に対して「誰が言っているかではなく、何を言っているか」のほうが大事であるということです。
自分の頭で考えずに「有名な人が言っているから」「google検索のトップに出てくるから」それが正しい、というような思考にイエローカードを出したかったのです。

私のようなゴミプログラマーの主張ですが「有名な人とは違う主張をもっているひとがいて、その人の主張は客観的に一理ある」と思ってくれる人が一人でもいてくれたら嬉しいです。

おまけ

私はもう6年も前にも伊藤淳一さんとQiita上でsubjectを書く/書かないでやりとりをしていました。↓記事です。

【アンチパターン】Arrange、Act、Assert（AAA）を意識できていないRSpecのコード例とその対処法

この記事はrspecを書くときはAAAというのを意識するといいよ、ということをサンプルコードを使ってわかりやすく解説してくれた素晴らしい記事です。
その記事の余談としてサンプルコードにsubjectが一切使われていないことに関して説明しています。
私は当時からsubject書く派だったので、それに対してサンプルコードをsubjectを使った形に書き換えるコメントをしました。そのコメントに対して伊藤淳一さんはコメントを返してくれたのですが、それに対して私はなんの返信もしませんでした（本当に申し訳ない！）。なのでこの場での伊藤淳一さんとのsubjectを書く/書かない論争は私の完全敗北という形で終わっています。

（正直私はそんなコメントしていたことを完全に忘れていました。たまたま最近、その私のコメントに対して「いいね」してくれた方がいて、その通知ではじめて思い出しました。）

さて、今回はリベンジとなるでしょうか（戦う前から負けてる気がしますが）。その辺も楽しんでいただけると面白いかも。

しかし、私が当時提案したコードは今見るとほんとクソコードですね。このテストの対象はキャンセル料（user.billings.first）ではなくキャンセル処理（reservation.cancel!）だと今は思う。

@akihitofujiwara さんコメントの

user.billings.firstをsubjectにするって発想は微妙に本題とずれてるし

っていうのはそのとおりだと思うし、

proceduralメソッドは素直にsubjectを使わんのがよいのでは。

というのも今はそのとおりだと思う。