Bug #8166: Since r39628 rspec-mock's and_call_original fail with SystemStackError

• Status: Rejected
• Assignee: nagachika (Tomoyuki Chikanaga)
• Target version: 2.1.0
• ruby -v: ruby 2.1.0dev (2013-03-24 trunk 39908) [x86_64-darwin10.8.0]

[nagachika (Tomoyuki Chikanaga)]

Hello,

By r39628 [Feature 8035], Module#ancestors for a singleton class contains singleton class itself.
It introduce a compatibility problem with rspec-mock's RSpec::Mocks::MessageExpectation#and_call_original.

and_call_original uses Module#ancestors to find the original method overridden by a singleton method. Because ancestors contains singleton class itself, `original_method' return stub method and fall into infinite recursive call.

I think hiding singleton class from ancestors is better even when a modules is pretended.

[hsbt (Hiroshi SHIBATA)]

• Assignee set to marcandre (Marc-Andre Lafortune)

marcandre

How do you think about this issue?

r39628 broke Mongoid tests, Please check http://ci.hsbt.org/view/trunk/job/mongoid/85/console

[marcandre (Marc-Andre Lafortune)]

Hi,

nagachika (Tomoyuki Chikanaga) wrote:

I think hiding singleton class from ancestors is better even when a modules is pretended.

Do you believe that only because you want to avoid all compatibility issues, or are there other reasons?

I believe it should be easy to fix the incompatibility of RSpec, for example by going through ancestors and skipping them until you encounter overriden_method.owner. It might even be easier if there was a builtin way to access super as suggested by John Mair [ruby-core:52266, no actual feature request].

I still believe that not hiding the singleton class is both the most consistent and the most helpful. With the advent of prepend, it makes even less sense to hide the singleton class.

Remember that this question arise only if you access the ancestors from the singleton class. I think it is fair to assume that one understands the existence of singleton classes in this case... I would guess that only fairly advanced libraries (e.g. rspec) would do so and they should be able to adapt easily.

I'm not familiar with rspec's code, but I'll have a look later today.

Ultimately, this will be Matz' decision.

[marcandre (Marc-Andre Lafortune)]

I looked at the code of rspec-mock.

I find it telling that the consistent singleton_class.ancestors makes the code much simpler. There is a special case handling that is no longer needed with it, see my pull request for rspec-mock: https://github.com/rspec/rspec-mocks/pull/257

[marcandre (Marc-Andre Lafortune)]

• Status changed from Open to Feedback
• Assignee changed from marcandre (Marc-Andre Lafortune) to nagachika (Tomoyuki Chikanaga)

Further study revealed that and_call_original does not work in some circumstances in Ruby 1.9/2.0 (see https://github.com/rspec/rspec-mocks/pull/258 ).

In Ruby 2.1 this bug is not even present, because of the consistency of singleton_class.ancestors!

I don't think it is easy to fix rspec-mock to pass the test I gave, as a direct consequence of singleton classes ancestors being hidden.

I consider this a good example that the 2.1 behavior is superior, would you not agree?

[nagachika (Tomoyuki Chikanaga)]

• Status changed from Feedback to Rejected

Hello,

Thank you for your investigation.

Do you believe that only because you want to avoid all compatibility issues, or are there other reasons?
Just because for compatibility issues.
And I agree it is more consistent and helpful that the singleton class exists in ancestors in case of self is a singleton class.

My concern was about Module#prepend with a singleton class. How to mock o.m1 in the following example?

module M
def m1
"M#m1"
end
end
o = Object.new
o.singleton_class.module_eval do
def m1
"C#m1"
end
prepend M
end
o.m1 # => "M#m1"
o.ancestors # => [M, #<Class:#Object:0x00000100b8a8e8>, Object, Kernel, BasicObject]

But, it was not the issue from the change made by r39628, and problem of rspec-mock. Sorry, I was confused.

I'll close the ticket in a while.