Merge pull request #151 from ekohl/rework-facterversion

Do not query for the exact facter version

lib/rspec-puppet-facts.rb

@@ -113,32 +113,31 @@ def on_supported_os_implementation(opts = {})
       end
     end
 
-    facterversion_obj = Gem::Version.new(facterversion)
+    strict_requirement = RspecPuppetFacts::facter_version_to_strict_requirement(facterversion)
+
+    loose_requirement = RspecPuppetFacts::facter_version_to_loose_requirement(facterversion)
 
     # 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|
-      facter_version_filter = RspecPuppetFacts.facter_version_to_filter(facterversion)
-      db = FacterDB.get_facts(filter_spec.merge({ :facterversion =>  facter_version_filter }))
-
-      if db.empty?
-        if RspecPuppetFacts.spec_facts_strict?
-          raise ArgumentError, "No facts were found in the FacterDB for Facter v#{facterversion} on #{filter_spec}, aborting"
-        end
+      versions = FacterDB.get_facts(filter_spec).map { |facts| Gem::Version.new(facts[:facterversion]) }.sort.reverse
+      next unless versions.any?
 
-        version = FacterDB.get_facts(filter_spec).map { |facts| Gem::Version.new(facts[:facterversion]) }.sort.reverse.detect { |v| v <= facterversion_obj }
+      version = versions.detect { |v| strict_requirement =~ v }
 
+      unless version
+        version = versions.detect { |v| loose_requirement =~ v } if loose_requirement
         next unless version
-        version = version.to_s
-        facter_version_filter = "/\\A#{Regexp.escape(version)}/"
 
-        unless version == facterversion
-          RspecPuppetFacts.warning "No facts were found in the FacterDB for Facter v#{facterversion} on #{filter_spec}, using v#{version} instead"
+        if RspecPuppetFacts.spec_facts_strict?
+          raise ArgumentError, "No facts were found in the FacterDB for Facter v#{facterversion} on #{filter_spec}, aborting"
         end
+
+        RspecPuppetFacts.warning "No facts were found in the FacterDB for Facter v#{facterversion} on #{filter_spec}, using v#{version} instead"
       end
 
-      filter_spec[:facterversion] = facter_version_filter
+      filter_spec[:facterversion] = "/\\A#{Regexp.escape(version.to_s)}\\Z/"
     end
 
     received_facts = FacterDB::get_facts(filter)
@@ -354,13 +353,47 @@ def self.reset
     @metadata = nil
   end
 
-  # Generates a JGrep statement expression for a specific facter version
-  # @return [String] JGrep statement expression
-  # @param version [String] the Facter version
+  # Construct the strict facter version requirement
+  # @return [Gem::Requirement] The version requirement to match
   # @api private
-  def self.facter_version_to_filter(version)
-    major, minor = version.split('.')
-    "/\\A#{major}\\.#{minor}\\./"
+  def self.facter_version_to_strict_requirement(version)
+    Gem::Requirement.new(facter_version_to_strict_requirement_string(version))
+  end
+
+  # Construct the strict facter version requirement string
+  # @return [String] The version requirement to match
+  # @api private
+  def self.facter_version_to_strict_requirement_string(version)
+    if /\A[0-9]+(\.[0-9]+)*\Z/.match?(version)
+      # Interpret 3 as ~> 3.0
+      "~> #{version}.0"
+    else
+      version
+    end
+  end
+
+  # Construct the loose facter version requirement
+  # @return [Optional[Gem::Requirement]] The version requirement to match
+  # @api private
+  def self.facter_version_to_loose_requirement(version)
+    string = facter_version_to_loose_requirement_string(version)
+    Gem::Requirement.new(string) if string
+  end
+
+  # Construct the facter version requirement string
+  # @return [String] The version requirement to match
+  # @api private
+  def self.facter_version_to_loose_requirement_string(version)
+    if (m = /\A(?<major>[0-9]+)\.(?<minor>[0-9]+)(?:\.(?<patch>[0-9]+))?\Z/.match(version))
+      # Interpret 3.1 as < 3.2 and 3.2.1 as < 3.3
+      "< #{m[:major]}.#{m[:minor].to_i + 1}"
+    elsif /\A[0-9]+\Z/.match?(version)
+      # Interpret 3 as < 4
+      "< #{version.to_i + 1}"
+    else
+      # This would be the same as the strict requirement
+      nil
+    end
   end
 
   def self.facter_version_for_puppet_version(puppet_version)

spec/rspec_puppet_facts_spec.rb

@@ -693,7 +693,7 @@
       end
 
       before do
-        RSpec.configuration.default_facter_version = '3.1.0'
+        RSpec.configuration.default_facter_version = '3.1.6'
       end
 
       after do
@@ -725,11 +725,10 @@
       end
 
 
-      it 'returns facts from a facter version matching future and below' do
-        major, minor = Facter.version.split('.')
+      it 'returns facts from a facter version matching version and below' do
         is_expected.to match(
           'centos-7-x86_64' => include(
-            :facterversion => /\A#{major}\.[#{minor}#{minor.to_i + 1}]\./,
+            :facterversion => /\A2\.[0-6]\./,
           ),
         )
       end
@@ -860,8 +859,10 @@
       before do
         allow(FacterDB).to receive(:get_facts).and_call_original
         allow(FacterDB).to receive(:get_facts).with(
-          a_hash_including(facterversion: "/\\A3\\.9\\./", operatingsystem: 'CentOS'),
-        ).and_return([])
+          {:operatingsystem=>"CentOS", :operatingsystemrelease=>"/^7/", :hardwaremodel=>"x86_64"},
+        ).and_wrap_original do |m, *args|
+          m.call(*args).reject { |facts| facts[:facterversion].start_with?('3.9.') }
+        end
       end
 
       it 'returns CentOS facts from a facter version matching 3.8' do
