rspec-style-guide icon indicating copy to clipboard operation
rspec-style-guide copied to clipboard

Check model validity

Open choosen opened this issue 3 years ago • 2 comments

Content below exists in guide from the initial version

Add an example ensuring that the model created with FactoryBot.create is valid.

describe Article do
  it 'is valid with valid attributes' do
    expect(article).to be_valid
  end
end

I would not add additional code to spec factory validity if factory bot is delivering that one (also with traits: true) see: https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#linting-factories

choosen avatar Jul 24 '21 07:07 choosen

It is very reasonable to use .lint, especially with traits: true. Very well worth it.

This, however, doesn't work in certain situations, still, workarounds exist.

  1. "Abstract" factories Disclaimer: abstract factories don't exist if FactoryBot. If there's an abstract model, that is not intended to be instantiated, the factory creation will fail.
factory :user do
  role { nil } # this makes the model invalid

  factory :admin do
    role { :admin } # valid now
  end
end

Solution: exclude :admin from linting by explicitly giving lint a list of factories to lint. Not so easy when the factory is designed in a way that a trait needs to be used, e.g. replace factory :admin with trait :admin.

  1. Other strategies lint only checks :create by default, and does not :build and :stub. With after(:create) callbacks, the spec would tell you that the factory is solid, while your build and build_stubbed models might be actually invalid.

  2. lint is slow In a number of projects, .lint spec example was by far the slowest one, taking up minutes even without traits: true. That made the spec suite poorly parallelizable. Solution:

  • split into several examples somehow, each linting a group of factories
  • include lint to a specific model spec Downsides: due to human error, some factories might be left out. It might be an idea for a rubocop-rspec cop to add a lint when a factory is used later in the spec, however, :product factory might be used in user_spec.rb, and for the cop it's impossible to distinguish if it's a user-related factory and it should be linted in the spec or not, and it should be linted in a corresponding model's spec.

Bottom line: would you like to amend the current recommendation, shifting it towards lint?

pirj avatar Jul 24 '21 08:07 pirj

Ad. 1 That's quite simple to reject them (see below) and abstract factories can be tested at model spec for validity as edge case.

factories_to_lint = FactoryBot.factories.reject do |factory|
  factory.name =~ /^old_/
end

FactoryBot.lint factories_to_lint

Ad. 2 You can pass strategy :build to .lint - but I did not test it actually

Ad. 3 Guard for linting only changed factories would be great :D (relations could be detected by reflection system) If lint is so slow, then you need to split your models into 2 rake tasks and run them in parallel. But nevertheless if lint is slow then checking all in model specs will not be faster.

// So I see that you have fast specs beside validations 👍

Ad. Bottom line I am not great writer with flair but I will draft something : )

choosen avatar Jul 24 '21 13:07 choosen