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

Mock#to_ary and Array#flatten problem... #31

Closed
lardawge opened this issue Nov 23, 2010 · 22 comments
Closed

Mock#to_ary and Array#flatten problem... #31

lardawge opened this issue Nov 23, 2010 · 22 comments

Comments

@lardawge
Copy link

No better way to describe this then:
http://floehopper.lighthouseapp.com/projects/22289-mocha/tickets/70

Ran into this while trying to run specs for Formtastic in 1.9.2. Problem with just stubbing it is it then breaks other tests because rails uses respond_to?(:to_ary) internally which breaks other tests in formtastic...

Rspec response:
Mock "user" received unexpected message :to_ary with (no args)
# ./lib/formtastic.rb:575:in flatten' # ./lib/formtastic.rb:575:ininputs_for_nested_attributes'
# ./lib/formtastic.rb:280:in inputs' # ./spec/inputs_spec.rb:439:inblock (6 levels) in <top (required)>'
# ./lib/formtastic.rb:1927:in block in semantic_form_for' # ./lib/formtastic.rb:1892:inwith_custom_field_error_proc'
# ./lib/formtastic.rb:1926:in semantic_form_for' # ./spec/inputs_spec.rb:438:inblock (5 levels) in <top (required)>'

Let me know what else is needed...

@lardawge
Copy link
Author

@dchelimsky, Have you had a chance to look at this? Even a work around would be nice...

Thanks

@dchelimsky
Copy link
Contributor

Not yet. Not sure when I will. Big backlog and it's the holidays.

@dchelimsky
Copy link
Contributor

@lardawge Do you have a solution in mind?

@lardawge
Copy link
Author

I don't have a solution for fixing rspec-mocks although there is a great suggestion by James Mead in that lighthouse ticket (the direction mocha is headed with a fix).

I was able to get the specs for formtastic passing by using an activemodel object which responds correctly to to_ary...

@medihack
Copy link

medihack commented Feb 2, 2011

I have the same problem when testing my controllers that use the authorize method of CanCan which seems to call flatten on an array of mocks models then.
Is there a complete solution in sight yet? For the moment I stub my mock model with :to_ary => nil as a workaround.

@AgileMantis
Copy link

Stubbing to_ary worked for me as well. I'm not senior enough to propose an overall solution, but couldn't we add Mock#to_ary which just returns nil?

@dchelimsky
Copy link
Contributor

What about returning [self]?

@kaiwren
Copy link
Contributor

kaiwren commented Mar 16, 2011

Returning [self] makes sense to me. Is this something small I could patch if everyone agrees that this is the way to go?

@amw
Copy link

amw commented Mar 24, 2011

You can't return [self]. flatten proceeds to array contents recursively. Returning [self] creates infinite loop.

@dchelimsky
Copy link
Contributor

A similar problem just came up with to_hash (#51). I don't think there is a one-size-fits-all solution here for implementing to_ary or to_hash. I'm going to close this as I don't see it as a problem for a mocking framework to solve - if you're going to use one, then you need to stub the methods the real object needs, even if they're deep in the API.

Now we could conceivably solve this for rails-specific problems in the context of rspec-rails, but even then I'm not sure how we can do it in a general way. If anybody has any ideas, please post them to the rspec-rails tracker.

@amw
Copy link

amw commented Apr 17, 2011

The problem is that mocked objects don't behave like a regular Object would. When flatten goes over an object it calls to_ary and the object throws NoMethodError. That's a desired behavior. API sees this and considers the object flat. The API doesn't expect the object to implement to_ary - it expects it to throw NoMethodError after a call to it. Mock objects throw an incorrect error. Consider this:

~$irb --version
irb 0.9.6(09/06/30)
~$irb
irb(main):001:0> o = Object.new
=> #<Object:0x00000101097d18>
irb(main):002:0> o.to_ary
NoMethodError: undefined method `to_ary' for #<Object:0x00000101097d18>
    from (irb):2
    from /opt/local/bin/irb:12:in `<main>'
irb(main):003:0> [o].flatten
=> [#<Object:0x00000101097d18>]

@dchelimsky
Copy link
Contributor

Perhaps we implement it to raise NoMethodError, even with as_null_object. I'm open to this assuming it solves the problem without raising new problems. WDYT?

@dchelimsky
Copy link
Contributor

I tried implementing to_ary thusly:

def to_ary
  raise NoMethodError
end

The following failed with the NoMethodError:

object = double('foo')
[object].flatten.should eq([object])

I then implemented it thusly:

def to_ary
  nil
end

Then the same example passed.

This suggests to me that the better choice is to return nil. It solves the flatten problem, and can always be overridden explicitly with object.stub(:to_ary).and_return(:some_other_value).

I'm going to go ahead and implement it this way, but I'd welcome any additional feedback or concerns about this approach.

@dchelimsky dchelimsky reopened this Apr 17, 2011
@amw
Copy link

amw commented Apr 17, 2011

I have gone through the same tests you did before. You can't raise NoMethodError from within to_ary. API expects NoMethodError during the call to to_ary. Those are two different things. One says "to_ary doesn't exist", the other says "to_ary is implemented incorrectly".

@dchelimsky
Copy link
Contributor

So are you saying that method_missing should just delegate to super if the method is to_ary?

@amw
Copy link

amw commented Apr 17, 2011

There two things you can do. Throw NoMethodError from method_missing or return nil from to_ary. For the first you can check this patch of mine for inherited_resources gem:

activeadmin/inherited_resources@c4468f4

@dchelimsky
Copy link
Contributor

Returning nil seems simpler and I've already got it working. Do you see any downside?

@amw
Copy link

amw commented Apr 17, 2011

Nope.

@amw
Copy link

amw commented Apr 17, 2011

Well, one. It's a bit less like a regular Object - which takes the other route. But right now I can't see this causing problems. We can always adjust if it turns out otherwise.

@dchelimsky
Copy link
Contributor

Actually, the NoMethodError implementation was easy to convert to (I was missing a step before), so let's go with that. I'd rather have it loudly cause a problem then do so quietly.

@alindeman
Copy link
Contributor

I think you need to add the new to_ary.feature to features/.nav

@dchelimsky
Copy link
Contributor

Done. Thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants