voxpupuli / rspec-puppet-facts

Simplify your unit tests by looping on every supported Operating System and populating facts.
Apache License 2.0
61 stars 53 forks source link

rspec-puppet-facts hangs searching for facter version #100

Closed alexjfisher closed 4 years ago

alexjfisher commented 5 years ago

Whilst trying to downgrade the facter version, rspec-puppet-facts will hang if it can't find a suitable version.

An example from puppet-ipset module.

with

RSpec.configure do |c|
  c.default_facter_version = '3.9'
end

and metadata.json

operatingsystem_support: [  
    {
      operatingsystem: Debian,    
      operatingsystemrelease: [  
        9,  
        8,  
        10  
      ]
    },
    {
      operatingsystem: Ubuntu,    
      operatingsystemrelease: [  
        16.04,  
        18.04  
      ]
    },
    {
      operatingsystem: RedHat,    
      operatingsystemrelease: [  
        7,  
        8  
      ]
    },
    {
      operatingsystem: CentOS,    
      operatingsystemrelease: [  
        7,  
        8  
      ]
    },
    {
      operatingsystem: OracleLinux,    
      operatingsystemrelease: [  
        7,  
        8  
      ]
    },
    {
      operatingsystem: Scientific,    
      operatingsystemrelease: [  
        7,  
        8  
      ]
    },
    {
      operatingsystem: Archlinux    
    }
  ],

Adding debug to https://github.com/mcanevet/rspec-puppet-facts/blob/4e07ec47787632ac3e3f8a8330ec074cc4b8905f/lib/rspec-puppet-facts.rb#L113 to print out version...

"3.9"
"3.8."
"3.7."
"3.6."
"3.5."
"3.4."
"3.3."
"3.2."
"3.1."
"3.0."
"3.-1."
"3.-2."
"3.-3."
"3.-4."
"3.-5."
"3.-6."
^C

The down_facter_version function is very simplistic. It only checks the minor version and doesn't check that it isn't already 0.

alexjfisher commented 5 years ago

Looks like the regex in while db.empty? && version !~ /\d+\.0($|\.\d+)/ should handle this, but assumes the full version is being used. ie 3.9.0 not just 3.9. The spec tests for facterversion suggest a version number without the revision part should work though.

alexjfisher commented 4 years ago

Correction: the regex assumes a full version or a version with just MAJOR.MINOR. It does not cover the case where the version returned by down_facter_version is incomplete though.

alexjfisher commented 4 years ago

/\d+\.0($|\.\d+)/ also needs to be anchored or it matches 3.9.0 as 9.0