From c3c6b7cbfd6f90876e484483551ed93b70d13930 Mon Sep 17 00:00:00 2001 From: Bogdan Irimie Date: Thu, 9 Apr 2020 17:27:12 +0300 Subject: [PATCH 1/6] (FACT-2538) Don't save facts in collection if they have nill value. --- lib/models/fact_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/fact_collection.rb b/lib/models/fact_collection.rb index 0ddfd31b2..4dbdee46a 100644 --- a/lib/models/fact_collection.rb +++ b/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 fact.value.nil? bury_fact(fact) end From 197c17f31d91b2649a96417cc451e67d36b656e6 Mon Sep 17 00:00:00 2001 From: Bogdan Irimie Date: Thu, 9 Apr 2020 17:55:38 +0300 Subject: [PATCH 2/6] (FACT-2538) Only exclude :core and :legacy facts with nill value. Add tests. --- lib/models/fact_collection.rb | 2 +- spec/facter/model/fact_collection_spec.rb | 81 ++++++++++++++++++----- 2 files changed, 64 insertions(+), 19 deletions(-) diff --git a/lib/models/fact_collection.rb b/lib/models/fact_collection.rb index 4dbdee46a..986fd94fa 100644 --- a/lib/models/fact_collection.rb +++ b/lib/models/fact_collection.rb @@ -8,7 +8,7 @@ def initialize def build_fact_collection!(facts) facts.each do |fact| - next if fact.value.nil? + next if (fact.type == :core || fact.type == :legacy) && fact.value.nil? bury_fact(fact) end diff --git a/spec/facter/model/fact_collection_spec.rb b/spec/facter/model/fact_collection_spec.rb index 968cc3376..3e3ae4f13 100644 --- a/spec/facter/model/fact_collection_spec.rb +++ b/spec/facter/model/fact_collection_spec.rb @@ -1,29 +1,74 @@ # 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 + context 'when fact has some value' do + let(:fact_value) { '1.2.3' } + let(:resolved_fact) { Facter::ResolvedFact.new('os.version', fact_value, :core) } - fact_collection.build_fact_collection!([resolved_fact]) - expected_hash = { 'os' => { 'version' => fact_value } } + before do + resolved_fact.filter_tokens = [] + resolved_fact.user_query = 'os' + end - expect(fact_collection).to eq(expected_hash) - end + 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(:resolved_fact) { Facter::ResolvedFact.new('os.version', nil, :legacy) } + + before do + resolved_fact.filter_tokens = [] + resolved_fact.user_query = 'os' + end + + 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(:resolved_fact) { Facter::ResolvedFact.new('os.version', nil, :core) } + + before do + resolved_fact.filter_tokens = [] + resolved_fact.user_query = 'os' + end + + 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 :custom' do + let(:resolved_fact) { Facter::ResolvedFact.new('os.version', nil, :custom) } - 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' + before do + resolved_fact.filter_tokens = [] + resolved_fact.user_query = 'operatingsystem' + end - fact_collection.build_fact_collection!([resolved_fact]) - expected_hash = {} + it 'adds fact to collection' do + fact_collection.build_fact_collection!([resolved_fact]) + expected_hash = { 'os' => { 'version' => nil } } - expect(fact_collection).to eq(expected_hash) + expect(fact_collection).to eq(expected_hash) + end + end + end end end From 87e85b9d886f86c1d6f908454c7d73f091d068a1 Mon Sep 17 00:00:00 2001 From: Bogdan Irimie Date: Thu, 9 Apr 2020 23:19:02 +0300 Subject: [PATCH 3/6] (FACT-2538) Update condition for rejecting facts. --- lib/models/fact_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/fact_collection.rb b/lib/models/fact_collection.rb index 986fd94fa..17045a46c 100644 --- a/lib/models/fact_collection.rb +++ b/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.type == :legacy) && fact.value.nil? + next if %i[core legacy].include?(fact.type) && fact.value.nil? bury_fact(fact) end From c98c19fc856afb760d44304a9fff9d6f62471400 Mon Sep 17 00:00:00 2001 From: Bogdan Irimie Date: Thu, 9 Apr 2020 23:22:01 +0300 Subject: [PATCH 4/6] (FACT-2538) Fix test for custom fact. --- spec/facter/model/fact_collection_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/facter/model/fact_collection_spec.rb b/spec/facter/model/fact_collection_spec.rb index 3e3ae4f13..93ef48641 100644 --- a/spec/facter/model/fact_collection_spec.rb +++ b/spec/facter/model/fact_collection_spec.rb @@ -55,7 +55,7 @@ end context 'when fact type is :custom' do - let(:resolved_fact) { Facter::ResolvedFact.new('os.version', nil, :custom) } + let(:resolved_fact) { Facter::ResolvedFact.new('operatingsystem', nil, :custom) } before do resolved_fact.filter_tokens = [] @@ -64,7 +64,7 @@ it 'adds fact to collection' do fact_collection.build_fact_collection!([resolved_fact]) - expected_hash = { 'os' => { 'version' => nil } } + expected_hash = { 'operatingsystem' => nil } expect(fact_collection).to eq(expected_hash) end From 40090118df0e0379cb2a43a7b5263d617d548c0d Mon Sep 17 00:00:00 2001 From: Bogdan Irimie Date: Fri, 10 Apr 2020 16:41:22 +0300 Subject: [PATCH 5/6] (FACT-2538) Refactor tests. --- spec/facter/model/fact_collection_spec.rb | 44 +++++++++++------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/spec/facter/model/fact_collection_spec.rb b/spec/facter/model/fact_collection_spec.rb index 93ef48641..7d7c78692 100644 --- a/spec/facter/model/fact_collection_spec.rb +++ b/spec/facter/model/fact_collection_spec.rb @@ -4,14 +4,18 @@ subject(:fact_collection) { Facter::FactCollection.new } describe '#build_fact_collection!' do + let(:user_query) { 'os' } + let(:resolved_fact) { Facter::ResolvedFact.new(fact_name, fact_value, type) } + + before do + resolved_fact.filter_tokens = [] + resolved_fact.user_query = user_query + end + context 'when fact has some value' do + let(:fact_name) { 'os.version' } let(:fact_value) { '1.2.3' } - let(:resolved_fact) { Facter::ResolvedFact.new('os.version', fact_value, :core) } - - before do - resolved_fact.filter_tokens = [] - resolved_fact.user_query = 'os' - end + let(:type) { :core } it 'adds fact to collection' do fact_collection.build_fact_collection!([resolved_fact]) @@ -23,12 +27,10 @@ context 'when fact value is nil' do context 'when fact type is legacy' do - let(:resolved_fact) { Facter::ResolvedFact.new('os.version', nil, :legacy) } - - before do - resolved_fact.filter_tokens = [] - resolved_fact.user_query = 'os' - end + let(:fact_name) { 'os.version' } + let(:fact_value) { nil } + let(:type) { :legacy } + # let(:resolved_fact) { Facter::ResolvedFact.new('os.version', nil, :legacy) } it 'does not add fact to collection' do fact_collection.build_fact_collection!([resolved_fact]) @@ -39,12 +41,9 @@ end context 'when fact type is core' do - let(:resolved_fact) { Facter::ResolvedFact.new('os.version', nil, :core) } - - before do - resolved_fact.filter_tokens = [] - resolved_fact.user_query = 'os' - end + 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]) @@ -55,12 +54,9 @@ end context 'when fact type is :custom' do - let(:resolved_fact) { Facter::ResolvedFact.new('operatingsystem', nil, :custom) } - - before do - resolved_fact.filter_tokens = [] - resolved_fact.user_query = 'operatingsystem' - end + let(:fact_name) { 'operatingsystem' } + let(:fact_value) { nil } + let(:type) { :custom } it 'adds fact to collection' do fact_collection.build_fact_collection!([resolved_fact]) From fa13dff8ca4ba74710cf200fefc43c40fd3585ba Mon Sep 17 00:00:00 2001 From: Bogdan Irimie Date: Mon, 13 Apr 2020 10:02:58 +0300 Subject: [PATCH 6/6] (FACT-2538) Remove commented line of code. --- spec/facter/model/fact_collection_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/facter/model/fact_collection_spec.rb b/spec/facter/model/fact_collection_spec.rb index 7d7c78692..8ad5183b9 100644 --- a/spec/facter/model/fact_collection_spec.rb +++ b/spec/facter/model/fact_collection_spec.rb @@ -30,7 +30,6 @@ let(:fact_name) { 'os.version' } let(:fact_value) { nil } let(:type) { :legacy } - # let(:resolved_fact) { Facter::ResolvedFact.new('os.version', nil, :legacy) } it 'does not add fact to collection' do fact_collection.build_fact_collection!([resolved_fact])