From 39f1dd7c938da03df86cda1c3ee30fab8613ab02 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Sat, 25 Jul 2020 17:28:19 -0600 Subject: [PATCH] Fix habtm matcher to support symbols for join_table (#1323) The fix is simple but required slightly rewriting the tests, as it turns out the existing tests weren't exercising all of the possible checks that join_table makes. --- .../active_record/association_matcher.rb | 6 +- .../join_table_matcher.rb | 4 +- .../active_record/association_matcher_spec.rb | 403 +++++++++++++++--- 3 files changed, 338 insertions(+), 75 deletions(-) diff --git a/lib/shoulda/matchers/active_record/association_matcher.rb b/lib/shoulda/matchers/active_record/association_matcher.rb index a4fd6a07..5c6a4095 100644 --- a/lib/shoulda/matchers/active_record/association_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matcher.rb @@ -920,21 +920,21 @@ module Shoulda # asserts that the table you're referring to actually exists. # # class Person < ActiveRecord::Base - # has_and_belongs_to_many :issues, join_table: 'people_tickets' + # has_and_belongs_to_many :issues, join_table: :people_tickets # end # # # RSpec # RSpec.describe Person, type: :model do # it do # should have_and_belong_to_many(:issues). - # join_table('people_tickets') + # join_table(:people_tickets) # end # end # # # Minitest (Shoulda) # class PersonTest < ActiveSupport::TestCase # should have_and_belong_to_many(:issues). - # join_table('people_tickets') + # join_table(:people_tickets) # end # # ##### validate diff --git a/lib/shoulda/matchers/active_record/association_matchers/join_table_matcher.rb b/lib/shoulda/matchers/active_record/association_matchers/join_table_matcher.rb index b214445f..83fa1648 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/join_table_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/join_table_matcher.rb @@ -29,7 +29,7 @@ module Shoulda if option_verifier.correct_for_string?(:join_table, options[:join_table_name]) true else - @failure_message = "#{name} should use '#{options[:join_table_name]}' for :join_table option" + @failure_message = "#{name} should use #{options[:join_table_name].inspect} for :join_table option" false end else @@ -38,7 +38,7 @@ module Shoulda end def join_table_exists? - if RailsShim.tables_and_views(connection).include?(join_table_name) + if RailsShim.tables_and_views(connection).include?(join_table_name.to_s) true else @failure_message = missing_table_message 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 0d762136..fa4fa314 100644 --- a/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_record/association_matcher_spec.rb @@ -1570,96 +1570,359 @@ Expected Parent to have a has_many association called children through conceptio end.to fail_with_message_including('missing columns: person_id, relative_id') end - context 'when the association is declared with a :join_table option' do - it 'accepts when testing with the same :join_table option' do - join_table_name = 'people_and_their_families' + context 'when qualified with join_table' do + context 'and it is a symbol' do + context 'and the association has been declared with a :join_table option' do + context 'which is the same as the matcher' do + context 'and the join table exists' do + context 'and the join table has the appropriate foreign key columns' do + it 'matches' do + define_model :relative - define_model :relative + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: :people_and_their_families + ) + end - define_model :person do - has_and_belongs_to_many(:relatives, join_table: join_table_name) + create_table(:people_and_their_families, id: false) do |t| + t.references :person + t.references :relative + end + + build_matcher = -> do + have_and_belong_to_many(:relatives). + join_table(:people_and_their_families) + end + + expect(&build_matcher). + to match_against(Person.new). + or_fail_with(<<-MESSAGE) +Did not expect Person to have a has_and_belongs_to_many association called relatives + MESSAGE + end + end + + context 'and the join table is missing columns' do + it 'does not match, producing an appropriate failure message' do + define_model :relative + + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: :people_and_their_families + ) + end + + create_table(:people_and_their_families) + + build_matcher = -> do + have_and_belong_to_many(:relatives). + join_table(:people_and_their_families) + end + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (join table people_and_their_families missing columns: person_id, relative_id) + MESSAGE + end + end + end + + context 'and the join table does not exist' do + it 'does not match, producing an appropriate failure message' do + define_model :relative + + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: :people_and_their_families + ) + end + + build_matcher = -> do + have_and_belong_to_many(:relatives). + join_table(:family_tree) + end + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (relatives should use :family_tree for :join_table option) + MESSAGE + end + end + end + + context 'which is the not the same as the matcher' do + it 'does not match, producing an appropriate failure message' do + define_model :relative + + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: :people_and_their_families + ) + end + + create_table(:people_and_their_families, id: false) do |t| + t.references :person + t.references :relative + end + + build_matcher = -> do + have_and_belong_to_many(:relatives).join_table(:family_tree) + end + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (relatives should use :family_tree for :join_table option) + MESSAGE + end + end end - create_table(join_table_name, id: false) do |t| - t.references :person - t.references :relative - end + context 'and the association has not been declared with a :join_table option' do + it 'does not match, producing an appropriate failure message' do + define_model :relative - expect(Person.new). - to have_and_belong_to_many(:relatives). - join_table(join_table_name) + define_model :person do + has_and_belongs_to_many(:relatives) + end + + create_table(:people_relatives, id: false) do |t| + t.references :person + t.references :relative + end + + build_matcher = -> do + have_and_belong_to_many(:relatives).join_table(:family_tree) + end + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (relatives should use :family_tree for :join_table option) + MESSAGE + end + end end - it 'accepts even when not explicitly testing with a :join_table option' do - join_table_name = 'people_and_their_families' + context 'and it is a string' do + context 'and the association has been declared with a :join_table option' do + context 'which is the same as the matcher' do + context 'and the join table exists' do + context 'and the join table has the appropriate foreign key columns' do + it 'matches' do + define_model :relative - define_model :relative + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: 'people_and_their_families' + ) + end - define_model :person do - has_and_belongs_to_many(:relatives, - join_table: join_table_name - ) + create_table(:people_and_their_families, id: false) do |t| + t.references :person + t.references :relative + end + + build_matcher = -> do + have_and_belong_to_many(:relatives). + join_table('people_and_their_families') + end + + expect(&build_matcher). + to match_against(Person.new). + or_fail_with(<<-MESSAGE) +Did not expect Person to have a has_and_belongs_to_many association called relatives + MESSAGE + end + end + + context 'and the join table is missing columns' do + it 'does not match, producing an appropriate failure message' do + define_model :relative + + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: 'people_and_their_families' + ) + end + + create_table(:people_and_their_families) + + build_matcher = -> do + have_and_belong_to_many(:relatives). + join_table('people_and_their_families') + end + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (join table people_and_their_families missing columns: person_id, relative_id) + MESSAGE + end + end + end + + context 'and the join table does not exist' do + it 'does not match, producing an appropriate failure message' do + define_model :relative + + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: 'people_and_their_families' + ) + end + + build_matcher = -> do + have_and_belong_to_many(:relatives). + join_table('family_tree') + end + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (relatives should use "family_tree" for :join_table option) + MESSAGE + end + end + end + + context 'which is the not the same as the matcher' do + it 'does not match, producing an appropriate failure message' do + define_model :relative + + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: 'people_and_their_families' + ) + end + + create_table(:people_and_their_families, id: false) do |t| + t.references :person + t.references :relative + end + + build_matcher = -> do + have_and_belong_to_many(:relatives).join_table('family_tree') + end + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (relatives should use "family_tree" for :join_table option) + MESSAGE + end + end end - create_table(join_table_name, id: false) do |t| - t.references :person - t.references :relative + context 'and the association has not been declared with a :join_table option' do + it 'does not match, producing an appropriate failure message' do + define_model :relative + + define_model :person do + has_and_belongs_to_many(:relatives) + end + + create_table(:people_relatives, id: false) do |t| + t.references :person + t.references :relative + end + + build_matcher = -> do + have_and_belong_to_many(:relatives).join_table('family_tree') + end + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (relatives should use "family_tree" for :join_table option) + MESSAGE + end end - - expect(Person.new).to have_and_belong_to_many(:relatives) - end - - it 'rejects when testing with a different :join_table option' do - join_table_name = 'people_and_their_families' - - define_model :relative - - define_model :person do - has_and_belongs_to_many( - :relatives, - join_table: join_table_name - ) - end - - create_table(join_table_name, id: false) do |t| - t.references :person - t.references :relative - end - - assertion = lambda do - expect(Person.new). - to have_and_belong_to_many(:relatives). - join_table('family_tree') - end - - expect(&assertion).to fail_with_message_including( - "relatives should use 'family_tree' for :join_table option" - ) end end - context 'when the association is not declared with a :join_table option' do - it 'rejects when testing with a :join_table option' do - define_model :relative + context 'when the matcher is not qualified with join_table but the association has still been declared with a :join_table option' do + context 'and the join table exists' do + context 'and the join table has the appropriate foreign key columns' do + it 'matches' do + define_model :relative - define_model :person do - has_and_belongs_to_many(:relatives) + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: :people_and_their_families + ) + end + + create_table(:people_and_their_families, id: false) do |t| + t.references :person + t.references :relative + end + + build_matcher = -> { have_and_belong_to_many(:relatives) } + + expect(&build_matcher). + to match_against(Person.new). + or_fail_with(<<-MESSAGE) +Did not expect Person to have a has_and_belongs_to_many association called relatives + MESSAGE + end end - create_table('people_relatives', id: false) do |t| - t.references :person - t.references :relative - end + context 'and the join table is missing columns' do + it 'does not match, producing an appropriate failure message' do + define_model :relative - assertion = lambda do - expect(Person.new). - to have_and_belong_to_many(:relatives). - join_table('family_tree') - end + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: :people_and_their_families + ) + end - expect(&assertion).to fail_with_message_including( - "relatives should use 'family_tree' for :join_table option" - ) + create_table(:people_and_their_families) + + build_matcher = -> { have_and_belong_to_many(:relatives) } + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (join table people_and_their_families missing columns: person_id, relative_id) + MESSAGE + end + end + end + + context 'and the join table does not exist' do + it 'does not match, producing an appropriate failure message' do + define_model :relative + + define_model :person do + has_and_belongs_to_many( + :relatives, + join_table: :people_and_their_families + ) + end + + build_matcher = -> { have_and_belong_to_many(:relatives) } + + expect(&build_matcher). + not_to match_against(Person.new). + and_fail_with(<<-MESSAGE) +Expected Person to have a has_and_belongs_to_many association called relatives (join table people_and_their_families doesn't exist) + MESSAGE + end end end