(FACT-2617) Fix for tests/external_facts/external_fact_stderr_message…

…s_output_to_stderr.rb (#522)

* (FACT-2617) If a logger is provided, use it for all log levels.

* (FACT-2617) Fix tests.

* (FACT-2617) Revert logger to implementation without send as it interferes with Kernel :warn

* (FACT-2617) Remove multi logger.

* (FACT-2617) Se default log level on default logger.

* (FACT-2617) Set logger to nil before it is initialised.

* (FACT-2617) Fix acceptance tests.

* (FACT-2617) Set log level to default when initialising default logger.

* (FACT-2617) Only set default log level if the logger was uninitialised.

* (FACT-2617) Fix rubocop.

* (FACT-2617) Make colorize method private.

lib/facter.rb

@@ -12,7 +12,7 @@ module Facter
   class ResolveCustomFactError < StandardError; end
 
   Options.init
-  Log.add_legacy_logger(STDOUT)
+  Log.output(STDOUT)
   @already_searched = {}
   @trace = false
 

lib/framework/cli/cli_launcher.rb

@@ -2,9 +2,8 @@
 # frozen_string_literal: true
 
 require 'thor'
-require "#{ROOT_DIR}/lib/framework/logging/multilogger.rb"
 require "#{ROOT_DIR}/lib/framework/logging/logger.rb"
-Facter::Log.add_legacy_logger(STDERR)
+Facter::Log.output(STDERR)
 require "#{ROOT_DIR}/lib/facter"
 require "#{ROOT_DIR}/lib/framework/cli/cli"
 

lib/framework/core/file_loader.rb

@@ -22,7 +22,7 @@ def load_lib_dirs(*dirs)
 
 load_lib_dirs('framework', 'core', 'options')
 require "#{ROOT_DIR}/lib/framework/core/options"
-require "#{ROOT_DIR}/lib/framework/logging/multilogger"
+require "#{ROOT_DIR}/lib/framework/logging/logger_helper"
 require "#{ROOT_DIR}/lib/framework/logging/logger"
 
 require "#{ROOT_DIR}/lib/util/file_helper"

lib/framework/logging/logger.rb

@@ -11,9 +11,7 @@ module Facter
   DEFAULT_LOG_LEVEL = :warn
 
   class Log
-    @@legacy_logger = nil
-    @@logger = MultiLogger.new([])
-    @@logger.level = :warn
+    @@logger = nil
     @@message_callback = nil
     @@has_errors = false
 
@@ -33,47 +31,36 @@ def level
       def errors?
         @@has_errors
       end
-    end
 
-    def initialize(logged_class)
-      determine_callers_name(logged_class)
-    end
+      def output(output)
+        return if @@logger
 
-    def self.add_legacy_logger(output)
-      return if @@legacy_logger
+        @@logger = Logger.new(output)
+        set_logger_format
+        @@logger.level = DEFAULT_LOG_LEVEL
+      end
 
-      @@legacy_logger = Logger.new(output)
-      @@legacy_logger.level = DEFAULT_LOG_LEVEL
-      set_format_for_legacy_logger
-      @@logger.add_logger(@@legacy_logger)
+      def set_logger_format
+        @@logger.formatter = proc do |severity, datetime, _progname, msg|
+          datetime = datetime.strftime(@datetime_format || '%Y-%m-%d %H:%M:%S.%6N ')
+          "[#{datetime}] #{severity} #{msg} \n"
+        end
+      end
     end
 
-    def determine_callers_name(sender_self)
-      @class_name = case sender_self
-                    when String
-                      sender_self
-                    when Class
-                      sender_self.name
-                    when Module
-                      sender_self.name
-                    else # when class is singleton
-                      sender_self.class.name
-                    end
-    end
+    def initialize(logged_class)
+      @class_name = LoggerHelper.determine_callers_name(logged_class)
+      return unless @@logger.nil?
 
-    def self.set_format_for_legacy_logger
-      @@legacy_logger.formatter = proc do |severity, datetime, _progname, msg|
-        datetime = datetime.strftime(@datetime_format || '%Y-%m-%d %H:%M:%S.%6N ')
-        "[#{datetime}] #{severity} #{msg} \n"
-      end
+      @@logger = Logger.new(STDOUT)
+      @@logger.level = DEFAULT_LOG_LEVEL
     end
 
     def debug(msg)
       return unless debugging_active?
 
       if msg.nil? || msg.empty?
-        invoker = caller(1..1).first.slice(/.*:\d+/)
-        empty_message_error(msg, invoker)
+        empty_message_error(msg)
       elsif @@message_callback
         @@message_callback.call(:debug, msg)
       else
@@ -83,38 +70,57 @@ def debug(msg)
     end
 
     def info(msg)
-      msg = colorize(msg, GREEN) if Options[:color]
-      @@logger.info(@class_name + ' - ' + msg)
+      if msg.nil? || msg.empty?
+        empty_message_error(msg)
+      elsif @@message_callback
+        @@message_callback.call(:info, msg)
+      else
+        msg = colorize(msg, GREEN) if Options[:color]
+        @@logger.info(@class_name + ' - ' + msg)
+      end
     end
 
     def warn(msg)
-      msg = colorize(msg, YELLOW) if Options[:color]
-
-      @@logger.warn(@class_name + ' - ' + msg)
+      if msg.nil? || msg.empty?
+        empty_message_error(msg)
+      elsif @@message_callback
+        @@message_callback.call(:warn, msg)
+      else
+        msg = colorize(msg, YELLOW) if Options[:color]
+        @@logger.warn(@class_name + ' - ' + msg)
+      end
     end
 
     def error(msg, colorize = false)
       @@has_errors = true
-      msg = colorize(msg, RED) if colorize || Options[:color]
-      @@logger.error(@class_name + ' - ' + msg)
+
+      if msg.nil? || msg.empty?
+        empty_message_error(msg)
+      elsif @@message_callback
+        @@message_callback.call(:error, msg)
+      else
+        msg = colorize(msg, RED) if colorize || Options[:color]
+        @@logger.error(@class_name + ' - ' + msg)
+      end
     end
 
+    private
+
     def colorize(msg, color)
       return msg if OsDetector.instance.identifier.eql?(:windows)
 
       "\e[0;#{color}m#{msg}\e[0m"
     end
 
-    private
-
     def debugging_active?
       return true unless Facter.respond_to?(:debugging?)
 
       Facter.debugging?
     end
 
-    def empty_message_error(msg, invoker)
-      self.warn "#{self.class}#debug invoked with invalid message #{msg.inspect}:#{msg.class} at #{invoker}"
+    def empty_message_error(msg)
+      invoker = caller(1..1).first.slice(/.*:\d+/)
+      @@logger.warn "#{self.class}#debug invoked with invalid message #{msg.inspect}:#{msg.class} at #{invoker}"
     end
   end
 end

lib/framework/logging/logger_helper.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class LoggerHelper
+  class << self
+    def determine_callers_name(sender_self)
+      class_name = case sender_self
+                   when String
+                     sender_self
+                   when Class
+                     sender_self.name
+                   when Module
+                     sender_self.name
+                   else # when class is singleton
+                     sender_self.class.name
+                   end
+      class_name
+    end
+  end
+end

spec/facter/util/file_helper_spec.rb

@@ -7,15 +7,15 @@
   let(:content) { 'file content' }
   let(:error_message) { 'Facter::Util::FileHelper - File at: /Users/admin/file.txt is not accessible.' }
   let(:array_content) { ['line 1', 'line 2', 'line 3'] }
-  let(:multi_logger_double) { instance_spy(Facter::MultiLogger, level: :warn) }
+  let(:logger_double) { instance_spy(Logger) }
 
   before do
-    Facter::Log.class_variable_set(:@@logger, multi_logger_double)
+    Facter::Log.class_variable_set(:@@logger, logger_double)
     allow(Facter).to receive(:debugging?).and_return(true)
   end
 
   after do
-    Facter::Log.class_variable_set(:@@logger, Facter::MultiLogger.new([]))
+    Facter::Log.class_variable_set(:@@logger, Logger.new(STDOUT))
   end
 
   shared_context 'when file is readable' do
@@ -57,7 +57,7 @@
       it "doesn't log anything" do
         file_helper.safe_read(path)
 
-        expect(multi_logger_double).not_to have_received(:debug)
+        expect(logger_double).not_to have_received(:debug)
       end
     end
 
@@ -87,7 +87,7 @@
       it 'logs a debug message' do
         file_helper.safe_read(path)
 
-        expect(multi_logger_double).to have_received(:debug)
+        expect(logger_double).to have_received(:debug)
           .with(error_message)
       end
     end
@@ -120,7 +120,7 @@
       it "doesn't log anything" do
         file_helper.safe_readlines(path)
 
-        expect(multi_logger_double).not_to have_received(:debug)
+        expect(logger_double).not_to have_received(:debug)
       end
     end
 
@@ -150,7 +150,7 @@
       it 'logs a debug message' do
         file_helper.safe_read(path)
 
-        expect(multi_logger_double).to have_received(:debug).with(error_message)
+        expect(logger_double).to have_received(:debug).with(error_message)
       end
     end
   end