FactoryBot(旧:FactoryGirl)アンチパターンを検出することは意味がありますか？

[ybiquitous]

こちらのblog postで紹介されているFactoryBotのアンチパターンを、RuboCopで検出することは意味がありそうでしょうか？

Rails アンチパターン - 錆びついたファクトリー (factory_girl) - アジャイルSEの憂鬱

特に、以下の例で示されているAssociationsのアンチパターンはテスト実行速度に影響があるため、検出できれば有益かなと思いました。

# アンチパターン：build_stubbed なのにレコードが生成される

# bad
FactoryBot.define do
  factory :book do
    author { create(:user) }
    published_at { '2017-04-10' }
  end
end

# good
FactoryBot.define do
  factory :book do
    association :author, factory: :user
    published_at { '2017-04-10' }
  end
end

[ybiquitous]

rubocop-rspec FactoryBot Cops に追加されることを想定してます。

[koic]

RubyKaigi 2019 にて口頭で話したログになります。build_stubbed を使っていないデータベースの状態に依存したテストというコンテキストで false positive が発生しそうです。アイデアとしてはとても興味深く実践的だと思うのですが、このあたりの false postive をクリアしていないとユーザーの対処が大変そうなため、author { create(:user) } の代わりに association :author, factory: :user を示唆するだけの Cop だと導入が難しそうです。

[ybiquitous]

コメントありがとうございます！もう一度、FactoryBotの挙動を見直してみます！

[ybiquitous]

大変失礼しました 🙇 私の理解が足りていなかったので、デモを作ってみました。 https://github.com/ybiquitous/factory-bot-demo#readme

READMEに test log を貼ったので見てもらえると分かると思うのですが、 association :author を使用した test_build_stubbed では INSERT が発行されていないのに対し、 author { create(:user) } を使用した test_build_stubbed_(invalid) では INSERT が発行されています。

つまり、 build_stubbed を使う側はDB accessが一切発生しないことを期待するはずですが、 誤った factory 定義を指定したために、意図せずDB accessが発生してしまいます。

factory を使う側としては、

• DB accessが必要なテストを書くとき => build or create を使う
• DB accessが必要ないテストを書くとき => build_stubbed を使う

になるかと思っています。なので、ほとんどのケースにおいて factory block 内に create or build を書くのは、アンチパターンになるかと。。。

＊参考 https://www.rubydoc.info/gems/factory_bot/file/GETTING_STARTED.md#Associations

[ybiquitous]

実際 false positives がどのくらい出るか不明な点もあるので、real applications で試してみます 💪 助言いただき、ありがとうございます！

[ybiquitous]

とりあえず、雑に作って eliotsykes/real-world-rails のいくつかで試してみました。

コードはこちら ⬇️ https://github.com/rubocop-hq/rubocop-rspec/compare/master...ybiquitous:factory_bot/prefer_association

$ rubocop --config .rubocop.yml --only FactoryBot/PreferAssociation apps/wiki_edu_dashboard/spec/factories
Inspecting 32 files
........C..C.......C............

Offenses:

apps/wiki_edu_dashboard/spec/factories/questions_factory.rb:20:5: C: FactoryBot/PreferAssociation: Use association instead of create.
    question_group  { FactoryBot.create(:question_group) }
    ^^^^^^^^^^^^^^
apps/wiki_edu_dashboard/spec/factories/answers_factory.rb:5:5: C: FactoryBot/PreferAssociation: Use association instead of create.
    answer_group  { FactoryBot.create(:answer_group) }
    ^^^^^^^^^^^^
apps/wiki_edu_dashboard/spec/factories/answers_factory.rb:6:5: C: FactoryBot/PreferAssociation: Use association instead of create.
    question      { FactoryBot.create(:q_long)       }
    ^^^^^^^^
apps/wiki_edu_dashboard/spec/factories/answer_groups_factory.rb:5:5: C: FactoryBot/PreferAssociation: Use association instead of create.
    question_group { FactoryBot.create(:question_group) }
    ^^^^^^^^^^^^^^

32 files inspected, 4 offenses detected

試した限りでは false positives は出なかったです。

ただし1点問題があって、FactoryBotはMinitestとかでも使えるので、rubocop-rspecにあるのはどうなの？？？みたいなのはあります 😅 （この問題は一朝一夕で済むものではないのですが…）

[koic]

ご確認ありがとうございます。

FactoryBotはMinitestとかでも使えるので、rubocop-rspecにあるのはどうなの？？？みたいなのはあります

FactoryBot に関連する Cop という意味では前例があるので大丈夫だと思っていますが、Cop の是非については最終的に RuboCop RSpec チームの判断が入ることになると思います。コードを仕上げて PR を開く際に、Cop として取り込むメリットに対する十分な説明があると良いと思います。