From 9103ead2147b012dabf7fdf9c68b9995d24697ae Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 23 Jan 2018 21:11:20 -0600 Subject: [PATCH] Fix deprecation warning for uniqueness matcher (Rails 4.2) The uniqueness matcher needs to set the attributes under test (which is not only the attribute you give to the matcher, but also any scope attributes as well) to arbitrary values so that it can test the underlying uniqueness validation logic. To set an attribute correctly, it needs to check what type of column that attribute is and then generate a value that matches the column (otherwise unpredictable results happen). The matcher was doing this for scope attributes, but not for the main attribute. One of the consequences of this inconsistency is that when setting a boolean attribute, it was using a string value, which generates a warning under Rails 4.2. This commit fixes this so that the main attribute is set the same way as scope attributes, thereby removing the warning. --- .../validate_uniqueness_of_matcher.rb | 2 +- .../unit/matchers/print_warning_including.rb | 40 +++++++++++-------- .../validate_uniqueness_of_matcher_spec.rb | 34 +++++++++++----- 3 files changed, 48 insertions(+), 28 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 647cd4f3..811ef17d 100644 --- a/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb +++ b/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb @@ -518,8 +518,8 @@ module Shoulda end def arbitrary_non_blank_value + non_blank_value = dummy_value_for(@attribute) limit = column_limit_for(@attribute) - non_blank_value = 'an arbitrary value' if limit && limit < non_blank_value.length 'x' * limit diff --git a/spec/support/unit/matchers/print_warning_including.rb b/spec/support/unit/matchers/print_warning_including.rb index 94a1a55c..03577b2d 100644 --- a/spec/support/unit/matchers/print_warning_including.rb +++ b/spec/support/unit/matchers/print_warning_including.rb @@ -11,43 +11,51 @@ module UnitTests def matches?(block) @captured_stderr = collapse_whitespace(capture(:stderr, &block)) + @was_negated = false captured_stderr.include?(expected_warning) end - def failure_message - "Expected block to #{expectation}\nbut actually printed#{actual_warning}" + def does_not_match?(block) + !matches?(block).tap do + @was_negated = true + end + end + + def failure_message + "Expected block to #{expectation}\n\nHowever, #{aberration}" end - alias_method :failure_message_for_should, :failure_message def failure_message_when_negated - "Expected block not to #{expectation}, but it did." + "Expected block not to #{expectation}\n\nHowever, #{aberration}" end - alias_method :failure_message_for_should_not, - :failure_message_when_negated def description - "should #{expectation}" + "should print a warning containing #{expected_warning.inspect}" end def supports_block_expectations? true end - protected + private attr_reader :expected_warning, :captured_stderr - private - - def expectation - "print a warning including:\n #{expected_warning}" + def was_negated? + @was_negated end - def actual_warning - if captured_stderr.empty? - " nothing." + def expectation + "print a warning containing:\n\n #{expected_warning}" + end + + def aberration + if was_negated? + 'it did.' + elsif captured_stderr.empty? + 'it actually printed nothing.' else - ":\n #{captured_stderr}" + "it actually printed:\n\n #{captured_stderr}" end end 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 cf06942e..af56f25e 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 @@ -929,11 +929,11 @@ unique. 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: + After taking the given Example, setting its :attr to ‹"dummy value"›, + and saving it as the existing record, then making a new Example and + setting its :attr to a different value, ‹"DUMMY 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 @@ -1355,12 +1355,12 @@ Example did not properly validate that :attr is case-sensitively unique. message = <<-MESSAGE.strip Example did not properly validate that :name is case-sensitively unique. - After taking the given Example, setting its :name to ‹"an arbitrary - value"› (read back as ‹"AN ARBITRARY VALUE"›), and saving it as the - existing record, then making a new Example and setting its :name to - ‹"an arbitrary value"› (read back as ‹"AN ARBITRARY VALUE"›) as well, - the matcher expected the new Example to be valid, but it was invalid - instead, producing these validation errors: + After taking the given Example, setting its :name to ‹"dummy value"› + (read back as ‹"DUMMY VALUE"›), and saving it as the existing record, + then making a new Example and setting its :name to ‹"dummy value"› + (read back as ‹"DUMMY VALUE"›) as well, the matcher expected the new + Example to be valid, but it was invalid instead, producing these + validation errors: * name: ["has already been taken"] @@ -1419,6 +1419,18 @@ Example did not properly validate that :name is case-sensitively unique. end end + context 'when the column is a boolean column' do + it 'accepts (and does not print a warning)' do + record = build_record_validating_uniqueness(attribute_type: :boolean) + running_validation = -> { expect(record).to validate_uniqueness } + message = + 'You attempted to assign a value which is not explicitly `true` or ' + + '`false`' + + expect(&running_validation).not_to print_warning_including(message) + end + end + let(:model_attributes) { {} } def default_attribute