search

puppetlabs / puppetlabs-stdlib

Puppet Labs Standard Library module
http://forge.puppetlabs.com/puppetlabs/stdlib
Apache License 2.0
348 stars 580 forks source link

Remove deprecated File.exists? #1357

Closed ekohl closed 1 year ago

ekohl commented 1 year ago

In 7999ff2aebbd2d85a231318f1e466b36d6dab84e the cops were disabled, but Ruby 3.2 has removed the method.

Fixes #1354

puppet-community-rangefinder[bot] commented 1 year ago

load_module_metadata is a function

Breaking changes to this file MAY impact these 10 modules (near match): * [theforeman-pulpcore](https://github.com/theforeman/puppet-pulpcore) * [puppetlabs-relay](https://github.com/puppetlabs/puppetlabs-relay) * [icinga-icinga2](https://github.com/icinga/puppet-icinga2) * [veepshosting-cloudwatchlogs](https://github.com/Veeps-Hosting/puppet-cloudwatchlogs) * [puppet-yum](https://github.com/voxpupuli/puppet-yum.git) * [simp-tcpwrappers](https://github.com/simp/pupmod-simp-tcpwrappers) * [simp-polkit](https://github.com/simp/pupmod-simp-polkit) * [geekify-cloudwatchlogs](https://github.com/dsappet/puppet-cloudwatchlogs) * [datadog-datadog_agent](https://github.com/DataDog/puppet-datadog-agent.git) * [danzilio-virtualbox](https://github.com/danzilio/danzilio-virtualbox)

loadjson is a function

Breaking changes to this file MAY impact these 1 modules (near match): * [shinesolutions-aem_curator](http://github.com/shinesolutions/puppet-aem-curator)

loadyaml is a function

Breaking changes to this file MAY impact these 2 modules (near match): * [kobybr-apmserver](https://github.com/kobybr/puppet-apmserver.git) * [maany-simple_grid](https://github.com/WLCG-Lightweight-Sites/simple_grid_puppet_module)

This module is declared in 318 of 580 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.

LukasAud commented 1 year ago

Hey @ekohl, it looks like the PR is generating a block of failures in the CI. I dont think we can merge this while in this state. Also, we are planning to merge the Puppet 8 update to main very soon (today/tomorrow morning, you are welcome to leave a review if you want), so this PR will need a rebase to ensure that CI covers testing properly.

I will try to investigate this issue again as well, to see if I can figure out whats going on or why CI is not failing on main despite .exists? being present there.

ekohl commented 1 year ago

That looks very suspicious, but I can't explain why it happens. I don't have time to investigate it now so feel free to take over if you have the cycles.

LukasAud commented 1 year ago

@ekohl Sure thing, I'll rebase the PR and play around with for a bit to see if I can figure out a reason for these failures.

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

ekohl commented 1 year ago

I rebased it myself because I suspect the merge commit tripped up the CLA check.

smortex commented 1 year ago

The mocked tests seems to "hide" the original code. Allowing all calls to File#exist? and calling the original seems to fix the issue.

LukasAud commented 1 year ago

I was looking into this yesterday. Looks like I was getting close to the solution but I couldnt quite figure out exactly how to fix it. Anyways, happy to merge this now.

ekohl commented 1 year ago

The mocked tests seems to "hide" the original code. Allowing all calls to File#exist? and calling the original seems to fix the issue.

That was also one of my suspicions, but was too busy with other work. Thanks @smortex!