From a9a2bf281257976105690a3aea7b340905fe2740 Mon Sep 17 00:00:00 2001 From: CodeMeister Date: Mon, 3 Feb 2025 11:21:41 +0000 Subject: [PATCH] Expanded trait error message to include list of defined traits. - Files changed: - lib/factory_bot/definition.rb - spec/acceptance/traits_spec.rb - spec/acceptance/enum_traits_spec.rb - Tests added: - added new use-case tests for: - no registered traits - single registered trait - multiple registered traits - multiple registered traits through multiple inheritance --- lib/factory_bot/definition.rb | 53 ++++--- spec/acceptance/enum_traits_spec.rb | 2 +- spec/acceptance/traits_spec.rb | 229 +++++++++++++++++++++++++++- 3 files changed, 256 insertions(+), 28 deletions(-) diff --git a/lib/factory_bot/definition.rb b/lib/factory_bot/definition.rb index a9d5fe5c1..c54e5170f 100644 --- a/lib/factory_bot/definition.rb +++ b/lib/factory_bot/definition.rb @@ -125,31 +125,8 @@ def callback(*names, &block) def base_traits @base_traits.map { |name| trait_by_name(name) } - rescue KeyError => error - raise error_with_definition_name(error) - end - - # detailed_message introduced in Ruby 3.2 for cleaner integration with - # did_you_mean. See https://bugs.ruby-lang.org/issues/18564 - if KeyError.method_defined?(:detailed_message) - def error_with_definition_name(error) - message = error.message + " referenced within \"#{name}\" definition" - - error.class.new(message, key: error.key, receiver: error.receiver) - .tap { |new_error| new_error.set_backtrace(error.backtrace) } - end - else - def error_with_definition_name(error) - message = error.message - message.insert( - message.index("\nDid you mean?") || message.length, - " referenced within \"#{name}\" definition" - ) - - error.class.new(message).tap do |new_error| - new_error.set_backtrace(error.backtrace) - end - end + rescue KeyError => key_error + raise error_with_definition_name key_error end def additional_traits @@ -158,6 +135,8 @@ def additional_traits def trait_by_name(name) trait_for(name) || Internal.trait_by_name(name, klass) + rescue KeyError => key_error + raise error_with_defined_traits key_error end def trait_for(name) @@ -205,5 +184,29 @@ def automatically_register_defined_enums?(klass) FactoryBot.automatically_define_enum_traits && klass.respond_to?(:defined_enums) end + + def error_with_definition_name(key_error) + message = key_error.message + ". Referenced within \"#{name}\" definition" + new_trait_error(message, key_error) + end + + def error_with_defined_traits(key_error) + trait_names = defined_traits_names.sort.map { |n| ":#{n}" }.join(", ") + message = "#{key_error.message}. Registered traits: [#{trait_names}]" + + new_trait_error(message, key_error) + end + + def new_trait_error(message, key_error) + # detailed_message introduced in Ruby 3.2 for cleaner integration with + # did_you_mean. See https://bugs.ruby-lang.org/issues/18564 + if KeyError.method_defined?(:detailed_message) + KeyError.new(message, key: key_error.key, receiver: key_error.receiver) + else + KeyError.new(message) + end.tap do |error| + error.set_backtrace(key_error.backtrace) + end + end end end diff --git a/spec/acceptance/enum_traits_spec.rb b/spec/acceptance/enum_traits_spec.rb index 5c6a20ecf..4a5fcec14 100644 --- a/spec/acceptance/enum_traits_spec.rb +++ b/spec/acceptance/enum_traits_spec.rb @@ -132,7 +132,7 @@ def each(&block) Task.statuses.each_key do |trait_name| expect { FactoryBot.build(:task, trait_name) }.to raise_error( - KeyError, "Trait not registered: \"#{trait_name}\"" + KeyError, /Trait not registered: "#{trait_name}"/ ) end diff --git a/spec/acceptance/traits_spec.rb b/spec/acceptance/traits_spec.rb index 710ccc647..94221ac78 100644 --- a/spec/acceptance/traits_spec.rb +++ b/spec/acceptance/traits_spec.rb @@ -333,9 +333,15 @@ def build_user_factory_with_admin_trait(trait_name) end end + expected_message = [ + "Trait not registered: \"inaccessible_trait\".", + "Registered traits: [].", + "Referenced within \"user\" definition" + ].join(" ") + expect { FactoryBot.build(:user) }.to raise_error( KeyError, - 'Trait not registered: "inaccessible_trait" referenced within "user" definition' + expected_message ) end @@ -370,14 +376,233 @@ def build_user_factory_with_admin_trait(trait_name) end end + expected_message = [ + "Trait not registered: \"inaccessible_trait\".", + "Registered traits: [:admin].", + "Referenced within \"admin\" definition" + ].join(" ") + expect { FactoryBot.build(:user, :admin) }.to raise_error( KeyError, - 'Trait not registered: "inaccessible_trait" referenced within "admin" definition' + expected_message ) end end end +describe "trait exception error messages" do + before { define_model("User", name: :string) } + + context "when a missing trait is defined within the factory" do + it "includes the definition name where the missing trait was called" do + FactoryBot.define do + factory :user do + inaccessible_trait + end + end + + expect { FactoryBot.build(:user) }.to raise_error( + KeyError, + /Referenced within "user" definition/ + ) + end + end + + context "when a missing trait is called by the user" do + it "excludes the factory definition name" do + FactoryBot.define do + factory :user + end + + expect { FactoryBot.build(:user, :missing_trait) }.to raise_error(KeyError) do |error| + expect(error.message).not_to include "Referenced within \"user\" definition" + end + end + end + + context "when no defined traits" do + context "includes an empty list" do + it "when the factory references a missing trait internally" do + FactoryBot.define do + factory :user do + inaccessible_trait + end + end + + expect { FactoryBot.build(:user) }.to raise_error( + KeyError, + /Registered traits: \[\]/ + ) + end + + it "when a missing trait is called by the user" do + FactoryBot.define do + factory :user + end + + expect { FactoryBot.build(:user, :missing_trait) }.to raise_error( + KeyError, + "Trait not registered: \"missing_trait\". Registered traits: []" + ) + end + end + end + + context "with a single defined trait" do + context "includes a list if the single trait" do + it "when the factory references a missing trait internally" do + FactoryBot.define do + factory :user do + trait :trait_1 + + inaccessible_trait + end + end + + expected_message = [ + "Trait not registered: \"inaccessible_trait\".", + "Registered traits: [:trait_1].", + "Referenced within \"user\" definition" + ].join(" ") + + expect { FactoryBot.build(:user) }.to raise_error( + KeyError, + expected_message + ) + end + + it "when a missing trait is called by the user" do + FactoryBot.define do + factory :user do + trait :trait_1 + end + end + + expect { FactoryBot.build(:user, :missing_trait) }.to raise_error( + KeyError, + "Trait not registered: \"missing_trait\". Registered traits: [:trait_1]" + ) + end + end + end + + context "with multiple defined traits" do + context "includes a list of all traits" do + it "when the factory references a missing trait internally" do + FactoryBot.define do + factory :user do + trait :trait_1 + trait :trait_2 + trait :trait_3 + trait :trait_4 + + inaccessible_trait + end + end + + expected_message = [ + "Trait not registered: \"inaccessible_trait\".", + "Registered traits: [:trait_1, :trait_2, :trait_3, :trait_4].", + "Referenced within \"user\" definition" + ].join(" ") + + expect { FactoryBot.build(:user) }.to raise_error( + KeyError, + expected_message + ) + end + + it "when a missing trait is called by the user" do + FactoryBot.define do + factory :user do + trait :trait_1 + trait :trait_2 + trait :trait_3 + trait :trait_4 + end + end + + expected_message = [ + "Trait not registered: \"missing_trait\".", + "Registered traits: [:trait_1, :trait_2, :trait_3, :trait_4]" + ].join(" ") + + expect { FactoryBot.build(:user, :missing_trait) }.to raise_error( + KeyError, + expected_message + ) + end + end + end + + context "with both defined and inherited traits" do + context "includes a list of all traits" do + it "when the factory references a missing trait internally" do + FactoryBot.define do + factory :user do + trait :user_trait_1 + trait :user_trait_2 + + factory :admin do + trait :admin_trait_1 + trait :admin_trait_2 + + factory :dev do + trait :dev_trait_1 + trait :dev_trait_2 + + inaccessible_trait + end + end + end + end + + expected_message = [ + "Trait not registered: \"inaccessible_trait\".", + "Registered traits: [:admin_trait_1, :admin_trait_2,", + ":dev_trait_1, :dev_trait_2, :user_trait_1, :user_trait_2].", + "Referenced within \"dev\" definition" + ].join(" ") + + expect { FactoryBot.build(:dev) }.to raise_error( + KeyError, + expected_message + ) + end + + it "when a missing trait is called by the user" do + FactoryBot.define do + factory :user do + trait :user_trait_1 + trait :user_trait_2 + + factory :admin do + trait :admin_trait_1 + trait :admin_trait_2 + + factory :dev do + trait :dev_trait_1 + trait :dev_trait_2 + end + end + end + end + + expected_message = [ + "Trait not registered: \"missing_trait\".", + "Registered traits: [:admin_trait_1, :admin_trait_2,", + ":dev_trait_1, :dev_trait_2, :user_trait_1, :user_trait_2]" + ].join(" ") + + expect { FactoryBot.build(:dev, :missing_trait) }.to raise_error( + KeyError, + expected_message + ) + end + end + end +end + describe "traits with callbacks" do before do define_model("User", name: :string)