From 5e3eee50b96c156434bfa452fe76e39824b3ddee Mon Sep 17 00:00:00 2001 From: Sydney Young Date: Thu, 4 Aug 2016 12:02:04 -0400 Subject: [PATCH] Refactor Catalog Controller Spec Resolves #1590 --- spec/controllers/catalog_controller_spec.rb | 180 +++++++----------- spec/support/controller_helpers.rb | 25 ++- spec/support/mockers/cart.rb | 12 ++ spec/support/mockers/category.rb | 21 ++ spec/support/mockers/equipment_item.rb | 21 ++ spec/support/mockers/equipment_model.rb | 32 ++++ spec/support/mockers/mocker.rb | 103 ++++++++++ spec/support/mockers/reservation.rb | 19 ++ spec/support/mockers/user.rb | 18 ++ .../shared_examples/controller_examples.rb | 13 ++ 10 files changed, 323 insertions(+), 121 deletions(-) create mode 100644 spec/support/mockers/cart.rb create mode 100644 spec/support/mockers/category.rb create mode 100644 spec/support/mockers/equipment_item.rb create mode 100644 spec/support/mockers/equipment_model.rb create mode 100644 spec/support/mockers/mocker.rb create mode 100644 spec/support/mockers/reservation.rb create mode 100644 spec/support/mockers/user.rb create mode 100644 spec/support/shared_examples/controller_examples.rb diff --git a/spec/controllers/catalog_controller_spec.rb b/spec/controllers/catalog_controller_spec.rb index 13cc5fd96..e98bd3182 100644 --- a/spec/controllers/catalog_controller_spec.rb +++ b/spec/controllers/catalog_controller_spec.rb @@ -1,164 +1,120 @@ +# frozen_string_literal: true require 'spec_helper' describe CatalogController, type: :controller do + let!(:user) { UserMock.new(traits: [:findable]) } + let!(:cart) { CartMock.new(reserver_id: user.id, items: {}) } before(:each) do - @app_config = FactoryGirl.create(:app_config) - @user = FactoryGirl.create(:user) - @cart = FactoryGirl.build(:cart, reserver_id: @user.id) - sign_in @user - # @controller.stub(:cart).and_return(session[@cart]) - # @controller.stub(:fix_cart_date) + mock_app_config + mock_user_sign_in(user) + allow_any_instance_of(described_class).to receive(:fix_cart_date) + allow_any_instance_of(ApplicationController).to receive(:fix_cart_date) + allow_any_instance_of(described_class).to \ + receive(:prepare_catalog_index_vars) + allow_any_instance_of(described_class).to receive(:cart).and_return(cart) end + describe 'GET index' do - before(:each) do - # the first hash passed here is params[] and the second is session[] - get :index, {} # , { cart: @cart } - end - it 'sets @reserver_id to the current cart.reserver_id' do - expect(assigns(:reserver_id)).to eq(@user.id) + before(:each) { get :index, {}, cart: cart } + it 'gets the reserver id' do + expect(cart).to have_received(:reserver_id).at_least(:once) end - it { is_expected.to respond_with(:success) } - it { is_expected.to render_template(:index) } - it { is_expected.not_to set_flash } + it_behaves_like 'successful request', :index end + describe 'PUT add_to_cart' do context 'valid equipment_model selected' do - before(:each) do - @equipment_model = FactoryGirl.create(:equipment_model) - put :add_to_cart, id: @equipment_model.id + let!(:eq_model) { EquipmentModelMock.new(traits: [:findable]) } + before do + allow(cart).to receive(:validate_all).and_return([]) + put :add_to_cart, { id: eq_model.id }, cart: cart end - it 'should call cart.add_item to add item to cart' do - expect do - put :add_to_cart, id: @equipment_model.id - end.to change { session[:cart].items[@equipment_model.id] }.by(1) + it 'calls cart.add_item to add item to cart' do + expect(cart).to have_received(:add_item).with(eq_model, any_args) end it 'should set flash[:error] if errors exist' do - allow(@cart).to receive(:validate_items).and_return('test') - allow(@cart).to receive(:validate_dates_and_items).and_return('test2') + allow(cart).to receive(:validate_all).and_return(['ERROR']) + put :add_to_cart, id: eq_model.id expect(flash[:error]).not_to be_nil end + it { is_expected.to set_flash[:notice] } it { is_expected.to redirect_to(root_path) } end - context 'invalid equipment_model selected' do + context 'no equipment_model selected' do before(:each) do - # there are no equipment models in the db so this is invalid - put :add_to_cart, id: 1 + allow(Rails.logger).to receive(:error) + put :add_to_cart, { id: 1 }, cart: cart end - it { is_expected.to redirect_to(root_path) } - it { is_expected.to set_flash } it 'should add logger error' do - expect(Rails.logger).to\ - receive(:error).with('Attempt to add invalid equipment model 1') - # this call has to come after the previous line - put :add_to_cart, id: 1 + expect(Rails.logger).to have_received(:error) + .with('Attempt to add invalid equipment model 1') end + it_behaves_like 'redirected request' end end describe 'POST submit_cart_updates_form on item' do - before(:each) do - @equipment_model = FactoryGirl.create(:equipment_model) - put :add_to_cart, id: @equipment_model.id + let!(:eq_model) { EquipmentModelMock.new(traits: [:findable]) } + let!(:attrs) { { id: eq_model.id, quantity: 2, reserver_id: user.id } } + it 'adjusts item quantity with cart#edit_cart_item' do + post :submit_cart_updates_form, attrs, cart: cart + expect(cart).to have_received(:edit_cart_item).with(eq_model, 2) end - it 'should adjust item quantity' do - params = { id: @equipment_model.id, - quantity: 2, - reserver_id: @user.id } - post :submit_cart_updates_form, params - expect(session[:cart].items[@equipment_model.id]).to eq(2) - expect(assigns(:errors)).to eq session[:cart].validate_all - is_expected.to redirect_to(new_reservation_path) + context 'newly empty cart' do + before do + allow(cart).to receive(:items).and_return(spy('Array', empty?: true)) + post :submit_cart_updates_form, attrs, cart: cart + end + it { is_expected.to redirect_to(root_path) } end - it 'should remove item when quantity is 0' do - params = { id: @equipment_model.id, - quantity: 0, - reserver_id: @user.id } - post :submit_cart_updates_form, params - # should remove the item after setting quantity to 0 - expect(session[:cart].items).to be_empty - is_expected.to redirect_to(root_path) + context 'non empty cart' do + before do + allow(cart).to receive(:items).and_return(spy('Array', empty?: false)) + post :submit_cart_updates_form, attrs, cart: cart + end + it { is_expected.to redirect_to(new_reservation_path) } end end describe 'PUT changing dates on confirm reservation page' do - before(:each) do - @equipment_model = FactoryGirl.create(:equipment_model) - put :add_to_cart, id: @equipment_model.id - end - it 'should set new dates' do - # sets start and due dates to tomorrow and day after tomorrow + # TODO: refactor update_cart so we can actually check that dates are + # being set + it 'calls update_cart' do tomorrow = Time.zone.today + 1.day params = { cart: { start_date_cart: tomorrow.strftime('%Y-%m-%d'), due_date_cart: (tomorrow + 1.day).strftime('%Y-%m-%d') }, - reserver_id: @user.id } - post :change_reservation_dates, params - expect(session[:cart].start_date).to eq(tomorrow) - expect(session[:cart].due_date).to eq(tomorrow + 1.day) + reserver_id: user.id } + expect_any_instance_of(described_class).to receive(:update_cart) + post :change_reservation_dates, params, cart: cart end end describe 'PUT update_user_per_cat_page' do - before(:each) do - put :update_user_per_cat_page - end - it 'should set session[:items_per_page] to params[items_per_page] '\ - 'if exists' do - put :update_user_per_cat_page, items_per_page: 20 - expect(session[:items_per_page]).to eq('20') - end - it 'should not alter session[:items_per_page] if '\ - 'params[:items_per_page] is nil' do - session[:items_per_page] = '15' - put :update_user_per_cat_page, items_per_page: nil - expect(session[:items_per_page]).not_to eq(nil) - expect(session[:items_per_page]).to eq('15') - end + before(:each) { put :update_user_per_cat_page, {}, cart: cart } it { is_expected.to redirect_to(root_path) } end - # I don't like that this test is actually searching the database, but - # unfortunately I couldn't get the model methods to stub correctly describe 'PUT search' do context 'query is blank' do - before(:each) do - put :search, query: '' - end + before(:each) { put :search, { query: '' }, cart: cart } it { is_expected.to redirect_to(root_path) } end context 'query is not blank' do - it 'should call catalog_search on EquipmentModel and return active '\ - 'equipment models' do - @equipment_model = FactoryGirl.create(:equipment_model, - active: true, - description: 'query') - # EquipmentModel.stub(:catelog_search).with('query') - # .and_return(@equipment_model) - put :search, query: 'query' - expect(assigns(:equipment_model_results)).to eq([@equipment_model]) - end - it 'should give unique results even with multiple matches' do - @equipment_model = FactoryGirl.create(:equipment_model, - active: true, - name: 'query', - description: 'query') - put :search, query: 'query' - expect(assigns(:equipment_model_results)).to eq([@equipment_model]) - expect(assigns(:equipment_model_results).uniq!).to eq(nil) # no dups + it 'calls catalog_search on EquipmentModel' do + expect(EquipmentModel).to \ + receive_message_chain(:active, :catalog_search) + put :search, { query: 'query' }, cart: cart end - it 'should call catalog_search on EquipmentItem' do - @equipment_item = - FactoryGirl.create(:equipment_item, serial: 'query') - # EquipmentItem.stub(:catelog_search).with('query') - # .and_return(@equipment_item) - put :search, query: 'query' - expect(assigns(:equipment_item_results)).to eq([@equipment_item]) + it 'calls catalog_search on EquipmentItem' do + allow(EquipmentItem).to receive(:catalog_search) + put :search, { query: 'query' }, cart: cart + expect(EquipmentItem).to have_received(:catalog_search).with('query') end - it 'should call catalog_search on Category' do - @category = FactoryGirl.create(:category, name: 'query') - # Category.stub(:catelog_search).with('query').and_return(@category) - put :search, query: 'query' - expect(assigns(:category_results)).to eq([@category]) + it 'calls catalog_search on Category' do + allow(Category).to receive(:catalog_search) + put :search, { query: 'query' }, cart: cart + expect(Category).to have_received(:catalog_search).with('query') end end end diff --git a/spec/support/controller_helpers.rb b/spec/support/controller_helpers.rb index 87e36a0f3..a90cf501c 100644 --- a/spec/support/controller_helpers.rb +++ b/spec/support/controller_helpers.rb @@ -1,14 +1,21 @@ -# some basic helpers to simulate devise controller methods in specs +# frozen_string_literal: true +require Rails.root.join('spec/support/mockers/user.rb') + module ControllerHelpers - def current_user - user_session_info = - response.request.env['rack.session']['warden.user.user.key'] - return unless user_session_info - user_id = user_session_info[0][0] - User.find(user_id) + def mock_user_sign_in(user = UserMock.new(traits: [:findable])) + pass_app_setup_check + allow(request.env['warden']).to receive(:authenticate!).and_return(user) + # necessary for permissions to work + allow(ApplicationController).to receive(:current_user).and_return(user) + allow(Ability).to receive(:new).and_return(Ability.new(user)) + allow_any_instance_of(described_class).to \ + receive(:current_user).and_return(user) end - def user_signed_in? - !current_user.nil? + private + + def pass_app_setup_check + allow(AppConfig).to receive(:first).and_return(true) unless AppConfig.first + allow(User).to receive(:count).and_return(1) unless User.first end end diff --git a/spec/support/mockers/cart.rb b/spec/support/mockers/cart.rb new file mode 100644 index 000000000..60ba4e47b --- /dev/null +++ b/spec/support/mockers/cart.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true +require Rails.root.join('spec/support/mockers/mocker.rb') + +class CartMock < Mocker + def self.klass + Cart + end + + def self.klass_name + 'Cart' + end +end diff --git a/spec/support/mockers/category.rb b/spec/support/mockers/category.rb new file mode 100644 index 000000000..9c4cc7521 --- /dev/null +++ b/spec/support/mockers/category.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true +require Rails.root.join('spec/support/mockers/mocker.rb') +require Rails.root.join('spec/support/mockers/equipment_model.rb') + +class CategoryMock < Mocker + def self.klass + Category + end + + def self.klass_name + 'Category' + end + + private + + def with_equipment_models(models: nil, count: 1) + models ||= Array.new(count) { EquipmentModelMock.new } + parent_has_many(mocked_children: models, parent_sym: :category, + child_sym: :equipment_models) + end +end diff --git a/spec/support/mockers/equipment_item.rb b/spec/support/mockers/equipment_item.rb new file mode 100644 index 000000000..87f69f37b --- /dev/null +++ b/spec/support/mockers/equipment_item.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true +require Rails.root.join('spec/support/mockers/mocker.rb') +require Rails.root.join('spec/support/mockers/equipment_model.rb') + +class EquipmentItemMock < Mocker + def self.klass + EquipmentItem + end + + def self.klass_name + 'EquipmentItem' + end + + private + + def with_model(model: nil) + model ||= EquipmentModelMock.new + child_of_has_many(mocked_parent: model, parent_sym: :equipment_model, + child_sym: :equipment_items) + end +end diff --git a/spec/support/mockers/equipment_model.rb b/spec/support/mockers/equipment_model.rb new file mode 100644 index 000000000..8433669ad --- /dev/null +++ b/spec/support/mockers/equipment_model.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true +require Rails.root.join('spec/support/mockers/mocker.rb') +require Rails.root.join('spec/support/mockers/category.rb') +require Rails.root.join('spec/support/mockers/equipment_item.rb') + +class EquipmentModelMock < Mocker + def self.klass + EquipmentModel + end + + def self.klass_name + 'EquipmentModel' + end + + private + + def with_item(item: EquipmentItemMock.new) + with_items(items: [item]) + end + + def with_items(items: nil, count: 1) + items ||= Array.new(count) { EquipmentItemMock.new } + parent_has_many(mocked_children: items, parent_sym: :equipment_model, + child_sym: :equipment_items) + end + + def with_category(cat: nil) + cat ||= CategoryMock.new + child_of_has_many(mocked_parent: cat, parent_sym: :category, + child_sym: :equipment_models) + end +end diff --git a/spec/support/mockers/mocker.rb b/spec/support/mockers/mocker.rb new file mode 100644 index 000000000..8b95b22a8 --- /dev/null +++ b/spec/support/mockers/mocker.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true +require 'rspec/mocks/standalone' + +# This class behaves as an extension of rspec-mocks' instance_spy. +# It is intended to be extended and used to make mocking models much simpler! +# +# To create a new subclass, the following methods must be overridden: +# - self.klass must return the class that the subclass is mocking +# - self.klass_name must return a string that matches the class being mocked +# +# Some examples using the EquipmentModelMock subclass: +# A mock that can be "found" with EquipmentModel#find: +# EquipmentModelMock.new(traits: [:findable]) +# A mock with a set of attributes: +# EquipmentModelMock.new(name: 'Camera', late_fee: 3) +# A mock with attributes and method stubs: +# EquipmentModelMock.new(name: 'Camera', model_restriced: false) +# A findable mock with attributes: +# EquipmentModelMock.new(traits: [:findable], name: 'Camera') +# +# A trait can be any method that exists on the mocker superclass or child class. +# To create an EquipmentModel that belongs to an existing category, camera: +# EquipmentModelMock.new(traits: [[:with_category, cat: camera]]) +# +# Use caution before adding methods -- any method defined here should be usable +# by all subclasses, with the exception of the association stub methods. + +class Mocker < RSpec::Mocks::InstanceVerifyingDouble + include RSpec::Mocks + + FIND_METHODS = [:find, :find_by_id].freeze + + def initialize(traits: [], **attrs) + # from RSpec::Mocks::ExampleMethods + # combination of #declare_verifying_double and #declare_double + ref = ObjectReference.for(self.class.klass_name) + RSpec::Mocks.configuration.verifying_double_callbacks.each do |block| + block.call(ref) + end + attrs ||= {} + super(ref, attrs) + as_null_object + process_traits(traits) + end + + def process_traits(traits) + traits.each { |t| send(*t) } + end + + private + + def klass + Object + end + + def klass_name + 'Object' + end + + def spy + self + end + + # lets us use rspec-mock syntax in mockers + def receive(method_name, &block) + Matchers::Receive.new(method_name, block) + end + + def allow(target) + AllowanceTarget.new(target) + end + + # Traits + def findable + id = FactoryGirl.generate(:unique_id) + allow(spy).to receive(:id).and_return(id) + FIND_METHODS.each do |method| + allow(self.class.klass).to receive(method) + allow(self.class.klass).to receive(method).with(id).and_return(spy) + allow(self.class.klass).to receive(method).with(id.to_s).and_return(spy) + end + end + + # Generalized association stubs + def child_of_has_many(mocked_parent:, parent_sym:, child_sym:) + allow(spy).to receive(parent_sym).and_return(mocked_parent) + children = if mocked_parent.send(child_sym).is_a? Array + mocked_parent.send(child_sym) << spy + else + [spy] + end + allow(mocked_parent).to receive(child_sym).and_return(children) + end + + def parent_has_many(mocked_children:, parent_sym:, child_sym:) + if mocked_children.is_a? Array + mocked_children.each do |child| + allow(child).to receive(parent_sym).and_return(spy) + end + end + allow(spy).to receive(child_sym).and_return(mocked_children) + end +end diff --git a/spec/support/mockers/reservation.rb b/spec/support/mockers/reservation.rb new file mode 100644 index 000000000..4c1f09914 --- /dev/null +++ b/spec/support/mockers/reservation.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true +require Rails.root.join('spec/support/mockers/mocker.rb') + +class ReservationMock < Mocker + def self.klass + Reservation + end + + def self.klass_name + 'Reservation' + end + + private + + def for_user(user:) + child_of_has_many(mocked_parent: user, parent_sym: :reserver, + child_sym: :reservations) + end +end diff --git a/spec/support/mockers/user.rb b/spec/support/mockers/user.rb new file mode 100644 index 000000000..1270ac456 --- /dev/null +++ b/spec/support/mockers/user.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +require Rails.root.join('spec/support/mockers/mocker.rb') + +class UserMock < Mocker + def initialize(role = :user, traits: [], **attrs) + attrs = FactoryGirl.attributes_for(role).merge attrs + traits = [:findable] if traits.empty? + super(traits: traits, **attrs) + end + + def self.klass + User + end + + def self.klass_name + 'User' + end +end diff --git a/spec/support/shared_examples/controller_examples.rb b/spec/support/shared_examples/controller_examples.rb new file mode 100644 index 000000000..3d645a7b7 --- /dev/null +++ b/spec/support/shared_examples/controller_examples.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true +require 'spec_helper' + +shared_examples_for 'successful request' do |template| + it { is_expected.to respond_with(:success) } + it { is_expected.to render_template(template) } + it { is_expected.not_to set_flash } +end + +shared_examples_for 'redirected request' do + it { expect(response).to be_redirect } + it { is_expected.to set_flash } +end