[rspec-devel] [ rspec-Bugs-8027 ] should_change works but should_not_change appears not to.

noreply at rubyforge.org noreply at rubyforge.org
Sun Jan 21 16:21:22 EST 2007


Bugs item #8027, was opened at 2007-01-20 18:46
You can respond by visiting: 
http://rubyforge.org/tracker/?func=detail&atid=3149&aid=8027&group_id=797

Category: None
Group: None
Status: Open
Resolution: None
Priority: 3
Submitted By: steve ross (cwd)
Assigned to: Nobody (None)
Summary: should_change works but should_not_change appears not to.

Initial Comment:
Rspec v. 0.8.0 (trunk)

Given the code:

context "Given a Bug (in general)" do
  specify "Should be able to add a valid record" do
    lambda { Bug.create(:title => 'a valid bug', :description => 'a valid description') }.should_change(Bug, :count).by(1)
  end
  
  specify "Should not be able to add an invalid record" do
    lambda { Bug.create(:description => 'a valid description') }.should_not_change(Bug, :count).by(1)
  end
end

The result is:

1)
'Given a Bug (in general) Should not be able to add an invalid record' FAILED
count should have been changed by 1, but was changed by 0
./spec/models/bug_spec.rb:18:

Model code, bug.rb, is:

class Bug < ActiveRecord::Base
  validates_presence_of :title
end


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

>Comment By: David Chelimsky (dchelimsky)
Date: 2007-01-21 21:21

Message:
Sorry... that should have been "should_not_change(obj, :sym).by(1)"

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

Comment By: David Chelimsky (dchelimsky)
Date: 2007-01-21 21:20

Message:
Steve, I agree that there are many cases where negative expectations make perfect sense. Generally, I think there should be a parallel negative. It's the weird ones like should_change(obj, :sym).by(1) that, in my view, make specs more confusing than explicit that make me think absolute parallelism is not a necessity.

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

Comment By: steve ross (cwd)
Date: 2007-01-21 20:29

Message:
Let's talk about how something this obtuse comes about. Typically, I spec 
positive expectations first to make sure what I think will happen does indeed 
happen. Then I add negative expectations to weed out some corner cases. 
This may seem duplicative, but the specs might read: "should be able to add 
a valid record" and "should not be able to add an invalid record."

Clearly copy/paste is a quick way to get this kind of template set up, but the 
change in sugar is why it doesn't work quite as simply as adding *not*. This is 
user error, but it might come up again.

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

Comment By: David Chelimsky (dchelimsky)
Date: 2007-01-21 20:22

Message:
I think it's OK to not be parallel. For example, these two mean the same thing:

team.should_not have_at_least(4).players
team.should have_at_most(3).players

Would you EVER want to use the first? Talk about making specs unclear!

Being able to say proc.should_not change(Model, :count).by(1) is like saying - it's OK for this to cause a change, but whatever you do, don't change it by 1. That's just a bit too non-deterministic for my stomach.

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

Comment By: steve ross (cwd)
Date: 2007-01-21 20:16

Message:
It would seem that most expectations also have a negative expectation 
associated with them (i.e., should_nnn has a corresponding should_not_nnn). 
The expected behavior, then would be that if the object changed by any number 
other than the specified one, it would fail.

Is there value to adding this? Am I right about the parallelism among 
expectations?

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

Comment By: Wilson Bilkovich (wilson)
Date: 2007-01-21 19:38

Message:
I should have disallowed it originally, in my opinion, making this a real bug.

However, if we assume for a moment that we wanted to support it, how would it work?

given:
lambda {}.should_not_change(Thing, :count)

The current code 'knows' that that expectation should fail as soon as it can compare the before and after values.

lambda {}.should_not_change(Thing, :count).by(2)
would, I would expect, pass if the count changed by 1.
This means that the bare version without 'by' would fail, but the 'by' version would pass.

Given that the bare 'should_not_change' has no way of knowing whether it will have 'by' called on it, I don't see a way to implement that.

Every other extension type makes the expectation MORE specific, not less.  This would violate that guideline, and I'm not sure it is implementable.  Any ideas?

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

Comment By: David Chelimsky (dchelimsky)
Date: 2007-01-21 11:14

Message:
should_not_change is only spec'd to work without any extensions. i.e.

Bug.create(:description => 'a valid description') }.should_not_change(Bug, :count)

(without to, from or by)

We have two choices to deal w/ this:
1 - disallow use of to, from or by with should_not_change
2 - support it

My feeling about this is that if you're using should_not_change you shouldn't be using the extensions, so I'd go for 1.

Other opinions?

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

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


More information about the rspec-devel mailing list