From 58fa0a429fb2d92363500e690cabcc4c4d3c034d Mon Sep 17 00:00:00 2001
From: Ewoud Kohl van Wijngaarden <ewoud@kohlvanwijngaarden.nl>
Date: Mon, 9 Nov 2020 19:35:08 +0100
Subject: [PATCH] Collect facts iteratively

Rather than getting the facts for every filter spec and then discarding
that, this stores the found facts in an array and uses that.

This eliminates a call to FacterDB::get_facts with a very complex
filter.

A quick test in puppet-nginx reduces loading of tests by about 2 seconds
(from ~9 to ~7).
---
 lib/rspec-puppet-facts.rb       | 11 +++++------
 spec/rspec_puppet_facts_spec.rb | 15 ++++++---------
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/lib/rspec-puppet-facts.rb b/lib/rspec-puppet-facts.rb
index e3ed4b3..326827c 100644
--- a/lib/rspec-puppet-facts.rb
+++ b/lib/rspec-puppet-facts.rb
@@ -116,18 +116,18 @@ def on_supported_os_implementation(opts = {})
     strict_requirement = RspecPuppetFacts::facter_version_to_strict_requirement(facterversion)
 
     loose_requirement = RspecPuppetFacts::facter_version_to_loose_requirement(facterversion)
+    received_facts = []
 
     # FacterDB may have newer versions of facter data for which it contains a subset of all possible
     # facter data (see FacterDB 0.5.2 for Facter releases 3.8 and 3.9). In this situation we need to
     # cycle through and downgrade Facter versions per platform type until we find matching Facter data.
     filter.each do |filter_spec|
-      versions = FacterDB.get_facts(filter_spec).map { |facts| Gem::Version.new(facts[:facterversion]) }.sort.reverse
-      next unless versions.any?
+      versions = FacterDB.get_facts(filter_spec).to_h { |facts| [Gem::Version.new(facts[:facterversion]), facts] }
 
-      version = versions.detect { |v| strict_requirement =~ v }
+      version, facts = versions.select { |v, _f| strict_requirement =~ v }.max_by { |v, _f| v }
 
       unless version
-        version = versions.detect { |v| loose_requirement =~ v } if loose_requirement
+        version, facts = versions.select { |v, _f| loose_requirement =~ v }.max_by { |v, _f| v } if loose_requirement
         next unless version
 
         if RspecPuppetFacts.spec_facts_strict?
@@ -137,10 +137,9 @@ def on_supported_os_implementation(opts = {})
         RspecPuppetFacts.warning "No facts were found in the FacterDB for Facter v#{facterversion} on #{filter_spec}, using v#{version} instead"
       end
 
-      filter_spec[:facterversion] = "/\\A#{Regexp.escape(version.to_s)}\\Z/"
+      received_facts << facts
     end
 
-    received_facts = FacterDB::get_facts(filter)
     unless received_facts.any?
       RspecPuppetFacts.warning "No facts were found in the FacterDB for: #{filter.inspect}"
       return {}
diff --git a/spec/rspec_puppet_facts_spec.rb b/spec/rspec_puppet_facts_spec.rb
index 5f1e79f..216fbe1 100644
--- a/spec/rspec_puppet_facts_spec.rb
+++ b/spec/rspec_puppet_facts_spec.rb
@@ -662,19 +662,16 @@
 
       before do
         allow(described_class).to receive(:warning).with(a_string_matching(/no facts were found/i))
-        allow(FacterDB).to receive(:get_facts).and_call_original
       end
 
       it 'escapes the parens in the filter' do
-        filter = [
-          include(
-            :operatingsystem        => "IOS",
-            :operatingsystemrelease => "/^12\\.2\\(25\\)EWA9/",
-            :hardwaremodel          => "x86_64",
-          ),
-        ]
+        filter = {
+          :operatingsystem        => "IOS",
+          :operatingsystemrelease => "/^12\\.2\\(25\\)EWA9/",
+          :hardwaremodel          => "x86_64",
+        }
 
-        expect(FacterDB).to receive(:get_facts).with(filter)
+        expect(FacterDB).to receive(:get_facts).with(filter).once
         subject
       end