@@ -980,69 +981,145 @@ module MCollectiveStub # rubocop:todo Lint/ConstantDefinitionInBlock
     end
   end
 
-  describe '.facter_version_to_filter' do
+  describe '.facter_version_to_strict_requirement' do
+    subject { described_class.facter_version_to_strict_requirement(version) }
+
+    context 'when passed a version that is a complex requirement' do
+      let(:version) { '~> 2.4' }
+
+      it { is_expected.to be_instance_of(Gem::Requirement) }
+    end
+
+    context 'when passed a version that is major' do
+      let(:version) { '1' }
+
+      it { is_expected.to be_instance_of(Gem::Requirement) }
+    end
+  end
+
+  describe '.facter_version_to_strict_requirement_string' do
+    subject { described_class.facter_version_to_strict_requirement_string(version) }
+
+    context 'when passed a version that is a complex requirement' do
+      let(:version) { '~> 2.4' }
+
+      it { is_expected.to eq('~> 2.4') }
+    end
+
+    context 'when passed a version that is major' do
+      let(:version) { '1' }
+
+      it { is_expected.to eq('~> 1.0') }
+    end
+
+    context 'when passed a version that is major.minor' do
+      let(:version) { '1.2' }
+
+      it { is_expected.to eq('~> 1.2.0') }
+    end
+
+    context 'when passed a version that is major.minor.patch' do
+      let(:version) { '1.2.3' }
+
+      it { is_expected.to eq('~> 1.2.3.0') }
+    end
+  end
+
+  describe '.facter_version_to_loose_requirement' do
+    subject { described_class.facter_version_to_loose_requirement(version) }
+
+    context 'when passed a version that is a complex requirement' do
+      let(:version) { '~> 2.4' }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'when passed a version that is major' do
+      let(:version) { '1' }
+
+      it { is_expected.to be_instance_of(Gem::Requirement) }
+    end
+  end
+
+  describe '.facter_version_to_loose_requirement_string' do
+    subject { described_class.facter_version_to_loose_requirement_string(version) }
+
+    context 'when passed a version that is a complex requirement (1)' do
+      let(:version) { '~> 2.4' }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'when passed a version that is a complex requirement (2)' do
+      let(:version) { '>= 3 < 5' }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'when passed a version that is major (1)' do
+      let(:version) { '1' }
+
+      it { is_expected.to eq('< 2') }
+    end
+
+    context 'when passed a version that is major (2)' do
+      let(:version) { '9' }
+
+      it { is_expected.to eq('< 10') }
+    end
+
+    context 'when passed a version that is major (3)' do
+      let(:version) { '10' }
+
+      it { is_expected.to eq('< 11') }
+    end
+
     context 'when passed a version that is major.minor (1)' do
-      subject { described_class.facter_version_to_filter('1.2') }
+      let(:version) { '1.2' }
 
-      it 'returns the correct JGrep statement expression' do
-        is_expected.to eq('/\A1\.2\./')
-      end
+      it { is_expected.to eq('< 1.3') }
     end
 
     context 'when passed a version that is major.minor (2)' do
-      subject { described_class.facter_version_to_filter('10.2') }
+      let(:version) { '10.2' }
 
-      it 'returns the correct JGrep statement expression' do
-        is_expected.to eq('/\A10\.2\./')
-      end
+      it { is_expected.to eq('< 10.3') }
     end
 
     context 'when passed a version that is major.minor (3)' do
-      subject { described_class.facter_version_to_filter('1.20') }
+      let(:version) { '1.20' }
 
-      it 'returns the correct JGrep statement expression' do
-        is_expected.to eq('/\A1\.20\./')
-      end
+      it { is_expected.to eq('< 1.21') }
     end
 
     context 'when passed a version that is major.minor (4)' do
-      subject { described_class.facter_version_to_filter('10.20') }
+      let(:version) { '10.20' }
 
-      it 'returns the correct JGrep statement expression' do
-        is_expected.to eq('/\A10\.20\./')
-      end
+      it { is_expected.to eq('< 10.21') }
     end
 
     context 'when passed a version that is major.minor.patch (1)' do
-      subject { described_class.facter_version_to_filter('1.2.3') }
+      let(:version) { '1.2.3' }
 
-      it 'returns the correct JGrep statement expression' do
-        is_expected.to eq('/\A1\.2\./')
-      end
+      it { is_expected.to eq('< 1.3') }
     end
 
     context 'when passed a version that is major.minor.patch (2)' do
-      subject { described_class.facter_version_to_filter('10.2.3') }
+      let(:version) { '10.2.3' }
 
-      it 'returns the correct JGrep statement expression' do
-        is_expected.to eq('/\A10\.2\./')
-      end
+      it { is_expected.to eq('< 10.3') }
     end
 
     context 'when passed a version that is major.minor.patch (3)' do
-      subject { described_class.facter_version_to_filter('1.20.3') }
+      let(:version) { '1.20.3' }
 
-      it 'returns the correct JGrep statement expression' do
-        is_expected.to eq('/\A1\.20\./')
-      end
+      it { is_expected.to eq('< 1.21') }
     end
 
     context 'when passed a version that is major.minor.patch (4)' do
-      subject { described_class.facter_version_to_filter('10.20.3') }
+      let(:version) { '10.20.3' }
 
-      it 'returns the correct JGrep statement expression' do
-        is_expected.to eq('/\A10\.20\./')
-      end
+      it { is_expected.to eq('< 10.21') }
     end
   end
 end