Skip to content

FactoryGirl::Declaration subclass == method assumes object of same type #440

Closed
wingrunr21 opened this Issue Sep 27, 2012 · 3 comments

2 participants

@wingrunr21

When FactoryGirl is used with activerecord-oracle_enhanced-adapter, you get the following errors on test runs:

undefined method `name' for "":String
@ vendor/bundle/ruby/1.9.1/gems/factory_girl-4.1.0/lib/factory_girl/declaration/implicit.rb:11:in `=='
vendor/bundle/ruby/1.9.1/gems/activerecord-oracle_enhanced-adapter-1.4.1/lib/active_record/connection_adapters/oracle_enhanced_dirty.rb:20:in `field_changed?'
vendor/bundle/ruby/1.9.1/gems/activerecord-3.0.17/lib/active_record/attribute_methods/dirty.rb:57:in `write_attribute'
vendor/bundle/ruby/1.9.1/gems/activerecord-3.0.17/lib/active_record/attribute_methods/write.rb:14:in `product_id='

The issue stems from the Oracle adapter needing to nil the value of a string column if the new value is an empty string (line 20 of oracle_enhanced_dirty.rb).

It appears FactoryGirl::Declaration::Implicit is making the assumption that any comparison object will be of type FactoryGirl::Declaration::Implicit as well. It appears other subclasses of FactoryGirl::Declaration also make this assumption.

I believe a type-check should be performed first:

def ==(other)
  other.is_a?(self.class) &&
    name == other.name &&
    factory == other.factory &&
    ignored == other.ignored
end
@joshuaclayton
thoughtbot, inc. member

Could you provide a bit more of a backtrace and a test that exhibits this failure? I'm not sure why the implicit instance would've been passed to field_changed? in the first place; it should convert the declaration to the "real" value before attempting to be persisted. Thanks!

@wingrunr21

Here is a full backtrace:

https://gist.github.com/3795200

The relevant portion of that factory is:

product_fulfillment_types do |pft|
  [
    pft.association(:product_fulfillment_type, :product_id => productid, :fulfillment_method_id => digital_fulfillment_method.id, :strategy => :build),
    pft.association(:screen_printing_product_fulfillment_type, :product_id => productid, :fulfillment_method_id => screen_fulfillment_method.id, :strategy => :build)
  ]
end

Upon reviewing that factory, it appears it did not get updated when we upgraded FactoryGirl and should be like this:

product_fulfillment_types do |pft|
  [
    pft.association(:product_fulfillment_type, :product_id => pft.productid, :fulfillment_method_id => pft.digital_fulfillment_method.id, :strategy => :build),
    pft.association(:screen_printing_product_fulfillment_type, :product_id => pft.productid, :fulfillment_method_id => pft.screen_fulfillment_method.id, :strategy => :build)
  ]
end

That does indeed fix the problem of the implicit being passed instead of the "real" value.

I am still of the opinion that other should be type-checked, however, it appears this is not a real issue with FactoryGirl. Feel free to close. Thanks for your help.

@joshuaclayton
thoughtbot, inc. member

Glad it's working!

I'm going to leave the type-checking out; FactoryGirl::Declaration::Implicit is a private class and won't get interacted with outside of FG, except in cases where there's something incorrect going on. It allows us to remain decoupled from the class itself and rely on interface only, which is what we really want here. Thanks again, and best of luck!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.