Merge branch 'master' into FACT-2550

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,13 @@
+PLEASE LABEL YOUR PULL REQUEST ACCORDINGLY!
+
+Choose only one of the following:
+
+"backwards-incompatible" - use this label for PRs that breaks some old functionality
+
+"feature" - use this label for new added functionality
+
+"bugfix" - use this label for PRs that contain fixes
+
+"maintenance" - use this label for PRs that contain trivial changes (eg. changes in unit tests)
+
+Also please add "community" additional label if you're part of puppet community (special attention will be provided for those PRs).

.travis.yml

@@ -23,6 +23,7 @@ jobs:
         - bundle exec rubocop
         - bundle exec rubycritic --no-browser -f console
         - bundle exec rake spec
+        - bundle exec rake commits
 
     - rvm: 2.5
       script:

CHANGELOG.md

@@ -1,5 +1,15 @@
 
 
+## [4.0.17](https://github.com/puppetlabs/facter-ng/tree/4.0.17) (2020-04-21)
+
+[Full Changelog](https://github.com/puppetlabs/facter-ng/compare/4.0.16...4.0.17)
+
+### Fixed
+
+- \(FACT-2562\) Correctly load custom and external fact directories [\#458](https://github.com/puppetlabs/facter-ng/pull/458) ([IrimieBogdan](https://github.com/IrimieBogdan))
+
+
+
 ## [4.0.16](https://github.com/puppetlabs/facter-ng/tree/4.0.16) (2020-04-15)
 
 [Full Changelog](https://github.com/puppetlabs/facter-ng/compare/4.0.15...4.0.16)

Rakefile

@@ -8,7 +8,39 @@ Dir.glob(File.join('tasks/**/*.rake')).each { |file| load file }
 
 task default: :spec
 
+desc 'verify that commit messages match CONTRIBUTING.md requirements'
+task(:commits) do
+  # This rake task looks at the summary from every commit from this branch not
+  # in the branch targeted for a PR. This is accomplished by using the
+  # TRAVIS_COMMIT_RANGE environment variable, which is present in travis CI and
+  # populated with the range of commits the PR contains. If not available, this
+  # falls back to `master..HEAD` as a next best bet as `master` is unlikely to
+  # ever be absent.
+  commit_range = ENV['TRAVIS_COMMIT_RANGE'].nil? ? 'master..HEAD' : ENV['TRAVIS_COMMIT_RANGE'].sub(/\.\.\./, '..')
+  puts "Checking commits #{commit_range}"
+  `git log --no-merges --pretty=%s #{commit_range}`.each_line do |commit_summary|
+    # This regex tests for the currently supported commit summary tokens: maint, doc, gem, or fact-<number>.
+    # The exception tries to explain it in more full.
+    if /^\((maint|doc|docs|gem|fact-\d+)\)|revert/i.match(commit_summary).nil?
+      raise "\n\n\n\tThis commit summary didn't match CONTRIBUTING.md guidelines:\n" \
+        "\n\t\t#{commit_summary}\n" \
+        "\tThe commit summary (i.e. the first line of the commit message) should start with one of:\n"  \
+        "\t\t(FACT-<digits>) # this is most common and should be a ticket at tickets.puppet.com\n" \
+        "\t\t(docs)\n" \
+        "\t\t(docs)(DOCUMENT-<digits>)\n" \
+        "\t\t(maint)\n" \
+        "\t\t(gem)\n" \
+        "\n\tThis test for the commit summary is case-insensitive.\n\n\n"
+    else
+      puts commit_summary.to_s
+    end
+    puts '...passed'
+  end
+end
+
 def retrieve_from_keyboard
+  return unless ARGV =~ /changelog/
+
   puts "Please provide the next release tag:\n"
   next_version = $stdin.gets.chomp
   raise(ArgumentError, ' The string that you entered is invalid!') unless /[0-9]+\.[0-9]+\.[0-9]+/.match?(next_version)

VERSION

@@ -1 +1 @@
-4.0.16
+4.0.17

lib/facter.rb

@@ -131,6 +131,8 @@ def fact(user_query)
     # @api public
     def reset
       LegacyFacter.reset
+      Options[:custom_dir] = []
+      Options[:external_dir] = []
       LegacyFacter.search(*Options.custom_dir)
       LegacyFacter.search_external(Options.external_dir)
       nil
@@ -145,6 +147,7 @@ def reset
     #
     # @api public
     def search(*dirs)
+      Options[:custom_dir] += dirs
       LegacyFacter.search(*dirs)
     end
 
@@ -156,6 +159,7 @@ def search(*dirs)
     #
     # @api public
     def search_external(dirs)
+      Options[:external_dir] += dirs
       LegacyFacter.search_external(dirs)
     end
 
@@ -165,7 +169,7 @@ def search_external(dirs)
     #
     # @api public
     def search_external_path
-      LegacyFacter.search_external_path
+      Options.external_dir
     end
 
     # Returns the registered search directories for custom facts.
@@ -174,7 +178,7 @@ def search_external_path
     #
     # @api public
     def search_path
-      LegacyFacter.search_path
+      Options.custom_dir
     end
 
     # Gets a hash mapping fact names to their values
@@ -186,7 +190,6 @@ def search_path
     def to_hash
       log_blocked_facts
 
-      reset
       resolved_facts = Facter::FactManager.instance.resolve_facts
       Facter::SessionCache.invalidate_all_caches
       Facter::FactCollection.new.build_fact_collection!(resolved_facts)

lib/framework/core/fact_loaders/external_fact_loader.rb

@@ -19,6 +19,7 @@ def all_facts
     # The search paths must be set before creating the fact collection.
     # If we set them after, they will not be visible.
     def load_search_paths
+      LegacyFacter.reset_search_path!
       LegacyFacter.search(*Options.custom_dir) if Options.custom_dir?
       LegacyFacter.search_external(Options.external_dir) if Options.external_dir?
     end

lib/framework/logging/logger.rb

@@ -65,7 +65,7 @@ def self.set_format_for_legacy_logger
     end
 
     def debug(msg)
-      return unless Facter.debugging?
+      return unless debugging_active?
 
       if msg.nil? || msg.empty?
         invoker = caller(1..1).first.slice(/.*:\d+/)
@@ -94,5 +94,13 @@ def error(msg, colorize = false)
     def colorize(msg, color)
       "\e[#{color}m#{msg}\e[0m"
     end
+
+    private
+
+    def debugging_active?
+      return true unless Facter.respond_to?(:debugging?)
+
+      Facter.debugging?
+    end
   end
 end

spec/facter/facter_spec.rb

@@ -285,8 +285,8 @@
   end
 
   describe '#search_path' do
-    it 'sends call to LegacyFacter' do
-      expect(LegacyFacter).to receive(:search_path).once
+    it 'sends call to Facter::Options' do
+      expect(Facter::Options).to receive(:custom_dir).once
       Facter.search_path
     end
   end
@@ -301,8 +301,8 @@
   end
 
   describe '#search_external_path' do
-    it 'sends call to LegacyFacter' do
-      expect(LegacyFacter).to receive(:search_external_path).once
+    it 'sends call to Facter::Options' do
+      expect(Facter::Options).to receive(:external_dir).once
       Facter.search_external_path
     end
   end

spec/facter/resolvers/utils/ssh_helper_spec.rb

@@ -6,15 +6,14 @@
   describe '#create_ssh' do
     let(:fingerprint) { instance_spy(Facter::FingerPrint) }
     let(:key) { load_fixture('rsa_key').read.strip }
-    let(:ssh_object) { Facter::Ssh.new(fingerprint, 'ssh-rsa', key, 'rsa') }
 
     before do
       allow(Facter::FingerPrint).to receive(:new).and_return(fingerprint)
-      allow(Facter::Ssh).to receive(:new).and_return(ssh_object)
     end
 
     it 'returns a ssh object' do
-      expect(ssh_helper.create_ssh('ssh-rsa', key)).to eql(ssh_object)
+      expect(ssh_helper.create_ssh('ssh-rsa', key)).to be_an_instance_of(Facter::Ssh).and \
+        have_attributes(name: 'rsa', type: 'ssh-rsa', fingerprint: fingerprint)
     end
   end
 end

spec/framework/logging/logger_spec.rb

@@ -3,91 +3,129 @@
 describe Logger do
   subject(:log) { Facter::Log.new(Class) }
 
-  let(:file_logger_double) { instance_spy(Logger) }
   let(:multi_logger_double) { instance_spy(Facter::MultiLogger, level: :warn) }
 
   before do
-    Facter::Log.class_variable_set(:@@file_logger, file_logger_double)
     Facter::Log.class_variable_set(:@@logger, multi_logger_double)
+  end
 
-    allow(file_logger_double).to receive(:formatter=)
+  after do
+    Facter::Log.class_variable_set(:@@logger, Facter::MultiLogger.new([]))
   end
 
   describe '#debug' do
     before do
       allow(Facter).to receive(:debugging?).and_return(true)
     end
 
-    let(:handler) { instance_spy(Logger) }
+    context 'when debugging is set to false' do
+      it 'no debug messages are sent' do
+        allow(Facter).to receive(:debugging?).and_return(false)
+
+        log.debug('info_message')
 
-    it 'noops of debugging is not set' do
-      allow(Facter).to receive(:debugging?).and_return(false)
-      log.debug('info_message')
-      expect(multi_logger_double).not_to have_received(:debug)
+        expect(multi_logger_double).not_to have_received(:debug)
+      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
 
-    it 'writes debug message' do
-      log.debug('debug_message')
-      expect(multi_logger_double).to have_received(:debug).with('Class - debug_message')
+    shared_examples 'writes debug message' do
+      it 'calls debug on multi_logger' do
+        log.debug('debug_message')
+
+        expect(multi_logger_double).to have_received(:debug).with('Class - debug_message')
+      end
     end
 
-    it 'provides on_message hook' do
-      Facter.on_message do |level, message|
-        handler.debug("on_message called with level: #{level}, message: #{message}")
+    it_behaves_like 'writes debug message'
+
+    context 'when message callback is provided' do
+      after do
+        Facter::Log.class_variable_set(:@@message_callback, nil)
       end
 
-      log.debug('test')
+      it 'provides on_message hook' do
+        logger_double = instance_spy(Logger)
+        Facter.on_message do |level, message|
+          logger_double.debug("on_message called with level: #{level}, message: #{message}")
+        end
+
+        log.debug('test')
 
-      expect(handler).to have_received(:debug).with('on_message called with level: debug, message: test')
+        expect(logger_double).to have_received(:debug).with('on_message called with level: debug, message: test')
+      end
+    end
+
+    context 'when call is made during os detection in os_detector.rb and facter.rb is not fully loaded' do
+      before do
+        allow(Facter).to receive(:respond_to?).with(:debugging?).and_return(false)
+      end
+
+      it_behaves_like 'writes debug message'
+
+      it 'does not call Facter.debugging?' do
+        log.debug('debug_message')
+
+        expect(Facter).not_to have_received(:debugging?)
+      end
     end
   end
 
   describe '#info' do
     it 'writes info message' do
       log.info('info_message')
+
       expect(multi_logger_double).to have_received(:info).with('Class - info_message')
     end
   end
 
   describe '#warn' do
     it 'writes warn message' do
       log.warn('warn_message')
+
       expect(multi_logger_double).to have_received(:warn).with('Class - warn_message')
     end
   end
 
   describe '#error' do
     it 'writes error message with color on macosx' do
       allow(OsDetector.instance).to receive(:detect).and_return(:macosx)
+
       log.error('error_message', true)
+
       expect(multi_logger_double).to have_received(:error).with("Class - \e[31merror_message\e[0m")
     end
 
     it 'writes error message not colorized on Windows' do
       allow(OsDetector.instance).to receive(:detect).and_return(:windows)
+
       log.error('error_message', true)
+
       expect(multi_logger_double).to have_received(:error).with('Class - error_message')
     end
 
     it 'writes error message' do
       log.error('error_message')
+
       expect(multi_logger_double).to have_received(:error).with('Class - error_message')
     end
   end
 
   describe '#level=' do
     it 'sets the log level' do
       Facter::Log.level = :error
+
       expect(multi_logger_double).to have_received(:level=).with(:error)
     end
   end