Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RSpec/MultipleExpectations: ひとつの example に 10件まで expect を書けるようにする #101

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tk0miya
Copy link
Member

@tk0miya tk0miya commented May 17, 2022

さまざまなデータを扱う Web ページでは複数の変更が行われるため、複数の
expect を並べることはよく行われていることです。

標準では 1件までしか expect を書くべきではないとする
RSpec/MultipleExpectations を 10件まで許容するように変更します。
(10件という値に特別な意味はありません。5件では少ないだろうという直感によ
るものです)

さまざまなデータを扱う Web ページでは複数の変更が行われるため、複数の
expect を並べることはよく行われていることです。

標準では 1件までしか expect を書くべきではないとする
RSpec/MultipleExpectations を 10件まで許容するように変更します。
(10件という値に特別な意味はありません。5件では少ないだろうという直感によ
るものです)
@tk0miya tk0miya requested a review from a team as a code owner May 17, 2022 09:53
@github-actions github-actions bot added the ruby label May 17, 2022
@epaew
Copy link
Member

epaew commented May 17, 2022

system spec / request spec では複数 expectation も分かるんですが、

それ以外の spec は基本「単体テスト相当のテストコード」となるべきで、複数 expectation が出現したら「メソッドの分割が甘い/副作用が大きすぎる」を疑うべきなのかなあと。

なので、

  • 無条件で 10 は大きすぎるんじゃないかな
  • 結合テストでは緩和していいと思うけど、Exclude: ["spec/requests/**/*"] をここに書く…?これはこれで微妙かな

が私の意見です

@epaew
Copy link
Member

epaew commented May 20, 2022

結合テストでは緩和していいと思うけど、Exclude: ["spec/requests/**/*"] をここに書く…?これはこれで微妙かな

rubocop-rspec のデフォルト設定で↓みたいに書かれてるのを見ちゃったので、
https://github.com/rubocop/rubocop-rspec/blob/17aded3d7a0de0bdac4fa2e7c74c251315952b3b/config/default.yml#L113-L119

https://github.com/timedia/styleguide/blob/master/ruby/rubocop/config/rspec.ymlExclude: ["spec/requests/**/*"] を追加することにはそんなにネガはなくなりました。
inherit_mode の指定はあったほうが良さそうですが

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants