Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

6.3.0: NoMethodError: undefined method '<' for nil:NilClass when using a factory with traits #431

Closed
tagliala opened this issue Nov 17, 2023 · 20 comments · Fixed by thoughtbot/factory_bot#1600, thoughtbot/factory_bot#1601 or sanger/traction-service#1148
Labels

Comments

@tagliala
Copy link

tagliala commented Nov 17, 2023

Description

After upgrading to 6.3.0, specs with traits started failing.

Downgrade to 6.2.0 works.

The problem seems to be triggered here:

if for_class < ActiveRecord::Base && for_class.primary_key == attribute.name.to_s

for_class is nil because the payload contains class: nil

Reproduction Steps

Please checkout the branch https://github.com/diowa/ruby3-rails7-bootstrap-heroku/tree/tmp/factory-bot-rails-issue

and run bundle exec rspec spec/model/user_spec.rb

Relevant commit: diowa/ruby3-rails7-bootstrap-heroku@3c83903

Expected behavior

Objects being created as 6.2.0

Actual behavior

       expected no Exception, got #<NoMethodError: undefined method `<' for nil:NilClass> with backtrace:
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot_rails-6.3.0/lib/factory_bot_rails/factory_validator/active_record_validator.rb:7:in `block in validate!'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/attribute_list.rb:19:in `each'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/attribute_list.rb:19:in `each'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot_rails-6.3.0/lib/factory_bot_rails/factory_validator/active_record_validator.rb:6:in `validate!'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot_rails-6.3.0/lib/factory_bot_rails/factory_validator.rb:27:in `block (2 levels) in rails_6_0_support'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot_rails-6.3.0/lib/factory_bot_rails/factory_validator.rb:27:in `each'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot_rails-6.3.0/lib/factory_bot_rails/factory_validator.rb:27:in `block in rails_6_0_support'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/definition.rb:61:in `compile'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/definition.rb:169:in `aggregate_from_traits_and_self'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/definition.rb:36:in `to_create'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/trait.rb:17:in `to_create'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/definition.rb:174:in `map'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/definition.rb:174:in `aggregate_from_traits_and_self'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/definition.rb:36:in `to_create'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/definition_hierarchy.rb:6:in `build_from_definition'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/factory.rb:125:in `build_hierarchy'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/factory.rb:88:in `compile'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/factory.rb:32:in `run'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/factory_runner.rb:29:in `block in run'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/factory_runner.rb:28:in `run'
         # ~/.rvm/gems/ruby-3.2.2/gems/factory_bot-6.4.0/lib/factory_bot/strategy_syntax_method_registrar.rb:28:in `block in define_singular_strategy_method'
         # ./spec/model/user_spec.rb:8:in `block (3 levels) in <top (required)>'
         # ./spec/model/user_spec.rb:7:in `block (2 levels) in <top (required)>'
         # ~/.rvm/gems/ruby-3.2.2/gems/webmock-3.19.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'

System configuration

factory_bot_rails version: 6.3.0
factory_bot version: 6.4.0
rails version: 7.1.2
ruby version: 3.2.2

@tagliala tagliala added the bug label Nov 17, 2023
@guperrot
Copy link

We have the same issue too, preventing us from upgrading the lib.
We are trying to call a method defined in a factory in another file:

FactoryBot.define do
  factory :bbb do
    aaa { create :aaa, status: "enabled" }

And we get the same error at the aaa line, which was working fine in 6.2.0.

factory_bot_rails version: 6.4.0
factory_bot version: 6.4.0
rails version: 7.1.2
ruby version: 3.1.3

@troya2
Copy link

troya2 commented Nov 19, 2023

I don't have a full grasp on what's happening, but I have traced the code and this happens when the factory_bot.compile_factory notification is sent from FacyoryBot with a nil class, which occurs when FactoryBot is sends this notification for defined traits. The ActiveRecordValidator class's validate! method processes the event and assumes class will not be nil and throws this exception.

makicamel added a commit to makicamel/factory_bot that referenced this issue Nov 20, 2023
andyundso added a commit to simplificator/renovate-preset that referenced this issue Nov 20, 2023
There is an issue in the newest release with traits:
thoughtbot/factory_bot_rails#431
thoughtbot/factory_bot#1600

as we use this library in many projects, I suggest to add a separate group for it in the preset until the bug is fixed, so we can continue to merge the non-major dependencies for this week.
andyundso added a commit to simplificator/renovate-preset that referenced this issue Nov 20, 2023
There is an issue in the newest release with traits:
thoughtbot/factory_bot_rails#431
thoughtbot/factory_bot#1600

as we use this library in many projects, I suggest to add a separate group for it in the preset until the bug is fixed, so we can continue to merge the non-major dependencies for this week.
@gaffneyc
Copy link

Just tried updating to FactoryBot to 6.4.1 and I'm still seeing this issue. The best I can figure out is that it happens the first time we create a factory which has a trait with the same name as an attribute (e.g. create(:user, :admin) where the User class has an admin trait and an admin column).

@mike-burns mike-burns reopened this Nov 20, 2023
@davidrunger
Copy link

FWIW, bumping to factory_bot (6.4.1) and factory_bot_rails (6.4.0) did resolve this issue on my repo. Thanks for the quick (partial?) fix!

thomasleese added a commit to DFE-Digital/apply-for-qualified-teacher-status that referenced this issue Nov 21, 2023
This fixes an conflict between factory_bot and factory_bot_rails:
thoughtbot/factory_bot_rails#431
@mudge
Copy link

mudge commented Nov 21, 2023

After upgrading to factory_bot 6.4.1 and factory_bot_rails 6.4.0, I'm still seeing the undefined method < for nil:NilClass error from lib/factory_bot_rails/factory_validator/active_record_validator.rb:7:in block in validate! when using a mixin trait, e.g.

# In the factory definition...
FactoryBot.define do
  factory :magazine do
    # ...
  end

  trait :with_latest_issue do
    # ...
  end
end

# And then in a spec...
create(:magazine, :with_latest_issue)

Adding a debugger after line 5 of the ActiveRecordValidator shows that the payload Hash passed to validate!(payload) for the :with_latest_issue trait has the :class key set to nil.

@sledge909
Copy link

FWIW, bumping to factory_bot (6.4.1) and factory_bot_rails (6.4.0) did resolve this issue on my repo. Thanks for the quick (partial?) fix!

This worked for me too 👍

@gmmcal
Copy link

gmmcal commented Nov 21, 2023

Bumping factory_bot to 6.4.1 also fixed for me on gmmcal/gmmcal.com.br#1129

@mike-burns
Copy link
Contributor

Note that due to a US holiday this week, and the family + travel leading up to and around that, I won't be able to debug this myself until December. But if someone opens a PR with a test, I can merge and release that.

@tagliala
Copy link
Author

tagliala commented Nov 21, 2023

I cannot replicate the failure mentioned in the opening post with 6.4.1.

the first time we create a factory which has a trait with the same name as an attribute (e.g. create(:user, :admin) where the User class has an admin trait and an admin column).

Literally the opening post (active trait with active field: diowa/ruby3-rails7-bootstrap-heroku@3c83903), but it works with 6.4.1

I've also tried enum's implicit traits and they worked

@PedroSena
Copy link

I'm noticing a weird behavior locally, on my first try it fails, on subsequent tries it works. This for factory_bot 6.4.1 and factory_bot_rails 6.4.0, this happens on rails console.

@tagliala
Copy link
Author

tagliala commented Nov 21, 2023

I'm noticing a weird behavior locally, on my first try it fails, on subsequent tries it works. This for factory_bot 6.4.1 and factory_bot_rails 6.4.0, this happens on rails console.

Thanks for the clarification!

Confirmed it happening in the console

Replicable on the opening post application:

