Rspec similar code for testing DELETE request gives different results

Question

Following Michael Hartl's book on Ruby on Rails, I can't seem to understand why one of the tests pass and another fails, considering they do mostly the same. This derives from testing UsersController#destroy from Exercise 9.6-9.

This is my spec/requests/user_pages_spec.rb:

require 'spec_helper'

describe "User pages" do

    subject { page }
    describe "index" do
        let(:user){ FactoryGirl.create(:user) }
        #before/after ALL TESTS, not USERS
        before(:all){ 30.times {FactoryGirl.create(:user) }}
        after(:all) {User.delete_all}

        #before EACH TEST, no user
        before(:each) do
            valid_signin user
            visit users_path
        end

        describe "delete links" do
            it { should_not have_link('delete') }
            describe "as an admin user" do
                let(:admin) { FactoryGirl.create(:admin) }
                let(:non_admin) { FactoryGirl.create(:user) }
                before do
                    valid_signin admin
                    visit users_path
                end
                it "should be able to delete another user" do
                    expect { delete user_path(user) }.to change(User, :count).by(-1)
                end
.
.
.
end

And this is my spec/requests/authentication_pages_spec.rb:

require 'spec_helper'

describe "AuthenticationPages" do
    subject{ page }
    describe "signin page" do
        before { visit signin_path }
        it { should have_selector('h1',text: 'Sign in') }
        it { should have_selector('title', text: full_title('Sign in')) }
    end
    describe "signin" do
        before { visit signin_path }

        #INVALID INFO
        describe "with invalid information" do
            before { click_button "Sign in"}

            it{ should have_selector('title', text: 'Sign in')}
            #it{ should have_selector('div.alert.alert-error', text: 'Invalid')}
            it{ should have_error_message('Invalid')}

            describe "after visiting another page" do
                before { click_link "Home" }
                #it{ should_not have_selector('div.alert.alert-error')}
                it{ should_not have_error_message()}
                #exercise 9.6-3
                it{ should_not have_link('Profile')}
                it{ should_not have_link('Settings')}
            end
        end

        #VALID INFO
        describe "with valid information" do
            let(:user) { FactoryGirl.create(:user) }
            before{ valid_signin(user) }

            it{ should have_selector('title', text: user.name)}
            it{ should have_link('Users', href: users_path)}
            it{ should have_link('Profile', href: user_path(user))}
            it{should have_link('Settings', href: edit_user_path(user))}
            it{ should have_link('Sign out', href: signout_path)}
            it{ should_not have_selector('Sign in', href:signin_path)}

            describe "followed by signout" do
                before{click_link "Sign out"}
                it{ should have_link('Sign in') }
            end
            #Exercise 9.6-6
            describe "accessing new and create actions" do
                describe "through website" do
                    before{visit signup_path}
                    it{ should_not have_selector('h1',text:"Sign up")} 
                    it{ should_not have_button("Create my account")} 
                end
                describe "through a POST request" do
                    before { post users_path}
                    specify { response.should redirect_to(root_path)}
                end
            end
        end
    end
    describe "authorization" do
        describe "as non-admin user" do
            let(:admin) {FactoryGirl.create(:admin)}
            let(:non_admin) {FactoryGirl.create(:user)}

            before{valid_signin non_admin}

            #Check that loggin to nonadmin works(debug ex.9.6-9)
            describe "should render the non-admin profile page" do
                it{ should have_selector('title', text: non_admin.name)}
            end

            describe "submitting a DELETE request to the Users#destroy action" do
                before do
                    delete user_path(admin)
                    #puts response.message
                    #puts response.success?
                end
                specify{ response.should redirect_to(root_path) }
                specify{ response.should_not be_success }
            end
        end
        #Exercise 9.6-9 prevent admin from destroying himself
        describe "as admin user" do
            let(:user){FactoryGirl.create(:user)}
            let(:admin){FactoryGirl.create(:admin)}
            let(:non_admin){FactoryGirl.create(:user)}

            before do 
                valid_signin admin
                visit users_path
            end
            it "should be able to delete another user" do
                expect { delete user_path(user) }.to change(User, :count).by(-1)
            end

.
.
.
end

I know from interacting with the application that deleting users does work, the problem is with testing it.The test of interest here is the one described as "should be able to delete another user", which is the same in both files user_pages_spec.rb and authentication_pages_spec.rb.

There's two things I cannot seem to understand:

1. At user_pages_spec.rb the test with expect { delete user_path(user) }.to change(User,:count).by(-1) does pass, however if I change it to expect { delete user_path(non_admin) }.to change(User,:count).by(-1), it fails. Why is that? They are both created with the same factory parameters.

2. Why is the test in the authentication_pages_spec.rb never passing? No matter whether it's user_path(user) or user_path(non_admin).

This is my factory:

FactoryGirl.define do
    factory :user do
        sequence(:name){ |n| "Person #{n}" }
        sequence(:email){ |n| "person_#{n}@example.com"}
        password "foobar"
        password_confirmation "foobar"

        factory :admin do
            admin true
        end
    end

