From caea941fbc6e92d081abe89ed52e0137d5fcf07a Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Mon, 22 May 2023 11:59:00 +0200 Subject: [PATCH 1/2] Remove deprecated File.exists? In 7999ff2aebbd2d85a231318f1e466b36d6dab84e the cops were disabled, but Ruby 3.2 has removed the method. --- lib/puppet/parser/functions/load_module_metadata.rb | 2 +- lib/puppet/parser/functions/loadjson.rb | 2 +- lib/puppet/parser/functions/loadyaml.rb | 2 +- spec/functions/load_module_metadata_spec.rb | 6 +++--- spec/functions/loadjson_spec.rb | 6 +++--- spec/functions/loadyaml_spec.rb | 6 +++--- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/puppet/parser/functions/load_module_metadata.rb b/lib/puppet/parser/functions/load_module_metadata.rb index 9a44b6186..d5c6672b8 100644 --- a/lib/puppet/parser/functions/load_module_metadata.rb +++ b/lib/puppet/parser/functions/load_module_metadata.rb @@ -23,7 +23,7 @@ module Puppet::Parser::Functions module_path = function_get_module_path([mod]) metadata_json = File.join(module_path, 'metadata.json') - metadata_exists = File.exists?(metadata_json) # rubocop:disable Lint/DeprecatedClassMethods : Changing to .exist? breaks the code + metadata_exists = File.exist?(metadata_json) if metadata_exists metadata = if Puppet::Util::Package.versioncmp(Puppet.version, '8.0.0').negative? PSON.load(File.read(metadata_json)) diff --git a/lib/puppet/parser/functions/loadjson.rb b/lib/puppet/parser/functions/loadjson.rb index 6ec9fae1d..a85cd18ee 100644 --- a/lib/puppet/parser/functions/loadjson.rb +++ b/lib/puppet/parser/functions/loadjson.rb @@ -55,7 +55,7 @@ module Puppet::Parser::Functions else JSON.parse(contents) || args[1] end - elsif File.exists?(args[0]) # rubocop:disable Lint/DeprecatedClassMethods : Changing to .exist? breaks the code + elsif File.exist?(args[0]) content = File.read(args[0]) if Puppet::Util::Package.versioncmp(Puppet.version, '8.0.0').negative? PSON.load(content) || args[1] diff --git a/lib/puppet/parser/functions/loadyaml.rb b/lib/puppet/parser/functions/loadyaml.rb index 9bf047a0a..bb2a7b00a 100644 --- a/lib/puppet/parser/functions/loadyaml.rb +++ b/lib/puppet/parser/functions/loadyaml.rb @@ -50,7 +50,7 @@ module Puppet::Parser::Functions args[1] end YAML.safe_load(contents) || args[1] - elsif File.exists?(args[0]) # rubocop:disable Lint/DeprecatedClassMethods : Changing to .exist? breaks the code + elsif File.exist?(args[0]) YAML.load_file(args[0]) || args[1] else warning("Can't load '#{args[0]}' File does not exist!") diff --git a/spec/functions/load_module_metadata_spec.rb b/spec/functions/load_module_metadata_spec.rb index f2d857f14..6a8124a0f 100644 --- a/spec/functions/load_module_metadata_spec.rb +++ b/spec/functions/load_module_metadata_spec.rb @@ -29,7 +29,7 @@ it 'jsons parse the file' do allow(scope).to receive(:function_get_module_path).with(['science']).and_return("#{prefix}/path/to/module/") - allow(File).to receive(:exists?).with("#{prefix}/path/to/module/metadata.json").and_return(true) + allow(File).to receive(:exist?).with("#{prefix}/path/to/module/metadata.json").and_return(true) allow(File).to receive(:read).with("#{prefix}/path/to/module/metadata.json").and_return('{"name": "spencer-science"}') result = subject.execute('science') @@ -38,13 +38,13 @@ it 'fails by default if there is no metadata.json' do allow(scope).to receive(:function_get_module_path).with(['science']).and_return("#{prefix}/path/to/module/") - allow(File).to receive(:exists?).with("#{prefix}/path/to/module/metadata.json").and_return(false) + allow(File).to receive(:exist?).with("#{prefix}/path/to/module/metadata.json").and_return(false) expect { subject.call(['science']) }.to raise_error(Puppet::ParseError) end it 'returns nil if user allows empty metadata.json' do allow(scope).to receive(:function_get_module_path).with(['science']).and_return("#{prefix}/path/to/module/") - allow(File).to receive(:exists?).with("#{prefix}/path/to/module/metadata.json").and_return(false) + allow(File).to receive(:exist?).with("#{prefix}/path/to/module/metadata.json").and_return(false) result = subject.execute('science', true) expect(result).to eq({}) end diff --git a/spec/functions/loadjson_spec.rb b/spec/functions/loadjson_spec.rb index b9f0df95a..7e809b6d5 100644 --- a/spec/functions/loadjson_spec.rb +++ b/spec/functions/loadjson_spec.rb @@ -26,7 +26,7 @@ end before(:each) do - allow(File).to receive(:exists?).with(filename).and_return(false).once + allow(File).to receive(:exist?).with(filename).and_return(false).once if Puppet::PUPPETVERSION[0].to_i < 8 allow(PSON).to receive(:load).never # rubocop:disable RSpec/ReceiveNever Switching to not_to receive breaks testing in this case else @@ -51,7 +51,7 @@ let(:json) { '{"key":"value", {"ķęŷ":"νậŀųề" }, {"キー":"値" }' } before(:each) do - allow(File).to receive(:exists?).with(filename).and_return(true).once + allow(File).to receive(:exist?).with(filename).and_return(true).once allow(File).to receive(:read).with(filename).and_return(json).once allow(File).to receive(:read).with(filename).and_return(json).once if Puppet::PUPPETVERSION[0].to_i < 8 @@ -75,7 +75,7 @@ let(:json) { '{"key":"value"}' } before(:each) do - allow(File).to receive(:exists?).with(filename).and_return(true).once + allow(File).to receive(:exist?).with(filename).and_return(true).once allow(File).to receive(:read).with(filename).and_return(json).once if Puppet::PUPPETVERSION[0].to_i < 8 allow(PSON).to receive(:load).with(json).once.and_raise StandardError, 'Something terrible have happened!' diff --git a/spec/functions/loadyaml_spec.rb b/spec/functions/loadyaml_spec.rb index cc08c477a..556ca17b4 100644 --- a/spec/functions/loadyaml_spec.rb +++ b/spec/functions/loadyaml_spec.rb @@ -10,7 +10,7 @@ let(:filename) { '/tmp/doesnotexist' } it "'default' => 'value'" do - expect(File).to receive(:exists?).with(filename).and_return(false).once + expect(File).to receive(:exist?).with(filename).and_return(false).once expect(YAML).not_to receive(:load_file) expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value') end @@ -21,7 +21,7 @@ let(:data) { { 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値' } } it "returns 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値'" do - expect(File).to receive(:exists?).with(filename).and_return(true).once + expect(File).to receive(:exist?).with(filename).and_return(true).once expect(YAML).to receive(:load_file).with(filename).and_return(data).once expect(subject).to run.with_params(filename).and_return(data) end @@ -31,7 +31,7 @@ let(:filename) { '/tmp/doesexist' } it 'filename /tmp/doesexist' do - expect(File).to receive(:exists?).with(filename).and_return(true).once + expect(File).to receive(:exist?).with(filename).and_return(true).once allow(YAML).to receive(:load_file).with(filename).once.and_raise(StandardError, 'Something terrible have happened!') expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value') end From 43fb939829bc6ace267ece3ab2831f16d0605534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Wed, 24 May 2023 13:17:24 -1000 Subject: [PATCH 2/2] Fix CI File#exist? is called in many locations. If we want to mock its behavior for some use cases, we must ensure that only the specific test case is mocked and the other calls still rely on the original implementation. --- spec/functions/loadjson_spec.rb | 3 +++ spec/functions/loadyaml_spec.rb | 3 +++ 2 files changed, 6 insertions(+) diff --git a/spec/functions/loadjson_spec.rb b/spec/functions/loadjson_spec.rb index 7e809b6d5..17d01e451 100644 --- a/spec/functions/loadjson_spec.rb +++ b/spec/functions/loadjson_spec.rb @@ -26,6 +26,7 @@ end before(:each) do + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with(filename).and_return(false).once if Puppet::PUPPETVERSION[0].to_i < 8 allow(PSON).to receive(:load).never # rubocop:disable RSpec/ReceiveNever Switching to not_to receive breaks testing in this case @@ -51,6 +52,7 @@ let(:json) { '{"key":"value", {"ķęŷ":"νậŀųề" }, {"キー":"値" }' } before(:each) do + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with(filename).and_return(true).once allow(File).to receive(:read).with(filename).and_return(json).once allow(File).to receive(:read).with(filename).and_return(json).once @@ -75,6 +77,7 @@ let(:json) { '{"key":"value"}' } before(:each) do + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with(filename).and_return(true).once allow(File).to receive(:read).with(filename).and_return(json).once if Puppet::PUPPETVERSION[0].to_i < 8 diff --git a/spec/functions/loadyaml_spec.rb b/spec/functions/loadyaml_spec.rb index 556ca17b4..99e87d8a9 100644 --- a/spec/functions/loadyaml_spec.rb +++ b/spec/functions/loadyaml_spec.rb @@ -10,6 +10,7 @@ let(:filename) { '/tmp/doesnotexist' } it "'default' => 'value'" do + allow(File).to receive(:exist?).and_call_original expect(File).to receive(:exist?).with(filename).and_return(false).once expect(YAML).not_to receive(:load_file) expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value') @@ -21,6 +22,7 @@ let(:data) { { 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値' } } it "returns 'key' => 'value', 'ķęŷ' => 'νậŀųề', 'キー' => '値'" do + allow(File).to receive(:exist?).and_call_original expect(File).to receive(:exist?).with(filename).and_return(true).once expect(YAML).to receive(:load_file).with(filename).and_return(data).once expect(subject).to run.with_params(filename).and_return(data) @@ -31,6 +33,7 @@ let(:filename) { '/tmp/doesexist' } it 'filename /tmp/doesexist' do + allow(File).to receive(:exist?).and_call_original expect(File).to receive(:exist?).with(filename).and_return(true).once allow(YAML).to receive(:load_file).with(filename).once.and_raise(StandardError, 'Something terrible have happened!') expect(subject).to run.with_params(filename, 'default' => 'value').and_return('default' => 'value')