My first tests, backwards. Please help


#1

Hello,

I’m trying to get a positive attitude towards testing. I wrote a
controller that seems to work, and I want to make some tests. I
populated the test db with an auction, some users, an item, and some
charities. But, here is my first test, can’t get it to pass. If you
please, can you just help me get this test to pass, and not dwell on how
wrong my approach is? I’ll appreciate it, thanks.

require File.dirname(FILE) + ‘/…/spec_helper’

describe “bidding on an item” do
controller_name :items
integrate_views

before :each do
@user = User.first
User.stub!(:current_user).with(@user)
end

it “should require a credit” do
@user.credits = 0
@user.save
post ‘bid’, :bid => {:auction_id => 1, :user_id => @user, :point =>
1}
Bid.count.should == 0
end

end


#2

On Tue, Dec 30, 2008 at 4:17 PM, Jesse C. removed_email_address@domain.invalid
wrote:

Hello,

I’m trying to get a positive attitude towards testing.

I think you already do just by virtue of this email.

controller_name :items
post ‘bid’, :bid => {:auction_id => 1, :user_id => @user, :point =>
1}
Bid.count.should == 0
end

end

What’s the failure message?


#3

David C. wrote:

On Tue, Dec 30, 2008 at 4:17 PM, Jesse C. removed_email_address@domain.invalid
wrote:

Hello,

I’m trying to get a positive attitude towards testing.

I think you already do just by virtue of this email.

No comment :slight_smile:

What’s the failure message?

I’m confused, because I don’t know if the stub is returning the
logged_in? @current_user (bort base app) and doing something I’m not
sure what.

However, I mainly get "You may be expecting an instance of
ActiveRecord::Base.

I’m so confused… this one is the most confusing.

it “should require a credit” do
@user.credits = 0
@user.save
post ‘bid’, :bid => {:auction_id => 1, :user_id => @user, :point =>
1}
assigns[:bid].should_not be_new_record
end

You have a nil object when you didn’t expect it!
You might have expected an instance of ActiveRecord::Base.
The error occurred while evaluating nil.new_record?

But the model and views are all written. The bid method is fairly
complex. This is the simplest test case I can think of.


#4

On Tue, Dec 30, 2008 at 4:41 PM, Jesse C. removed_email_address@domain.invalid
wrote:

I’m so confused… this one is the most confusing.
You might have expected an instance of ActiveRecord::Base.
The error occurred while evaluating nil.new_record?

But the model and views are all written. The bid method is fairly
complex. This is the simplest test case I can think of.

Can you post the bid method?


#5

I guess it speaks for itself.

def bid
auction = Auction.new
p_value = params[:bid][:point]
@auction = Auction.find_by_auction_id(params[:bid][:auction_id])

if @completed = (@auction.bid_count == @auction.bid_limit)
  flash[:warning] = "Other bidding has completed the auction.  Your 

bid has been returned."
elsif current_user.credits > 0
if params[:bid][:point].blank?
flash[:error] = “Sorry, you must enter a valid bid.”
elsif p_value.to_i < 1 or p_value.to_i >
auction.point_range(params[:bid][:auction_id])
flash[:error] = “Sorry, you attempted to place a bid out of
range.”
elsif not Bid.find_by_user_id_and_point(params[:bid][:user_id],
params[:bid][:point]).nil?
flash[:error] = “You have already placed this point
(#{p_value.to_i}).”
else
#place bid
@bid = Bid.new(params[:bid])
point = @bid.point
if @bid.save
# update winning charities and pref charity places

      # TODO make sure that the top bids are used for place (double 

check)

      update_charities_on_bid(current_user, @auction)

      # set auction winners
      if @completed
        winners = Bid.new.winners(params[:bid][:auction_id])
        a.winner_first_id  = winners.first[:user_id]
        a.winner_second_id = winners.second[:user_id]
        a.winner_third_id  = winners.third[:user_id]
        a.duration_hours = ((Time.now - a.scheduled_start) / 60.0 / 

60.0).to_i
end

      current_user.credits -= 1
      current_user.save

      user_bids = 