rails c
Loading development environment (Rails 7.1.2)
[1] pry(main)> FactoryBot.create :user, :active
NoMethodError: undefined method `<' for nil:NilClass

EDIT

Still can't replicate on 6.4.1

$ rails c
Loading development environment (Rails 7.1.2)
[1] pry(main)> FactoryBot::VERSION
=> "6.4.1"
[2] pry(main)> FactoryBot.create :user, :active
  TRANSACTION (0.2ms)  BEGIN
  User Create (1.8ms)  INSERT INTO "users" ("name", "active", "created_at", "updated_at") VALUES ($1, $2, $3, $4) RETURNING "id"  [["name", "User"], ["active", true], ["created_at", "2023-11-21 17:14:35.732503"], ["updated_at", "2023-11-21 17:14:35.732503"]]
  TRANSACTION (0.6ms)  COMMIT

Do you have something like spring which may have cached old gems?

@PedroSena
Copy link

PedroSena commented Nov 21, 2023

I do have spring, but I'm able to replicate the issue even when I remove it. One difference that I noticed is the rails version.
I'm on Rails 6.1.7.6. I'll try to isolate the problem with a standalone rails 6 app and update here soon

[1] pry(main)> FactoryBot.create(:pipeline_run_step_result, :discard)
NoMethodError: undefined method `<' for nil:NilClass
from /Users/psena/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/factory_bot_rails-6.4.0/lib/factory_bot_rails/factory_validator/active_record_validator.rb:7:in `block in validate!'
[2] pry(main)> FactoryBot::VERSION
=> "6.4.1"
[3] pry(main)> Rails.version
=> "6.1.7.6"

EDIT

I created a new rails 6.1.7.6 app but was unable to replicate the issue there, I'll dig deeper into my app in order to better understand if some other gem is affecting this behavior. I tried to remove spring and bootsnap but no luck so far

@tagliala
Copy link
Author

I've also tried to replicate with 6.1.7.6 without success. At the beginning it was failing, but I opened a new bash console and it stopped. I run it the first time with Spring

tagliala added a commit to diowa/ruby3-rails6-bootstrap-heroku that referenced this issue Nov 21, 2023
@Petercopter
Copy link

This is failing consistently for me locally and in the CI, but what's interesting is that the same factory with the same attributes/traits is being used in multiple tests, but only some of them are failing. I haven't found the rhyme or the reason yet.

@gaffneyc
Copy link

When I tested it out it appeared to be the first time a factory was created with a given trait or set of arguments so it would move around based on test execution order.

makicamel added a commit to makicamel/factory_bot that referenced this issue Nov 21, 2023
To pass their class to ActiveSuport::Notifications
Re-Fix thoughtbot/factory_bot_rails#431
@elia
Copy link

elia commented Nov 22, 2023

@mike-burns @tagliala #437 is a hot fix, maybe until thoughtbot/factory_bot#1601 is merged, even if #437 is not merged the spec in consistently fails without the patch and might be worth adding as regression avoidance.

elia added a commit to solidusio/solidus that referenced this issue Nov 22, 2023
elia added a commit to solidusio/solidus that referenced this issue Nov 22, 2023
elia added a commit to solidusio/solidus that referenced this issue Nov 22, 2023
elia added a commit to solidusio/solidus that referenced this issue Nov 22, 2023
elia added a commit to solidusio/solidus that referenced this issue Nov 22, 2023
@mike-burns
Copy link
Contributor

mike-burns commented Nov 22, 2023

Thank you to @makicamel for stepping up with a second fix, with great tests. This fix is released as 6.4.2.

chrisroos added a commit to alphagov/signon that referenced this issue Nov 22, 2023
Although top level traits (mixins) are allowed according to the docs[1],
they appear to be broken in factory bot 6.4.0[2].

We were seeing the following error:

NoMethodError: undefined method `<' for nil:NilClass
  if for_class < ActiveRecord::Base && for_class.primary_key == attribute.name.to_s

When running either of the tests that used the
`with_expired_confirmation_token` trait:

- test/controllers/account/emails_controller_test.rb:99
- test/controllers/users/emails_controller_test.rb:277

Given that we're only using this trait in User factories I've moved it
inside the User factory definition so that we can upgrade.

[1]: https://thoughtbot.github.io/factory_bot/traits/mixins.html
[2]: thoughtbot/factory_bot_rails#431
chrisroos added a commit to alphagov/signon that referenced this issue Nov 22, 2023
Although top level traits (mixins) are allowed according to the docs[1],
they appear to be broken in factory bot 6.4.0[2].

We were seeing the following error:

NoMethodError: undefined method `<' for nil:NilClass
  if for_class < ActiveRecord::Base && for_class.primary_key == attribute.name.to_s

When running the tests that used the `in_organisation` trait.

Given that we're only using this trait in User factories I've moved it
inside the User factory definition so that we can upgrade.

[1]: https://thoughtbot.github.io/factory_bot/traits/mixins.html
[2]: thoughtbot/factory_bot_rails#431
@rnestler
Copy link

This fix is released as 6.4.2.

Where can we find this 6.4.2 release? The latest tag on GitHub and version on rubygems.org is 6.4.0.

@mike-burns
Copy link
Contributor

Sorry, the 6.4.2 release of factory_bot, which factory_bot_rails depends on. If you bundle update factory_bot it should pull in the latest dependency.

@rnestler
Copy link

Sorry, the 6.4.2 release of factory_bot, which factory_bot_rails depends on. If you bundle update factory_bot it should pull in the latest dependency.

Ah this would actually have been clear if I read the comments above. It works now fine again with factory_bot 6.4.2 and factory_bot_rails 6.4.0, thanks 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment