From 360477dac02cf47375d11a7326aab0dc5a75c3e0 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Sun, 13 Dec 2015 16:48:32 -0700 Subject: [PATCH] Refactor uniqueness matcher This is part of a collection of commits that aim to improve failure messages across the board, in order to make matchers easier to debug when something goes wrong. * Have the failure message describe what the matcher was trying to do when it failed. * Make the description of the matcher more readable. * Refactor the matcher so that it no longer unnecessarily creates records, but re-uses the existing record if it can instead. This makes the logic that the matcher performs slightly easier to follow. * Fix or fill in tests involving failure messages and descriptions to match these changes and recent changes to ValidationMatcher. --- .../validate_uniqueness_of_matcher.rb | 356 ++++++++++---- .../validate_uniqueness_of_matcher_spec.rb | 465 +++++++++++++++--- 2 files changed, 649 insertions(+), 172 deletions(-) diff --git a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb index a3c34b2b..12744534 100644 --- a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb +++ b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb @@ -57,7 +57,7 @@ module Shoulda # # Failures: # - # 1) Post should require case sensitive unique value for title + # 1) Post should validate :title to be case-sensitively unique # Failure/Error: it { should validate_uniqueness_of(:title) } # ActiveRecord::StatementInvalid: # SQLite3::ConstraintException: posts.content may not be NULL: INSERT INTO "posts" ("title") VALUES (?) @@ -217,7 +217,13 @@ module Shoulda def initialize(attribute) super(attribute) + @expected_message = :taken @options = {} + @existing_record = nil + @existing_record_created = false + @original_existing_value = nil + @failure_reason = nil + @failure_reason_when_negated = nil end def scoped_to(*scopes) @@ -225,11 +231,6 @@ module Shoulda self end - def with_message(message) - @expected_message = message - self - end - def case_insensitive @options[:case_insensitive] = true self @@ -240,27 +241,44 @@ module Shoulda self end + def expects_to_allow_nil? + @options[:allow_nil] + end + def allow_blank @options[:allow_blank] = true self end - def description - result = "require " - result << "case sensitive " unless @options[:case_insensitive] - result << "unique value for #{@attribute}" - result << " scoped to #{@options[:scopes].join(', ')}" if @options[:scopes].present? - result + def expects_to_allow_blank? + @options[:allow_blank] end - def matches?(subject) - @original_subject = subject - @subject = subject.class.new - @expected_message ||= :taken - @all_records = @subject.class.all + def simple_description + description = "validate that :#{@attribute} is" - scopes_match? && - set_scoped_attributes && + if @options[:case_insensitive] + description << ' case-insensitively' + else + description << ' case-sensitively' + end + + description << ' unique' + + if @options[:scopes].present? + description << " within the scope of #{inspected_expected_scopes}" + end + + description + end + + def matches?(given_record) + @given_record = given_record + @all_records = model.all + + existing_record_valid? && + validate_scopes_present? && + scopes_match? && validate_everything_except_duplicate_nils_or_blanks? && validate_case_sensitivity? && validate_after_scope_change? && @@ -270,42 +288,81 @@ module Shoulda Uniqueness::TestModels.remove_all end + protected + + def failure_reason + @failure_reason || super + end + + def failure_reason_when_negated + @failure_reason_when_negated || super + end + + def build_allow_or_disallow_value_matcher(args) + super.tap do |matcher| + matcher.failure_message_preface = method(:failure_message_preface) + end + end + private + def new_record + @_new_record ||= build_new_record + end + alias_method :subject, :new_record + def validation - @subject.class._validators[@attribute].detect do |validator| + model._validators[@attribute].detect do |validator| validator.is_a?(::ActiveRecord::Validations::UniquenessValidator) end end def scopes_match? - expected_scopes = Array.wrap(@options[:scopes]) - - if validation - actual_scopes = Array.wrap(validation.options[:scope]) - else - actual_scopes = [] - end - if expected_scopes == actual_scopes true else - @failure_message = "Expected validation to be scoped to " + - "#{expected_scopes}" + @failure_reason = 'Expected the validation' - if actual_scopes.present? - @failure_message << ", but it was scoped to #{actual_scopes}." + if expected_scopes.empty? + @failure_reason << ' not to be scoped to anything' else - @failure_message << ", but it was not scoped to anything." + @failure_reason << " to be scoped to #{inspected_expected_scopes}" + end + + if actual_scopes.empty? + @failure_reason << ', but it was not scoped to anything.' + else + @failure_reason << ', but it was scoped to ' + @failure_reason << "#{inspected_actual_scopes} instead." end false end end + def expected_scopes + Array.wrap(@options[:scopes]) + end + + def inspected_expected_scopes + expected_scopes.map(&:inspect).to_sentence + end + + def actual_scopes + if validation + Array.wrap(validation.options[:scope]) + else + [] + end + end + + def inspected_actual_scopes + actual_scopes.map(&:inspect).to_sentence + end + def allows_nil? - if @options[:allow_nil] - ensure_nil_record_in_database + if expects_to_allow_nil? + update_existing_record(nil) allows_value_of(nil, @expected_message) else true @@ -313,51 +370,79 @@ module Shoulda end def allows_blank? - if @options[:allow_blank] - ensure_blank_record_in_database + if expects_to_allow_blank? + update_existing_record('') allows_value_of('', @expected_message) else true end end + def existing_record_valid? + if existing_record.valid? + true + else + @failure_reason = + "Given record could not be set to #{value.inspect}: " + + existing_record.errors.full_messages + false + end + end + def existing_record - @existing_record ||= first_instance + @existing_record ||= find_or_create_existing_record end - def first_instance - @subject.class.first || create_record_in_database - end - - def ensure_nil_record_in_database - unless existing_record_is_nil? - create_record_in_database(nil_value: true) + def find_or_create_existing_record + if find_existing_record + find_existing_record + else + create_existing_record.tap do |existing_record| + @existing_record_created = true + end end end - def ensure_blank_record_in_database - unless existing_record_is_blank? - create_record_in_database(blank_value: true) + def find_existing_record + record = model.first + + if valid_existing_record?(record) + record.tap do |existing_record| + @original_existing_value = existing_record.public_send(@attribute) + end + else + nil end end - def existing_record_is_nil? - @existing_record.present? && existing_value.nil? + def valid_existing_record?(record) + record.present? && + record_has_nil_when_required?(record) && + record_has_blank_when_required?(record) end - def existing_record_is_blank? - @existing_record.present? && existing_value.strip == '' + def record_has_nil_when_required?(record) + !expects_to_allow_nil? || record.public_send(@attribute).nil? end - def create_record_in_database(options = {}) - @original_subject.tap do |instance| - instance.__send__("#{@attribute}=", value_for_new_record(options)) - ensure_secure_password_set(instance) - instance.save(validate: false) - @created_record = instance + def record_has_blank_when_required?(record) + !expects_to_allow_blank? || + record.public_send(@attribute).to_s.strip.empty? + end + + def create_existing_record + @given_record.tap do |existing_record| + @original_existing_value = value = arbitrary_non_blank_value + existing_record.public_send("#{@attribute}=", value) + ensure_secure_password_set(existing_record) + existing_record.save end end + def update_existing_record(value) + existing_record.update_column(attribute, value) + end + def ensure_secure_password_set(instance) if has_secure_password? instance.password = "password" @@ -365,39 +450,75 @@ module Shoulda end end - def value_for_new_record(options = {}) - case - when options[:nil_value] then nil - when options[:blank_value] then '' - else 'a' + def arbitrary_non_blank_value + limit = column_limit_for(@attribute) + non_blank_value = 'an arbitrary value' + + if limit && limit < non_blank_value.length + 'x' * limit + else + non_blank_value end end def has_secure_password? - @subject.class.ancestors.map(&:to_s).include?('ActiveModel::SecurePassword::InstanceMethodsOnActivation') + model.ancestors.map(&:to_s).include?( + 'ActiveModel::SecurePassword::InstanceMethodsOnActivation' + ) end - def set_scoped_attributes - if @options[:scopes].present? - @options[:scopes].all? do |scope| - setter = :"#{scope}=" - if @subject.respond_to?(setter) - @subject.__send__(setter, existing_record.__send__(scope)) - true - else - @failure_message = "#{class_name} doesn't seem to have a #{scope} attribute." - false - end + def build_new_record + existing_record.dup.tap do |new_record| + new_record.public_send("#{@attribute}=", existing_value) + + expected_scopes.each do |scope| + new_record.public_send( + "#{scope}=", + existing_record.public_send(scope) + ) end - else - true end end + def validate_scopes_present? + if all_scopes_present_on_model? + true + else + reason = '' + + reason << inspected_missing_scopes.to_sentence + + if inspected_missing_scopes.many? + reason << " do not seem to be attributes" + else + reason << " does not seem to be an attribute" + end + + reason << " on #{model.name}." + + @failure_reason = reason + + false + end + end + + def all_scopes_present_on_model? + missing_scopes.none? + end + + def missing_scopes + @_missing_scopes ||= expected_scopes.select do |scope| + !@given_record.respond_to?("#{scope}=") + end + end + + def inspected_missing_scopes + missing_scopes.map(&:inspect) + end + def validate_everything_except_duplicate_nils_or_blanks? - if (@options[:allow_nil] && existing_value.nil?) || - (@options[:allow_blank] && existing_value.blank?) - create_record_with_value + if existing_value.nil? || (expects_to_allow_blank? && existing_value.blank?) + update_existing_record(arbitrary_non_blank_value) end disallows_value_of(existing_value, @expected_message) @@ -406,7 +527,7 @@ module Shoulda def validate_case_sensitivity? value = existing_value - if value.respond_to?(:swapcase) + if value.respond_to?(:swapcase) && !value.empty? swapcased_value = value.swapcase if @options[:case_insensitive] @@ -414,7 +535,7 @@ module Shoulda else if value == swapcased_value raise NonCaseSwappableValueError.create( - model: @subject.class, + model: model, attribute: @attribute, value: value ) @@ -427,10 +548,6 @@ module Shoulda end end - def create_record_with_value - @existing_record = create_record_in_database - end - def model_class?(model_name) model_name.constantize.ancestors.include?(::ActiveRecord::Base) rescue NameError @@ -438,10 +555,10 @@ module Shoulda end def validate_after_scope_change? - if @options[:scopes].blank? || all_scopes_are_booleans? + if expected_scopes.empty? || all_scopes_are_booleans? true else - @options[:scopes].all? do |scope| + expected_scopes.all? do |scope| previous_value = @all_records.map(&scope).compact.max next_value = @@ -451,16 +568,12 @@ module Shoulda next_value_for(scope, previous_value) end - @subject.__send__("#{scope}=", next_value) + new_record.public_send("#{scope}=", next_value) if allows_value_of(existing_value, @expected_message) - @subject.__send__("#{scope}=", previous_value) - - @failure_message_when_negated << - " (with different value of #{scope})" + new_record.public_send("#{scope}=", previous_value) true else - @failure_message << " (with different value of #{scope})" false end end @@ -534,8 +647,8 @@ module Shoulda end def defined_as_enum?(scope) - @subject.class.respond_to?(:defined_enums) && - @subject.defined_enums[scope.to_s] + model.respond_to?(:defined_enums) && + new_record.defined_enums[scope.to_s] end def polymorphic_type_attribute?(scope, previous_value) @@ -543,21 +656,62 @@ module Shoulda end def available_enum_values_for(scope, previous_value) - @subject.defined_enums[scope.to_s].reject do |key, _| + new_record.defined_enums[scope.to_s].reject do |key, _| key == previous_value end end def existing_value - existing_record.__send__(@attribute) + existing_record.public_send(@attribute) end - def class_name - @subject.class.name + def model + @given_record.class end def column_for(scope) - @subject.class.columns_hash[scope.to_s] + model.columns_hash[scope.to_s] + end + + def column_limit_for(attribute) + column_for(attribute).try(:limit) + end + + def failure_message_preface + prefix = '' + + if @existing_record_created + prefix << "After taking the given #{model.name}," + prefix << " setting its :#{attribute} to" + prefix << " #{existing_value.inspect}," + prefix << " and saving it as the existing record," + prefix << " then" + elsif @original_existing_value != existing_value + prefix << "Given an existing #{model.name}," + prefix << " after setting its :#{attribute} to" + prefix << " #{existing_value.inspect}, then" + else + prefix << "Given an existing #{model.name} whose :#{attribute}" + prefix << " is #{existing_value.inspect}, after" + end + + prefix << " making a new #{model.name} and setting its" + prefix << " :#{attribute} to" + + if last_value_set_on_new_record == existing_value + prefix << " #{last_value_set_on_new_record.inspect} as well" + else + prefix << " a different value," + prefix << " #{last_value_set_on_new_record.inspect}" + end + + prefix << ", the matcher expected the new #{model.name} to be" + + prefix + end + + def last_value_set_on_new_record + last_submatcher_run.last_value_set end # @private diff --git a/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb index f2c3ff0c..9d083d32 100644 --- a/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb @@ -66,67 +66,164 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo end context 'when too narrow of a scope is specified' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( scopes: [ build_attribute(name: :scope1), - { name: :scope2 } + build_attribute(name: :scope2) ], + additional_attributes: [:other] ) - expect(record). - not_to validate_uniqueness. - scoped_to(:scope1, :scope2, :other) + + assertion = lambda do + expect(record). + to validate_uniqueness. + scoped_to(:scope1, :scope2, :other) + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique +within the scope of :scope1, :scope2, and :other. + Expected the validation to be scoped to :scope1, :scope2, and :other, + but it was scoped to :scope1 and :scope2 instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) end end context 'when too broad of a scope is specified' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( scopes: [ build_attribute(name: :scope1), - { name: :scope2 } + build_attribute(name: :scope2) ], ) - expect(record). - not_to validate_uniqueness. - scoped_to(:scope1) + + assertion = lambda do + expect(record). + to validate_uniqueness. + scoped_to(:scope1) + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique +within the scope of :scope1. + Expected the validation to be scoped to :scope1, but it was scoped to + :scope1 and :scope2 instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) end end context 'when a different scope is specified' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( - scopes: [ build_attribute(name: :scope) ], - additional_attributes: [:other] + scopes: [ build_attribute(name: :other) ], + additional_attributes: [:scope] ) - expect(record). - not_to validate_uniqueness. - scoped_to(:other) + assertion = lambda do + expect(record). + to validate_uniqueness. + scoped_to(:scope) + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique +within the scope of :scope. + Expected the validation to be scoped to :scope, but it was scoped to + :other instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) end end context 'when no scope is specified' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( scopes: [ build_attribute(name: :scope) ] ) - expect(record).not_to validate_uniqueness + + assertion = lambda do + expect(record).to validate_uniqueness + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique. + Expected the validation not to be scoped to anything, but it was + scoped to :scope instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) end - it 'rejects if the scope is unset beforehand' do - record = build_record_validating_uniqueness( - scopes: [ build_attribute(name: :scope, value: nil) ] - ) - expect(record).not_to validate_uniqueness + context 'if the scope attribute is unset in the record given to the matcher' do + it 'rejects with an appropriate failure message' do + record = build_record_validating_uniqueness( + scopes: [ build_attribute(name: :scope, value: nil) ] + ) + + assertion = lambda do + expect(record).to validate_uniqueness + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique. + Expected the validation not to be scoped to anything, but it was + scoped to :scope instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) + end end end context 'when a non-existent attribute is specified as a scope' do - it 'rejects' do - record = build_record_validating_uniqueness( - scopes: [ build_attribute(name: :scope) ] - ) - expect(record).not_to validate_uniqueness.scoped_to(:non_existent) + context 'when there is one scope' do + it 'rejects with an appropriate failure message (and does not raise an error)' do + record = build_record_validating_uniqueness( + scopes: [ build_attribute(name: :scope) ] + ) + + assertion = lambda do + expect(record).to validate_uniqueness.scoped_to(:non_existent) + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique +within the scope of :non_existent. + :non_existent does not seem to be an attribute on Example. + MESSAGE + + expect(&assertion).to fail_with_message(message) + end + end + + context 'when there is more than scope' do + it 'rejects with an appropriate failure message (and does not raise an error)' do + record = build_record_validating_uniqueness( + scopes: [ build_attribute(name: :scope) ] + ) + + assertion = lambda do + expect(record).to validate_uniqueness.scoped_to( + :non_existent1, + :non_existent2 + ) + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique +within the scope of :non_existent1 and :non_existent2. + :non_existent1 and :non_existent2 do not seem to be attributes on + Example. + MESSAGE + + expect(&assertion).to fail_with_message(message) + end end end @@ -140,14 +237,22 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo end context 'when the model does not have a uniqueness validation' do - it 'rejects' do - model = define_model(:example, attribute_name => :string) do |m| - m.attr_accessible attribute_name + it 'rejects with an appropriate failure message' do + model = define_model_without_validation + model.create!(attribute_name => 'value') + + assertion = lambda do + expect(model.new).to validate_uniqueness_of(attribute_name) end - model.create!(attr: 'value') + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique. + Given an existing Example whose :attr is "value", after making a new + Example and setting its :attr to "value" as well, the matcher expected + the new Example to be invalid, but it was valid instead. + MESSAGE - expect(model.new).not_to validate_uniqueness_of(attribute_name) + expect(&assertion).to fail_with_message(message) end end @@ -179,13 +284,25 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo end context 'when the validation has no scope and a scope is specified' do - it 'rejects' do + it 'rejects with an appropriate failure message' do model = define_model_validating_uniqueness( additional_attributes: [:other] ) create_record_from(model) record = build_record_from(model) - expect(record).not_to validate_uniqueness.scoped_to(:other) + + assertion = lambda do + expect(record).to validate_uniqueness.scoped_to(:other) + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique +within the scope of :other. + Expected the validation to be scoped to :other, but it was not scoped + to anything. + MESSAGE + + expect(&assertion).to fail_with_message(message) end end end @@ -219,23 +336,58 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo context 'and the validation has a custom message' do context 'when no message is specified' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( validation_options: { message: 'bad value' } ) - expect(record).not_to validate_uniqueness + + assertion = lambda do + expect(record).to validate_uniqueness + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique. + After taking the given Example, setting its :attr to "an arbitrary + value", and saving it as the existing record, then making a new + Example and setting its :attr to "an arbitrary value" as well, the + matcher expected the new Example to be invalid and to produce the + validation error "has already been taken" on :attr. The record was + indeed invalid, but it produced these validation errors instead: + + * attr: ["bad value"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end context 'given a string' do context 'when the given and actual messages do not match' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( - validation_options: { message: 'bad value' } + validation_options: { message: 'something else entirely' } ) - expect(record). - not_to validate_uniqueness. - with_message('something else entirely') + + assertion = lambda do + expect(record). + to validate_uniqueness. + with_message('some message') + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique, +producing a custom validation error on failure. + After taking the given Example, setting its :attr to "an arbitrary + value", and saving it as the existing record, then making a new + Example and setting its :attr to "an arbitrary value" as well, the + matcher expected the new Example to be invalid and to produce the + validation error "some message" on :attr. The record was indeed + invalid, but it produced these validation errors instead: + + * attr: ["something else entirely"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end @@ -253,13 +405,31 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo context 'given a regex' do context 'when the given and actual messages do not match' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( - validation_options: { message: 'Bad value' } + validation_options: { message: 'something else entirely' } ) - expect(record). - not_to validate_uniqueness. - with_message(/something else entirely/) + + assertion = lambda do + expect(record). + to validate_uniqueness. + with_message(/some message/) + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique, +producing a custom validation error on failure. + After taking the given Example, setting its :attr to "an arbitrary + value", and saving it as the existing record, then making a new + Example and setting its :attr to "an arbitrary value" as well, the + matcher expected the new Example to be invalid and to produce a + validation error matching /some message/ on :attr. The record was + indeed invalid, but it produced these validation errors instead: + + * attr: ["something else entirely"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end @@ -334,27 +504,49 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo end context 'when too narrow of a scope is specified' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_scoped_uniqueness_with_enum( enum_scope: :scope1, additional_scopes: [:scope2], additional_attributes: [:other] ) - expect(record). - not_to validate_uniqueness. - scoped_to(:scope1, :scope2, :other) + + assertion = lambda do + expect(record). + to validate_uniqueness. + scoped_to(:scope1, :scope2, :other) + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique +within the scope of :scope1, :scope2, and :other. + Expected the validation to be scoped to :scope1, :scope2, and :other, + but it was scoped to :scope1 and :scope2 instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) end end context 'when too broad of a scope is specified' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_scoped_uniqueness_with_enum( enum_scope: :scope1, additional_scopes: [:scope2] ) - expect(record). - not_to validate_uniqueness. - scoped_to(:scope1) + + assertion = lambda do + expect(record).to validate_uniqueness.scoped_to(:scope1) + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique +within the scope of :scope1. + Expected the validation to be scoped to :scope1, but it was scoped to + :scope1 and :scope2 instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) end end end @@ -467,26 +659,55 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo end context 'when the matcher is qualified with case_insensitive' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( attribute_type: :string, validation_options: { case_sensitive: true } ) - expect(record).not_to validate_uniqueness.case_insensitive + assertion = lambda do + expect(record).to validate_uniqueness.case_insensitive + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-insensitively +unique. + After taking the given Example, setting its :attr to "an arbitrary + value", and saving it as the existing record, then making a new + Example and setting its :attr to a different value, "AN ARBITRARY + VALUE", the matcher expected the new Example to be invalid, but it was + valid instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) end end end context 'when the model has a case-insensitive validation' do context 'when case_insensitive is not specified' do - it 'rejects' do + it 'rejects with an appropriate failure message' do record = build_record_validating_uniqueness( attribute_type: :string, validation_options: { case_sensitive: false } ) - expect(record).not_to validate_uniqueness + assertion = lambda do + expect(record).to validate_uniqueness + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique. + After taking the given Example, setting its :attr to "an arbitrary + value", and saving it as the existing record, then making a new + Example and setting its :attr to a different value, "AN ARBITRARY + VALUE", the matcher expected the new Example to be valid, but it was + invalid instead, producing these validation errors: + + * attr: ["has already been taken"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end @@ -543,18 +764,50 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo context 'when the validation is not declared with allow_nil' do context 'given a new record whose attribute is nil' do - it 'rejects' do + it 'rejects with an appropriate failure message' do model = define_model_validating_uniqueness record = build_record_from(model, attribute_name => nil) - expect(record).not_to validate_uniqueness.allow_nil + + assertion = lambda do + expect(record).to validate_uniqueness.allow_nil + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique, +but only if it is not nil. + After taking the given Example, setting its :attr to nil, and saving + it as the existing record, then making a new Example and setting its + :attr to nil as well, the matcher expected the new Example to be + valid, but it was invalid instead, producing these validation errors: + + * attr: ["has already been taken"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end context 'given an existing record whose attribute is nil' do - it 'rejects' do + it 'rejects with an appropriate failure message' do model = define_model_validating_uniqueness record = create_record_from(model, attribute_name => nil) - expect(record).not_to validate_uniqueness.allow_nil + + assertion = lambda do + expect(record).to validate_uniqueness.allow_nil + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique, +but only if it is not nil. + Given an existing Example whose :attr is nil, after making a new + Example and setting its :attr to nil as well, the matcher expected the + new Example to be valid, but it was invalid instead, producing these + validation errors: + + * attr: ["has already been taken"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end end @@ -640,38 +893,102 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo context 'when the validation is not declared with allow_blank' do context 'given a new record whose attribute is nil' do - it 'rejects' do + it 'rejects with an appropriate failure message' do model = define_model_validating_uniqueness record = build_record_from(model, attribute_name => nil) - expect(record).not_to validate_uniqueness.allow_blank + + assertion = lambda do + expect(record).to validate_uniqueness.allow_blank + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique, +but only if it is not blank. + After taking the given Example, setting its :attr to "", and saving it + as the existing record, then making a new Example and setting its + :attr to "" as well, the matcher expected the new Example to be valid, + but it was invalid instead, producing these validation errors: + + * attr: ["has already been taken"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end context 'given an existing record whose attribute is nil' do - it 'rejects' do + it 'rejects with an appropriate failure message' do model = define_model_validating_uniqueness record = create_record_from(model, attribute_name => nil) - expect(record).not_to validate_uniqueness.allow_blank + + assertion = lambda do + expect(record).to validate_uniqueness.allow_blank + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique, +but only if it is not blank. + Given an existing Example, after setting its :attr to "", then making + a new Example and setting its :attr to "" as well, the matcher + expected the new Example to be valid, but it was invalid instead, + producing these validation errors: + + * attr: ["has already been taken"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end context 'given a new record whose attribute is empty' do - it 'rejects' do + it 'rejects with an appropriate failure message' do model = define_model_validating_uniqueness( attribute_type: :string ) record = build_record_from(model, attribute_name => '') - expect(record).not_to validate_uniqueness.allow_blank + + assertion = lambda do + expect(record).to validate_uniqueness.allow_blank + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique, +but only if it is not blank. + After taking the given Example, setting its :attr to "", and saving it + as the existing record, then making a new Example and setting its + :attr to "" as well, the matcher expected the new Example to be valid, + but it was invalid instead, producing these validation errors: + + * attr: ["has already been taken"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end context 'given an existing record whose attribute is empty' do - it 'rejects' do + it 'rejects with an appropriate failure message' do model = define_model_validating_uniqueness( attribute_type: :string ) record = create_record_from(model, attribute_name => '') - expect(record).not_to validate_uniqueness.allow_blank + + assertion = lambda do + expect(record).to validate_uniqueness.allow_blank + end + + message = <<-MESSAGE +Example did not properly validate that :attr is case-sensitively unique, +but only if it is not blank. + Given an existing Example whose :attr is "", after making a new + Example and setting its :attr to "" as well, the matcher expected the + new Example to be valid, but it was invalid instead, producing these + validation errors: + + * attr: ["has already been taken"] + MESSAGE + + expect(&assertion).to fail_with_message(message) end end end @@ -909,6 +1226,12 @@ describe Shoulda::Matchers::ActiveRecord::ValidateUniquenessOfMatcher, type: :mo build_record_from(model) end + def define_model_without_validation + define_model(:example, attribute_name => :string) do |model| + model.attr_accessible(attribute_name) + end + end + def validate_uniqueness validate_uniqueness_of(attribute_name) end