Rails初級者が一ヶ月研修を受けて得た学びをまとめた~GithubActions・RSpec~

この記事は「[Rails初級者が一ヶ月研修を受けて得た学びをまとめた - Qiita](https://qiita.com/Nifu-Uchi/items/f676d9f082e8c02bd88d)」のうち分量が多くなりすぎたコード品質担保（LinterやRSpecなど）に関する項目を分割した記事です。
概要などについては親記事を参照してください。



# code-checkとGithubAction
- PR時にRSpectとともにLinter/Formatterが走るようにした
- 導入したものは以下
  - rubocop
  - erblint
  - brakeman
  - rspec


## GithubActions
- [こちらを参照](https://zenn.dev/link/comments/7ece3f5df6eac9)

##  rubocop
### action-rubocopを使おう
- 指摘箇所があるとPRにコメントしてくれる
- [reviewdog/action-rubocop: Run rubocop with reviewdog 🐶](https://github.com/reviewdog/action-rubocop)
- そして先輩の一人がメンテナをされているらしい、心強すぎる
### pre-commitで走らせると良さげ
- [【Ruby on Rails】rubocop と pre-commit を利用して git commit 時にコーディングチェックを行う - きゃまなかのブログ](https://techblog.kyamanak.com/entry/2018/06/19/221910)

### frozen_string_literal: true について
- 「Ruby3 では文字列がデフォルトで immutable になる」と書いたが、「Ruby3 では文字列をデフォルトで immutable にはしない」という決定がMatzによってなされた。
[Ruby2.3で導入されたfrozen_string_literalマジックコメントでImmutable Stringを実現する - Hack Your Design!](https://blog.toshimaru.net/ruby-immutable-string/)

### application.rbに`require_relative 'boot'`をつける
[Rails の初期化プロセス - Railsガイド](https://railsguides.jp/initialization.html)


### unique_index
- model_validates`uniqueness`をつけるカラムはunique indexをつける
	```bash
	app/models/label.rb:4:3: C: Rails/UniqueValidationWithoutIndex: Uniqueness validation should have a unique index on the database column.
	  validates :label_name, presence: true, uniqueness: true, length: { maximum: 10 }
	  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	```
- 参考：[[Rails] uniquness バリデーションの警告 - Qiita](https://qiita.com/shougo120305/items/4a080821c893baf05fa0)

### Style/OptionalArguments は入れる
- オプショナル引数は省略できることに意味があるので、最後にないと意味がない（先輩のサンプルをそのまま引用。）
	```rb
	# このメソッドは引数`a`, `b`両方を与えなければいけないので`a`をオプショナルにしている意味がない
	def bad(a = '', b)
	end

	# このメソッドには引数`b`を与えなくてもよい
	def good(a, b = '')
	end
	```
	>  オプショナル引数を最後に置くほうがメソッドの使い勝手が良いので従いたいルールですね。



### Rails/HelperInstanceVariable 入れよう
- これはhelperメソッドで`@インスタンス変数`を定義すると怒られるもの
- 可能な限りメソッドを局所化すべきであり、あちこちで同じ変数を参照できる状態が増えると、複雑性や可読負荷が高まりバグの原因になる
    > プログラムにおいては「状態を持つ」というのが複雑さや認知負荷（読みづらい・保守しづらい）を生む大きな要因です。なので極力状態を気にするモジュールを局所化するのが良いプラクティスとされています。
たとえばcontroller, view, helperすべてで同じ変数を参照していると、いつどこで変数が代入されるかによって影響を受ける箇所が多くなってしまいます。こうした事態を避けるためhelperではメソッドをただの関数となるようにしたく、こうしたルールが存在しています。

### なにを有効化すべきか？
- 基本はデフォルトベースで調整する
- Linterは問題点検出が目的なので可能な限り全て有効化する
- formatterはプロジェクト内で統一したフォーマットにするために必要なものであるので状況による
  - 基本的にはデフォルトベースで良い
  - **フォーマット（インデントなど…）は個人の好みであり、そこで議論することは開発の本質ではない**
  - 特にプロジェクトに途中から参加してformatterを有効にするタスクを任されたときはデフォルトのままで行くのが望ましい
  - 自分の好みを入れるとそこで議論が起きてしまう

### 余談：rubocopのバグにぶち当たる
<img width="480px"  src="https://qiita-image-store.s3.ap-northeast-1.amazonaws.com/0/421823/bcbc52e8-e57a-a86c-5598-90ef5ece5b47.jpeg">

*https://minne.com/items/2715511*

- Rubocopの導入中、突然動かなくなった。（何もしていないのに壊れました）
- おま環かと思い慌てたがどうもバグっぽい
- ビビりながらrubocopにissue立てたら、バグで爆速で直してもらえた。
- [Can't execute v1.34.0 with error "cannot load such file -- rubocop-rails" · Issue #10893 · rubocop/rubocop](https://github.com/rubocop/rubocop/issues/10893)
- 教訓：慣れない環境構築中はversionが上がってないか注意しようね。問題が切り分けられなくなるぞ






## Rspec
### ファイル名はわかりやすく
- `task_controller_spec.rb ` とかにしておくとIDEでの検索時にmodelなのかcontrollerなのかわかりやすい
### `.rspec`のオプション変更
- `--require rails_helper `を記述することで、specファイルの`require 'rails_helper'`は書かなくてよくなる

### describe/context/example/itをしっかり使い分けよう
- [RSpecの(describe/context/example/it)の使い分け - Qiita](https://qiita.com/uchiko/items/d34c5d1298934252f58f)
- Jestの経験があるのでなんとなくは掴んでいた。PJごとに文化などあったら確認

### controllerSpecよりrequestSpecの方が推奨
- 今回テスト方針としてendpointが設定されているcontrollerメソッドのカバレッジを100％にすることが目標となっていた
- [RSpec - Controller or Request Specs?](https://medium.com/just-tech/rspec-controller-or-request-specs-d93ef563ef11)

### requestSpecでチェックした項目
- HTTPstatus, flash, bodyレンダリング結果
	```rb
    expect(response).to have_http_status(200)
    expect(flash[:notice]).to be_present
    expect(response.body).to include('flashの内容')
    expect(response.body).to include('xxx')
	```
- redirectされる場合は`redirect_to`の宛先も確認する
- 記述順序は統一しよう

 
### エンドポイントテストでカバーできない範囲は別メソッドに切り出して単体テスト、あるいはmodel側のテストでカバーする
- 切り出さないと検索クエリなどのテストがやりづらい
  ```rb
   def index
    @user = current_user
    @tasks = @user.tasks.includes(:labels)
    unless params[:reset]
      @tasks = @tasks.where({ status: Task.statuses[params[:status]] }.compact)
      if params[:search_word]
        @tasks = @tasks.where('task_name LIKE ? OR detail LIKE ?', params[:search_word],
                              params[:search_word])
      end
    end
    @tasks = @tasks.order("#{sort_column(params[:sort])} #{sort_direction}").page(params[:page]).per(20)
  end
  ```
- この状態だとindexにリクエストを送るrequestSpecでは、responseのHTMLをperseして期待した順番にアイテムが並んでいるかチェック…としなければいけず、良い状態とは言えない
- 別メソッドに切り出してファイルも分けて管理した方がテストはしやすい

### 使い回す値はletとfactory_botで生成
- `let(:valName) { command.. }`で定義できる
- factory_botはテストデータの定義をしてくれており、それを呼び出すだけで簡単に作成できる
- letと組み合わせて使うと便利
- [factory_bot/GETTING_STARTED.md at master · thoughtbot/factory_bot](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md) ←読み込んでおこう


### HTTPステータスだけでなく描画結果も確認しよう
- `expect(response).to have_http_status(200)` だけチェックしていたが中身も見るべき

### 実行しないテストは`xit`、`xcontect`にしておく
- 空行のみや、コメントのみの状態で放置しない

### controllerテストではテストデータ作成の記述をそこまで実際のものに寄せる必要はない
- controllerテストの目的はrequestを行い、正しいレスポンスが帰ってくるかの確認
- なので、letでのモデルインスタンス作成時の記述にこだわる必要はなく、可読性が担保されていればよい
- テストとコントローラーでインスンタンスの初期化方法が同じであることのメリットはない
### Factory_bot初期値は実際に入りそうな文字列にしておくとイメージしやすい
- たとえば　`task_name { 'testTask1' }`ではなく`task_name { 'お米を買う' }`のような感じ
### expectの値はハードコードした方が良い場合がある
- bodyレンダリング結果を次のように検証するばあい、可読性の悪さ、テスト自体が間違っている際に気づきにくい というデメリットがある
  - bad: `expect(response.body).to include(default_task1.task_name,...`
  - good: `expect(response.body).to include('testTask1', 'testTask2', ...)`
- 前職でも言われていたことだったが完全に忘れてやってしまっていた…
### redirect先の検証は`follow_redirect!`で
- redirectを返すエンドポイントテストでは`follow_redirect!`を使うとリダイレクト先のresponseまで確認できる
### 共通処理はshared_contextで共通化しよう
- 例えばログインしてない状態で任意のurlに飛ぶと、全てログインページにリダイレクトさせるような処理をチェックする場合,このようにcontroller_specの中でshared_contextを定義してあげて
  ```rb
    shared_context 'redirect_to_login-page' do
      it 'is redirect to /users/sign_in' do
        send_request

        expect(response).to have_http_status(302)
        expect(response).to redirect_to('/users/sign_in')
        follow_redirect!
        expect(response).to have_http_status(200)
        expect(response.body).to include('Log in', 'Email', 'Password')
      end
    end
  ```
- 任意の場所で`it_behaves_like`で呼び出してやる
  ```rb
    context 'is redirect to /users/sign_in' do
        context 'request GET #index' do
            subject(:send_request) { get '/' }
            it_behaves_like 'redirect_to_login-page'
        end
    ...
    end
  ```
- subjectと組み合わせて使うと良い
### 共通処理はsubjetで定義する
- `subject(:subject-name) { get 'xx', params: {...} }` のように使う

### expectとis_expectedの違いなどなど
- [RSpec で subject & expect する時の注意点 - Qiita](https://qiita.com/mm36/items/453d71d48dd8f462344c)

## modelSpec
### shouldaを入れる
- validationなどのmodel定義のテストをワンライナーでかける。
- 以下のサンプルはshouldaを利用したもの
- ref: [shoulda-matchersが便利すぎる - Qiita](https://qiita.com/mightysosuke/items/62967c9a7d24f59344f9)
- ref: [File: README — Documentation by YARD 0.9.20](http://matchers.shoulda.io/docs/v4.3.0/)
### validationができているかのテストなどはしなくて良い
- rails自体のテストになってしまうため
- validationが適用されているかを確認したければ `validate_presence_of`をmodeltestで行う

### enum定義されたもの以外が渡ったときエラーになるかの検証はしなくていい
- 先輩のサンプル
	```rb
	describe XXX do
	  describe 'Model attributes definition' do
	    let(:task) { build(:task) } # factory_bot

	    it { is_expected.to validate_presence_of(:priority) }

	    it {
	      is_expected.to define_enum_for(:priority)
		.with_values({
		  '低': 0,
		  '標準': 1,
		  '高': 2
	      })
	    }
	  end
	end
	```
	 > validatesやenumのテストは、正しく設定ができていることを確認すれば良いので、下記のようなテストを実装することが多いですね


### test改行ルールも統一
- これは好みだがルールは統一しておくべき。
- 改行はLinterがやってくれないので人間が気をつけなければいけない
- 今回はこんな感じにした
  ```rb
    RSpec.describe LabelsController, type: :request do
    let(:user) { create(:user) }
    ...

	    describe 'XXXX' do
		before do
		...
		end

		context 'YYYY' do
			it 'ZZZZZ' do
			    ...
			end

			it 'ZZZZZ' do
			    ...
			end
		end

		context 'YYY2' do
		    ...
		end
	    end

	    describe 'XXX' do
	    ...
	    end
    end
  ```
- ブロックの開始と終わりは一行開ける
- `let`などの初期化行との間も一行開ける
- end同士は改行しない