LoginSignup
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 1 year has passed since last update.

@Nifu-Uchi

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

Last updated at Posted at 2022-09-03

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

code-checkとGithubAction

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

GithubActions

rubocop

action-rubocopを使おう

pre-commitで走らせると良さげ

frozen_string_literal: true について

application.rbにrequire_relative 'boot'をつける

Rails の初期化プロセス - Railsガイド

unique_index

  • model_validatesuniquenessをつけるカラムはunique indexをつける 
     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

Style/OptionalArguments は入れる

  • オプショナル引数は省略できることに意味があるので、最後にないと意味がない（先輩のサンプルをそのまま引用。） 
     # このメソッドは引数`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のバグにぶち当たる

https://minne.com/items/2715511

Rspec

ファイル名はわかりやすく

  • task_controller_spec.rb とかにしておくとIDEでの検索時にmodelなのかcontrollerなのかわかりやすい

.rspecのオプション変更

  • --require rails_helper を記述することで、specファイルのrequire 'rails_helper'は書かなくてよくなる

describe/context/example/itをしっかり使い分けよう

controllerSpecよりrequestSpecの方が推奨

requestSpecでチェックした項目

  • HTTPstatus, flash, bodyレンダリング結果 
     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側のテストでカバーする

  • 切り出さないと検索クエリなどのテストがやりづらい 
     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で生成

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を定義してあげて 
      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で呼び出してやる 
      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の違いなどなど

modelSpec

shouldaを入れる

validationができているかのテストなどはしなくて良い

  • rails自体のテストになってしまうため
  • validationが適用されているかを確認したければ validate_presence_ofをmodeltestで行う

enum定義されたもの以外が渡ったときエラーになるかの検証はしなくていい

  • 先輩のサンプル 
     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がやってくれないので人間が気をつけなければいけない
  • 今回はこんな感じにした 
      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同士は改行しない
0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?