From 9eaf782d47ae08380d9772e069f835a084e11849 Mon Sep 17 00:00:00 2001 From: Jeff Whitmire Date: Sat, 6 Sep 2014 10:53:39 -0400 Subject: [PATCH] Adding error messages to lint output Seeing the error messages as to why a factory is invalid makes it much easier to diagnose the issue and fix the factory than just getting a list of factories that failed. --- lib/factory_girl/linter.rb | 18 +++++++++--------- spec/acceptance/lint_spec.rb | 26 ++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/lib/factory_girl/linter.rb b/lib/factory_girl/linter.rb index 93c7c5ba4..dbffe5fa9 100644 --- a/lib/factory_girl/linter.rb +++ b/lib/factory_girl/linter.rb @@ -6,34 +6,34 @@ def self.lint!(factories_to_lint) def initialize(factories_to_lint) @factories_to_lint = factories_to_lint - @invalid_factories = calculate_invalid_factories + @invalid_factory_messages = calculate_invalid_factory_messages end def lint! - if invalid_factories.any? + if invalid_factory_messages.any? raise InvalidFactoryError, error_message end end private - attr_reader :factories_to_lint, :invalid_factories + attr_reader :factories_to_lint, :invalid_factory_messages - def calculate_invalid_factories - factories_to_lint.select do |factory| + def calculate_invalid_factory_messages + factories_to_lint.map do |factory| built_factory = FactoryGirl.build(factory.name) - if built_factory.respond_to?(:valid?) - !built_factory.valid? + if built_factory.respond_to?(:valid?) && !built_factory.valid? + "* #{factory.name} -- #{built_factory.errors.full_messages.join('; ')}" end - end + end.compact end def error_message <<-ERROR_MESSAGE.strip The following factories are invalid: -#{invalid_factories.map {|factory| "* #{factory.name}" }.join("\n")} +#{invalid_factory_messages.join("\n")} ERROR_MESSAGE end end diff --git a/spec/acceptance/lint_spec.rb b/spec/acceptance/lint_spec.rb index f64fbde6c..415fa94b8 100644 --- a/spec/acceptance/lint_spec.rb +++ b/spec/acceptance/lint_spec.rb @@ -19,8 +19,30 @@ error_message = <<-ERROR_MESSAGE.strip The following factories are invalid: -* user -* admin_user +* user -- Name can't be blank +* admin_user -- Name can't be blank + ERROR_MESSAGE + + expect do + FactoryGirl.lint + end.to raise_error FactoryGirl::InvalidFactoryError, error_message + end + + it 'lists all errors when multiple are present' do + define_model 'Person', first_name: :string, last_name: :string do + validates :first_name, presence: true + validates :last_name, presence: true + end + + FactoryGirl.define do + factory :person do + end + end + + error_message = <<-ERROR_MESSAGE.strip +The following factories are invalid: + +* person -- First name can't be blank; Last name can't be blank ERROR_MESSAGE expect do