Merge pull request #2565 from AriaXLi/FACT-3192/--puppet_--no-external-facts

(FACT-3192) facter -p --no-external-facts should not raise

Author: joshcooper

lib/facter/framework/core/options/options_validator.rb

@@ -55,8 +55,15 @@ def self.conflicting_configs(options)
       no_custom_facts = !options[:custom_facts]
       puppet = options[:puppet]
       custom_dir = options[:custom_dir].nil? ? false : options[:custom_dir].any?
-      external_dir = options[:external_dir].nil? ? false : options[:external_dir].any?
-
+      default_external_dir = Facter::OptionStore.default_external_dir
+      # --puppet/-p option adds an external directory and is not an explicitly
+      # set external_dir from cli or config file
+      default_external_dir += [Puppet[:pluginfactdest]] if puppet && const_defined?('Puppet')
+      external_dir = if options[:external_dir].nil? || options[:external_dir].none?
+                       false
+                     else
+                       options[:external_dir] != default_external_dir
+                     end
       [
         { 'no-ruby' => no_ruby, 'custom-dir' => custom_dir },
         { 'no-external-facts' => !options[:external_facts], 'external-dir' => external_dir },

spec/fixtures/invalid_option_pairs.conf

@@ -0,0 +1,4 @@
+global : {
+    external-dir : ""
+    no-external-facts : true,
+}

spec/framework/core/options/options_validator_spec.rb

@@ -8,7 +8,7 @@
       allow(Facter::Log).to receive(:new).and_return(logger)
     end
 
-    context 'when options are invalid pairs' do
+    context 'when CLI options are invalid pairs' do
       let(:options) { ['--puppet', '--no-ruby'] }
       let(:error_code) { 1 }
 
@@ -24,6 +24,17 @@
       end
     end
 
+    context 'when config file options are invalid pairs' do
+      let(:error_code) { 1 }
+
+      it 'writes message and exit' do
+        expect { Facter::Options.init_from_cli({ config: 'spec/fixtures/invalid_option_pairs.conf' }) }.to raise_error(
+          an_instance_of(SystemExit)
+              .and(having_attributes(status: error_code))
+        )
+      end
+    end
+
     context 'when options are duplicated' do
       let(:options) { ['--puppet', '-p'] }
       let(:error_code) { 1 }
@@ -47,5 +58,19 @@
         expect { Facter::OptionsValidator.validate(options) }.not_to raise_error
       end
     end
+
+    context 'when parsing resolved options' do
+      # rubocop:disable Style/BlockDelimiters
+      let(:options) {
+        { puppet: true, external_facts: false, external_dir: Facter::OptionStore.default_external_dir + [''],
+          ruby: true, custom_facts: true }
+      }
+      # rubocop:enable Style/BlockDelimiters
+
+      it 'writes message and exit' do
+        stub_const('Puppet', { pluginfactdest: '' })
+        expect { Facter::OptionsValidator.validate_configs(options) }.not_to raise_error
+      end
+    end
   end
 end