search

brightin-archive / bretels

Base rails application used at Brightin
MIT License
1 stars 2 forks source link

Remove factory_girl model spec #21

Open florish opened 9 years ago

florish commented 9 years ago

factory_girl has a FactoryGirl.lint method since version 4.0 which can be run automatically before running a spec suite. See the getting started guide.

Bretels still generates a separate spec for factories, which isn't necessary anymore.

stefanroex commented 9 years ago

I'm not sure FactoryGirl.lint is better. The getting started guide states you should add it as a rspec before(:suite). I just did a quick benchmark, this adds ~300ms per test-run. This overhead is added on every run, even when FactoryGirl isn't used.

This extra cost would be beneficial when there was a high chance of catching a invalid factory, however I think the changes are actually quite low.

I'll close this issue, but if you're convinced this is better, please let me know :wink:.

florish commented 9 years ago

I got into trouble with database_cleaner after adding the before(:suite) lint task and had to dive into this a couple of days ago. It turns out the getting started guide has beenupdated, see the linting section. (this commit is only 11 days ago).

In my project, I added the rake task and updated the CI workflow to run the factory_girl:lint task as part of the CI suite:

desc 'Run all tests and static analysis checks, as the CI runner does.'
task ci: %i(static_analysis factory_girl:lint spec)

(static_analysis runs brakeman, reek and rubocop).

When added as such, I'm thinking it should be an improvement over the auto-generated model spec. What do you think?

avdgaag commented 9 years ago

:+1: for replacing the generated spec/models/factories_spec.rb with a separate task.