Avoid usage of create/build in factory associations

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Close this issue

Preamble

In #240929 (closed), we've changed FactoryBot's strategy to use the same build strategy as their parent object.

@engwan mentioned the following problem in #240929 (comment 422724486).

Problem

Our factories make use of create / build in associations, for example project's creator:

FactoryBot.define do
  factory :project, class: 'Project' do
    ...

    # Associations
    namespace
    creator { group ? create(:user) : namespace&.owner }

    ...
  end
end

group = FactoryBot.build(:group)
group.persisted? # => false

project = FactoryBot.build(:project, group: group)
project.persisted? # => false

# However:
project.creator.persisted? # => true

We are persisting a user record even if the build strategy is used. This behaviour potentially leads to a higher number of SQL queries which slows down our test suite.

Solution

Avoid the use of create / build in factory associations and prefer implicit or explicit association definitions.

To fix the example above we could use:

diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 091c9d5a245..87e4a8e355d 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -15,7 +15,7 @@ FactoryBot.define do
 
     # Associations
     namespace
-    creator { group ? create(:user) : namespace&.owner }
+    creator { group ? association(:user) : namespace&.owner }
 
     transient do
       # Nest Project Feature attributes

Proposed solution

• Mention explicit/implicit associations in Testing best practices > Factories - !44447 (merged)
• Create a 👮 to flag
  • create / build in factory associations - !44840 (merged)
    • prefer inline association definition such as association(:model)
    • Fix current occurrences - #267606 (closed)
  • create / build in after callbacks - #262624
    • prefer plain assocations
  • create_list and build_list in factory associations - !102552 (merged)
    • prefer Array.new(x) { association(:model) } instead
    • Fix current occurrences - ?
• Watch total amount of SQL queries go down in https://gitlab-org.gitlab.io/rspec_profiling_stats/ 🎉

Risks

We might see specs starting to fail if they assume that associations are always persisted.

In the case of the example above we are safe though. See !44429 (merged).