From 30d6d37bab9807a9e8758e2e4a44dca9018d69c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 24 Nov 2009 23:19:12 -0200 Subject: [PATCH] Refactor tests a little bit and gain more speed (from 12s to 9s in my machine). --- lib/devise/models.rb | 21 ++++++++++ lib/devise/models/authenticatable.rb | 36 ++++------------ test/models/authenticatable_test.rb | 57 ++++++++++--------------- test/models/confirmable_test.rb | 63 +++++++++++----------------- test/models/recoverable_test.rb | 32 ++++++-------- test/models/rememberable_test.rb | 55 ++++++++++++------------ test/models/timeoutable_test.rb | 1 + 7 files changed, 114 insertions(+), 151 deletions(-) diff --git a/lib/devise/models.rb b/lib/devise/models.rb index 0e3f3bba..4a171e8d 100644 --- a/lib/devise/models.rb +++ b/lib/devise/models.rb @@ -97,5 +97,26 @@ module Devise def devise_modules @devise_modules ||= [] end + + # Find an initialize a record setting an error if it can't be found + def find_or_initialize_with_error_by(attribute, value, error=:invalid) + if value.present? + conditions = { attribute => value } + record = find(:first, :conditions => conditions) + end + + unless record + record = new + + if value.present? + record.send(:"#{attribute}=", value) + record.errors.add(attribute, error, :default => error.to_s.gsub("_", " ")) + else + record.errors.add(attribute, :blank) + end + end + + record + end end end diff --git a/lib/devise/models/authenticatable.rb b/lib/devise/models/authenticatable.rb index e4ea1d59..2097e7b1 100644 --- a/lib/devise/models/authenticatable.rb +++ b/lib/devise/models/authenticatable.rb @@ -38,18 +38,19 @@ module Devise end end - # Regenerates password salt and encrypted password each time password is - # setted. + # Regenerates password salt and encrypted password each time password is set. def password=(new_password) @password = new_password - self.password_salt = Devise.friendly_token - self.encrypted_password = password_digest(@password) + + if @password.present? + self.password_salt = Devise.friendly_token + self.encrypted_password = password_digest(@password) + end end - # Verifies whether an incoming_password (ie from login) is the user - # password. + # Verifies whether an incoming_password (ie from login) is the user password. def valid_password?(incoming_password) - !incoming_password.blank? && password_digest(incoming_password) == encrypted_password + password_digest(incoming_password) == encrypted_password end protected @@ -108,27 +109,6 @@ module Devise resource if resource.valid_password?(attributes[:password]) end - # Find an initialize a record setting an error if it can't be found - def find_or_initialize_with_error_by(attribute, value, error=:invalid) - if value - conditions = { attribute => value } - record = find(:first, :conditions => conditions) - end - - unless record - record = new - - if value - record.send(:"#{attribute}=", value) - record.errors.add(attribute, error, :default => error.to_s.gsub("_", " ")) - else - record.errors.add(attribute, :blank) - end - end - - record - end - Devise::Models.config(self, :pepper, :stretches, :encryptor, :authentication_keys) end end diff --git a/test/models/authenticatable_test.rb b/test/models/authenticatable_test.rb index b841fe66..c61e61e7 100644 --- a/test/models/authenticatable_test.rb +++ b/test/models/authenticatable_test.rb @@ -13,11 +13,10 @@ class AuthenticatableTest < ActiveSupport::TestCase assert user.respond_to?(:password_confirmation) end - test 'should generate salt while setting password' do - assert_present new_user.password_salt - assert_present new_user(:password => nil).password_salt - assert_present new_user(:password => '').password_salt - assert_present create_user.password_salt + test 'should generate encrypted password and salt while setting password' do + user = new_user + assert_present user.password_salt + assert_present user.encrypted_password end test 'should not change password salt when updating' do @@ -33,20 +32,14 @@ class AuthenticatableTest < ActiveSupport::TestCase assert_equal 'friendly_token', new_user.password_salt end - test 'should never generate the same salt for different users' do - password_salts = [] - 10.times do - salt = create_user.password_salt - assert_not password_salts.include?(salt) - password_salts << salt - end + test 'should not generate salt if password is blank' do + assert_blank new_user(:password => nil).password_salt + assert_blank new_user(:password => '').password_salt end - test 'should generate encrypted password while setting password' do - assert_present new_user.encrypted_password - assert_present new_user(:password => nil).encrypted_password - assert_present new_user(:password => '').encrypted_password - assert_present create_user.encrypted_password + test 'should not generate encrypted password if password is blank' do + assert_blank new_user(:password => nil).encrypted_password + assert_blank new_user(:password => '').encrypted_password end test 'should encrypt password again if password has changed' do @@ -57,31 +50,28 @@ class AuthenticatableTest < ActiveSupport::TestCase assert_not_equal encrypted_password, user.encrypted_password end - test 'should fallback to Sha1 as default encryption' do + test 'should fallback to sha1 as default encryption' do user = new_user assert_equal encrypt_password(user), user.encrypted_password end - test 'should fallback to devise pepper default configuring' do + test 'should fallback to devise pepper default configuration' do begin Devise.pepper = '' user = new_user assert_equal encrypt_password(user), user.encrypted_password assert_not_equal encrypt_password(user, 'another_pepper'), user.encrypted_password + Devise.pepper = 'new_pepper' user = new_user assert_equal encrypt_password(user, 'new_pepper'), user.encrypted_password assert_not_equal encrypt_password(user, 'another_pepper'), user.encrypted_password - Devise.pepper = '123456' - user = new_user - assert_equal encrypt_password(user, '123456'), user.encrypted_password - assert_not_equal encrypt_password(user, 'another_pepper'), user.encrypted_password ensure Devise.pepper = nil end end - test 'should fallback to devise stretches default configuring' do + test 'should fallback to devise stretches default configuration' do swap Devise, :stretches => 1 do user = new_user assert_equal encrypt_password(user, nil, 1), user.encrypted_password @@ -140,11 +130,6 @@ class AuthenticatableTest < ActiveSupport::TestCase assert_not_nil Admin.authenticate(:email => admin.email, :password => admin.password) end - test 'should never authenticate an account' do - account = Account.create!(valid_attributes) - assert_nil Account.authenticate(:email => account.email, :password => account.password) - end - test 'should serialize user into session' do user = create_user assert_equal [User, user.id], User.serialize_into_session(user) @@ -155,16 +140,16 @@ class AuthenticatableTest < ActiveSupport::TestCase assert_equal user.id, User.serialize_from_session([User, user.id]).id end - test 'should not serialize another klass from session' do + test 'should serialize another klass from session if it is an ancestors' do + user = create_user + klass = Class.new(User) + assert_equal user.id, User.serialize_from_session([klass, user.id]).id + end + + test 'should not serialize another klass from session if not an ancestors' do user = create_user assert_raise RuntimeError, /ancestors/ do User.serialize_from_session([Admin, user.id]) end end - - test 'should serialize another klass from session' do - user = create_user - klass = Class.new(User) - assert_equal user.id, User.serialize_from_session([klass, user.id]).id - end end diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb index 71762ad1..b594cac9 100644 --- a/test/models/confirmable_test.rb +++ b/test/models/confirmable_test.rb @@ -22,14 +22,14 @@ class ConfirmableTest < ActiveSupport::TestCase test 'should never generate the same confirmation token for different users' do confirmation_tokens = [] - 10.times do + 3.times do token = create_user.confirmation_token assert !confirmation_tokens.include?(token) confirmation_tokens << token end end - test 'should confirm a user updating confirmed at' do + test 'should confirm a user by updating confirmed at' do user = create_user assert_nil user.confirmed_at assert user.confirm! @@ -51,32 +51,32 @@ class ConfirmableTest < ActiveSupport::TestCase assert user.confirmed? end - test 'should not confirm a user already confirmed and add an error to email' do + test 'should not confirm a user already confirmed' do user = create_user assert user.confirm! assert_nil user.errors[:email] + assert_not user.confirm! - assert_not_nil user.errors[:email] assert_equal 'already confirmed', user.errors[:email] end test 'should find and confirm an user automatically' do user = create_user confirmed_user = User.confirm!(:confirmation_token => user.confirmation_token) - assert_not_nil confirmed_user assert_equal confirmed_user, user assert user.reload.confirmed? end - test 'should return a new user with errors if no user exists while trying to confirm' do + test 'should return a new record with errors when a invalid token is given' do confirmed_user = User.confirm!(:confirmation_token => 'invalid_confirmation_token') assert confirmed_user.new_record? + assert_equal "is invalid", confirmed_user.errors[:confirmation_token] end - test 'should return errors for a new user when trying to confirm' do - confirmed_user = User.confirm!(:confirmation_token => 'invalid_confirmation_token') - assert_not_nil confirmed_user.errors[:confirmation_token] - assert_equal 'is invalid', confirmed_user.errors[:confirmation_token] + test 'should return a new record with errors when a blank token is given' do + confirmed_user = User.confirm!(:confirmation_token => '') + assert confirmed_user.new_record? + assert_equal "can't be blank", confirmed_user.errors[:confirmation_token] end test 'should generate errors for a user email if user is already confirmed' do @@ -91,7 +91,6 @@ class ConfirmableTest < ActiveSupport::TestCase user = create_user user.confirm! authenticated_user = User.authenticate(:email => user.email, :password => user.password) - assert_not_nil authenticated_user assert_equal authenticated_user, user end @@ -112,13 +111,11 @@ class ConfirmableTest < ActiveSupport::TestCase test 'should find a user to send confirmation instructions' do user = create_user confirmation_user = User.send_confirmation_instructions(:email => user.email) - assert_not_nil confirmation_user assert_equal confirmation_user, user end test 'should return a new user if no email was found' do confirmation_user = User.send_confirmation_instructions(:email => "invalid@email.com") - assert_not_nil confirmation_user assert confirmation_user.new_record? end @@ -173,65 +170,53 @@ class ConfirmableTest < ActiveSupport::TestCase user.confirm! assert_not user.reset_confirmation! assert user.confirmed? - assert user.errors[:email].present? assert_equal 'already confirmed', user.errors[:email] end test 'confirm time should fallback to devise confirm in default configuration' do - begin - confirm_within = Devise.confirm_within - Devise.confirm_within = 1.day + swap Devise, :confirm_within => 1.day do user = new_user user.confirmation_sent_at = 2.days.ago assert_not user.active? + Devise.confirm_within = 3.days assert user.active? - ensure - Devise.confirm_within = confirm_within end end test 'should be active when confirmation sent at is not overpast' do - Devise.confirm_within = 5.days - user = create_user - user.confirmation_sent_at = 4.days.ago - assert user.active? + swap Devise, :confirm_within => 5.days do + Devise.confirm_within = 5.days + user = create_user + + user.confirmation_sent_at = 4.days.ago + assert user.active? + + user.confirmation_sent_at = 5.days.ago + assert_not user.active? + end end test 'should be active when already confirmed' do user = create_user assert_not user.confirmed? assert_not user.active? + user.confirm! assert user.confirmed? assert user.active? end - test 'should not be active when confirmation was sent within the limit' do - Devise.confirm_within = 5.days - user = create_user - user.confirmation_sent_at = 5.days.ago - assert_not user.active? - end - - test 'should be active when confirm in is zero' do + test 'should not be active when confirm in is zero' do Devise.confirm_within = 0.days user = create_user user.confirmation_sent_at = Date.today assert_not user.active? end - test 'should not be active when confirmation was sent before confirm in time' do - Devise.confirm_within = 4.days - user = create_user - user.confirmation_sent_at = 5.days.ago - assert_not user.active? - end - test 'should not be active without confirmation' do user = create_user user.update_attribute(:confirmation_sent_at, nil) assert_not user.reload.active? end - end diff --git a/test/models/recoverable_test.rb b/test/models/recoverable_test.rb index ff20e7d2..f7804ddd 100644 --- a/test/models/recoverable_test.rb +++ b/test/models/recoverable_test.rb @@ -8,7 +8,6 @@ class RecoverableTest < ActiveSupport::TestCase test 'should not generate reset password token after creating a record' do assert_nil new_user.reset_password_token - assert_nil create_user.reset_password_token end test 'should regenerate reset password token each time' do @@ -22,7 +21,7 @@ class RecoverableTest < ActiveSupport::TestCase test 'should never generate the same reset password token for different users' do reset_password_tokens = [] - 10.times do + 3.times do user = create_user user.send_reset_password_instructions token = user.reset_password_token @@ -45,6 +44,7 @@ class RecoverableTest < ActiveSupport::TestCase test 'should clear reset password token while reseting the password' do user = create_user assert_nil user.reset_password_token + user.send_reset_password_instructions assert_present user.reset_password_token assert user.reset_password!('123456789', '123456789') @@ -77,55 +77,47 @@ class RecoverableTest < ActiveSupport::TestCase test 'should find a user to send instructions by email' do user = create_user reset_password_user = User.send_reset_password_instructions(:email => user.email) - assert_not_nil reset_password_user assert_equal reset_password_user, user end - test 'should return a new user if no email was found' do + test 'should return a new record with errors if user was not found by e-mail' do reset_password_user = User.send_reset_password_instructions(:email => "invalid@email.com") - assert_not_nil reset_password_user assert reset_password_user.new_record? - end - - test 'should add error to new user email if no email was found' do - reset_password_user = User.send_reset_password_instructions(:email => "invalid@email.com") - assert reset_password_user.errors[:email] assert_equal 'not found', reset_password_user.errors[:email] end - test 'should reset reset password token before send the reset instructions email' do + test 'should reset reset_password_token before send the reset instructions email' do user = create_user token = user.reset_password_token reset_password_user = User.send_reset_password_instructions(:email => user.email) assert_not_equal token, user.reload.reset_password_token end - test 'should send email instructions to the user reset it\'s password' do + test 'should send email instructions to the user reset his password' do user = create_user assert_email_sent do User.send_reset_password_instructions(:email => user.email) end end - test 'should find a user to reset it\'s password based on reset_password_token' do + test 'should find a user to reset his password based on reset_password_token' do user = create_user user.send :generate_reset_password_token! reset_password_user = User.reset_password!(:reset_password_token => user.reset_password_token) - assert_not_nil reset_password_user assert_equal reset_password_user, user end - test 'should return a new user when trying to reset it\'s password if no reset_password_token is found' do + test 'should a new record with errors if no reset_password_token is found' do reset_password_user = User.reset_password!(:reset_password_token => 'invalid_token') - assert_not_nil reset_password_user assert reset_password_user.new_record? + assert_equal 'is invalid', reset_password_user.errors[:reset_password_token] end - test 'should add error to new user email if no reset password token was found' do - reset_password_user = User.reset_password!(:reset_password_token => "invalid_token") - assert reset_password_user.errors[:reset_password_token] - assert_equal 'is invalid', reset_password_user.errors[:reset_password_token] + test 'should a new record with errors if reset_password_token is blank' do + reset_password_user = User.reset_password!(:reset_password_token => '') + assert reset_password_user.new_record? + assert_equal "can't be blank", reset_password_user.errors[:reset_password_token] end test 'should reset successfully user password given the new password and confirmation' do diff --git a/test/models/rememberable_test.rb b/test/models/rememberable_test.rb index 396f35c0..7e7e123e 100644 --- a/test/models/rememberable_test.rb +++ b/test/models/rememberable_test.rb @@ -82,49 +82,48 @@ class RememberableTest < ActiveSupport::TestCase end test 'remember for should fallback to devise remember for default configuration' do - begin - remember_for = Devise.remember_for + swap Devise, :remember_for => 1.day do user = create_user - Devise.remember_for = 1.day user.remember_me! assert_not user.remember_expired? - Devise.remember_for = 0.days - user.remember_me! - assert user.remember_expired? - ensure - Devise.remember_for = remember_for end end test 'remember expires at should sum date of creation with remember for configuration' do - Devise.remember_for = 3.days - user = create_user - user.remember_me! - assert_equal 3.days.from_now.to_date, user.remember_expires_at.to_date - Devise.remember_for = 5.days - assert_equal 5.days.from_now.to_date, user.remember_expires_at.to_date + swap Devise, :remember_for => 3.days do + user = create_user + user.remember_me! + assert_equal 3.days.from_now.to_date, user.remember_expires_at.to_date + + Devise.remember_for = 5.days + assert_equal 5.days.from_now.to_date, user.remember_expires_at.to_date + end end test 'remember should be expired if remember_for is zero' do - Devise.remember_for = 0.days - user = create_user - user.remember_me! - assert user.remember_expired? + swap Devise, :remember_for => 0.days do + Devise.remember_for = 0.days + user = create_user + user.remember_me! + assert user.remember_expired? + end end test 'remember should be expired if it was created before limit time' do - Devise.remember_for = 1.day - user = create_user - user.remember_me! - user.update_attribute(:remember_created_at, 2.days.ago) - assert user.remember_expired? + swap Devise, :remember_for => 1.day do + user = create_user + user.remember_me! + user.update_attribute(:remember_created_at, 2.days.ago) + assert user.remember_expired? + end end test 'remember should not be expired if it was created whitin the limit time' do - Devise.remember_for = 30.days - user = create_user - user.remember_me! - user.update_attribute(:remember_created_at, 30.days.ago + 2.minutes) - assert_not user.remember_expired? + swap Devise, :remember_for => 30.days do + user = create_user + user.remember_me! + user.update_attribute(:remember_created_at, 30.days.ago + 2.minutes) + assert_not user.remember_expired? + end end end diff --git a/test/models/timeoutable_test.rb b/test/models/timeoutable_test.rb index dcf871f4..af1fb9f8 100644 --- a/test/models/timeoutable_test.rb +++ b/test/models/timeoutable_test.rb @@ -19,6 +19,7 @@ class TimeoutableTest < ActiveSupport::TestCase user = new_user assert user.timeout?(2.minutes.ago) assert_not user.timeout?(30.seconds.ago) + Devise.timeout = 5.minutes assert_not user.timeout?(2.minutes.ago) assert user.timeout?(6.minutes.ago)