Tama.rb に行ってモブプロをやって feature spec をリファクタリングした話

[ブログ記事](http://secret-garden.hatenablog.com/entry/2019/10/20/021738)からの転載です。

と、いうことで最近噂の Tama.rb が前々から気になったので初めて参加してモブプロをやってきました！

* [Tama.rb #17 Everyday Rails - RSpecによるRailsテスト入門 - connpass](https://tamarb.connpass.com/event/150383/)

今回の Tama.rb の内容は [Everyday Rails - RSpecによるRailsテスト入門](https://leanpub.com/everydayrailsrspec-jp) を読みながらグループに分かれてモブプロを行う、という内容でした。
ぶっちゃけモブプロはやったことがなかったのでどうなるのか不安でしたが、Vim グループというとても居心地がいいグループに入った事でもう好き勝手言いまくってました！！

そしてその結果、書籍の内容そっちのけで最初から最後までひたすら feature spec のコードをリファクタリングしていただけという結果になり…。
もう途中からモブプロはそっちのけで全員であーだこーだ言いながらみんなでわいわいいいながらコードを書く会になっていましたね…。
はやモブプロとは…という感じだったんですが、結果的には RSpec の知識を共有したりわいわい言いながらみんなでコードを書くことが出来てめっちゃ楽しかったです！

最終的には個人的にかなりいい感じのリファクタリングになったので、そこに至るまでの流れを簡単にまとめてみたいと思います。
結構うろ覚えなので話の流れとか間違ってたらごめんなさい！


## 元の feature spec

書籍に載っていたのは元々以下のような rspec でした。

```ruby
require 'rails_helper'

RSpec.feature "Projects", type: :feature do
  # ユーザーは新しいプロジェクトを作成する
  scenario "user creates a new project" do
    user = FactoryBot.create(:user)

    visit root_path
    click_link "Sign in"
    fill_in "Email", with: user.email
    fill_in "Password", with: user.password
    click_button "Log in"

    expect {
      click_link "New Project"
      fill_in "Name", with: "tama.rb"
      fill_in "Description", with: "omotesando"
      click_button "Create Project"

      # MEMO: click_button とはまだ処理が完了していない可能性がある
      #       そこで以下のようなテストを実行することで「次のページがレンダリングされている」ことを保証する
      expect(page).to have_content "Project was successfully created"
      expect(page).to have_content "tama.rb"
      expect(page).to have_content "Owner: #{user.name}"
    }.to change(user.projects, :count).by(1)
  end
end
```

このテスト自体はそこまで複雑ではなくて以下のような流れになっています。

1． テストで使用する `User` を生成する  
2． `visit root_path` で root にアクセスして生成した `User` で `email` と `password` を入力して `"Log in"` ボタンをクリックする  
3． (`expect` のブロック内に入る)  
4． `"New Project"` をクリックして `New Project` のページに移動する  
5． `"Name"` と `"Description"` のフォームを入力して `"Create Project"` をクリックする  
6． クリック後に `"Project was successfully created"` 等が表示されているかどうかのテストを行う  
7． 最後に `user.projects` が増えていることを確認する

これはこれで流れがわかりやすくて悪くはないと思うんですが、最初にコードを見たときに以下の点が気になりました。

* `scenario` の中身が長い
  * 別 `context` のテストを追加しようとするとつらい
* `expect {}` の中で `expect` している…
  * これは待ち処理を行っているのでしょうがない部分もあるがうーん…

と、いうような話をモブプロ内で行っていたら「じゃあ、リファクタリングするべ」という流れになり書籍を読むのはやめてひたすらこのコードのリファクタリングを行っていました。
このときの共通の意識としては、

* フォームに変な入力を行った場合のテストを書きたいよね
* 失敗したときのテストを書きたいよね
* `expect {}` の中で `expect` してるのがきもい

っていうのがあったと思うのでこのあたりを中心にリファクタリングしていきました。


## `user` を `let` 化する

まず最初は、

「`user.name` が空の場合のテストとかしたいよねー」
「じゃあ、`let(:name)` とかに切り出すべー」

みたいな話になり、まずは `user` を `let` に切り出しました。

```ruby
require 'rails_helper'

RSpec.feature "Projects", type: :feature do
  # コンテキストで切り分けそうな値を切り出す
  let(:user) { FactoryBot.create(:user) }
  let(:email){ user.email }
  let(:password){ user.password }
  let(:name){ user.name }

  scenario "user creates a new project" do
    visit root_path
    click_link "Sign in"
    fill_in "Email", with: email
    fill_in "Password", with: password
    click_button "Log in"

    expect {
      click_link "New Project"
      fill_in "Name", with: "tama.rb"
      fill_in "Description", with: "omotesando"
      click_button "Create Project"

      # MEMO: click_button とはまだ処理が完了していない可能性がある
      #       そこで以下のようなテストを実行することで「次のページがレンダリングされている」ことを保証する
      expect(page).to have_content "Project was successfully created"
      expect(page).to have_content "tama.rb"
      expect(page).to have_content "Owner: #{name}"
    }.to change(user.projects, :count).by(1)
  end
end
```

割と妥当なやり方ですね。


## ログイン処理を `before` に切り出す

今回テストするのは『`New Project` からプロジェクトを生成した場合にどうなるのか』を検証するテストになります。
なので最初の方にある『ログイン処理』は `scenario` から切り出して `before` で定義する事になりました。
また、ここで気づいたんですが

「あれ、ログイン処理が今回のテストに関係ないなら別に `email` とか `password` は `let` で切り出さなくてよくね」
「じゃあ、無駄な `let` は定義しないで `user.email` とかで参照するべ」

という流れになり、今回必要がない `let` は消すことにしました。

```ruby
require 'rails_helper'

RSpec.feature "Projects", type: :feature do
  let(:user) { FactoryBot.create(:user) }
  let(:name){ user.name }

  # ログイン処理を before に切り出し
  before do
    visit root_path
    click_link "Sign in"
    # let 経由ではなくて user 経由で参照する
    fill_in "Email", with: user.email
    fill_in "Password", with: user.password
    click_button "Log in"
  end

  scenario "user creates a new project" do
    expect {
      click_link "New Project"
      fill_in "Name", with: "tama.rb"
      fill_in "Description", with: "omotesando"
      click_button "Create Project"

      # MEMO: click_button とはまだ処理が完了していない可能性がある
      #       そこで以下のようなテストを実行することで「次のページがレンダリングされている」ことを保証する
      expect(page).to have_content "Project was successfully created"
      expect(page).to have_content "tama.rb"
      expect(page).to have_content "Owner: #{name}"
    }.to change(user.projects, :count).by(1)
  end
end
```

めっちゃライブ感がありますね！  


## `プロジェクトの生成` を subject に切り出す

これはわたしが強い気持ちで、

「`scenario` の中身が長いので `subject` に切り出そう！！！」

と言って『プロジェクトを生成する処理』をまるっと `subject` に切り出しました。
ここでポイントなのは『`subject` が `lambda` を返している点』です。
これは `subject` が `lambda` を返すことで

```ruby
scenario "user creates a new project" do
  expect { subject.call }.to change(user.projects, :count).by(1)
end
```

を

```ruby
scenario "user creates a new project" do
  is_expected.to change(user.projects, :count).by(1)
end
```

と言う風に `is_expected` で記述することが出来るからです。
また、


「もう `scenario` の中身が1行しかないから `it` のほうがよくない？」


みたいな話になり全員が特に強い気持ちがなかったので `scenario` から `it` に変更し、更に引数も省略しました。
「あーみんなもそういう意識があるんだー」と「自分の書き方もそこまで間違ってないんだなー」と思いました。
一人で書いていると何が正しいのかがわからなくなるのでこういう『共通の認識』みたいなのを共有できるとだいぶ安心感があっていいですねー。
で、以下のようになりました。

```ruby
it { is_expected.to change(user.projects, :count).by(1) }
```

結果的に出来上がったのは以下のような `subject` になります。

```ruby
require 'rails_helper'

RSpec.feature "Projects", type: :feature do
  let(:user) { FactoryBot.create(:user) }
  let(:name){ user.name }

  before do
    visit root_path
    click_link "Sign in"
    fill_in "Email", with: user.email
    fill_in "Password", with: user.password
    click_button "Log in"
  end

  # 『プロジェクトを生成する』という処理を subject に切り出し
  subject {
    -> {
      click_link "New Project"
      fill_in "Name", with: "tama.rb"
      fill_in "Description", with: "omotesando"
      click_button "Create Project"

      # MEMO: click_button とはまだ処理が完了していない可能性がある
      #       そこで以下のようなテストを実行することで「次のページがレンダリングされている」ことを保証する
      expect(page).to have_content "Project was successfully created"
      expect(page).to have_content "tama.rb"
      expect(page).to have_content "Owner: #{name}"
    }
  }

  # その結果、 scenario はやめて it 1行だけに
  it { is_expected.to change(user.projects, :count).by(1) }
end
```

各々の処理の役割が明確になってきましたね。
`it` (`scenario`)が1行だけになりだいぶスッキリしてきました。


## `describe` で別スコープ化

ここまで来ると
「上位スコープにあんまり `let` とか定義したくないよねー」
「`describe` でテストの意味を切り分けよう！」
ということで `describe` でテストを切り分けました。

```ruby
require 'rails_helper'

RSpec.feature "Projects", type: :feature do
  # 『プロジェクトを生成した場合』のテストとして切り分ける
  describe "project create" do
    let(:user) { FactoryBot.create(:user) }
    let(:name){ user.name }

    before do
      visit root_path
      click_link "Sign in"
      fill_in "Email", with: user.email
      fill_in "Password", with: user.password
      click_button "Log in"
    end

    subject {
      -> {
        click_link "New Project"
        fill_in "Name", with: "tama.rb"
        fill_in "Description", with: "omotesando"
        click_button "Create Project"

        # MEMO: click_button とはまだ処理が完了していない可能性がある
        #       そこで以下のようなテストを実行することで「次のページがレンダリングされている」ことを保証する
        expect(page).to have_content "Project was successfully created"
        expect(page).to have_content "tama.rb"
        expect(page).to have_content "Owner: #{name}"
      }
    }

    it { is_expected.to change(user.projects, :count).by(1) }
  end
end
```

こうすることで今後『プロジェクトを削除した場合のテスト』などを追加する場合に `let` が切り分けることが出来るので便利です。


## `click_button "Create Project"` したあとの待ち処理をどうするか

さて、今回一番悩んだ点です。
`click_button "Create Project"` した後にどうやって『ページ遷移が終了したのか』の判定を行うのか、です。
現状は `expect(page).to have_content "Project was successfully created"` を行い『`"Project was successfully created"` が content に表示されるまで待つ』という処理を行っています。

「現状は `"Project was successfully created"` で判定しているけどバリデーションエラーになった場合とかに必ずしも `"Project was successfully created"` が表示されるとは限らない」
「失敗した時のページ遷移の条件をどうするか…」
「今回の程度ならもう `sleep 1` でいいんじゃね？」
「待ち処理の為に `expect` 使いたくなりなり〜」

みたいな話をしていたんですが、結果的には『`have_content` に渡す文字列をコンテキストによって変える( `:let` で定義しておく)』という事にしました。
また、

「`expect {}` 内で `expect` を行っているのはあくまでも『待ち処理を行う』という目的じゃん？」
「`expect {}` 内でテストを行う必要性はないんじゃなかろうか」

と、言うことで一旦『 `expect {}` 内で行う `expect` は1回だけ』にすることにしました。
ついでに `fill_in "Name"` に渡す文字列もコンテキストによって変えたいので `let` で定義することに。

```ruby
require 'rails_helper'

RSpec.feature "Projects", type: :feature do
  describe "project create" do
    let(:user) { FactoryBot.create(:user) }
    let(:name){ user.name }
    # コンテキストで変わりそうな値を let で定義する
    let(:have_content_notice) { "Project was successfully created" }
    let(:project_name) { "tama.rb" }
    let(:project_description) { "omotesando" }

    before do
      visit root_path
      click_link "Sign in"
      fill_in "Email", with: user.email
      fill_in "Password", with: user.password
      click_button "Log in"
    end

    subject {
      -> {
        click_link "New Project"
        fill_in "Name", with: project_name
        fill_in "Description", with: project_description
        click_button "Create Project"

        # MEMO: click_button とはまだ処理が完了していない可能性がある
        #       そこで以下のようなテストを実行することで「次のページがレンダリングされている」ことを保証する
        expect(page).to have_content have_content_notice
      }
    }

    it { is_expected.to change(user.projects, :count).by(1) }
  end
end
```

なかなか整ってきましたね


## `context` を追加する

もう少しです！  

* `project_name` が空の場合
* `project_description` が空の場合

のテストを追加していきたいと思います！  

```ruby
require 'rails_helper'

RSpec.feature "Projects", type: :feature do
  describe "project create" do
    let(:user) { FactoryBot.create(:user) }
    let(:name){ user.name }
    let(:have_content_notice) { "Project was successfully created" }
    let(:project_name) { "tama.rb" }
    let(:project_description) { "omotesando" }

    before do
      visit root_path
      click_link "Sign in"
      fill_in "Email", with: user.email
      fill_in "Password", with: user.password
      click_button "Log in"
    end

    subject {
      -> {
        click_link "New Project"
        fill_in "Name", with: project_name
        fill_in "Description", with: project_description
        click_button "Create Project"

        # MEMO: click_button とはまだ処理が完了していない可能性がある
        #       そこで以下のようなテストを実行することで「次のページがレンダリングされている」ことを保証する
        expect(page).to have_content have_content_notice
      }
    }

    # project_name と description が存在する場合のテスト
    context "with project name and description" do
      it { is_expected.to change(user.projects, :count).by(1) }
    end

    # project_name が空の場合のテスト
    context "with empty project name" do
      let(:have_content_notice) { "Name can't be blank" }
      let(:project_name) { "" }

      it { is_expected.not_to change(user.projects, :count) }
    end

    # project_description が空の場合のテスト
    context "with empty project description" do
      let(:project_description) { "" }

      it { is_expected.to change(user.projects, :count).by(1) }
    end
  end
end
```

おおーだいぶ簡単にテストが追加できましたね。
余談ですが、アプリ側の挙動が全然わかってなくて

「`project_description` が空だとエラーになるよねー」
「いや、でも `description` が空でも別に大丈夫なんじゃない？」
「じゃあ、試しにやってみよう！」

と、言う感じでやってみたら実は `description` が空でもプロジェクトが生成されるということがわかりました。
テストからアプリ側の挙動がわかるのがちょっとおもしろかったです。


## `let` は DRY にしない

これも RSpec あるあるかもしれませんが `let` はあまり DRY にしたくありません。
と、いうのも

「`let` はデフォルトで定義するんじゃなくて各 `context` で明示的に定義したいよね」
「`context` の外に `let` があると目線が上に言ったりきたりしてつらい」
「コードが重複してもいいのでなるべく狭い範囲(`context`)で `let` を定義しよう！」

と、言うことで `let(:have_content_notice)` などは各 `context` 内で明示的に定義するようにしました。
これも強い反対意見などなくて割とみんな共通の意識なのかなーと思いました。

```ruby
require 'rails_helper'

RSpec.feature "Projects", type: :feature do
  describe "project create" do
    let(:user) { FactoryBot.create(:user) }
    let(:name){ user.name }

    before do
      visit root_path
      click_link "Sign in"
      fill_in "Email", with: user.email
      fill_in "Password", with: user.password
      click_button "Log in"
    end

    subject {
      -> {
        click_link "New Project"
        fill_in "Name", with: project_name
        fill_in "Description", with: project_description
        click_button "Create Project"

        # MEMO: click_button とはまだ処理が完了していない可能性がある
        #       そこで以下のようなテストを実行することで「次のページがレンダリングされている」ことを保証する
        expect(page).to have_content have_content_notice
      }
    }

    context "with project name and description" do
      let(:have_content_notice) { "Project was successfully created" }
      let(:project_name) { "tama.rb" }
      let(:project_description) { "omotesando" }

      it { is_expected.to change(user.projects, :count).by(1) }
    end

    context "with empty project name" do
      let(:have_content_notice) { "Name can't be blank" }
      let(:project_name) { "" }
      let(:project_description) { "omotesando" }

      it { is_expected.not_to change(user.projects, :count) }
    end

    context "with empty project description" do
      let(:have_content_notice) { "Project was successfully created" }
      let(:project_name) { "tama.rb" }
      let(:project_description) { "" }

      it { is_expected.to change(user.projects, :count).by(1) }
    end
  end
end
```

## `have_content` のテストを追加する

最後に先程削除した `expect(page).to have_content "tama.rb"` のテストをどうするのか考えました。
これ、`expect(page)` を呼び出しているのでちょっとテストの仕方に工夫が必要になります。
最初は、

```ruby
it { is_expected.to change { page }.to have_content "tama.rb" }
```

みたいに書いてみたんですが、これだとうまく行きませんでした。

「これで動くといいなー」
「あー動かなかったー」
「`page` が返す値が同じになっているのかなあ…」
「じゃあ、 `subject` を直接呼び出せばいいんじゃない？」

で、まずは以下のようなテストにしました。

```ruby
it "shows project_name" do
  subject.call
  expect(page).to have_content project_name
end
```

これはこれでいいのですが、

「`subject.call` っていう呼び出しが意味不明過ぎる」
「これはつらい」
「じゃあ `subject` に名前つければいいんじゃね？」
「「それだ！」」

という事で `subject(:new_project)` と定義し、 `new_project.call` と呼び出すことに。
ぶっちゃけ `subject` に引数が渡せることは完全に失念していたいので「なるほどなー」とめっちゃ関心してました。
そして、最終的に出来上がったので次のコードになります！

```ruby
require 'rails_helper'

RSpec.feature "Projects", type: :feature do
  describe "project create" do
    let(:user) { FactoryBot.create(:user) }
    let(:name){ user.name }

    before do
      visit root_path
      click_link "Sign in"
      fill_in "Email", with: user.email
      fill_in "Password", with: user.password
      click_button "Log in"
    end

    subject(:new_project) {
      -> {
        click_link "New Project"
        fill_in "Name", with: project_name
        fill_in "Description", with: project_description
        click_button "Create Project"

        # MEMO: click_button とはまだ処理が完了していない可能性がある
        #       そこで以下のようなテストを実行することで「次のページがレンダリングされている」ことを保証する
        expect(page).to have_content have_content_notice
      }
    }

    context "with project name and description" do
      let(:have_content_notice) { "Project was successfully created" }
      let(:project_name) { "tama.rb" }
      let(:project_description) { "omotesando" }

      it { is_expected.to change(user.projects, :count).by(1) }
      it "shows project_name" do
        new_project.call
        expect(page).to have_content project_name
      end
    end

    context "with empty project name" do
      let(:have_content_notice) { "Name can't be blank" }
      let(:project_name) { "" }
      let(:project_description) { "omotesando" }

      it { is_expected.not_to change(user.projects, :count) }
    end

    context "with empty project description" do
      let(:have_content_notice) { "Project was successfully created" }
      let(:project_name) { "tama.rb" }
      let(:project_description) { "" }

      it { is_expected.to change(user.projects, :count).by(1) }
    end
  end
end
```

もう最初のコードのかけらもないですね！  
ちょうどここでモブプロの時間も終了して個人的にはかなり満足したコードがかけたと思います。


## まとめ

はじめてモブプロをやったんですが、これがモブプロとして正しいのかどうかは置いておいて多人数でコードを書くのはやっぱり楽しいですね。
普段は基本的に1人だけでコードを書いているのでこういう『他人のコードの書き方』や『コードを書く上での知見』みたいなのを得るのはとても貴重な機会でした。
意外というか意見を出しながらコードを書いていたんですが、あんまりコードの書き方の方向性で<del>殴り合い</del>揉めなかったことですね。
  
これは自分の声がデカかったのかもしれませんが、『あーみんなもそう思っているんだー』みたいな安心感の方が大きかった気がします。
1人でコードを書いていると何が正しいのかがわからなくなる事が多いんですが、こういう機会があると『自分のコードの書き方は間違ってなかったんや！！』みたいに思えるのでめっちゃいいですね。
  
あと今回は 1つの PC をみんなで使いまわしていたんですが、自分の Vim に慣れすぎてて他人の Vim がめっちゃつらいという事が体験できました…。
いかにしてに自分の Vim が自分の使いやすいようにカスタマイズされているのか改めて実感しました。
  
今回参加してみてモブプロ自体がかなりよくて、そのあともいろいろと Ruby について話が出来たので次もぜひぜひ参加してみたいですね！  
これは Ruby の話に飢えているんだ…もっと Ruby の話がしてえんだ…。
   
そんな感じで簡単に初参加した Tama.rb の内容をまとめてみました。
同じグループでワイワイしていた方々、運営の方々ありがとうございましたー！  


## その後

で、いまブログ記事を書きながら先程動かねーと言っていた

```ruby
it { is_expected.to change { page }.to have_content "tama.rb" }
```

をどうにか出来ないかいろいろと試していたんですが、`page` ではなくて `page.html` を参照することで解決出来ました。

```ruby
it { is_expected.to change { page.html }.to include "tama.rb" }
```

これでかなりすっきりしましたね！