Bid.find_all_by_user_id_and_auction_id(current_user.id,
@auction.auction_id).count
if user_bids == 1
@auction =
Auction.find_by_auction_id(params[:bid][:auction_id])
@auction.total_bidders += 1
end
@auction.bid_count += 1
@auction.save

      if Bid.find_all_by_point(point).count > 1
        flash[:notice] = "Auction Completed.  We have winners!!" if 

@completed
flash[:warning] = “Oh! Someone else has bid the same point.
Please try again.” if not @completed
flash[:warning] = “Bravery! However, your bid was not
unique. Please view the auction results.” if @completed
else
flash[:notice] = “Success! You bid for point #{point}. It
is unique!”
flash[:notice] << " Auction Completed. We have winners!!"
if @completed
end
else
flash[:error] = “Bleep, bloop. System error, please try
again.”
end
end
else
flash[:error] = “Whoops, you have zero credits available to bid.”
end
redirect_back_or_default :show
end

def update_charities_on_bid(user, auction)
# best_bids: top 3 winning bids
best_bids = Bid.all.uniq
best_bids.sort! {|a, b| b.point <=> a.point}[0…2]

best_bids[1]  = 0 if best_bids.second.nil?
best_bids[2]  = 0 if best_bids.third.nil?

# TODO check timestamp for place ??

if best_bids.first.user_id == user.id
  auction.charity_first_id  = user.pref_charity_one_id
elsif best_bids.second != 0 && best_bids.second.user_id == user.id
  auction.charity_second_id = user.pref_charity_two_id
elsif best_bids.third != 0 && best_bids.third.user_id == user.id
  auction.charity_third_id  = user.pref_charity_three_id
end

auction.save

end


#6

What seems to be happening is that the action is executing without
error (otherwise you’d get an error on the line with the post) but is
failing to actually save the record. There are quite a few potential
reasons for the record not to be saved in this case. What I’d do if I
were working side by side with you, in this case, is just start
writing puts statements to see what is in what state at different
points in the method.

Try that and see where it leads. We’ll still be here if you find
anything you want to share or get help with.

HTH,
David


#7

On Wed, Dec 31, 2008 at 12:02 AM, Jesse C.
removed_email_address@domain.invalidwrote:

I guess it speaks for itself.

Yes. It says “I’m too hard to test”. More specifically, it has a
http://en.wikipedia.org/wiki/Cyclomatic_complexity of ca 14, which I
consider very high. I usually try to strive for max 5.

