thoughtbot / factory_bot

A library for setting up Ruby objects as test data.
https://thoughtbot.com
MIT License
7.92k stars 2.61k forks source link

`before(:create)` not executed in expected order #1573

Open samanthawritescode opened 1 year ago

samanthawritescode commented 1 year ago

Description

The before(:create) block is not getting executed in the order I expect when using instance in one of the factory's associations.

I'm trying to update an existing factory I have (organization) to allow a :build strategy. It currently makes use of a block like this:

after(:create) do |instance, evaluator|
      instance.update!(
        account_type: evaluator.account_type || build(:account_type, organization: instance),
      )
    end

Which obviously won't work when using build. So I've updated it by making an association:

account_type { association :account_type, organization: instance }

But now I can't find evidence that the factory's before(:create) block that I need to run is running (at least in the order I expect). See repro steps below.

Reproduction Steps

This fails because the http call is not getting stubbed out:

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "factory_bot", "~> 6.0"
  gem "activerecord"
  gem "sqlite3"
  gem 'webmock'
end

require "active_record"
require "factory_bot"
require "minitest/autorun"
require "logger"
require 'uri'
require 'net/http'
require 'webmock/minitest'

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :organizations, force: true do |t|
    t.integer :account_type_id
  end

  create_table :account_types, force: true do |t|
    t.integer :organization_id
  end
end

class Organization < ActiveRecord::Base
  belongs_to :account_type, optional: true

  before_create :get_nasa_image

  # [callback] before_create
  def get_nasa_image
    uri = URI('https://api.nasa.gov/planetary/apod?api_key=DEMO_KEY')
    Net::HTTP.get_response(uri)
  end
end

class AccountType < ActiveRecord::Base
  belongs_to :organization
end

FactoryBot.define do
  factory :organization do
    account_type { association :account_type, organization: instance }

    before(:create) do
      WebMock.stub_request(:get, "https://api.nasa.gov/planetary/apod?api_key=DEMO_KEY").
        with(
          headers: {
            'Accept'=>'*/*',
            'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
            'Host'=>'api.nasa.gov',
            'User-Agent'=>'Ruby'
          }).
        to_return(status: 200, body: "", headers: {})
    end
  end

  factory :account_type do
    organization
  end
end

class FactoryBotTest < Minitest::Test
  def test_factory_bot_stuff
    org = FactoryBot.create(:organization)

    assert true
  end
end

# Run the tests with `ruby <filename>`

However, if you change the organization factory to this, the http call does in fact get stubbed out and everything works as expected:

  factory :organization do
    before(:create) do
      WebMock.stub_request(:get, "https://api.nasa.gov/planetary/apod?api_key=DEMO_KEY").
        with(
          headers: {
            'Accept'=>'*/*',
            'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
            'Host'=>'api.nasa.gov',
            'User-Agent'=>'Ruby'
          }).
        to_return(status: 200, body: "", headers: {})
    end

    after(:create) do |instance, evaluator|
      instance.update!(
        account_type: evaluator.account_type || build(:account_type, organization: instance),
      )
    end
  end

Expected behavior

I expected the factory's before(:create) block to run before the model's after(:create) block.

Actual behavior

The factory's before(:create) block is not run, the HTTP call is not stubbed out, and the test fails because the model is running it's after(:create).

System configuration

factory_bot version: 6.2.1 rails version: 6.1.7.3 ruby version: 2.7.8

J-Swift commented 9 months ago

I think I'm running into something similar, where factory use in a before(:all) block fails because http calls arent being stubbed. Nothing to add other than that so far 😅

milgner commented 8 months ago

In my project, before(:create) doesn't seem to be run at all. This leads to a situation where

FactoryBot.build(:my_factory).save!

works - the after(:build) callback runs as expected and fills in some bidirectional associations for delegated types correctly.

But

FactoryBot.create(:my_factory)

throws

Validation failed: Data source can't be blank (ActiveRecord::RecordInvalid)

because neither after(:build) nor before(:create) are invoked. This is contrary to the documentation which suggests that FactoryBot.create should run both after(:build) and before(:create) callbacks.

smaboshe commented 4 days ago

Thank you, @samanthawritescode, we were able to reproduce this with Factory Bot 6.5.0 (current version). We don't have a solution yet but are investigating.

unused commented 4 days ago

First of all the issue description provided is impressive @samanthawritescode Having a one-ruby-file script to execute leveraging inline-bundler and sqlite from memory is awesome :clap: :rocket:

Following the path of execution, this is not a problem with FactoryBot but caused because ActiveRecord saves the association given on the go. This results into an confusing, though properly defined, behavior. Let me try to sketch the steps that should explain this in detail:

FB - account_type { association :account_type, organization: instance } # this runs first from defined Factory
FB - organization#initialize # that's instance initialized for association
FB - account_type#initialize # that's actual association build itself
FB - account_type#save! # FactoryBot tells the ORM to save the association
AR - organization#save # ActiveRecord now creates the association of account first
AR - organization#get_nasa_image # <-- here the execution causes problems
FB - organization#before_create # the expected callback with webmock#stub
FB - organization#save # here an update would occur on the already persisted object

Note that ActiveRecord storing the association is out of the domain of FactoryBot. The behavior of the ORM shouldn't be assumed by the gem and, as you shared with your alternative solution, can be solved in proper ways.

CodeMeister commented 4 days ago

The issue is in the associations. You have Organisation :belongs_to AccountType & you also have AccountType :belongs_to Organisation. These relationships are also referenced in each factory.

So what happens is FactoryBot creates and runs the callbacks on AccountType BEFORE running them on Organisation .... BUT ... because AccountType also references Organisation, the Organisation model is created before the factory callbacks are run.

In practice, both models belonging to each other is a recipe for disaster. If you change AccountType to :have_many organisations it works in the correct order.

With both models using :belongs_to relationships, the callback order is:

1: AccountType Factory After Build
2: AccountType Factory Before Create
3: Organisation Model Before Create
4: Organisation Model After Create
5: AccountType Model Before Create
6: AccountType Model After Create
7: AccountType Factory After Create
8: Organisation Factory After Build
9: Organisation Factory Before Create
10: Organisation Factory After Create

With AcountType changed to :has_many :organizations, the callback order becomes:

1: AccountType Factory After Build
2: AccountType Factory Before Create
3: AccountType Model Before Create
4: AccountType Model After Create
5: AccountType Factory After Create
6: Organisation Factory After Build
7: Organisation Factory Before Create
8: Organisation Model Before Create
9: Organisation Model After Create
10: Organisation Factory After Create

Hope this helps.