From 1ab1935a9157dae0e024c339199e80dc01b9698b Mon Sep 17 00:00:00 2001 From: Andrei Filipovici Date: Fri, 11 Sep 2020 16:05:51 +0300 Subject: [PATCH 1/2] (FACT-2389) Modified the win32 mocks to better reflect reality --- spec/facter/resolvers/windows/netkvm_resolver_spec.rb | 2 +- spec/mocks/win32_mock.rb | 6 +++++- spec/mocks/win32_registry_error_mock.rb | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/spec/facter/resolvers/windows/netkvm_resolver_spec.rb b/spec/facter/resolvers/windows/netkvm_resolver_spec.rb index f30d008264..9ef85676f2 100644 --- a/spec/facter/resolvers/windows/netkvm_resolver_spec.rb +++ b/spec/facter/resolvers/windows/netkvm_resolver_spec.rb @@ -2,7 +2,7 @@ describe Facter::Resolvers::NetKVM do describe '#resolve' do - let(:reg) { instance_double('Win32::Registry::HKEY_LOCAL_MACHINE') } + let(:reg) { instance_spy('Win32::Registry') } before do allow(reg).to receive(:keys).and_return(reg_value) diff --git a/spec/mocks/win32_mock.rb b/spec/mocks/win32_mock.rb index b5df44d3d1..06ae7c924f 100644 --- a/spec/mocks/win32_mock.rb +++ b/spec/mocks/win32_mock.rb @@ -1,7 +1,11 @@ # frozen_string_literal: true module Win32 - module Registry + class Registry + def keys; end + + def close; end + class HKEY_LOCAL_MACHINE def self.open(*); end diff --git a/spec/mocks/win32_registry_error_mock.rb b/spec/mocks/win32_registry_error_mock.rb index a26caec873..a753c2c400 100644 --- a/spec/mocks/win32_registry_error_mock.rb +++ b/spec/mocks/win32_registry_error_mock.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Win32 - module Registry + class Registry class Error < RuntimeError end end From 4849218722fd5606582c6469196c22c7e0a54375 Mon Sep 17 00:00:00 2001 From: Andrei Filipovici Date: Mon, 14 Sep 2020 15:48:48 +0300 Subject: [PATCH 2/2] (FACT-2319) added debugonce method --- lib/facter.rb | 27 ++++++++++-- lib/facter/framework/logging/logger.rb | 4 +- spec/facter/facter_spec.rb | 60 ++++++++++++++++++++++++++ spec/framework/logging/logger_spec.rb | 12 ------ 4 files changed, 84 insertions(+), 19 deletions(-) diff --git a/lib/facter.rb b/lib/facter.rb index 16fac3093c..2e8cede94b 100644 --- a/lib/facter.rb +++ b/lib/facter.rb @@ -13,6 +13,7 @@ class ResolveCustomFactError < StandardError; end Options.init Log.output(STDOUT) @already_searched = {} + @debug_once = [] class << self def clear_messages @@ -54,6 +55,7 @@ def add(name, options = {}, &block) # @api public def clear @already_searched = {} + @debug_once = [] LegacyFacter.clear Options[:custom_dir] = [] LegacyFacter.collection.invalidate_custom_facts @@ -68,16 +70,33 @@ def core_value(user_query) fact_collection.dig(*splitted_user_query) end - # Prints out a debug message when debug option is set to true - # @param msg [String] Message to be printed out + # Logs debug message when debug option is set to true + # @param message [Object] Message object to be logged # # @return [nil] # # @api public - def debug(msg) + def debug(message) return unless debugging? - logger.debug(msg) + logger.debug(message.to_s) + nil + end + + # Logs the same debug message only once when debug option is set to true + # @param message [Object] Message object to be logged + # + # @return [nil] + # + # @api public + def debugonce(message) + return unless debugging? + + message_string = message.to_s + return if @debug_once.include? message_string + + @debug_once << message_string + logger.debug(message_string) nil end diff --git a/lib/facter/framework/logging/logger.rb b/lib/facter/framework/logging/logger.rb index 0c314e35bb..0234808955 100644 --- a/lib/facter/framework/logging/logger.rb +++ b/lib/facter/framework/logging/logger.rb @@ -60,9 +60,7 @@ def initialize(logged_class) def debug(msg) return unless debugging_active? - if msg.nil? || msg.empty? - empty_message_error(msg) - elsif @@message_callback + if @@message_callback @@message_callback.call(:debug, msg) else msg = colorize(msg, CYAN) if Options[:color] diff --git a/spec/facter/facter_spec.rb b/spec/facter/facter_spec.rb index 4759bd49d2..0b065e8751 100644 --- a/spec/facter/facter_spec.rb +++ b/spec/facter/facter_spec.rb @@ -380,4 +380,64 @@ def mock_collection(method, os_name = nil, error = nil) end end end + + describe '#debugonce' do + context 'when debugging is active' do + before do + allow(logger).to receive(:debug) + Facter.debugging(true) + end + + after do + Facter.debugging(false) + end + + it 'calls logger with the debug message' do + message = 'Some error message' + + Facter.debugonce(message) + + expect(logger).to have_received(:debug).with(message) + end + + it 'writes the same debug message only once' do + message = 'Some error message' + + Facter.debugonce(message) + Facter.debugonce(message) + + expect(logger).to have_received(:debug).once.with(message) + end + + it 'writes empty message when message is nil' do + Facter.debugonce(nil) + + expect(logger).to have_received(:debug).with('') + end + + it 'when message is a hash' do + Facter.debugonce({ warn: 'message' }) + + expect(logger).to have_received(:debug).with('{:warn=>"message"}') + end + + it 'returns nil' do + result = Facter.debugonce({ warn: 'message' }) + + expect(result).to be_nil + end + end + end + + context 'when debugging is inactive' do + before do + allow(logger).to receive(:debug) + end + + it 'does not call the logger' do + Facter.debugonce('message') + + expect(logger).not_to have_received(:debug) + end + end end diff --git a/spec/framework/logging/logger_spec.rb b/spec/framework/logging/logger_spec.rb index 40b358664d..a289963c49 100644 --- a/spec/framework/logging/logger_spec.rb +++ b/spec/framework/logging/logger_spec.rb @@ -29,18 +29,6 @@ end end - it 'logs a warn if message is nil' do - log.debug(nil) - - expect(multi_logger_double).to have_received(:warn).with(/debug invoked with invalid message/) - end - - it 'logs a warn if message is empty' do - log.debug('') - - expect(multi_logger_double).to have_received(:warn).with(/debug invoked with invalid message/) - end - shared_examples 'writes debug message' do it 'calls debug on multi_logger' do log.debug('debug_message')