Skip to content

Strange DSL behavior in ruby-1.9.3-preview1 #167

Closed
chaffeqa opened this Issue Aug 1, 2011 · 15 comments

5 participants

@chaffeqa
chaffeqa commented Aug 1, 2011

As I commented in the previous issue, I am noting some strange behavior in ruby 1.9.3.

The specs ran fine under 1.9.2, but when i ran them under 1.9.3 I got:

gist

(Note the factories.rb syntax is attached)

After checking out lib/factory_girl/syntax/default.rb, I noted that your DSL desires different syntax for my factories.rb file, so I went about implementing that:

gist

After noting the DEPRECATION WARNING: Change Factory.build to FactoryGirl.build and changing my test syntaxes, I ran into the problem of failing tests due to:

Failure/Error: @www_site = FactoryGirl(:site)
NoMethodError:
undefined method `FactoryGirl' for #<RSpec::Core::ExampleGroup::Nested_3::Nested_1:0x00000005d80540>

At that point I decided to cut my losses and switch back to passing tests and 1.9.2 :(

I would love to help in any way possible, and I think I'm going to dive further into FG's innards to try and unearth the issue. Let me know if theres anything I should know (like: DSL in the process of a change... ect)

@chaffeqa
chaffeqa commented Aug 2, 2011

Ok I think I figured it out... It seems that the method_missing call in your depricated.rb uses *args. So when FactoryGirl.send(:define,*args, &block) is called, I'm guessing ruby-1.9.3 doesn't ignore an empty *args... hence the to many variables error.

So this seems to indeed be a 1.9.3 bug, not a FG bug. I'll report it to them, and I don't know if/how you wish to deal with it on your end... monkey-patch ftw? :P

@chaffeqa chaffeqa closed this Aug 2, 2011
@chaffeqa chaffeqa reopened this Aug 2, 2011
@chaffeqa
chaffeqa commented Aug 2, 2011

Well after diving much deeper in, I found that ruby-1.9.3 was actually not finding the Factory#define method with the vintage syntax... I was unable to figure out why that was, and its late... and I'm going to look like a zombie in the morning.

So in summary:

  • 1.9.3 loses the definition of Factory#define and, via Factory.method_missing invokes FactoryGirl#define which errors.

Beeeeeed

@joshuaclayton
thoughtbot, inc. member

This looks to be a bug in how 1.9.3 handles module inclusion, scope, and method_missing. Here's a gist: https://gist.github.com/1119516 Where method missing is called, it's because the module Another, while included and referencing Something::Another, doesn't find the method when it should.

The same thing is happening here: Factory.define has been defined already (within the vintage syntax file), accepting two arguments, and FactoryGirl.define accepts a named block. method_missing is getting called erroneously on the Factory module, even though define is defined on the module, and calling FactoryGirl.define anyways, which has a different method signature and the reason you're seing 2 for 0 arguments.

I'd encourage you to file a ticket with them and point them to this thread in case they have any insight.

Thanks.

@gnufied
gnufied commented Aug 3, 2011

Hit by the same issue today, have filed a bug with ruby-core, http://redmine.ruby-lang.org/issues/5154

@stepheneb

It also doesn't work with Ruby 1.9.2 p303 (rev r32917) and higher

rev 32916 on the ruby_1_9_2 branch works.

see: backport #3422 into Ruby 1.9.2

@dgm
dgm commented Jun 30, 2012

I'm running ruby 1.9.2p0 (2010-08-18 revision 29036) [x86_64-darwin11.0.1] and get this error:

     Failure/Error: let(:p1) { FactoryGirl(:person) }
     NoMethodError:
       undefined method `FactoryGirl' for #<RSpec::Core::ExampleGroup::Nested_1::Nested_1:0x007fdb1bcf9358>
@gnufied
gnufied commented Jun 30, 2012

@dgm I think 1.9.2-p0 is a unsupported ruby version, even by ruby-core team.

@dgm
dgm commented Jul 1, 2012

I just upgraded my app to ruby 1.9.3-p194, and it appears to have the same error

@joshuaclayton
thoughtbot, inc. member

@dgm FactoryGirl should work fine in the latest patchlevel of 1.9.3. Can you paste the full trace you're seeing? Thanks!

@dgm
dgm commented Jul 1, 2012

In the rails console:

