1

Background: I've got an after_action callback in my controller, which takes the string address, processes it and stores longitude and latitude in corresponding fields. I want to test this.

This SO question, as well as this article only consider update methods, but at least, they are quite clear, because I've already got an object to work with.

So my question is - how to find this newly created record? This SO question led me to this code:

require 'rails_helper'

RSpec.describe Admin::Settings::GeneralSettingsController, type: :controller do

  context "POST methods" do
    describe "#edit and #create" do
      it "encodes and stores lang/lot correctly" do
          post :create, general_setting: FactoryGirl.attributes_for(:general_setting)
          expect(assigns(:general_setting).long).to eq(37.568021)
          # expect(general_setting.long).to eq(37.568021)
          # expect(general_setting.lat).to eq(55.805553)
      end
    end
  end
end

But using the code in the answer, I get this error:

Failure/Error: expect(assigns(:general_setting).long).to eq(37.568021)

     NoMethodError:
       undefined method `long' for nil:NilClass

Update #1: This is my new controller spec code:

RSpec.describe Admin::Settings::GeneralSettingsController, type: :controller do

  context 'POST methods' do
    before(:each) do
      allow(subject).to receive(:set_long_lat)
    end

    describe 'post create' do
      before(:each) do
        post :create, params: { general_setting: FactoryGirl.attributes_for(:general_setting) }
      end

      it "saves the record with valid attributes" do
        expect{subject}.to change{GeneralSetting.count}.by(1)
      end

      it 'calls :set_long_lat' do
        expect(subject).to have_received(:set_long_lat)
      end
    end
  end

  describe '#set_long_lat' do
    # spec for method
  end
end

Update #2:

Here is my controller code:

class Admin::Settings::GeneralSettingsController < AdminController
  include CrudConcern

  before_action :find_general_setting, only: [:edit, :destroy, :update, :set_long_lat]
  after_action :set_long_lat

  def index
    @general_settings = GeneralSetting.all
  end

  def new
    @general_setting = GeneralSetting.new
    # Билдим для того, что бы было видно сразу одно поле и пользователь не должен
    # кликать на "добавить телефон"
    @general_setting.phones.build
    @general_setting.opening_hours.build
  end

  def edit
    # Тоже самое, что и с нью - если телефонов нет вообще, то показываем одно пустое поле
    if @general_setting.phones.blank?
      @general_setting.phones.build
    end

    if @general_setting.opening_hours.blank?
      @general_setting.opening_hours.build
    end
  end

  def create
    @general_setting = GeneralSetting.new(general_setting_params)
    create_helper(@general_setting, "edit_admin_settings_general_setting_path")
  end

  def destroy
    destroy_helper(@general_setting, "admin_settings_general_settings_path")
  end

  def update
    # debug
    # @general_setting.update(language: create_hash(params[:general_setting][:language]))
    @general_setting.language = create_hash(params[:general_setting][:language])
    update_helper(@general_setting, "edit_admin_settings_general_setting_path", general_setting_params)
  end


  private

  def set_long_lat
    geocoder = Geocoder.new
    data = geocoder.encode!(@general_setting.address)
    @general_setting.update!(long: data[0], lat: data[1])
  end

  def find_general_setting
    @general_setting = GeneralSetting.find(params[:id])
  end

  def general_setting_params
    params.require(:general_setting).permit(GeneralSetting.attribute_names.map(&:to_sym).push(
      phones_attributes: [:id, :value, :_destroy, :general_setting_id ]).push(
      opening_hours_attributes: [:id, :title, :value, :_destroy, :deneral_setting_id]) )
  end

  def create_hash(params)
    language_hash = Hash.new

    params.each do |param|
      language_hash[param.to_sym] = param.to_sym
    end
    return language_hash
  end
end

(If it helps - I've got a lot of similar crud-actions, that is why I've put them all in a concern controller)

module CrudConcern 
  extend ActiveSupport::Concern
  include Language

  included do
    helper_method :create_helper, :update_helper, :destroy_helper, :get_locales
  end

  def get_locales
    @remaining_locales = Language.get_remaining_locales
  end

  def create_helper(object, path)
    if object.save!
      respond_to do |format|
        format.html {
          redirect_to send(path, object)
          flash[:primary] = "Well done!"
        }
      end
    else
      render :new
      flash[:danger] = "Something not quite right"
    end
    @remaining_locales = Language.get_remaining_locales
  end

  def update_helper(object, path, params)
    if object.update!(params)
      respond_to do |format|
        format.html {
          redirect_to send(path, object)
          flash[:primary] = "Well done!"
        }
      end
    else
      render :edit
      flash[:danger] = "Something's not quite right"
    end
  end

  def destroy_helper(object, path)
    if object.destroy
      respond_to do |format|
        format.html {
          redirect_to send(path)
          flash[:primary] = "Well done"    
        }
      end
    else
      render :index
      flash[:danger] = "Something's not quite right"
    end
  end

end

Update #3 It's not the ideal solution, but, somehow, controller tests just won't work. I've moved my callback into the model and updated my general_setting_spec test.

class GeneralSetting < ApplicationRecord
  after_save :set_long_lat    
  validates :url, presence: true

  private

  def set_long_lat
    geocoder = Geocoder.new
    data = geocoder.encode(self.address)
    self.update_column(:long, data[0])
    self.update_column(:lat, data[1])
  end
end

My tests now:

RSpec.describe GeneralSetting, type: :model do

  let (:regular) { FactoryGirl.build(:general_setting) } 

  describe "checking other validations" do
    it "is invalid with no url" do
      expect{
        invalid.save
      }.not_to change(GeneralSetting, :count)
    end

    it 'autofills the longitude' do
      expect{ regular.save }.to change{ regular.long }.from(nil).to(37.568021)
    end

    it 'autofills the latitude' do
      expect{ regular.save }.to change{ regular.lat }.from(nil).to(55.805078)
    end
  end
end
Community
  • 1
  • 1
mohnstrudel
  • 639
  • 8
  • 22

1 Answers1

0

I would test expectation that controller calls method specified in after_action and make a separate test for that method.

Something like:

context 'POST methods' do
  before(:each) do
    allow(subject).to receive(:method_from_callback)
  end

  describe 'post create' do
    before(:each) do
      post :create, params: { general_setting: attributes_for(:general_setting) }
    end

    it 'calls :method_from_callback' do
      expect(subject).to have_received(:method_from_callback)
    end
  end
end

describe '#method_from_callback' do
  # spec for method
end

Be sure to use your method name instead of :method_from_callback pay attention that I've used rspec 3.5 syntax (wrapped request request parameters into params).

Maxim Khan-Magomedov
  • 1,326
  • 12
  • 15
  • Sorry for a bit of off-topic, but I can't additionally make the simple create action pass: `describe 'post create' do before(:each) do post :create, params: { general_setting: FactoryGirl.attributes_for(:general_setting) } end it "saves the record with valid attributes" do expect{subject}.to change{GeneralSetting.count}.by(1) end it 'calls :set_long_lat' do expect(subject).to have_received(:set_long_lat) end end` It says `expected result to have changed by 1, but was changed by 0` Could the callback be the issue? – mohnstrudel Mar 16 '17 at 12:27
  • Probably. I usually use bang versions (`create!` or `save!`) in actual controller when I think that it works, but rspec shows that it doesn't. This will at least throw exception into console. – Maxim Khan-Magomedov Mar 16 '17 at 12:31
  • It's a bit hard to read in a comment, I've updated my question. Besides, for the callback, I now receive this error: `expected: 1 time with any arguments received: 0 times with any arguments` – mohnstrudel Mar 16 '17 at 12:31
  • Then maybe it is not called afterwards :) Can you also add controller code? – Maxim Khan-Magomedov Mar 16 '17 at 12:33
  • Besides, I'm not sure that `expect {subject}.to change{GeneralSetting.count}` will do what you expect it to do, because `before` callback will fire before `it`. I usually use other form: `let(:action) { -> { post :create, ....} }` and then `expect(action).to change(ModelName, :count)`. – Maxim Khan-Magomedov Mar 16 '17 at 12:37
  • Everything looks like it should work. I think there is some problem with rspec syntax. What version of rspec and rails do you use? Because for rails 4 one should not wrap request parameters into `params`. So it will be just `post :create, general_setting: attributes_for(:general_setting)`. And the rest would be the same. – Maxim Khan-Magomedov Mar 16 '17 at 12:42
  • I'm using: `rspec -v 3.5.4 rails -v Rails 5.0.2` – mohnstrudel Mar 16 '17 at 12:45
  • You also have `set_long_lat` in `before_action` in `only`. Should it be there? And I think that is is the key as it responds with http 404. Add checking happy path: `it 'responds with HTTP 200' { expect(response).to have_http_status(:success) }` – Maxim Khan-Magomedov Mar 16 '17 at 12:53
  • I think yes, it should be here because I call :find_general_setting for this method (need the @general_setting variable to access the address field). However, now, I moved all the code from controller to the model (updated my question) and moved my tests from controller to the model spec. Now it works. I still receive the error, that upon saving the GeneralSetting count was not increased by 1, even if no callback are now involved. Strange... – mohnstrudel Mar 16 '17 at 13:20
  • But it uses `find` with `params[:id]` and there is no `params[:id]` in `create`. So I think you get the following sequence: `create` and then `find_general_setting`, which raises `ActiveRecord::RecordNotFound`, so `set_long_lat` is never called indeed. – Maxim Khan-Magomedov Mar 16 '17 at 13:36
  • I've added the path check `Add checking happy path: it 'responds with HTTP 200' { expect(response).to have_http_status(:success) }` and the output is `expected the response to have a success status code (2xx) but it was 302`. My `expect(subject).to have_received` test results in: `expected: 1 time with any arguments received: 0 times with any arguments` – mohnstrudel Mar 17 '17 at 12:23
  • That should be actually redirect for `create`, yes. You can also try to use `expect(subject).to receive(:set_long_lat)` in `bafore(:each)` and remove expectation that it has received from `it`, but I think you'll get the same result. That's come kind of black magic or we both miss something obvious. – Maxim Khan-Magomedov Mar 18 '17 at 16:51