(FACT-2538) Don't save core and legacy facts in collection if they have no value

lib/models/fact_collection.rb

@@ -8,7 +8,7 @@ def initialize
 
     def build_fact_collection!(facts)
       facts.each do |fact|
-        next if fact.type == :core && fact.value.nil?
+        next if %i[core legacy].include?(fact.type) && fact.value.nil?
 
         bury_fact(fact)
       end

spec/facter/model/fact_collection_spec.rb

@@ -1,29 +1,69 @@
 # frozen_string_literal: true
 
 describe Facter::FactCollection do
-  it 'adds elements to fact collection' do
-    fact_value = '1.2.3'
+  subject(:fact_collection) { Facter::FactCollection.new }
 
-    fact_collection = Facter::FactCollection.new
-    resolved_fact = Facter::ResolvedFact.new('os.version', fact_value)
-    resolved_fact.filter_tokens = []
-    resolved_fact.user_query = 'os'
+  describe '#build_fact_collection!' do
+    let(:user_query) { 'os' }
+    let(:resolved_fact) { Facter::ResolvedFact.new(fact_name, fact_value, type) }
 
-    fact_collection.build_fact_collection!([resolved_fact])
-    expected_hash = { 'os' => { 'version' => fact_value } }
+    before do
+      resolved_fact.filter_tokens = []
+      resolved_fact.user_query = user_query
+    end
 
-    expect(fact_collection).to eq(expected_hash)
-  end
+    context 'when fact has some value' do
+      let(:fact_name) { 'os.version' }
+      let(:fact_value) { '1.2.3' }
+      let(:type) { :core }
+
+      it 'adds fact to collection' do
+        fact_collection.build_fact_collection!([resolved_fact])
+        expected_hash = { 'os' => { 'version' => fact_value } }
+
+        expect(fact_collection).to eq(expected_hash)
+      end
+    end
+
+    context 'when fact value is nil' do
+      context 'when fact type is legacy' do
+        let(:fact_name) { 'os.version' }
+        let(:fact_value) { nil }
+        let(:type) { :legacy }
+
+        it 'does not add fact to collection' do
+          fact_collection.build_fact_collection!([resolved_fact])
+          expected_hash = {}
+
+          expect(fact_collection).to eq(expected_hash)
+        end
+      end
+
+      context 'when fact type is core' do
+        let(:fact_name) { 'os.version' }
+        let(:fact_value) { nil }
+        let(:type) { :core }
+
+        it 'does not add fact to collection' do
+          fact_collection.build_fact_collection!([resolved_fact])
+          expected_hash = {}
+
+          expect(fact_collection).to eq(expected_hash)
+        end
+      end
 
-  it 'does not add elements to fact collection if fact value is nil' do
-    fact_collection = Facter::FactCollection.new
-    resolved_fact = Facter::ResolvedFact.new('os.version', nil)
-    resolved_fact.filter_tokens = []
-    resolved_fact.user_query = 'os'
+      context 'when fact type is :custom' do
+        let(:fact_name) { 'operatingsystem' }
+        let(:fact_value) { nil }
+        let(:type) { :custom }
 
-    fact_collection.build_fact_collection!([resolved_fact])
-    expected_hash = {}
+        it 'adds fact to collection' do
+          fact_collection.build_fact_collection!([resolved_fact])
+          expected_hash = { 'operatingsystem' => nil }
 
-    expect(fact_collection).to eq(expected_hash)
+          expect(fact_collection).to eq(expected_hash)
+        end
+      end
+    end
   end
 end