Do not query for the exact facter version

[ekohl]

This assumes that no exact match can be found in most cases. This saves a call to FacterDB. Some testing on puppet-nginx this reduced the time for a dry run by about 1 to 2 seconds, from about ~11 to ~9 seconds.

Implementation wise this switches to using Gem::Requirement to detect matching versions. This means you can also specify ~> 3.8 as a valid facterversion, or >= 3 < 5.

This is a reworked version of a patch in #124 that focuses on only reworking the facterversion selection.

There are some implementation differences that can be considered backwards incompatible. Right now this is not ready to be merged, but we should talk about what is desired. Please see the test cases and their diffs for how the behavior will change.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.30 🎉

Comparison is base (8d9370c) 94.93% compared to head (2596ab7) 95.23%.

❗ Current head 2596ab7 differs from pull request most recent head d690765. Consider uploading reports for the commit d690765 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
+ Coverage   94.93%   95.23%   +0.30%     
==========================================
  Files           2        2              
  Lines         158      168      +10     
==========================================
+ Hits          150      160      +10     
  Misses          8        8              

Impacted Files	Coverage Δ
lib/rspec-puppet-facts.rb	98.15% <100.00%> (+0.12%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[ekohl]

This isn't = #{version} because 3.3 should allow 3.3.1.

[ekohl]

For me it does produce more warnings. Without the patch:

No facts were found in the FacterDB for Facter v4.2.8 on {:operatingsystem=>"RedHat", :operatingsystemrelease=>"/^6/", :hardwaremodel=>"x86_64"}, using v3.14.1 instead
No facts were found in the FacterDB for Facter v4.2.8 on {:operatingsystem=>"CentOS", :operatingsystemrelease=>"/^6/", :hardwaremodel=>"x86_64"}, using v3.14.1 instead
No facts were found in the FacterDB for Facter v4.2.8 on {:operatingsystem=>"Scientific", :operatingsystemrelease=>"/^6/", :hardwaremodel=>"x86_64"}, using v3.14.1 instead

With the patch:

No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"RedHat", :operatingsystemrelease=>"/^6/", :hardwaremodel=>"x86_64"}, using v3.14.1 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"RedHat", :operatingsystemrelease=>"/^7/", :hardwaremodel=>"x86_64"}, using v4.2.13 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"RedHat", :operatingsystemrelease=>"/^8/", :hardwaremodel=>"x86_64"}, using v4.2.13 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"CentOS", :operatingsystemrelease=>"/^6/", :hardwaremodel=>"x86_64"}, using v3.14.1 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"CentOS", :operatingsystemrelease=>"/^7/", :hardwaremodel=>"x86_64"}, using v4.2.2 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"CentOS", :operatingsystemrelease=>"/^8/", :hardwaremodel=>"x86_64"}, using v4.2.2 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"Scientific", :operatingsystemrelease=>"/^6/", :hardwaremodel=>"x86_64"}, using v3.14.1 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"Scientific", :operatingsystemrelease=>"/^7/", :hardwaremodel=>"x86_64"}, using v4.2.2 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"Debian", :operatingsystemrelease=>"/^9/", :hardwaremodel=>"x86_64"}, using v4.2.2 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"Debian", :operatingsystemrelease=>"/^10/", :hardwaremodel=>"x86_64"}, using v4.2.2 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"Ubuntu", :operatingsystemrelease=>"/^16\\.04/", :hardwaremodel=>"x86_64"}, using v4.2.5 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"Ubuntu", :operatingsystemrelease=>"/^18\\.04/", :hardwaremodel=>"x86_64"}, using v4.2.5 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"FreeBSD", :operatingsystemrelease=>"/^11/", :hardwaremodel=>"amd64"}, using v4.2.1 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"FreeBSD", :operatingsystemrelease=>"/^12/", :hardwaremodel=>"amd64"}, using v4.2.4 instead
No facts were found in the FacterDB for Facter v4.2.11 on {:operatingsystem=>"Archlinux", :hardwaremodel=>"x86_64"}, using v4.2.9 instead

You could argue it's more correct that if you specify 4.2.11 that you should also get 4.2.11 facts.

Overall the (very unscientific) performance difference is:

Finished in 2.01 seconds (files took 10.32 seconds to load)

vs:

Finished in 1.55 seconds (files took 8.34 seconds to load)

So about 2 seconds or about 20% faster. #152 shaves off another 2 seconds and brings it down to 6.

[ekohl]

I'd like to see this merged. Should I merge it myself?

I wonder if this qualifies as backwards-incompatible or not. I think it isn't, though it generates more warnings. How strict should the warnings be? It looks to me that 4.2.11 != 4.2.9, but on the other hand: that's probably ok.

[bastelfreak]

I like the 'better safe than sorry' and suggest to merge this as backwards-incompatible. Then people won't pull it in automatically.