Cannot figure out why Shoulda test is failing

69 views
Skip to first unread message

Bharat

unread,
Jun 18, 2009, 9:11:57 PM6/18/09
to shoulda
I am trying to test the User model shown below:

class User < ActiveRecord::Base
...
attr_accessor :password

validates_presence_of :username, :email, :display_name
validates_uniqueness_of :username, :email
validates_format_of :email, :with => /^[-a-z0-9_+\.]+\@([-a-
z0-9]+\.)+[a-z0-9]{2,4}$/i
validates_presence_of :password, :if
=> :password_required?
validates_presence_of :password_confirmation, :if
=> :password_required?
validates_confirmation_of :password, :if
=> :password_required?
validates_presence_of :role_id
...
private

def encrypt_password
return if password.blank?
self.password_salt = Digest::SHA1.hexdigest("--#{Time.now.to_s}--#
{username}--") if new_record?
self.password_hash = encrypt(password)
end

def password_required?
password_hash.blank? || !password.blank?
end

end

Here is the Factory_Girl factories.rb:

# This will guess the User class
Factory.define :user do |u|
u.username {'system'}
u.display_name {'Administrator'}
u.email {'sys...@ssrxgrp.com'}
u.password {'important'}
u.password_confirmation {'important'}
u.association :role
end

And Here is the user_test.rb:

require 'test_helper'
class UserTest < ActiveSupport::TestCase
context 'A User instance' do
setup do
Factory(:user)
end

should_validate_presence_of :username, :display_name, :email, :password, :password_confirmation, :role_id
should_validate_uniqueness_of :username, :email
end
end

This works fine. No problem. Now I am trying to save the instance in
an user instance variable (@user) as shown below:

require 'test_helper'
class UserTest < ActiveSupport::TestCase
context 'A User instance' do
setup do
@user = Factory(:user)
end

should_validate_presence_of :username, :display_name, :email, :password, :password_confirmation, :role_id
should_validate_uniqueness_of :username, :email
end
end

If I run this test however, I get an error as shown below:

1) Failure:
A User instance should require password to be set(UserTest):
Expected errors to include "can't be blank" when password is set to
nil, got no errors
/usr/local/lib/ruby/gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/
shoulda/assertions.rb:50:in `assert_accepts'
/usr/local/lib/ruby/gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/
shoulda/active_record/macros.rb:46:in `__bind_1245373838_879599'
/usr/local/lib/ruby/gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/
shoulda/context.rb:253:in `call'
/usr/local/lib/ruby/gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/
shoulda/context.rb:253:in `test: A User instance should require
password to be set. '

Finished in 0.301 seconds.
8 tests, 1 failures, 0 errors

Why is it failing now?
Thanks in advance for your time.

Bharat

Bharat

unread,
Jun 18, 2009, 9:40:52 PM6/18/09
to shoulda
It looks like a bug in Shoulda. I have narrowed it down to the
following statement in the User model shown above:

class User < ActiveRecord::Base
...
validates_presence_of :password, :if => :password_required?
...
private

def password_required?
password_hash.blank? || !password.blank?
end

end

If I remove the conditional, i.e., I just have

validates_presence_of :password

Then Shoulda is happy.

I may be wrong though.

Bharat

Ryan McGeary

unread,
Jun 19, 2009, 10:08:21 AM6/19/09
to sho...@googlegroups.com
Bharat,

You actually have a lot of moving parts.

First, take a look at the password_required? conditional.  You're basically saying that you want to validate that a password is present, but only when password_hash is blank -or- the password is already present (redundant, but reused for the other validations).  In summary, "Only require a password if you don't already have a password assigned."

Shoulda, by default, uses the instance variable named the same as the class your testing as the subject for the test.  When you assign a @user instance variable, you're telling the macros to use it to test against.  In later versions of shoulda, this will likely be replaced by a `subject` block inside a context.

Shoulda asserts validates_presence_of by assigning nil to the attribute and then expecting an error.  You'll never get an error unless the object your testing at least has a nil password_hash.  My guess is that you have a before_save callback that assigns a password_hash, because your user factory has a filled password and your saving a new instance instead of just building one in memory.  You'll be better off testing this only when password_hash is blank.  This is also why your original tests passed when you weren't yet assigning to a @user instance variable.

context "A user without a password set" do
  setup { @user = Factory.build(:user, :password_hash => nil, :password => nil) }
  should_validate_presence_of :password
end