1.9.2p0 :001 > Factory(:person)
   (0.2ms)  BEGIN
  SQL (0.3ms)  INSERT INTO `people` (`common_name`, `created_at`, `dob`, `dod`, `dup_of_id`, `first_name`, `gender`, `last_name`, `maiden_name`, `metaphone_name`, `middle_name`, `needs_review`, `primary_email_id`, `race`, `re_id`, `review_requested_by`, `soundex_first_name`, `soundex_last_name`, `suffix`, `title`, `tshirt`, `updated_at`, `updated_by`) VALUES (NULL, '2012-07-01 15:36:32', NULL, NULL, NULL, 'John', NULL, 'Doe', NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, '2012-07-01 15:36:32', 1)
   (0.4ms)  SELECT MAX(`audits`.`version`) AS max_id FROM `audits` WHERE ((`audits`.`auditable_id` = 12342 AND `audits`.`auditable_type` = 'Person'))
  SQL (0.3ms)  INSERT INTO `audits` (`action`, `auditable_id`, `auditable_type`, `audited_changes`, `comment`, `created_at`, `remote_address`, `user_id`, `user_type`, `username`, `version`) VALUES ('create', 12342, 'Person', '---\ntitle: !!null \nfirst_name: John\nmiddle_name: !!null \nlast_name: Doe\ncommon_name: !!null \nsuffix: !!null \ndob: !!null \ngender: !!null \ntshirt: !!null \nre_id: !!null \nmaiden_name: !!null \nrace: !!null \ndod: !!null \ndup_of_id: !!null \nupdated_by: 1\nsoundex_first_name: !!null \nsoundex_last_name: !!null \nmetaphone_name: !!null \nprimary_email_id: !!null \nneeds_review: !!null \nreview_requested_by: !!null \n', NULL, '2012-07-01 15:36:32', NULL, NULL, NULL, NULL, 1)
   (0.6ms)  COMMIT
 => #<Person id: 12342, title: nil, first_name: "John", middle_name: nil, last_name: "Doe", common_name: nil, suffix: nil, dob: nil, gender: nil, tshirt: nil, updated_at: "2012-07-01 15:36:32", created_at: "2012-07-01 15:36:32", re_id: nil, maiden_name: nil, race: nil, dod: nil, dup_of_id: nil, updated_by: 1, soundex_first_name: nil, soundex_last_name: nil, metaphone_name: nil, primary_email_id: nil, needs_review: nil, review_requested_by: nil> 
1.9.2p0 :002 > FactoryGirl(:person)
NoMethodError: undefined method `FactoryGirl' for main:Object
    from (irb):2
    from /Users/davidmorton/workspace/cumission/vendor/ruby/1.9.1/gems/railties-3.2.6/lib/rails/commands/console.rb:47:in `start'
    from /Users/davidmorton/workspace/cumission/vendor/ruby/1.9.1/gems/railties-3.2.6/lib/rails/commands/console.rb:8:in `start'
    from /Users/davidmorton/workspace/cumission/vendor/ruby/1.9.1/gems/railties-3.2.6/lib/rails/commands.rb:41:in `<top (required)>'
    from script/rails:6:in `require'
    from script/rails:6:in `<main>'

I do see a constant defined:

FactoryGirl.class
 => Module 
@dgm
dgm commented Jul 1, 2012

It appears I can also do: FactoryGirl.Factory(:person)

@dgm
dgm commented Jul 1, 2012

Sorry for all the noise ... config.include FactoryGirl::Syntax::Methods also works well, and is more readable in specs anyway.

@joshuaclayton
thoughtbot, inc. member

@dgm hey, no worries! Using FactoryGirl(:person) doesn't actually work at all, as you've just seen. The Factory(:person) syntax is considered "vintage" (in that it was used in FG 1.x but deprecated in 2.x) and we never supported the other version ever; so, that's why you were seeing the NoMethodError. Re: including the FactoryGirl::Syntax::Methods, that's definitely your best shot and it'll end up being less noisy in your tests!

@dgm
dgm commented Jul 2, 2012

Ahhh I see. There are way too many tutorials, blogs, and SO questions that are out of date. :( Going from using Factory(:user) and getting a depreciation waring to use FactoryGirl instead, I assumed it meant FactoryGirl(:user). Could that depreciation message be altered a bit or point to a url for more information? I'll submit a patch if you want. :)

@joshuaclayton
thoughtbot, inc. member

The deprecation warning actually does tell the user to use FactoryGirl.create(:name): https://github.com/thoughtbot/factory_girl/blob/master/lib/factory_girl/syntax/vintage.rb#L123 So, don't worry about a pull request, it should be all set!

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.