rspec users - Oct 2010 - 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rspec-users/attachments/20101028/5b5564b8/attachment-0001.html>

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 Ilyichev <bluesman.alex at gmail.com>
Date: Thu, Oct 28, 2010 at 12:44 PM
Subject: Issue with AR::Base descendants with certain argument
To: rspec-users at rubyforge.org


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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rspec-users/attachments/20101028/dd5fb489/attachment.html>

On Oct 28, 7:40?am, Alexey Ilyichev <bluesman.a... at gmail.com>
wrote:
> 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 Ilyichev <bluesman.a... at gmail.com>
> Date: Thu, Oct 28, 2010 at 12:44 PM
> Subject: Issue with AR::Base descendants with certain argument
> To: rspec-us... at rubyforge.org
>
> 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.
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

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:

1)
''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:

2)
''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, dchelimsky at gmail.com
<dchelimsky at gmail.com>wrote:
>
>
> On Oct 28, 7:40 am, Alexey Ilyichev <bluesman.a... at gmail.com>
wrote:
> > 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 Ilyichev <bluesman.a... at gmail.com>
> > Date: Thu, Oct 28, 2010 at 12:44 PM
> > Subject: Issue with AR::Base descendants with certain argument
> > To: rspec-us... at rubyforge.org
> >
> > 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.
>
> 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
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rspec-users/attachments/20101101/5d13fae5/attachment.html>

On Mon, Nov 1, 2010 at 8:55 AM, Alexey Ilyichev <bluesman.alex at
gmail.com> 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
http://compexplicita.wordpress.com
http://compexplicita.tumblr.com

On Mon, Nov 1, 2010 at 8:55 AM, Alexey Ilyichev <bluesman.alex at
gmail.com> 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?
> 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:
>
> 1)
> ''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:
>
> 2)
> ''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.rb
> for 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, dchelimsky at gmail.com <dchelimsky at
gmail.com>
> wrote:
>>
>>
>> On Oct 28, 7:40?am, Alexey Ilyichev <bluesman.a... at gmail.com>
wrote:
>> > 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 Ilyichev <bluesman.a... at gmail.com>
>> > Date: Thu, Oct 28, 2010 at 12:44 PM
>> > Subject: Issue with AR::Base descendants with certain argument
>> > To: rspec-us... at rubyforge.org
>> >
>> > 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.
>>
>> 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
>> _______________________________________________
>> rspec-users mailing list
>> rspec-users at rubyforge.org
>> http://rubyforge.org/mailman/listinfo/rspec-users
>
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On Mon, Nov 1, 2010 at 9:44 AM, Bira <u.alberton at gmail.com>
wrote:
> On Mon, Nov 1, 2010 at 8:55 AM, Alexey Ilyichev <bluesman.alex at
gmail.com> 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.
Can you post a real example of this complete with spec,
implementation, and failure message?

Thx
> 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
> http://compexplicita.wordpress.com
> http://compexplicita.tumblr.com
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>

On Mon, Nov 1, 2010 at 10:32 AM, David Chelimsky <dchelimsky at gmail.com>
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
http://compexplicita.wordpress.com
http://compexplicita.tumblr.com

On Mon, Nov 1, 2010 at 10:32 AM, David Chelimsky <dchelimsky at gmail.com>
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:

1)
''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
http://compexplicita.wordpress.com
http://compexplicita.tumblr.com

> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rspec-users/attachments/20101101/0a992194/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.diff
Type: text/x-patch
Size: 3377 bytes
Desc: not available
URL:
<http://rubyforge.org/pipermail/rspec-users/attachments/20101101/0a992194/attachment-0001.bin>

On Nov 1, 2010, at 4:47 PM, Alexey Ilyichev 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://rubyforge.org/pipermail/rspec-users/attachments/20101101/05313b6e/attachment.html>