Writing valuable controller tests - Rspec

Question

I am looking to thoroughly test my controller method before moving onto my next part of code but would like to know how to break this following method down into testable chunks

private

 def create_real_user
   return unless current_or_guest_user.is_guest?
   generated_password = Devise.friendly_token.first(8)
   @user = User.new(
     is_guest: false,
     first_name: params[:first_name],
     last_name: params[:last_name],
     email: params[:email],
     password: generated_password,
     password_confirmation: generated_password,
     braintree_id: @result.transaction.customer_details.id
   )
   @user.save(validate: false)
   RegistrationMailer.welcome(@user, generated_password).deliver_now
 end 

This is called after a user of the site has completed a transaction

def create
  @result = Braintree::Transaction.sale(
              amount: @transaction_total,
              payment_method_nonce: params[:payment_method_nonce],
          )
  if @result.success?
    create_real_user
    update_completed_transaction
    guest_user.destroy
    redirect_to thank_you_path
  else
    update_transaction
    @error_message = BraintreeErrors::Errors.new.error_message(@result)
    flash.now[:alert] = @error_message
    flash.keep
    redirect_to new_transaction_path
  end
end

As you can see there are a few method calls but i would like to test them individually.

How would i go about setting this up with rspec

Thanks

Answer 1

I think @Nabeel is on the right track in terms of breaking your code down into testable chunks (first part of your question). I'll get to your second part (how to set up in RSpec) in a bit.

I suggest further refactoring. Personally, I want almost no logic in my controllers. I prefer to move logic into a 'manager' (just a PORO that has a parallel name to the controller). I like this because I find testing POROs a lot easier than testing controllers. I don't know what the name of the above controller is, so let's just call it FooController. The manager would be FooManager.

#controllers/foo_controller.rb
class FooController < ApplicationController

  def create
    # sometimes I have to do pre-processing of params, but I 
    # try to keep this to a minimum as is violates my 'dumb 
    # controller' mantra. My preference is to pass them is
    # 'as is' whenever possible.
    @results = FooManager.create(params)
    redirect_to @results[:success] ? thank_you_path : new_transaction_path
  end

end

#managers/foo_manager.rb
class FooManager 
  class << self

    # my managers' public methods are always named exactly
    # the same as their paired controller methods so I
    # don't have to remember what I named things.
    def create(params)
      @params = params # make params available to all subsequent methods
      {success: process_braintree_sale}
    end

    private

    # as written, this method will fail because @transaction_total
    # hasn't been defined yet.
    def process_braintree_sale
      @braintree_sale = Braintree::Transaction.sale(
        amount: @transaction_total,
        payment_method_nonce: @params[:payment_method_nonce],
      )
      @braintree_sale.success ? post_process_success : post_process_failure
    end

    # other methods as Nabeel outlined above.

  end
end

Now, for your test, you could do (I like to break my tests up by method):

#spec/managers/foo_manager/create
require 'rails_helper'

RSpec.describe FooManager do
  describe "#create" do # I will only test the #create method in this file
    context "when using a good params" do
      before(:each) do
        @params = ActionController.parameters.new(
          good: 'parameters',
          provided: 'here'
        ) # create a set of ActionController parameters
      end
      it "creates a User" do 
        expect{FooManager.create(@params)}.to change{User.count}.by(1)
      end
      it "does other useful stuff" do
        # write another post-condition test here
      end
      it "does even more useful stuff" do
        # write another post-condition test here
      end
      # until you have comprehensive post-condition tests
    end
  end
end

I think there is some discussion about whether the private methods should be tested. As long as your post-condition tests are comprehensive, then you should only have to test the one public method, FooManager.create in this case.

If you follow this path, then all your controller is doing is calling FooManager.create(params) and then redirecting. You could test your redirects in a separate RSpec test file. Personally, I tend to skip controller testing in RSpec when my methods are super skinny and defer redirect testing to integration tests with Cucumber/Capybara.

Answer 2

Perhaps something like this? (untested)

def create
  @result = Braintree::Transaction.sale(
    amount: @transaction_total,
    payment_method_nonce: params[:payment_method_nonce],
  )

  @result.success? ? successful_transaction : transaction_error
end

def successful_transaction
  setup_user
  update_completed_transaction
  guest_user.destroy

  redirect_to thank_you_path
end

def transaction_error
  update_transaction
  @error_message = BraintreeErrors::Errors.new.error_message(@result)
  flash.now[:alert] = @error_message
  flash.keep

  redirect_to new_transaction_path
end

private

def setup_user
  return unless current_or_guest_user.is_guest?

  generated_password = generate_password
  @user = create_user(generated_password)
  save_user!

  RegistrationMailer.welcome(@user, generated_password).deliver_now
end 


def generate_password
  Devise.friendly_token.first(8)
end

def create_user(generated_password)
  User.new(
     is_guest: false,
     first_name: params[:first_name],
     last_name: params[:last_name],
     email: params[:email],
     password: generated_password,
     password_confirmation: generated_password,
     braintree_id: @result.transaction.customer_details.id
   )
end

def save_user!
  @user.save(validate: false)
end

However you shouldn't really test private methods unless they're complicated (it's still good to break them up for readability and keeping the ABC to a minimum).