Revert to FactoryBot initialization without Rails lifecycle methods (was: Create factories earlier in the lifecycle)

[shageman]

#71 breaks gusto's build. By loading factories earlier in the lifecycle, we see more? full? success.

@matiaseche would you check out if this branch works for your use case?

[matiaseche]

This is exactly what breaks my use case 😢
Does this actually work at Gusto? I mean the loading of the factories file paths.

Here is the output to Rails.application.config.factory_bot.definition_file_paths on a test environment:

On this PR's branch:

On main branch:

[yesthesoup]

Hope you don't mind if I join in on this conversation, I can make a new issue if it makes sense, but I wanted to add on to my custom dir not being loaded (on main as well)

I have it under packs/my_pack/spec/factories/my_factory.rb

I am seeing it in application class's definition_file_paths via pack-rails' integration

> MyService::Application.config.factory_bot.definition_file_paths

=> ["factories", "test/factories", "spec/factories", "packs/my_pack/spec/factories", "packs/my_pack/test/factories"]

BUT the key part seems to be FactoryBot.definition_file_paths does not have it:

> FactoryBot.definition_file_paths
=> [#<Pathname:/Users/me/work/my-service/factories>, #<Pathname:/Users/me/work/my-service/test/factories>, #<Pathname:/Users/me/work/my-service/spec/factories>]

I am currently working around this by manually adding the "packs/my_pack/spec/factories" at the end of config in application.rb (or just test.rb):

config.factory_bot.definition_file_paths << "packs/my_pack/spec/factories"

then it appears in FactoryBot.definition_file_paths

[shageman]

Hm... we're in a bit of a pickle here...

• Main before the after_initialize change worked for gusto but for neither of you. Am I read your comment right, @yesthesoup?
• After the after_initialize change it works for @matiaseche but for neither of us.
• With this change it again works for gusto but for neither of you.

Where to go from here?

[shageman]

What versions of factory bot is everyone on?

[matiaseche]

We could call it twice: on before_configuration and on after_initialize, and then do .uniq to remove duplicates. But I don't like that at all.

Other idea is to add a config parameter to tell when to load FactoryBot.

[matiaseche]

What versions of factory bot is everyone on?

I am using:

• factory_bot 5.1.2
• ruby 3.2.2
• rails 6.1.7.6

[shageman]

factory bot 6.4.2
ruby 3.2.2
rails 7.0.8

[shageman]

@matiaseche in your spec helper, what comes first: loading of factories vs loading of the app environment?

[shageman]

@matiaseche check the Ruby/Rails modularity slack

[shageman]

@yesthesoup would you check whether this sha works for you: 81bc3ff

(without the workaround in test.rb)

[alexevanczuk]

Other idea is to add a config parameter to tell when to load FactoryBot.

@matiaseche I agree with this idea. It's more flexible to different folks' implementations and configuration and is a simple fix.

If a future upstream change makes one or the other implementation no longer practical to maintain (e.g. Rails or factorybot api change), we could release a breaking change that favors one or the other.

Although I'd still be curious why our environments are so different! This configuration affects the point in time at which factory bot is assumed to have loaded all factories, I guess? Could be worth filing an issue question over at factory bot!

Cc @shageman

[yesthesoup]

@shageman same versions as yourself:

factory_bot_rails / factory_bot 6.4.2
ruby 3.2.2
rails 7.0.8

maybe the new behaviour differs based on rails/factory bot major versions

let me try that sha thanks, will update.

[ngan]

Can you paste your Gemfile here? Or where is factory_bot_rails in relation to your rails entry? What does your factory bot gem declaration look like?

[yesthesoup]

Downgrading to 81bc3ff does work!

Here's an abbreviated version of my gemfile:

source 'https://rubygems.org'
ruby '3.2.2'

# Core Gems
# ... aws ...

gem 'bootsnap', require: false
gem 'bullet'
gem 'dogstatsd-ruby'
gem 'dotenv-rails'
gem 'graphql'
gem 'lograge'
gem 'logstash-event'
gem 'logstash-logger'
gem "packs-rails", github: "rubyatscale/packs-rails", ref: "81bc3ff" # was just 'pack-rails` latest before
gem 'pg'
gem 'pry-rails'
gem 'puma'
gem 'rack-timeout', require: 'rack/timeout/base'
gem 'rails', '~> 7.0.1'
gem 'rake'
gem 'rbtrace'
gem 'redis-reconnect_with_readonly'
gem 'request_store'
gem 'rollbar'

# sidekiq gems
...

source 'nexus' do 
# private gems
...
end

group :development, :test do
  gem 'brakeman'
  gem 'bundler-audit'
  gem 'byebug'
  gem 'factory_bot_rails'
  gem 'fakeredis', require: false
  gem 'fuubar'
  gem 'guard'
  gem 'guard-bundler'
  gem 'guard-rspec'
  gem 'guard-rubocop'
  gem 'listen'
  gem 'pry-byebug'
  gem 'rails-erd'
  gem 'rspec'
  gem 'rspec-rails'
  gem 'simplecov'
  gem 'simplecov_json_formatter'
  gem 'timecop'
  gem 'webmock'
  gem 'ws-style'
  gem 'packwerk'
end

spec/support/factory_bot.rb:

RSpec.configure do |config|
  config.include FactoryBot::Syntax::Methods
end

no other factory bot config in application.rb or test.rb

.rspec

--color
--format Fuubar
--require packs/rails/rspec
--require rails_helper

@matiaseche looking at your original PR #71 just wanted to throw it out there that respond_to?(:factory_bot) was initially always false for me as well because I had gem 'factory_bot_rails', require: false in the dev/test gem group. Just in case you have that as well and didn't already think of it.

[matiaseche]

Yes! @yesthesoup that was my problem!
My Gemfile has the following:

  gem 'factory_bot', require: false
  gem 'factory_bot_rails', require: false

If I remove the require: false then it works as expected on this branch 🚀
@shageman I think we are ready to move forward with this change! Thank you both!

[yesthesoup]

@matiaseche awesome! I think this is probably something that could be easily overlooked that should be added to the README, I'll make a PR :)

EDIT: PR #78

[alexevanczuk]

Thanks team for getting to the bottom of this!

[shageman]

Yay! to all the progress. And... I am unsure how to proceed :D

For @yesthesoup the downgraded version (i.e., before initializer stuff at all worked).
For @matiaseche what worked? This PR, yes! The downgrade?

If both work for all of us now... which one is better? I am leaning towards removing the initializer code (i.e., "downgrade" rather then this PR) because it is simpler.

Given that we know how to fix in all situations, I won't proceed before your feedback.

Thanks all!

[shageman]

Oh wait... I had pushed another change onto this branch that is effectively the downgrade... Since that's now confirmed to be working for folks, I will merge this.