Starting to write RSpec for this is starting in the wrong end. This code
is
so complex (both in terms of readability and number of branches and
paths)
that trying to wrap a unit test (RSpec spec) around it is going to be a
huge
headache. I recommend you do some serious extract method refactorings. (
http://www.refactoring.com/catalog/extractMethod.html). While
refactoring
you should try to push the logic from the controller into the model:
http://weblog.jamisbuck.org/2006/10/18/skinny-controller-fat-model

This can seem like a chicken and egg problem. How do you refactor
without
the fear that you have broken something? For this I recommend you slap
some
Cucumber around this piece of code. It allows you to write tests at a
much
higher level. When you have 3-10 green Cucumber scenarios for this you
can
start refactoring without too much fear. Then you can start using RSpec.

Teams that get into the habit of writing specs before the code end up
with
much more readable and maintainable code. :slight_smile:

Aslak


#8

Good new and good news.

First, I’m starting from the floor up. (The groundwork is still past my
anxiety threshhold.) I’ve saved all the preexisting work to a
refactor_me file in the rails_root; starting over:

== test code

require File.dirname(FILE) + ‘/…/spec_helper’

describe “bidding on an item” do
controller_name :items

before :each do
@user = User.first
User.stub!(:current_user).with(@user)
puts @user.id, @user.login
end

it “should require a credit” do
@user.credits = 0
@user.save
post ‘bid’, :bid => {:auction_id => 1, :user_id => @user, :point =>
1}
assigns[:bid].should_not be_new_record
end

it “should be allowed with a credit” do
@user.credits = 1
@user.save
post ‘bid’, :bid => {:auction_id => 1, :user_id => @user, :point =>
1}
assigns[:bid].should be_new_record
end

end

== application code

def bid

if can_bid?(current_user)
  @bid = Bid.new(params[:bid])
  @bid.save
end

end

def can_bid?(user)
user.credits > 0
end

== Errors

1
quentin
F1
quentin

== Was expecting
1
A
F1
A

(rather than the bort fixtures)

== auction_spec.rake

namespace :db do
desc “Erase and fill the test database for auction spec”
task :auction_spec => “test” do
require ‘populator’
require ‘faker’

[Item, Auction, User, Charity].each(&:delete_all)
 .
 .
 .
# Create activated non-admin user
user = User.create do |u|
  u.login = 'A'
  u.password = u.password_confirmation = 'foobar'
  u.email = 'removed_email_address@domain.invalid'
  u.pref_charity_one_id = 1
  u.pref_charity_two_id = 2
  u.pref_charity_three_id = 3
  u.credits = 10
end

# Activate non-admin user
user.register!
user.activate!
.
.
.

end
end

==

I feel open to constructive criticism, now, as to the method of using
test records. Though I’m not sure what my question is…

Thanks in advance.


#9

I’d put

puts “========> Bid is #{Bid.inspect}” and try to learn why Bid.count
is not == 0.

HTH,

Tim


#10

I hope to be on my feet after a few more basic questions…

Take a look at these:

http://github.com/flogic/object_daddy/tree/master
http://github.com/thoughtbot/factory_girl/tree/master
http://github.com/notahat/machinist/tree/master

Deciding on factory_girl due to it’s popularity, I wrote a factories
file:

== spec/factories.rb

This will guess the User class

Factory.define :user do |u|
u.login ‘John’
u.password ‘foobar’
u.password_confirmation ‘foobar’
u.email ‘removed_email_address@domain.invalid’
u.pref_charity_one_id 1
u.pref_charity_two_id 2
u.pref_charity_three_id 3
u.credits 0
u.register! # do these work?
u.activate! #
end

== but the test still gets ‘quentin’ from the bort fixtures for user 1
require File.dirname(FILE) + ‘/…/spec_helper’
require ‘factory_girl’ # don’t know if this works

Do I need to require the file like the spec_helper for factory_girl?

For instance, will you write a simple case that puts the factory user id
and login? I think that would help a lot.

Also, your patience is appreciated. I may have a New years resolution
knocked out before the year starts :slight_smile:


#11

Jesse C. wrote:

I hope to be on my feet after a few more basic questions…

I’m sorry, but this is beginning to feel like kicking myself in the
head.

=== here is what I’m working with
http://github.com/fudgestudios/bort/tree/master

=== Errors
FF

NoMethodError in ‘bidding on an item should be allowed with a credit’
You have a nil object when you didn’t expect it!
You might have expected an instance of ActiveRecord::Base.
The error occurred while evaluating nil.new_record?
spec/controllers/auction_controller_spec.rb:23:
spec/controllers/auction_controller_spec.rb:6:

NoMethodError in ‘bidding on an item should require a credit’
You have a nil object when you didn’t expect it!
You might have expected an instance of ActiveRecord::Base.
The error occurred while evaluating nil.new_record?
spec/controllers/auction_controller_spec.rb:17:
spec/controllers/auction_controller_spec.rb:6:

Finished in 0.074996 seconds

2 examples, 2 failures

=== here is my controller action

def bid

  @bid = Bid.new(params[:bid])
  @bid.save

end

=== here is my test

require File.dirname(FILE) + ‘/…/spec_helper’
include ApplicationHelper
include UsersHelper
include AuthenticatedTestHelper

describe “bidding on an item” do
controller_name :items

before(:each) do
  @user = mock_user
  stub!(:current_user).and_return(@user)
end

it “should require a credit” do
@user.stub!(:credits).and_return(0)
post ‘bid’, :bid => { :auction_id => 1, :user_id => @user.id, :point
=> 1 }
assigns[:bid].should_not be_new_record
end

it “should be allowed with a credit” do
@user.stub!(:credits).and_return(1)
post ‘bid’, :bid => { :auction_id => 1, :user_id => @user.id, :point
=> 1 }
assigns[:bid].should be_new_record
end

end

=== spec_helper
http://github.com/fudgestudios/bort/tree/master/spec/spec_helper.rb

It’s very disheartening to wake for work at 3 a.m. and accomplish
nothing for the day. Please understand.


#12

On Thu, Jan 1, 2009 at 5:31 AM, Jesse C. removed_email_address@domain.invalid
wrote:

FF
NoMethodError in ‘bidding on an item should require a credit’

 stub!(:current_user).and_return(@user)

Calling stub! with no receiver means it’s being called on the example
itself. Assuming you’re using RestfulAuth, you’ll want this to be on
the controller itself:

controller.stub!(:current_user).and_return(@user)

Although, looking at the ‘bid’ action, it doesn’t appear to be doing
anything with the user yet, so I’m not sure why this is here at this
point.

end

it “should require a credit” do
@user.stub!(:credits).and_return(0)
post ‘bid’, :bid => { :auction_id => 1, :user_id => @user.id, :point
=> 1 }

If you’re using the mock_user, which uses mock_model (I’m assuming
here, since you’re not posting complete files),

assigns[:bid].should_not be_new_record

I’m guessing this is line 17 and the "assigns[:bid] line below is 23.

What this means is that assigns[:bid] is nil, which means that somehow
the action is failing to generate the bit object. I’m at a bit of a
loss as to why that might be, but that’s what is indicated.

HTH,
David


#13

On Wed, Dec 31, 2008 at 3:52 AM, Jesse C. removed_email_address@domain.invalid
wrote:

Good new and good news.

First, I’m starting from the floor up. (The groundwork is still past my
anxiety threshhold.) I’ve saved all the preexisting work to a
refactor_me file in the rails_root; starting over:

Congrats!

User.stub!(:current_user).with(@user)
Assuming this is a controller spec, you’re going to want to stub
controller.current_user, not User.current user:

controller.stub!(:current_user).and_return(@user)

== application code
def can_bid?(user)
== Was expecting
1
A
F1
A

Not sure why you’re expecting “A” here. In before(:each) the #puts
statement prints the id of User.first (which is 1) and the login of
that same user (which is “quentin”). The “F” is for a failing example
and there should be another one somewhere in the output (or a “.” if
one is passing).

[Item, Auction, User, Charity].each(&:delete_all)

If you use transactional_fixtures, this should not be necessary.
transactional_fixtures is the wrong name - it really means
transactional examples - each example is run in a transaction that is
rolled back. So if you start w/ a truncated db, each example fires up
a transaction, loads up fixtures, runs the example, rolls back the
transaction. Make sense?

 u.pref_charity_three_id = 3
 u.credits = 10

end

Activate non-admin user

user.register!
user.activate!

You can keep these lines in the block - I would, though I wouldn’t
populate fixture data in a custom rake task like this.

If you’re using fixtures, just make this a fixture in
spec/fixtures/users.yml.

But I would recommend using something other than fixtures for sample
data. There are several problems with fixtures, some of which have
been resolved by the rails team over time, but the one that still
remains is that you have to look at another file to understand your
example data.

There are several libraries that help with this problem by letting you
set up default model builders that know how to build a valid model
with reasonable (what reasonable means is up to you - they are user
defined) defaults, but then override those defaults in your examples.

Take a look at these:

http://github.com/flogic/object_daddy/tree/master
http://github.com/thoughtbot/factory_girl/tree/master
http://github.com/notahat/machinist/tree/master

All three are very popular, work well, and offer different approaches
to setting up the default data, but all three let you override that
data in your code examples. Take a look at their READMEs and see if
this makes sense. Feel free to post back if you have questions.

Cheers,
David


#14

On Thu, Jan 1, 2009 at 11:32 AM, Jesse C. removed_email_address@domain.invalid
wrote:

I’ve finally made some progress thanks to Otto at stackoverflow. I’m
very exited. Thanks so much for trying to help. If you’ve got a
minute, please browse my question and Otto’s answer, and correct me if I
seem to be going in the wrong direction.

http://stackoverflow.com/questions/404887

I responded there to keep things in context.

Cheers,
David


#15

I’ve finally made some progress thanks to Otto at stackoverflow. I’m
very exited. Thanks so much for trying to help. If you’ve got a
minute, please browse my question and Otto’s answer, and correct me if I
seem to be going in the wrong direction.

http://stackoverflow.com/questions/404887

Yee haw, my first passing spec!