end

Here my users_controller.rb:

class UsersController < ApplicationController
    before_filter :signed_in_user, only: [:index, :edit, :update]
    before_filter :correct_user, only: [:edit, :update]
    before_filter :admin_user, only: [:destroy]
    def new
        #change for exercise 9.6-6
        if signed_in?
            redirect_to root_path
        else
            @user=User.new
        end
    end
    def show
        @user=User.find(params[:id])
    end
    def create
        if signed_in?
            redirect_to root_path
        else
            @user = User.new(params[:user])
            if @user.save
                sign_in @user
                flash[:success]="Welcome to the Sample App!"
                # Handle a successful save.
                redirect_to @user
            else
                render 'new'
            end
        end
    end

    def edit
        #@user= User.find(params[:id]) <----we can delete this because the before filter correct_user now defines @user variable
    end

    def update
        #@user = User.find(params[:id])
        if @user.update_attributes(params[:user])
            # Handle a successful update.
            flash[:success]="Profile updated"
            sign_in @user
            redirect_to @user
        else
            render 'edit'
        end
    end

    def index
        #@users= User.all
        @users= User.paginate(page: params[:page])
    end

    def destroy
        puts "The current user is:"+current_user.name
            puts "Is user admin?:"+current_user.admin.to_s
            User.find(params[:id]).destroy
        flash[:success]="User destroyed."
        redirect_to users_path
    end

    private
    def signed_in_user
        unless signed_in?
            store_location
            redirect_to signin_path, notice: "Please sign in."
        end
    end

    def correct_user
        @user =User.find(params[:id])
        redirect_to(root_path) unless current_user?(@user)
    end

    def admin_user
        redirect_to(root_path) unless current_user.admin?
    end
end

And the model user.rb:

# == Schema Information
#
# Table name: users
#
#  id         :integer         not null, primary key
#  name       :string(255)
#  email      :string(255)
#  created_at :datetime        not null
#  updated_at :datetime        not null
#

class User < ActiveRecord::Base
  attr_accessible :email, :name, :password, :password_confirmation
  has_secure_password
  before_save { self.email.downcase! }
  #callback for session token generation
  before_save :create_remember_token

  validates :name, presence: true, length: {maximum: 50}
  VALID_EMAIL_REGEX = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i
  validates :email, presence: true, format: { with: VALID_EMAIL_REGEX }, uniqueness: { case_sensitive: false}
  #validates :password, presence: true, length:{minimum:6}
  validates :password, length:{minimum:6}
  validates :password_confirmation, presence: true


  private
    def create_remember_token
        self.remember_token=SecureRandom.urlsafe_base64
    end

end

This is the support file where valid_signin is defined:

include ApplicationHelper
def valid_signin(user)
    visit signin_path
    fill_in "Email", with: user.email
    fill_in "Password", with: user.password
    click_button "Sign in"
    # Sign in when not using Capybara as well.
    cookies[:remember_token] = user.remember_token
end

def valid_signup(user)
    fill_in "Name", with: user.name
    fill_in "Email", with: user.email
    fill_in "Password", with: user.password
    fill_in "Confirm Password", with: user.password_confirmation
    #click_button "Sign in"
end

RSpec::Matchers.define :have_error_message do |message|
    match do |page|
        page.should have_selector('div.alert.alert-error', text: message)
    end
end
RSpec::Matchers.define :have_success_message do |message|
    match do |page|
        page.should have_selector('div.alert.alert-success', text: message)
    end
end

" Note to others: This is a follow-on to Rails response.should be_success is never true "

Edit

This is the SessionsHelper( spec/helpers/sessions_helper.rb):

module SessionsHelper

    def sign_in(user)
        cookies.permanent[:remember_token]=user.remember_token
        current_user=user
    end
    def signed_in?
        !current_user.nil?
    end

    def current_user=(user)
        @current_user = user
    end

    def current_user
        @current_user ||= User.find_by_remember_token(cookies[:remember_token])
    end

    def current_user?(user)
        user == current_user
    end

    def sign_out
        current_user=nil
        cookies.delete(:remember_token)
    end
    def redirect_back_or(default)
        redirect_to(session[:return_to] || default)
        session.delete(:return_to)
    end
    def store_location
        session[:return_to] = request.fullpath
    end

end

EDIT 2 Added puts to UsersController#destroy.

This is the output for the different test running:

rspec on spec/requests/user_pages_spec.rb trying to delete user:

..The current user is: Person 35
Is user admin?: true
..

Finished in 8.16 seconds
4 examples, 0 failures

rspec on spec/requests/user_pages_spec.rb trying to delete non_admin:

The current user is: Person 35
Is user admin?: true
F.

Failures:

  1) User pages index delete links as an admin user should be able to delete another user
     Failure/Error: expect { delete user_path(non_admin) }.to change(User, :count).by(-1)
       count should have been changed by -1, but was changed by 0
     # ./spec/requests/user_pages_spec.rb:48:in `block (5 levels) in <top (required)>'

