Background
---
In issues #64 and #66 (from 2010) people expressed surprise
that using the `build` strategy would still `create` any associations.
I remember being similarly surprised by that behavior when I first
started using factory_bot.
The main reason for this behavior is to ensure the built instance will
be valid if there were any foreign key validations
(like `validates_presence_of :user_id`). If we don't save the
associations, there won't be any ids present.
PR #191 (from 2011) offers a workaround for people who don't want
records to be saved automatically. Passing `strategy: :build`
(originally `method: :build`, but later renamed) when declaring the
association will prevent it from being saved when using `build`.
But #191 isn't really a complete solution (as discussed on the PR
after it was merged). `strategy: :build` may do
the right thing when building objects with the `build` strategy, but it
can cause new problems when using the `create` strategy.
A better solution would be something like:
`strategy: <whatever strategy I was already using>`.
PRs #749 and #961 (merged in 2016) introduce something like that,
with the `use_parent_strategy` configuration option.
With this turned on `build` end up being generally [a little faster][]
than `build_stubbed`, since it no longer needs to hit the database for
each association.
[a little faster]: https://gist.github.com/composerinteralia/d4796df9140f431e36f88dfb6fe9733a
I have set `use_parent_strategy` on several projects now. I also added
it to suspenders in thoughtbot/suspenders#952.
On newer projects I have not run into any problems. On existing
projects I have seen the occasional test failure, which are easy enough
to fix by changing `build` to `create`.
Unfortunately I don't think `use_parent_strategy` is widely known,
since it wasn't documented until #1234 (about a month ago).
I also learned in #1234 that the `use_parent_strategy` setting gets
wiped out with `FactoryBot.reload`, which can be problematic when using
factory_bot_rails.
To summarize, we have been exploring having a `build` strategy that uses
`build` all the way down since 2010, but there still isn't a totally
reliable way to get that.
This PR
---
* Default to using the parent strategy for factory_bot 5, as suggested
in #749.
* In the original PR for this (#1240) I had removed
`use_parent_strategy`, but to make the transition smoother I ended up
fixing the reload problem in #1244
Once factory\_bot 5 is released and the dust has settled, it may make
sense to deprecate `use_parent_strategy`, and maybe also the strategy
option for associations.
Before this PR, `use_parent_strategy` was set on the configuration
instance. Since `FactoryBot.reload` wipes out the configuration, it also
ends up resetting `use_parent_strategy` back to `nil`.
This can cause problems when using factory_bot_rails with Spring, since
it calls `FactoryBot.reload` each time Spring forks. If
`use_parent_strategy` is set in a file that Spring preloads, like in an
initializer, then the value will get wiped out.
With this PR, we set `use_parent_strategy` directly on FactoryBot,
rather than on the configuration instance, and so reloading no longer
has any effect.
This has come up a few times, and I can see why it might be helpful to
have access to full backtraces when debugging a factory error uncovered
by `FactoryBot.lint`. But since most of the time I don't want the extra
noise from the backtrace, I added this as a verbose option.
The default message is still:
```
The following factories are invalid:
* user - undefined method `save!' for #<User:0x00007ff0cbc89100>
* admin - undefined method `save!' for #<User:0x00007ff0cbc73e40>
```
And with the verbose option (usually with more lines of backtrace):
```
The following factories are invalid:
* user - undefined method `save!' for #<User:0x00007ff0cbc89100>
/Users/.../thoughtbot/factory_bot/lib/factory_bot/evaluation.rb:18:in `create'
/Users/.../factory_bot/lib/factory_bot/strategy/create.rb:12:in `block in result'
* admin - undefined method `save!' for #<User:0x00007ff0cbc73e40>
/Users/.../thoughtbot/factory_bot/lib/factory_bot/evaluation.rb:18:in `create'
/Users/.../factory_bot/lib/factory_bot/strategy/create.rb:12:in `block in result'
```
I moved the linting option defaults out of the FactoryBot.lint method
and into keyword argument defaults in Linter#initialize. This seems a
bit cleaner, and now we will get an error if we pass an option we don't
understand (before 6e511597 we had a test that passed in a bogus
option)
Closes#710Closes#1124
I am opening a new PR since the original PR is years old and it seemed
unkind to request changes after so long. Instead I will list the authors
as co-authors.
Co-authored-by: Jack Kinsella <jack.kinsella@gmail.com>
Co-authored-by: Jasper Woudenberg <mail@jasperwoudenberg.com>
This refactoring work should make adding the verbose linting option
(started in PR #1233) fairly simple.
I moved the linting option defaults into keyword argument defaults for
Linter#initialize. With keyword arguments we will now get an error if we
pass an option the Linter doesn't understand (we were doing exactly this
in one of our tests before 6e511597). Adding the verbose option will now
be as simple as adding another keyword argument, then adding backtraces
to the error messages when the option is set to true.
Closes#1196
We deprecated looking up factories by class in #877. We introduced
`allow_class_lookup` option so that people could disable the behavior
entirely after fixing the deprecation warning.
In preparation for factory_bot 5, I am removing the deprecation warning
and the `allow_class_lookup` option. It is no longer possible to look
up factories by class. This has also made the ClassKeyHash decorator
unnecessary. The behavior is technically a little different now
that we are using HashWithIndifferentAccess
instead of calling to_sym on the key, but it
should behave identically for any standard,
documented factory_bot usage.
Daniel Colson