From 1d6170af2bf593528a604a63981b054803aa01a8 Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Wed, 21 Nov 2018 15:28:28 -0500 Subject: [PATCH] Add option for verbose linting This has come up a few times, and I can see why it might be helpful to have access to full backtraces when debugging a factory error uncovered by `FactoryBot.lint`. But since most of the time I don't want the extra noise from the backtrace, I added this as a verbose option. The default message is still: ``` The following factories are invalid: * user - undefined method `save!' for # * admin - undefined method `save!' for # ``` And with the verbose option (usually with more lines of backtrace): ``` The following factories are invalid: * user - undefined method `save!' for # /Users/.../thoughtbot/factory_bot/lib/factory_bot/evaluation.rb:18:in `create' /Users/.../factory_bot/lib/factory_bot/strategy/create.rb:12:in `block in result' * admin - undefined method `save!' for # /Users/.../thoughtbot/factory_bot/lib/factory_bot/evaluation.rb:18:in `create' /Users/.../factory_bot/lib/factory_bot/strategy/create.rb:12:in `block in result' ``` I moved the linting option defaults out of the FactoryBot.lint method and into keyword argument defaults in Linter#initialize. This seems a bit cleaner, and now we will get an error if we pass an option we don't understand (before 6e511597 we had a test that passed in a bogus option) Closes #710 Closes #1124 I am opening a new PR since the original PR is years old and it seemed unkind to request changes after so long. Instead I will list the authors as co-authors. Co-authored-by: Jack Kinsella Co-authored-by: Jasper Woudenberg --- GETTING_STARTED.md | 7 +++++++ lib/factory_bot.rb | 1 + lib/factory_bot/linter.rb | 20 ++++++++++++++++++-- spec/acceptance/lint_spec.rb | 21 +++++++++++++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md index f435520..9747e1f 100644 --- a/GETTING_STARTED.md +++ b/GETTING_STARTED.md @@ -1110,6 +1110,13 @@ You can also specify the strategy used for linting: FactoryBot.lint strategy: :build ``` +Verbose linting will include full backtraces for each error, which can be +helpful for debugging: + +```ruby +FactoryBot.lint verbose: :true +``` + Custom Construction ------------------- diff --git a/lib/factory_bot.rb b/lib/factory_bot.rb index 873959e..2330397 100644 --- a/lib/factory_bot.rb +++ b/lib/factory_bot.rb @@ -60,6 +60,7 @@ module FactoryBot # options: # traits: true - to lint traits as well as factories # strategy: :create - to specify the strategy for linting + # verbose: true - to include full backtraces for each linting error def self.lint(*args) options = args.extract_options! factories_to_lint = args[0] || FactoryBot.factories diff --git a/lib/factory_bot/linter.rb b/lib/factory_bot/linter.rb index b5ccf56..82ed413 100644 --- a/lib/factory_bot/linter.rb +++ b/lib/factory_bot/linter.rb @@ -1,9 +1,10 @@ module FactoryBot class Linter - def initialize(factories, strategy: :create, traits: false) + def initialize(factories, strategy: :create, traits: false, verbose: false) @factories_to_lint = factories @factory_strategy = strategy @traits = traits + @verbose = verbose @invalid_factories = calculate_invalid_factories end @@ -36,6 +37,13 @@ module FactoryBot "* #{location} - #{message} (#{@wrapped_error.class.name})" end + def verbose_message + <<~MESSAGE + #{message} + #{@wrapped_error.backtrace.join("\n ")} + MESSAGE + end + def location @factory.name end @@ -85,7 +93,7 @@ module FactoryBot def error_message lines = invalid_factories.map do |_factory, exceptions| - exceptions.map(&:message) + exceptions.map(&error_message_type) end.flatten <<~ERROR_MESSAGE.strip @@ -94,5 +102,13 @@ module FactoryBot #{lines.join("\n")} ERROR_MESSAGE end + + def error_message_type + if @verbose + :verbose_message + else + :message + end + end end end diff --git a/spec/acceptance/lint_spec.rb b/spec/acceptance/lint_spec.rb index 0e16623..e055859 100644 --- a/spec/acceptance/lint_spec.rb +++ b/spec/acceptance/lint_spec.rb @@ -176,4 +176,25 @@ describe "FactoryBot.lint" do end.not_to raise_error end end + + describe "verbose linting" do + it "prints the backtrace for each factory error" do + define_class("InvalidThing") do + def save! + raise "invalid" + end + end + + FactoryBot.define do + factory :invalid_thing + end + + expect do + FactoryBot.lint(verbose: true) + end.to raise_error( + FactoryBot::InvalidFactoryError, + %r{#{__FILE__}:\d*:in `save!'}, + ) + end + end end