From 5d88b906cc2ca827347a99e8beb9bc16498f35eb Mon Sep 17 00:00:00 2001 From: Stef Schenkelaars Date: Tue, 11 Jun 2019 13:00:43 +0200 Subject: [PATCH] Refactored specs for HaveAttachedMatcher (#1102) --- .../active_record/have_attached_matcher.rb | 103 +++--- .../unit/helpers/active_record_versions.rb | 4 + .../have_attached_matcher_spec.rb | 304 ++++++++++++++---- 3 files changed, 299 insertions(+), 112 deletions(-) diff --git a/lib/shoulda/matchers/active_record/have_attached_matcher.rb b/lib/shoulda/matchers/active_record/have_attached_matcher.rb index 92ff19196..edfa48f51 100644 --- a/lib/shoulda/matchers/active_record/have_attached_matcher.rb +++ b/lib/shoulda/matchers/active_record/have_attached_matcher.rb @@ -18,20 +18,25 @@ def initialize(macro, name) @name = name end - def description - "has_#{macro}_attached #{name}" + def description + "have a has_#{macro}_attached called #{name}" end def failure_message - "Expected #{expectation} (#{@failure})" + <<-MESSAGE +Expected #{expectation}, but this could not be proved. + #{@failure} + MESSAGE end def failure_message_when_negated - "Did not expect #{expectation}" + <<-MESSAGE +Did not expect #{expectation}, but it does. + MESSAGE end def expectation - "#{model_class.name} to have a has_#{macro} attached called #{name}" + "#{model_class.name} to #{description}" end def matches?(subject) @@ -47,83 +52,93 @@ def matches?(subject) attr_reader :subject, :macro + def reader_attribute_exists? + if subject.respond_to?(name) + true + else + @failure = "#{model_class.name} does not have a :#{name} method." + false + end + end + + def writer_attribute_exists? + if subject.respond_to?("#{name}=") + true + else + @failure = "#{model_class.name} does not have a :#{name}= method." + false + end + end + def attachments_association_exists? if attachments_association_matcher.matches?(subject) true else - @failure = "no valid association called #{attachments_association_name}" + @failure = attachments_association_matcher.failure_message false end end def attachments_association_matcher - @attachments_association_matcher ||= - AssociationMatcher.new(:"has_#{macro}", attachments_association_name). + @_attachments_association_matcher ||= + AssociationMatcher.new( + :"has_#{macro}", + attachments_association_name, + ). conditions(name: name). class_name('ActiveStorage::Attachment'). inverse_of(:record). dependent(false) end + def attachments_association_name + case macro + when :one then + "#{name}_attachment" + when :many then + "#{name}_attachments" + end + end + def blobs_association_exists? if blobs_association_matcher.matches?(subject) true else - @failure = "no valid association called #{blobs_association_name}" + @failure = blobs_association_matcher.failure_message false end end def blobs_association_matcher - @blobs_association_matcher ||= - AssociationMatcher.new(:"has_#{macro}", blobs_association_name). + @_blobs_association_matcher ||= + AssociationMatcher.new( + :"has_#{macro}", + blobs_association_name, + ). through(attachments_association_name). class_name('ActiveStorage::Blob'). source(:blob) end + def blobs_association_name + case macro + when :one then + "#{name}_blob" + when :many then + "#{name}_blobs" + end + end def eager_loading_scope_exists? if model_class.respond_to?("with_attached_#{name}") true else - @failure = "no scope with_attached_#{name} found" + @failure = "#{model_class.name} does not have a " \ + ":with_attached_#{name} scope." false end end - def reader_attribute_exists? - if !subject.respond_to?(name) - @failure = "no association #{name} found" - false - else - true - end - end - - def writer_attribute_exists? - if !subject.respond_to?("#{name}=") - @failure = "no method #{name}= found" - false - else - true - end - end - - def attachments_association_name - case macro - when :one then "#{name}_attachment" - when :many then "#{name}_attachments" - end - end - - def blobs_association_name - case macro - when :one then "#{name}_blob" - when :many then "#{name}_blobs" - end - end - def model_class subject.class end diff --git a/spec/support/unit/helpers/active_record_versions.rb b/spec/support/unit/helpers/active_record_versions.rb index 1d35db635..fe871aa39 100644 --- a/spec/support/unit/helpers/active_record_versions.rb +++ b/spec/support/unit/helpers/active_record_versions.rb @@ -44,5 +44,9 @@ def active_record_supports_optional_for_associations? def active_record_supports_expression_indexes? active_record_version >= 5 end + + def active_record_supports_active_storage? + active_record_version >= 5.2 + end end end diff --git a/spec/unit/shoulda/matchers/active_record/have_attached_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/have_attached_matcher_spec.rb index f41a5c3d2..d1fa18368 100644 --- a/spec/unit/shoulda/matchers/active_record/have_attached_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/have_attached_matcher_spec.rb @@ -1,116 +1,284 @@ require 'unit_spec_helper' -# rubocop:disable Metrics/BlockLength -describe Shoulda::Matchers::ActiveRecord::HaveAttachedMatcher, - type: :model do - - if rails_gte_5_2? +describe Shoulda::Matchers::ActiveRecord::HaveAttachedMatcher, type: :model do + if active_record_supports_active_storage? before do create_table :active_storage_blobs do |t| - t.string :key, null: false - t.string :filename, null: false - t.string :content_type - t.text :metadata - t.bigint :byte_size, null: false - t.string :checksum, null: false + t.string :key, null: false + t.string :filename, null: false + t.string :content_type + t.text :metadata + t.bigint :byte_size, null: false + t.string :checksum, null: false t.datetime :created_at, null: false - t.index [ :key ], unique: true + t.index [:key], unique: true end create_table :active_storage_attachments do |t| - t.string :name, null: false - t.references :record, null: false, polymorphic: true, index: false - t.references :blob, null: false + t.string :name, null: false + t.references :record, null: false, polymorphic: true, index: false + t.references :blob, null: false t.datetime :created_at, null: false - t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true + t.index [:record_type, :record_id, :name, :blob_id], + name: 'index_active_storage_attachments_uniqueness', unique: true t.foreign_key :active_storage_blobs, column: :blob_id end - - create_table(:users) do |t| - end end - context 'have_one_attached' do + describe 'have_one_attached' do describe '#description' do it 'returns the message with the name of the association' do matcher = have_one_attached(:avatar) expect(matcher.description). - to eq('has_one_attached avatar') + to eq('have a has_one_attached called avatar') end end - it 'accepts when the subject has has_many_attached' do - valid_model = define_model_class(:User) do - has_one_attached :avatar + context 'when the attached exists on the model' do + let(:record) do + record_has_one_attached(:avatar) end - - expected_message = 'Did not expect User to have a has_one attached called avatar' - - aggregate_failures do - expect(valid_model.new).to have_one_attached(:avatar) - - expect { expect(valid_model.new).to_not have_one_attached(:avatar) }. - to fail_with_message(expected_message) + it 'matches' do + expect { have_one_attached(:avatar) }. + to match_against(record). + or_fail_with(<<-MESSAGE) +Did not expect User to have a has_one_attached called avatar, but it does. + MESSAGE end - end - - it 'rejects when association not defined' do - invalid_model = define_model_class(:User) do - has_one_attached :picture + context 'and the reader attribute does not exist' do + let(:record) do + record_has_one_attached(:avatar, remove_reader: true) + end + it 'matches' do + expect { have_one_attached(:avatar) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected User to have a has_one_attached called avatar, but this could not be proved. + User does not have a :avatar method. + MESSAGE + end end - - expected_message = 'Expected User to have a has_one attached called avatar (no association avatar found)' - - aggregate_failures do - expect(invalid_model.new).to_not have_one_attached(:avatar) - - expect { expect(invalid_model.new).to have_one_attached(:avatar) }. - to fail_with_message(expected_message) + context 'and the writer attribute does not exist' do + let(:record) do + record_has_one_attached(:avatar, remove_writer: true) + end + it 'matches' do + expect { have_one_attached(:avatar) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected User to have a has_one_attached called avatar, but this could not be proved. + User does not have a :avatar= method. + MESSAGE + end + end + context 'and the attachments association does not exist' do + let(:record) do + record_has_one_attached(:avatar, remove_attachments: true) + end + it 'matches' do + expect { have_one_attached(:avatar) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected User to have a has_one_attached called avatar, but this could not be proved. + Expected User to have a has_one association called avatar_attachment (no association called avatar_attachment) + MESSAGE + end + end + context 'and the blobs association is invalid' do + let(:record) do + record_has_one_attached(:avatar, invalidate_blobs: true) + end + it 'matches' do + expect { have_one_attached(:avatar) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected User to have a has_one_attached called avatar, but this could not be proved. + Expected User to have a has_one association called avatar_blob (avatar_blob should resolve to ActiveStorage::Blob for class_name) + MESSAGE + end + end + context 'and the eager loading scope does not exist' do + let(:record) do + record_has_one_attached(:avatar, remove_eager_loading_scope: true) + end + it 'matches' do + expect { have_one_attached(:avatar) }. + not_to match_against(record). + and_fail_with <<-MESSAGE +Expected User to have a has_one_attached called avatar, but this could not be proved. + User does not have a :with_attached_avatar scope. + MESSAGE + end end end end - context 'have_many_attached' do + describe 'have_many_attached' do describe '#description' do it 'returns the message with the name of the association' do matcher = have_many_attached(:avatars) expect(matcher.description). - to eq('has_many_attached avatars') + to eq('have a has_many_attached called avatars') end end - it 'accepts when the subject has has_many_attached' do - valid_model = define_model_class(:User) do - has_many_attached :pictures + context 'when the attached exists on the model' do + let(:record) do + record_has_many_attached(:avatars) + end + it 'matches' do + expect { have_many_attached(:avatars) }. + to match_against(record). + or_fail_with(<<-MESSAGE) +Did not expect User to have a has_many_attached called avatars, but it does. + MESSAGE + end + context 'and the reader attribute does not exist' do + let(:record) do + record_has_many_attached(:avatars, remove_reader: true) + end + it 'matches' do + expect { have_many_attached(:avatars) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected User to have a has_many_attached called avatars, but this could not be proved. + User does not have a :avatars method. + MESSAGE + end + end + context 'and the writer attribute does not exist' do + let(:record) do + record_has_many_attached(:avatars, remove_writer: true) + end + it 'matches' do + expect { have_many_attached(:avatars) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected User to have a has_many_attached called avatars, but this could not be proved. + User does not have a :avatars= method. + MESSAGE + end + end + context 'and the attachments association does not exist' do + let(:record) do + record_has_many_attached(:avatars, remove_attachments: true) + end + it 'matches' do + expect { have_many_attached(:avatars) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected User to have a has_many_attached called avatars, but this could not be proved. + Expected User to have a has_many association called avatars_attachments (no association called avatars_attachments) + MESSAGE + end + end + context 'and the blobs association is invalid' do + let(:record) do + record_has_many_attached(:avatars, invalidate_blobs: true) + end + it 'matches' do + expect { have_many_attached(:avatars) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected User to have a has_many_attached called avatars, but this could not be proved. + Expected User to have a has_many association called avatars_blobs (avatars_blobs should resolve to ActiveStorage::Blob for class_name) + MESSAGE + end end - expected_message = 'Did not expect User to have a has_many attached called pictures' - - aggregate_failures do - expect(valid_model.new).to have_many_attached(:pictures) - - expect { expect(valid_model.new).to_not have_many_attached(:pictures) }. - to fail_with_message(expected_message) + context 'and the eager loading scope does not exist' do + let(:record) do + record_has_many_attached(:avatars, remove_eager_loading_scope: true) + end + it 'matches' do + expect { have_many_attached(:avatars) }. + not_to match_against(record). + and_fail_with(<<-MESSAGE) +Expected User to have a has_many_attached called avatars, but this could not be proved. + User does not have a :with_attached_avatars scope. + MESSAGE + end end end + end + end +end - it 'rejects when association not defined' do - invalid_model = define_model_class(:User) do - has_many_attached :avatars - end +def record_has_one_attached( + attached_name, + model_name: 'User', + remove_reader: false, + remove_writer: false, + remove_attachments: false, + invalidate_blobs: false, + remove_eager_loading_scope: false +) + model = define_model(model_name) do + has_one_attached attached_name - expected_message = 'Expected User to have a has_many attached called pictures (no association pictures found)' + if remove_reader + undef_method attached_name + end - aggregate_failures do - expect(invalid_model.new).to_not have_many_attached(:pictures) + if remove_writer + undef_method "#{attached_name}=" + end - expect { expect(invalid_model.new).to have_many_attached(:pictures) }. - to fail_with_message(expected_message) - end - end + if remove_attachments + reflections.delete("#{attached_name}_attachment") + end + + if invalidate_blobs + reflections["#{attached_name}_blob"].options[:class_name] = 'User' + end + + if remove_eager_loading_scope + instance_eval <<-CODE, __FILE__, __LINE__ + 1 +undef with_attached_#{attached_name} + CODE + end + end + + model.new +end + +def record_has_many_attached( + attached_name, + model_name: 'User', + remove_reader: false, + remove_writer: false, + remove_attachments: false, + invalidate_blobs: false, + remove_eager_loading_scope: false +) + model = define_model(model_name) do + has_many_attached attached_name + + if remove_reader + undef_method attached_name + end + + if remove_writer + undef_method "#{attached_name}=" + end + + if remove_attachments + reflections.delete("#{attached_name}_attachments") + end + + if invalidate_blobs + reflections["#{attached_name}_blobs"].options[:class_name] = 'User' + end + + if remove_eager_loading_scope + instance_eval <<-CODE, __FILE__, __LINE__ + 1 +undef with_attached_#{attached_name} + CODE end end + + model.new end