From f241c1cf273a5dfaf8d85d175bde514862e82c1e Mon Sep 17 00:00:00 2001 From: Sean Devine Date: Sun, 28 Dec 2014 08:04:49 -0500 Subject: [PATCH 1/4] add true option for dependent matcher and test all options --- .gitignore | 1 + .../active_record/association_matcher.rb | 8 +++- .../association_matchers/dependent_matcher.rb | 17 +++++++-- .../active_record/association_matcher_spec.rb | 37 +++++++++++++++++-- 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 19d1b8fc7..4e635e8f5 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ pkg tags test/*/log/*.log tmp +.ruby-version diff --git a/lib/shoulda/matchers/active_record/association_matcher.rb b/lib/shoulda/matchers/active_record/association_matcher.rb index a336661bd..840759b50 100644 --- a/lib/shoulda/matchers/active_record/association_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matcher.rb @@ -162,6 +162,12 @@ module ActiveRecord # should belong_to(:world).dependent(:destroy) # end # + # To test that any :dependent option was specified, pass true. + # # RSpec + # describe Person do + # it { should belong_to(:world).dependent(true) } + # end + # # ##### counter_cache # # Use `counter_cache` to assert that the `:counter_cache` option was @@ -993,7 +999,7 @@ def macro_correct? end def macro_supports_primary_key? - macro == :belongs_to || + macro == :belongs_to || ([:has_many, :has_one].include?(macro) && !through?) end diff --git a/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb b/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb index 4c68364cc..f129f5880 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb @@ -18,11 +18,10 @@ def description def matches?(subject) self.subject = ModelReflector.new(subject, name) - - if option_verifier.correct_for_string?(:dependent, dependent) + if correct_for?(dependent) true else - self.missing_option = "#{name} should have #{dependent} dependency" + self.missing_option = missing_option_for(name, dependent) false end end @@ -34,6 +33,18 @@ def matches?(subject) def option_verifier @option_verifier ||= OptionVerifier.new(subject) end + + def correct_for?(dependent=dependent) + case dependent + when true, false then option_verifier.correct_for_boolean?(:dependent, dependent) + else option_verifier.correct_for_string?(:dependent, dependent) + end + end + + def missing_option_for(name=name, dependent=dependent) + "#{name} should have #{dependent == true ? "a" : dependent} dependency" + end + end end end diff --git a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb index b0ab03076..47e3c6227 100644 --- a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb @@ -1,5 +1,16 @@ require 'unit_spec_helper' +def each_dependent_option(rails_version) + case rails_version + when /\A3/ + [:destroy, :delete, :nullify, :restrict] + when /\A4/ + [:destroy, :delete, :nullify, :restrict_with_exception, :restrict_with_error] + end.each do |option| + yield option + end +end + describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do context 'belong_to' do it 'accepts a good association with the default foreign key' do @@ -645,9 +656,29 @@ def having_many_non_existent_class(model_name, assoc_name, options = {}) expect(Person.new).not_to have_one(:detail) end - it 'accepts an association with a valid :dependent option' do - expect(having_one_detail(dependent: :destroy)). - to have_one(:detail).dependent(:destroy) + each_dependent_option(Rails.version) do |option| + it 'accepts an association with a valid :dependent option' do + expect(having_one_detail(dependent: option)). + to have_one(:detail).dependent(option) + end + end + + it 'accepts any dependent option if true' do + each_dependent_option(Rails.version) do |option| + expect(having_one_detail(dependent: option)). + to have_one(:detail).dependent(true) + end + end + + it 'rejects any dependent options if false' do + each_dependent_option(Rails.version) do |option| + expect(having_one_detail(dependent: option)). + to_not have_one(:detail).dependent(false) + end + end + + it 'accepts a nil dependent option if false' do + expect(having_one_detail).to have_one(:detail).dependent(false) end it 'rejects an association with a bad :dependent option' do From 2756dc23ff3ac9a50e99386228013568d3bac073 Mon Sep 17 00:00:00 2001 From: Sean Devine Date: Sat, 3 Jan 2015 11:34:34 -0500 Subject: [PATCH 2/4] address comments --- .gitignore | 1 - .../active_record/association_matcher_spec.rb | 30 +++++++++---------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/.gitignore b/.gitignore index 4e635e8f5..19d1b8fc7 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,3 @@ pkg tags test/*/log/*.log tmp -.ruby-version diff --git a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb index 47e3c6227..578198423 100644 --- a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb @@ -1,16 +1,5 @@ require 'unit_spec_helper' -def each_dependent_option(rails_version) - case rails_version - when /\A3/ - [:destroy, :delete, :nullify, :restrict] - when /\A4/ - [:destroy, :delete, :nullify, :restrict_with_exception, :restrict_with_error] - end.each do |option| - yield option - end -end - describe Shoulda::Matchers::ActiveRecord::AssociationMatcher, type: :model do context 'belong_to' do it 'accepts a good association with the default foreign key' do @@ -656,22 +645,22 @@ def having_many_non_existent_class(model_name, assoc_name, options = {}) expect(Person.new).not_to have_one(:detail) end - each_dependent_option(Rails.version) do |option| - it 'accepts an association with a valid :dependent option' do + it 'accepts an association with a valid :dependent option' do + dependent_options(Rails.version).each do |option| expect(having_one_detail(dependent: option)). - to have_one(:detail).dependent(option) + to have_one(:detail).dependent(option) end end it 'accepts any dependent option if true' do - each_dependent_option(Rails.version) do |option| + dependent_options(Rails.version).each do |option| expect(having_one_detail(dependent: option)). to have_one(:detail).dependent(true) end end it 'rejects any dependent options if false' do - each_dependent_option(Rails.version) do |option| + dependent_options(Rails.version).each do |option| expect(having_one_detail(dependent: option)). to_not have_one(:detail).dependent(false) end @@ -1148,4 +1137,13 @@ def define_association_with_order(model, macro, name, order, other_options={}) args << options model.__send__(macro, name, *args) end + + def dependent_options(rails_version) + case rails_version + when /\A3/ + [:destroy, :delete, :nullify, :restrict] + when /\A4/ + [:destroy, :delete, :nullify, :restrict_with_exception, :restrict_with_error] + end + end end From 510050902c34c89334e2c9de908a9bc99a6bf503 Mon Sep 17 00:00:00 2001 From: Sean Devine Date: Sun, 4 Jan 2015 08:30:50 -0500 Subject: [PATCH 3/4] remove default values --- .../active_record/association_matchers/dependent_matcher.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb b/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb index f129f5880..934277aea 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb @@ -34,14 +34,14 @@ def option_verifier @option_verifier ||= OptionVerifier.new(subject) end - def correct_for?(dependent=dependent) + def correct_for?(dependent) case dependent when true, false then option_verifier.correct_for_boolean?(:dependent, dependent) else option_verifier.correct_for_string?(:dependent, dependent) end end - def missing_option_for(name=name, dependent=dependent) + def missing_option_for(name, dependent) "#{name} should have #{dependent == true ? "a" : dependent} dependency" end From ff98ab35bfec1acc36086d53dd9724e79baa7286 Mon Sep 17 00:00:00 2001 From: Sean Devine Date: Fri, 9 Jan 2015 08:22:01 -0500 Subject: [PATCH 4/4] Remove rails_version argument and just use Rails.version --- .../matchers/active_record/association_matcher_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb index 578198423..da8f20bab 100644 --- a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb @@ -646,21 +646,21 @@ def having_many_non_existent_class(model_name, assoc_name, options = {}) end it 'accepts an association with a valid :dependent option' do - dependent_options(Rails.version).each do |option| + dependent_options.each do |option| expect(having_one_detail(dependent: option)). to have_one(:detail).dependent(option) end end it 'accepts any dependent option if true' do - dependent_options(Rails.version).each do |option| + dependent_options.each do |option| expect(having_one_detail(dependent: option)). to have_one(:detail).dependent(true) end end it 'rejects any dependent options if false' do - dependent_options(Rails.version).each do |option| + dependent_options.each do |option| expect(having_one_detail(dependent: option)). to_not have_one(:detail).dependent(false) end @@ -1138,8 +1138,8 @@ def define_association_with_order(model, macro, name, order, other_options={}) model.__send__(macro, name, *args) end - def dependent_options(rails_version) - case rails_version + def dependent_options + case Rails.version when /\A3/ [:destroy, :delete, :nullify, :restrict] when /\A4/