Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expanded trait error message to include list of defined traits. #1732

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CodeMeister
Copy link
Contributor

Summary

Fixes: #1727

Expands trait error messages to include a list of the factory's defined traits.

Included

The message now includes: Registered traits: [:trait_1, :trait_2, :trait_3]

Example

Internal Definition Call

factory: :user
defined traits: :accessible_trait
error: internal reference to a missing trait :inaccessible_trait

Error Message:
Trait not registered: "inaccessible_trait". Registered traits: [:accessible_trait]. Referenced within "user" definition

User Trait Specified

factory: :user
defined traits: :accessible_trait
error: user calls an non-existent trait with build(:user, :missing_trait)

Error Message:
Trait not registered: "missing_trait". Registered traits: [:accessible_trait]

Note

When the error is because a missing trait was called from within a factory's definition, the message still includes the original:
Referenced within "user" definition

Changes

  • lib/factory_bot/definition.rb

    • error-rescue moved from :base_traits to :trait_by_name to catch both definition and calling errors.
  • spec/acceptance/enum_traits_spec.rb

    • updated specs for new error message layout.
  • spec/acceptance/traits_spec.rb updated specs for new error message layout. added new use-case tests with:

    • no registered traits
    • single registered trait
    • multiple registered traits
    • multiple registered traits through multiple inheritance

- 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
@neilvcarvalho
Copy link
Member

Hey @CodeMeister, I owe you a follow-up: while reviewing this PR and trying to understand how Factory Bot works with DidYouMean (and also how RSpec and Minitest work with it), and noticed that we re-raise the "Did you mean?" messages for factory definitions, but not for traits. I wonder if ultimately showing all the traits is the best solution (as there's DidYouMean).

At the same time, I can only see the Did you mean? message when running Factory Bot in an irb session, not on RSpec or Minitest. Now I got into this rabbit hole 😄

@CodeMeister
Copy link
Contributor Author

Hi @neilvcarvalho, it's a conundrum for sure!

DidYouMean is great when it's a close match, but not if it's completely wrong. I think it's good feedback to list all the available traits (including inherited ones), but DidYouMean would keep it consistent with factory definitions.

How about we park this for now and I'll re-do it using DidYouMean?

@neilvcarvalho
Copy link
Member

Sounds good! In the meantime, I'll check with the RSpec and MiniTest folks how to show the error's detailed message on Ruby >= 3.2. Since this version introduced the detailed_message method for errors (with a good reason), we're missing valuable information in the error messages. It's funny how no one seemed to notice - I even thought that it was some setting or mistake on my end.

@neilvcarvalho
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show the registered traits in the "Trait not registered" error message
2 participants