Issue with AR::Base descendants with certain argument

Hi!

I am trying to upgrade rspec-rails to 1.3.3, and one of my specs fails.

In before :each I stub finder method like this:
Payment.stub!(:find_by_id).with(@payment.id.to_s).and_return @payment
Payment class is derived from AR::Base

And then in one of the examples I call Payment.find_by_id((@payment.id +
1).to_s), and I am expecting it to return null, as it used to with 1.2.9
version, however it leads to an exception in active_record/base.rb.
Analyzing the stack trace I figured that in line 115 of mock/proxy.rb
method
find_by_id is called for ActiveRecord::Base, which is not correct.

I’m not familiar with rspec internals, so I don’t have an idea on where
in
code the actual problem is. So any help would be greatly appreciated.

I have made 2 examples for this and I can’t figure out how to make both
pass. Take a look please:

require ‘spec_helper’

describe “Mock” do
class A
def self.method_missing(name, *args, &block)
a_method_missing
end

def self.present_method(arg)
  '*present_method*'
end

end

class B < A
def self.method_missing(name, *args, &block)
b_method_missing
end
end

it ‘should call derived class method_missing when args differ from
what is
stubbed’ do
B.stub!(:missing_method).with(1).and_return ‘stub
B.missing_method(2).should == ‘b_method_missing
end

it ‘should call present_method when it is defined in parent class and
args
differ from what is stubbed’ do
B.stub!(:present_method).with(1).and_return ‘stub
B.present_method(2).should == ‘present_method
end
end

---------- Forwarded message ----------
From: Alexey I. [email protected]
Date: Thu, Oct 28, 2010 at 12:44 PM
Subject: Issue with AR::Base descendants with certain argument
To: [email protected]

Hi!

I am trying to upgrade rspec-rails to 1.3.3, and one of my specs fails.

In before :each I stub finder method like this:
Payment.stub!(:find_by_id).with(@payment.id.to_s).and_return @payment
Payment class is derived from AR::Base

And then in one of the examples I call Payment.find_by_id((@payment.id +
1).to_s), and I am expecting it to return null, as it used to with 1.2.9
version, however it leads to an exception in active_record/base.rb.
Analyzing the stack trace I figured that in line 115 of mock/proxy.rb
method
find_by_id is called for ActiveRecord::Base, which is not correct.

I’m not familiar with rspec internals, so I don’t have an idea on where
in
code the actual problem is. So any help would be greatly appreciated.

On Oct 28, 7:40am, Alexey I. [email protected] wrote:

end
I am trying to upgrade rspec-rails to 1.3.3, and one of my specs fails.

I’m not familiar with rspec internals, so I don’t have an idea on where in
code the actual problem is. So any help would be greatly appreciated.

The expected behaviour is that when you stub or mock a method on a
real object (we call this partial stubbing/mocking), the framework
takes over that method for the duration of the example. So the
expectation that when the wrong args are received the call is
delegated to the original object is incorrect, and making this change
would introduce potential problems for other users who count on this
behaviour.

There is a request to add an explicit way to call to the original in
rspec-mocks-2’s issue tracker:
http://github.com/rspec/rspec-mocks/issues#issue/23.

HTH,
David

On Mon, Nov 1, 2010 at 8:55 AM, Alexey I.
[email protected] wrote:

David, I got your point, however I find behavior in version 1.3.1
inconsistent. In my opinion, if I stub a method with a certain parameter
value, and then call with another value, it should either delegate to
original method, return nil, or throw an exception. I’d be happy with either
of these. But what it actually does is delegating to the original method in
the superclass, which doesn’t make sense to me at all.

<…>

And also, it behaved differently in 1.2.9. My original problem arisen after
updating to 1.3.1

I have a similar problem here at work. We use a variant of the
rails-settings plugin, which stores configuration settings in a DB
table and relies heavily on a class-level method_missing to fetch
them. So, for example, you could call Settings.welcome_message and
expect to get a string back.

Our specs stub specific methods from this class often, to test how the
application behaves given different values for them. The stubs work
properly the first time, but afterwards they somehow go missing. On
1.3.0, this caused the actual method to be called, which is incorrect,
but went unnoticed as the specs still passed.

On 1.3.1, due to commit 2753b492e00078c92f9fe3b9e879ea83e3536753, the
“missing” stub method is now actually called on the superclass, rather
than on the class itself. So we get lots of failures of the “undefined
method welcome_message for ActiveRecord::Base”.

I tried to reproduce this on rspec alone, but there things seemed to
work perfectly. So it looks like rspec-rails plays a part on this as
well. I’ll see if I can reproduce it there.


Bira


David, I got your point, however I find behavior in version 1.3.1
inconsistent. In my opinion, if I stub a method with a certain parameter
value, and then call with another value, it should either delegate to
original method, return nil, or throw an exception. I’d be happy with
either
of these. But what it actually does is delegating to the original method
in
the superclass, which doesn’t make sense to me at all. This is what my
spec
results to if I comment out the fix in code:

‘stubbing with arguments should call derived class method_missing when
args
differ from what is stubbed and the method is missing’ FAILED
expected: “b_method_missing”,
got: “a_method_missing” (using ==)

Diff:
@@ -1,2 +1,2 @@
-b_method_missing
+a_method_missing

/home/alex/rspec/spec/spec/mocks/stubbing_with_arguments_spec.rb:26:

‘stubbing with arguments should call present method when it is defined
in
derived class and args differ from what is stubbed’ FAILED
expected: “b_present_method”,
got: “a_method_missing” (using ==)

Diff:
@@ -1,2 +1,2 @@
-b_present_method
+a_method_missing

/home/alex/rspec/spec/spec/mocks/stubbing_with_arguments_spec.rb:36:

(see
http://github.com/alexz77/rspec/blob/master/spec/spec/mocks/stubbing_with_arguments_spec.rbfor
the spec code)
And also, it behaved differently in 1.2.9. My original problem arisen
after
updating to 1.3.1

On Sun, Oct 31, 2010 at 3:14 PM, [email protected]

On Mon, Nov 1, 2010 at 8:55 AM, Alexey I.
[email protected] wrote:

David, I got your point, however I find behavior in version 1.3.1
inconsistent. In my opinion, if I stub a method with a certain parameter
value, and then call with another value, it should either delegate to
original method, return nil, or throw an exception.

In RSpec-2 it raises an informative error, so I think this is what we
should do in RSpec-1. Would you like to submit a patch to make it do
that?

On Mon, Nov 1, 2010 at 9:44 AM, Bira [email protected] wrote:

And also, it behaved differently in 1.2.9. My original problem arisen after
application behaves given different values for them. The stubs work
properly the first time, but afterwards they somehow go missing.

Can you post a real example of this complete with spec,
implementation, and failure message?

Thx

On Mon, Nov 1, 2010 at 10:32 AM, David C. [email protected]
wrote:

Can you post a real example of this complete with spec,
implementation, and failure message?

Thx

I’m trying :)! So far it’s been hard to isolate.

Looks like many of the failures I get in my application actually
happen “far away” from the original stub statement. So we stub the
class method on one point of the spec file, and then on a subsequent
unrelated context it gets called by some observer code. Due to the
changes from 2753b492e, the message gets dispatched to
ActiveRecord::Base instead of Settings. I’ll try to reproduce this
within rspec-rails’ own specs.


Bira


In RSpec-2 it raises an informative error, so I think this is what we
should do in RSpec-1. Would you like to submit a patch to make it do
that?

Sure. Attaching a patch. Also submitted a pull request on github -
whatever.

On Mon, Nov 1, 2010 at 10:32 AM, David C. [email protected]
wrote:

Can you post a real example of this complete with spec,
implementation, and failure message?

Thx

This is the closest I could get with my limited knowledge of RSpec’s
and RSpec-rails’s internals. I tried to replicate a bit of
rails-settings’ “fancy footwork” with method_missing, and I use a
before(:all) block here to simulate what happens in my app, where a
stub happens within a normal before block in one point and causes a
similar failure 700 lines later.

require ‘spec_helper’

describe “stubbing a class method” do

class Settings < ActiveRecord::Base
@@defaults = {
“foo” => “a”,
“bar” => “b”
}
def self.method_missing(sym, *args)
super
rescue NoMethodError
@@defaults[sym.to_s]
end
end

before(:all) do
Settings.stub!(:foo).and_return(“c”)
end

specify “works when the stub is invoked for the first time” do
Settings.foo.should == “c”
Settings.bar.should == “b”
end

specify “should not raise an error after the first time” do
lambda { Settings.foo.should == “a”}.should_not raise_exception
end

end

Running this along with RSpec-rails’ other specs gives me the following
failure:

‘stubbing a class method should not raise an error after the first time’
FAILED
expected no Exception, got #<NoMethodError: undefined method `foo’ for
ActiveRecord::Base:Class>
./spec/spec/rails/mocks/bug_report_xxx_spec.rb:30:

I’m not sure if the correct behavior would be to call the actual
implementation of Settings.foo, or to give a different error message.
Currently, it tries to call ActiveRecord::Base.foo .

Not using a before(:all) block prevents the failure, but as I said
it’s only here as a “shorthand” for the complex specs I have on my
app. Actually declaring a .foo class method on Settings also makes it
pass, so method_missing seems to be part of the problem.

On my app’s specs, I found out that calling “unstub!” explicitly on an
after block makes some specs pass as well.


Bira


On Nov 1, 2010, at 4:47 PM, Alexey I. wrote:

In RSpec-2 it raises an informative error, so I think this is what we
should do in RSpec-1. Would you like to submit a patch to make it do
that?

Sure. Attaching a patch. Also submitted a pull request on github - whatever.
<patch.diff>_______________________________________________

Already received and merged the pull request.

Thanks for the contribution!

Cheers,
David

This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.

| Privacy Policy | Terms of Service | Remote Ruby Jobs