Note the use of Factory.build instead of the implicit Factory.create.  Two birds here: no callbacks and no unnecessary database access.  (Assigning password to nil in the setup above is probably redundant too, but I don't have access to your entire model, so playing it safe.)

-Ryan

Bharat

unread,
Jun 19, 2009, 11:15:30 AM6/19/09
to shoulda
Ryan,
You are awesome man! Thanks for the fantastic explanation! You folks
are fast-forwarding my education in Ruby and Rails and I am enjoying
it immensely.
Regards,
Bharat

Cynthia Kiser

unread,
Jun 19, 2009, 4:27:04 PM6/19/09
to sho...@googlegroups.com
Quoting Ryan McGeary <ryan.m...@gmail.com>:
> Shoulda, by default, uses the instance variable named the same as the class
> your testing as the subject for the test. When you assign a @user instance
> variable, you're telling the macros to use it to test against. In later
> versions of shoulda, this will likely be replaced by a `subject` block
> inside a context.

Thanks Ryan. Your answer to Bharat's question solved my problem with
upgrading tests of an ActiveRecord singleton to Shoulda 2.10.1. You
talk about the subject block in the future tense - but the rdocs at
http://rdoc.info/projects/thoughtbot/shoulda seem to indicate that
using the subject class method is the current preferred way to deal
with this.

subject(&block)

Sets the return value of the subject instance method:

class UserTest < Test::Unit::TestCase
subject { User.first }

# uses the existing user
should_validate_uniqueness_of :email
end

I tried an example that is essentially like that (I used the
should_allow_values_for method but...). I tried it with and without an
enclosing context and in both cases I got:

undefined method `subject' for SitePreferenceTest:Class (NoMethodError)

Am I just running into a version difference between what I installed
as a gem (2.10.1) and what version the rdocs are referring to
(Generated at 06/10/09 00:38:05 +00:00 from 1806451b...)? Or is there
something fundamental I am missing?

--
Cynthia Kiser
c...@caltech.edu

Dan Croak

unread,
Jun 19, 2009, 4:34:06 PM6/19/09
to sho...@googlegroups.com
Cynthia,

On Fri, Jun 19, 2009 at 4:27 PM, Cynthia Kiser <c...@caltech.edu> wrote:
 You
talk about the subject block in the future tense - but the rdocs at
http://rdoc.info/projects/thoughtbot/shoulda seem to indicate that
using the subject class method is the current preferred way to deal
with this.

undefined method `subject' for SitePreferenceTest:Class (NoMethodError)

Am I just running into a version difference between what I installed
as a gem (2.10.1) and what version the rdocs are referring to
(Generated at 06/10/09 00:38:05 +00:00 from 1806451b...)? 

Yes, the rdoc.info is slightly ahead of the most recently release Shoulda gem. We're going to move to a model of doing all work in branches, and only merging into master for gem releases so the newly official RDoc at rdoc.info stays in sync.

In the meantime, we should bump that to a new version soon (next week, fellas?) so people aren't confused about subject, should_create, and should_destroy.

--
Dan

Cynthia Kiser

unread,
Jun 19, 2009, 8:25:04 PM6/19/09
to sho...@googlegroups.com
Quoting Dan Croak <dcr...@thoughtbot.com>:
> In the meantime, we should bump that to a new version soon (next week,
> fellas?) so people aren't confused about subject, should_create, and
> should_destroy.

Cool! (even though I think that means it will break a number of my
tests).

So I have what smells to me like the same sort of problem - except
that I went out and generated my own copy of the rdocs from my
installed version of shoulda. I see the macro I am calling
(should_redirect_to) documented, with an example that looks like what
I am going - but my tests can't use it. What am I missing?

I am upgrading an app from Rails 2.1 -> 2.3.2 and more relevant for
this discussion from Shoulda 2.0.6 installed as a plugin to Shoulda
2.10.1 installed as a gem (+ config.gem line in environment.rb). Yes I
should have done these sequentially - starting with upgrading Shoulda,
but.... After a few syntax changes, my unit tests all pass and now I
am starting in on the functional tests. FWIW, manual testing via a
browser confirms that the user index page does indeed redirect if you
are not logged in first.

require File.dirname(__FILE__) + '/../test_helper'

class UsersControllerTest < ActionController::TestCase
context "User actions" do
setup do
# check to see if login_url is evaluated to what I expect
puts login_url
end

should "test the test" do
get :index
should_redirect_to("the login page") { login_url }
end

end
end

$ ruby -I test test/functional/users_controller_test.rb

1) Error: test: User actions should test the test. (UsersControllerTest):
NoMethodError: undefined method `should_redirect_to' for #<UsersControllerTest:0xb145978>
test/functional/users_controller_test.rb:16:in `__bind_1245455972_115540'
/software/stow/ruby-enterprise-1.8.6-20080709/lib/ruby/gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/context.rb:253:in `call'
/software/stow/ruby-enterprise-1.8.6-20080709/lib/ruby/gems/1.8/gems/thoughtbot-shoulda-2.10.1/lib/shoulda/context.rb:253:in `test: \
User actions should test the test. '

--
Cynthia Kiser

Cynthia Kiser

unread,
Jun 23, 2009, 12:29:46 PM6/23/09
to sho...@googlegroups.com
This issue turned out to be a partial conversion issue. I had failed
to move the configuration for my authorization plugin into the new
project. So of course the tests were not redirecting properly. I don't
understand why this was giving the error I see. Perhaps there are
some other, compouning issues that I have fixed along the way. But now
my functional tests are passing.
--
Cynthia Kiser
Reply all
Reply to author
Forward
0 new messages