rspec on spec/requests/authentication_pages_spec.rb trying to delete either user or non_admin:

Run options: include {:full_description=>/(?-mix:as\ admin\ user)/}
The current user is: Person 1
Is user admin?: true
FThe current user is: Person 3
Is user admin?: true
FThe current user is: Person 5
Is user admin?: true
.

Failures:

  1) AuthenticationPages authorization as admin user should be able to delete another user
     Failure/Error: expect { delete user_path(user) }.to change(User, :count).by(-1)
       count should have been changed by -1, but was changed by 0
     # ./spec/requests/authentication_pages_spec.rb:99:in `block (4 levels) in <top (required)>'

I'm still not sure why it does it three times, one for each let(...)?

Final Edit

See solution as last answer.

Answer 1

Well, apparently I managed to solve the issue. The root of the problem was that let(...) is lazily evaluated, which means that "it is not evaluated until the first time the method it defines is invoked.". Documentation here.

Instead, let!(..:) can be used to force its evaluation. I have to thank NemesisD at the channel #rubyonrails for pointing this out, and also Peter Alfvin here on Stackoverflow.

The final code with tests passing looks like(see change from let to let!):

require 'spec_helper'

describe "AuthenticationPages" do
    subject{ page }
    describe "signin page" do
        before { visit signin_path }
        it { should have_selector('h1',text: 'Sign in') }
        it { should have_selector('title', text: full_title('Sign in')) }
    end
    describe "signin" do
        before { visit signin_path }

        #INVALID INFO
        describe "with invalid information" do
            before { click_button "Sign in"}

            it{ should have_selector('title', text: 'Sign in')}
            #it{ should have_selector('div.alert.alert-error', text: 'Invalid')}
            it{ should have_error_message('Invalid')}

            describe "after visiting another page" do
                before { click_link "Home" }
                #it{ should_not have_selector('div.alert.alert-error')}
                it{ should_not have_error_message()}
                #exercise 9.6-3
                it{ should_not have_link('Profile')}
                it{ should_not have_link('Settings')}
            end
        end

        #VALID INFO
        describe "with valid information" do
            let(:user) { FactoryGirl.create(:user) }
            before{ valid_signin(user) }

            it{ should have_selector('title', text: user.name)}
            it{ should have_link('Users', href: users_path)}
            it{ should have_link('Profile', href: user_path(user))}
            it{should have_link('Settings', href: edit_user_path(user))}
            it{ should have_link('Sign out', href: signout_path)}
            it{ should_not have_selector('Sign in', href:signin_path)}

            describe "followed by signout" do
                before{click_link "Sign out"}
                it{ should have_link('Sign in') }
            end
            #Exercise 9.6-6
            describe "accessing new and create actions" do
                describe "through website" do
                    before{visit signup_path}
                    it{ should_not have_selector('h1',text:"Sign up")} 
                    it{ should_not have_button("Create my account")} 
                end
                describe "through a POST request" do
                    before { post users_path}
                    specify { response.should redirect_to(root_path)}
                end
            end
        end
    end
    describe "authorization" do
        describe "as non-admin user" do
            let(:admin) {FactoryGirl.create(:admin)}
            let(:non_admin) {FactoryGirl.create(:user)}

            before{valid_signin non_admin}

            #Check that loggin to nonadmin works(debug ex.9.6-9)
            describe "should render the non-admin profile page" do
                it{ should have_selector('title', text: non_admin.name)}
            end

            describe "submitting a DELETE request to the Users#destroy action" do
                before do
                    delete user_path(admin)
                    #puts response.message
                    #puts response.success?
                end
                specify{ response.should redirect_to(root_path) }
                specify{ response.should_not be_success }
            end
        end
        #Exercise 9.6-9 prevent admin from destroying himself
        describe "as admin user" do
            let!(:user){FactoryGirl.create(:user)}
            let!(:admin){FactoryGirl.create(:admin)}
            let!(:non_admin){FactoryGirl.create(:user)}

            before{ valid_signin admin }

            it "should be able to delete another user" do
                expect { delete user_path(user) }.to change(User, :count).by(-1)
            end

.
.
.
end

Answer 2

In response to your first question, the difference between user and non-admin is that you logged in as user in the outer describe block. If your subsequent attempt to log-in as admin is failing and you remain logged in as user, that would explain the behavior you're seeing. You haven't provided the definition of valid_signin, but if it doesn't work independent of whether you are already signed in and have navigated to the sign in page, then this would explain what's going on.

Similarly, your authentication_pages_spec.rb examples rely entirely on valid_signin working successfully. While you haven't logged in previously in this example, you haven't done any navigation either, so if valid_signin is a simple form fill-in (as it was defined in the 3.2 version of the tutorial at http://ruby.railstutorial.org/book/ruby-on-rails-tutorial?version=3.2), then that would explain why it's failing in this case.

As an aside, if you're mixing 3.2 code snippets with 4.0 code snippets, you're going to encounter a lot of problems.

Note to others: This is a follow-on to Rails response.should be_success is never true