Compare with major release version

[ekohl]

CentOS 8 stream doesn't have a minor version so this check likely fails. However, it's irrelevant here and we can use the major release version fact instead.

There are a lot more odd cases with facts, but I don't want to touch too much code.

[puppet-community-rangefinder]

apache::default_mods is a class

that may have no external impact to Forge modules.

apache::mod::fastcgi is a class

that may have no external impact to Forge modules.

apache::params is a class

Breaking changes to this file WILL impact these 11 modules (exact match):

• xdrum-puppet
• brucem-ezpublish
• desalvo-bacula
• jlondon-phpmyadmin
• osc-openondemand
• openstack-mistral
• openstack-cinder
• openstack-manila
• openstack-zaqar
• openstack-ironic
• openstack-keystone

Breaking changes to this file MAY impact these 16 modules (near match):

• ULHPC-passenger
• camptocamp-catalog_diff
• openstack-cloudkitty
• openstack-vitrage
• openstack-octavia
• openstack-barbican
• openstack-magnum
• openstack-watcher
• openstack-gnocchi
• openstack-sahara
• openstack-panko
• openstack-neutron
• openstack-aodh
• willdurand-bazinga
• openstack-tripleo
• openstack-heat

This module is declared in 175 of 576 indexed public Puppetfiles.

---

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

[sanfrancrisko]

Thanks for the tweak @ekohl - it appears as though we have test failures on Travis due to the versioncmp being passed an undefined variable: https://travis-ci.com/github/puppetlabs/puppetlabs-apache/jobs/464049567

[ekohl]

That's probably because the module doesn't use rspec-puppet-facts but rather specifies them manually. I had a look at the module, but its test infra is in serious need of an update.

[sanfrancrisko]

That's probably because the module doesn't use rspec-puppet-facts but rather specifies them manually. I had a look at the module, but its test infra is in serious need of an update.

Agreed - it would make things a lot nicer to use rspec-puppet-facts consistently across the module. I took a look at spec/classes/apache_spec.rb and frankly, it's going to be quite a while to fix that one test class up - it's not going to be as simple as encapsulating the tests within a on_supported_os.each block (not that I ever thought it would!). There's a heck of a lot of duplication and specific tests coupled to specific OSs. I haven't even looked in to the other test classes failing either - I suspect a similar scenario awaits in them.

I'm not saying that it isn't the right thing to do, but, it would be quite an undertaking and a bit more surgical than a bulk edit. I'll discuss with some of the team on how we want to proceed. If you have any suggestions, we'd be keen to hear, of course 👍

[ekohl]

I added another commit I wrote while looking into this. I think the main pain point comes from remaining compatible with Facter 1.6 for too long. puppetlabs/puppetlabs-stdlib#1154 is another thing that should have died long ago with Facter 1.6.

About the conversion to rspec-puppet-facts. I think you're right that this won't be a trivial conversion. Another thing to note is that the test runtime will go up a lot since it'll test a lot more combinations. Perhaps a decent compromise is to define a default test OS in some shared file and use that instead of defining the same facts over and over again.

[ekohl]

In #2120 I took a stab at a the preparation. Not a full conversion to rspec-puppet-facts, but by using shared examples a big step forward is taken to make it easier.

[ekohl]

This now includes #2120 since hopefully that's sufficient to make the tests pass. Once that's merged, I'll rebase for easier review.

[codecov-io]

Codecov Report

Merging #2110 (49572f4) into main (13f55bd) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2110   +/-   ##
=======================================
  Coverage   56.36%   56.36%           
=======================================
  Files          12       12           
  Lines         220      220           
=======================================
  Hits          124      124           
  Misses         96       96           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1a830b...49572f4. Read the comment docs.

[ekohl]

Rebased and squashed into a single commit. Also updated some fact sets to provide a major version.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[david22swan]

LGTM

[david22swan]

@ekohl Thanks for the excellent work as always
Looking forward to more in the future.