[rspec-devel] [ rspec-Bugs-12845 ] Typo when using instance variables in specs leads to setting expectations on the nil object

[noreply at rubyforge.org]

Bugs item #12845, was opened at 2007-08-06 11:04
You can respond by visiting: 
http://rubyforge.org/tracker/?func=detail&atid=3149&aid=12845&group_id=797

Category: mock module
Group: None
Status: Closed
Resolution: Rejected
Priority: 3
Submitted By: Ashley Moran (ashley_moran)
Assigned to: David Chelimsky (dchelimsky)
Summary: Typo when using instance variables in specs leads to setting expectations on the nil object

Initial Comment:
Just spent ages bashing my head against a wall on this, wondering why my spec was failing.  Basically summed up in this example (not the shortest but similar to my code):

class Computer
  attr_reader :power_indicator
  
  def initialize(power_indicator)
    @power_indicator = power_indicator
  end
  
  def turn_off
    power_indicator.dim
  end
end

describe Computer, "created with a PowerIndicator" do
  before(:each) do
    @power_indicator = mock("PowerIndicator")
    @computer = Computer.new(@power_indicator)
  end
  
  it "should dim the power indicator when sent :turn_off" do
    @power_indciator.should_receive(:dim)
    @computer.turn_off
  end
end

Basically I don't see a reason why you would ever set an expectation on "nil", so I wondered, would the following be better behaviour?

describe NilClass do
  it "should raise an exception when sent :should_receive" do
    lambda { nil.should_receive(:some_message) }.should raise_error(StandardError)
  end
end

A bit like the way when you do "nil.id" in ActiveRecord you get the warning about calling nil.id in case you meant nil.object_id.  WDYT?

----------------------------------------------------------------------

>Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-06 15:01

Message:
CURRENT means the latest release, not trunk. So this must be fixed in trunk. Release coming soon.

At least no you know that " expected :foo ..." means "nil expected :foo ...".

----------------------------------------------------------------------

Comment By: Ashley Moran (ashley_moran)
Date: 2007-08-06 14:57

Message:
No I don't get that...

  nil.should_receive(:foo)

produces

  " expected :foo with (any args) once, but received it 0 times"

This is my svn:externals on vendor/plugins:

rspec svn://rubyforge.org/var/svn/rspec/tags/CURRENT/rspec
rspec_on_rails 
svn://rubyforge.org/var/svn/rspec/tags/CURRENT/rspec_on_rails


----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-06 14:47

Message:
Really? I just did this:

@foo.should_receive(:bar)

and got the following in the TextMate results window:

nil expected :bar with (any args) once, but received it 0 times

You're not getting that? Are you on edge? I don't think this is something that changed recently, but it could be

----------------------------------------------------------------------

Comment By: Ashley Moran (ashley_moran)
Date: 2007-08-06 14:41

Message:
I see your point David

Actually I don't get "nil accepted..." I get " accepted..." (using the spec runner 
from TextMate) which is possibly why I thought this was a bigger issue than it 
is...



----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2007-08-06 11:45

Message:
While I can't think of a good reason to set expectations on nil either, I also think that the first guy who does is going to stop using rspec as soon as he sees an error or warning for doing so. I would!

Also, in fairness, the feedback you get right now says "nil expected :dim ...". It does tell you that you're dealing with nil.

----------------------------------------------------------------------

Comment By: Ashley Moran (ashley_moran)
Date: 2007-08-06 11:28

Message:
Scott - actually it was only :should_receive I was thinking about, not :should in 
general.  I think making :should work differently for nil would be a mistake, but 
I can still see an argument for :should_receive.  See where I'm coming from?

----------------------------------------------------------------------

Comment By: Scott Taylor (smtlaissezfaire)
Date: 2007-08-06 11:23

Message:
Oh - just to follow up.  Of course that code could go into your spec_helper file, so you wouldn't need to carry around a different version of rspec, or make your production environment dependent on RSpec.

----------------------------------------------------------------------

Comment By: Scott Taylor (smtlaissezfaire)
Date: 2007-08-06 11:20

Message:
Should this spec raise an error:

describe "NilClass"
  it "should have an instance which is equal to nil" do
    nil.should == nil
  end
end

or how about this (in the case that method_one returns nil):

describe "method_one" do
  it "should return the same value as method_two" do
    method_one.should == method_two
  end
end


It is in an interesting idea, although raising an error probably isn't.  Maybe a warning.  

Luckily for you (and all of us), this is easy enough to do in Ruby:

module Kernel
  alias_method :__old_should, :should

  def should(*args, &blk)
    if self.class == NilClass
      raise RuntimeError, "this is a nil!!"
    else
      __old_should(*args, &blk)
    end
  end
end

----------------------------------------------------------------------

You can respond by visiting: 
http://rubyforge.org/tracker/?func=detail&atid=3149&aid=12845&group_id=797