Merge pull request #379 from puppetlabs/CAT-2453

pmcmaw · web-flow · commit 73d75d1cd578 · 2025-09-30T15:46:47.000+01:00
(CAT-2453) Update rubocop to remove rexml dependency
diff --git a/.gitignore b/.gitignore
@@ -14,4 +14,4 @@ vendor/
 .rspec_status
 
 # puppetlabs_spec_helper automatic fixtures
-spec/fixtures/modules/test_module
+spec/fixtures/modules/
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -1,7 +1,12 @@
 ---
 inherit_from: .rubocop_todo.yml
 
-require: rubocop-rspec
+plugins: 
+  - rubocop-rspec
+  - rubocop-rspec_rails
+  - rubocop-performance
+  - rubocop-factory_bot
+  - rubocop-capybara
 AllCops:
   NewCops: enable
   TargetRubyVersion: '3.1'
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2025-03-10 20:03:26 UTC using RuboCop version 1.70.0.
+# on 2025-09-30 14:08:02 UTC using RuboCop version 1.73.2.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -24,6 +24,13 @@ Lint/EmptyClass:
     - 'spec/puppet/resource_api/transport_spec.rb'
     - 'spec/puppet/resource_api_spec.rb'
 
+# Offense count: 5
+# Configuration parameters: MinSize.
+Performance/CollectionLiteralInLoop:
+  Exclude:
+    - 'lib/puppet/resource_api.rb'
+    - 'spec/puppet/resource_api/data_type_handling_spec.rb'
+
 # Offense count: 25
 # Configuration parameters: IgnoredMetadata.
 RSpec/DescribeClass:
@@ -70,8 +77,6 @@ RSpec/SubjectStub:
 
 # Offense count: 24
 # This cop supports unsafe autocorrection (--autocorrect-all).
-# Configuration parameters: EnforcedStyle.
-# SupportedStyles: constant, string
 RSpec/VerifiedDoubleReference:
   Exclude:
     - 'spec/integration/resource_api_spec.rb'
diff --git a/Gemfile b/Gemfile
@@ -19,9 +19,12 @@ group :tests do
   # since the Resource API runs inside the puppetserver, test against the JRuby versions we ship
   # these require special dependencies to have everything load properly
   # rubocop 1.48 supports JRuby 9.3+, which includes coverage for versions we support
-  gem 'rubocop', '~> 1.70.0', require: false
-  gem 'rubocop-rspec', '~> 2.20.0', require: false
-  gem 'rubocop-performance', '~> 1.17.1', require: false
+  gem 'rubocop', '~> 1.73.0', require: false
+  gem 'rubocop-rspec', '~> 3.5.0', require: false
+  gem 'rubocop-performance', '~> 1.24.0', require: false
+  gem 'rubocop-rspec_rails', '~> 2.31.0', require: false
+  gem 'rubocop-factory_bot', '~> 2.27.0', require: false
+  gem 'rubocop-capybara', '~> 2.22.0', require: false
 end
 
 group :development do
diff --git a/spec/integration/resource_api_spec.rb b/spec/integration/resource_api_spec.rb
@@ -239,9 +239,7 @@ def get(_context)
       end
 
       before do
-        allow(catalog).to receive(:resource).and_return nil
-        allow(catalog).to receive(:alias).and_return nil
-        allow(catalog).to receive(:host_config?).and_return true
+        allow(catalog).to receive_messages(resource: nil, alias: nil, host_config?: true)
       end
 
       it('flushes') { expect { instance.flush }.not_to raise_exception }
diff --git a/spec/puppet/resource_api/glue_spec.rb b/spec/puppet/resource_api/glue_spec.rb
@@ -136,7 +136,7 @@
 
       it { expect(value <=> 'string').to eq(-1) }
       # Avoid this test on jruby 1.7, where it is hitting a implementation inconsistency and `'string' <=> value` returns `nil`
-      it('compares to a string', j17_exclude: true) { expect('string' <=> value).to eq 1 }
+      it('compares to a string', :j17_exclude) { expect('string' <=> value).to eq 1 }
       it { expect(value <=> described_class[b: 'b', c: 'c']).to eq 0 }
       it { expect(value <=> described_class[d: 'd']).to eq(-1) }
       it { expect(value <=> described_class[a: 'a']).to eq 1 }
diff --git a/spec/puppet/resource_api/property_spec.rb b/spec/puppet/resource_api/property_spec.rb
@@ -203,8 +203,7 @@
           allow(resource).to receive(:rsapi_canonicalized_target_state)
           allow(resource).to receive(:rsapi_current_state)
           allow(resource).to receive(:rsapi_title)
-          allow(referrable_type_custom_insync).to receive(:context).and_return(context)
-          allow(referrable_type_custom_insync).to receive(:my_provider).and_return(test_provider_with_insync)
+          allow(referrable_type_custom_insync).to receive_messages(context: context, my_provider: test_provider_with_insync)
         end
 
         context 'when the property is not rsapi_custom_insync_trigger' do
diff --git a/spec/puppet/resource_api/simple_provider_spec.rb b/spec/puppet/resource_api/simple_provider_spec.rb
@@ -445,8 +445,7 @@ def delete(context, _name); end
     before do
       allow(context).to receive(:updating).with('title').and_yield
       allow(type_def).to receive(:feature?).with('simple_get_filter')
-      allow(type_def).to receive(:ensurable?).and_return(false)
-      allow(type_def).to receive(:namevars).and_return([:name])
+      allow(type_def).to receive_messages(ensurable?: false, namevars: [:name])
     end
 
     it { expect { provider.set(context, changes) }.to raise_error(/SimpleProvider cannot be used with a Type that is not ensurable/) }
diff --git a/spec/puppet/resource_api/transport/wrapper_spec.rb b/spec/puppet/resource_api/transport/wrapper_spec.rb
@@ -6,7 +6,7 @@
 require 'puppet/resource_api/transport/wrapper'
 require_relative '../../../fixtures/test_module/lib/puppet/transport/test_device'
 
-RSpec.describe Puppet::ResourceApi::Transport::Wrapper, agent_test: true do
+RSpec.describe Puppet::ResourceApi::Transport::Wrapper, :agent_test do
   describe '#initialize(name, url_or_config)' do
     before do
       module Puppet::Transport
@@ -64,8 +64,7 @@ class SomethingSomethingDarkside; end
       let(:transport) { instance_double(Puppet::Transport::TestDevice, 'transport') }
 
       it 'returns the facts provided by the transport' do
-        allow(Puppet::ResourceApi::Transport).to receive(:connect).and_return(transport)
-        allow(Puppet::ResourceApi::Transport).to receive(:list).and_return(schema: :dummy)
+        allow(Puppet::ResourceApi::Transport).to receive_messages(connect: transport, list: { schema: :dummy })
         allow(Puppet::ResourceApi::PuppetContext).to receive(:new).and_return(context)
         allow(transport).to receive(:facts).with(context).and_return(facts)
 
diff --git a/spec/puppet/resource_api/transport_spec.rb b/spec/puppet/resource_api/transport_spec.rb
@@ -178,7 +178,7 @@ def change_environment(name = nil)
     end
   end
 
-  describe '#connect(name, connection_info)', agent_test: true do
+  describe '#connect(name, connection_info)', :agent_test do
     let(:name) { 'test_target' }
     let(:schema) do
       {
@@ -274,7 +274,7 @@ class Wibble; end
     end
   end
 
-  describe '#validate(name, connection_info)', agent_test: true do
+  describe '#validate(name, connection_info)', :agent_test do
     context 'when the transport does not exist' do
       it { expect { described_class.send(:validate, 'wibble', {}) }.to raise_error LoadError, %r{(no such file to load|cannot load such file) -- puppet/transport/schema/wibble} }
     end
diff --git a/spec/puppet/resource_api_spec.rb b/spec/puppet/resource_api_spec.rb
@@ -38,12 +38,12 @@
       subject(:type) { Puppet::Type.type(:minimal) }
 
       it { is_expected.not_to be_nil }
-      it { is_expected.to be_respond_to :instances }
+      it { is_expected.to respond_to :instances }
       it { expect(type.apply_to).to eq(:host) }
     end
 
     describe Puppet::Provider do
-      it('has a module prepared for the provider') { expect(described_class.const_get('Minimal', false).name).to eq 'Puppet::Provider::Minimal' }
+      it('has a module prepared for the provider') { expect(described_class.const_get(:Minimal, false).name).to eq 'Puppet::Provider::Minimal' }
     end
   end
 
@@ -64,7 +64,7 @@
       subject(:type) { Puppet::Type.type(:both) }
 
       it { is_expected.not_to be_nil }
-      it { is_expected.to be_respond_to :instances }
+      it { is_expected.to respond_to :instances }
       it { expect(type.instance_variable_get(:@doc)).to eq 'the docs' }
     end
   end
@@ -279,7 +279,7 @@ def extract_values(function)
         # rubocop:enable Lint/BooleanSymbol
       end
 
-      context 'with a basic provider', agent_test: true do
+      context 'with a basic provider', :agent_test do
         let(:provider_class) do
           Class.new do
             def get(_context)
@@ -322,7 +322,7 @@ def set(_context, _changes); end
           test_enum: 'a', test_variant_pattern: '0xAEF123FF', test_url: 'http://example.com' }
       end
 
-      context 'with a bad provider', agent_test: true do
+      context 'with a bad provider', :agent_test do
         before do
           stub_const('Puppet::Provider::TypeCheck', Module.new)
           stub_const('Puppet::Provider::TypeCheck::TypeCheck', provider_class)
@@ -408,7 +408,7 @@ def set(_context, _changes); end
 
       it('has the secret value is set correctly') { expect(instance[:secret]).to be_a Puppet::Pops::Types::PSensitiveType::Sensitive }
 
-      context 'with a basic provider', agent_test: true do
+      context 'with a basic provider', :agent_test do
         let(:provider_class) do
           Class.new do
             def get(_context)
@@ -471,7 +471,7 @@ def set(_context, _changes); end
     end
   end
 
-  context 'when registering a type that is ensurable', agent_test: true do
+  context 'when registering a type that is ensurable', :agent_test do
     context 'when ensurable is correctly declared' do
       let(:definition) do
         {
@@ -787,7 +787,7 @@ def set(_context, _changes); end
     end
   end
 
-  context 'when registering a type with an `init_only` attribute', agent_test: true do
+  context 'when registering a type with an `init_only` attribute', :agent_test do
     let(:definition) do
       {
         name: 'init_behaviour',
@@ -913,7 +913,7 @@ def set(_context, _changes); end
     end
   end
 
-  context 'when registering a type with an `read_only` attribute', agent_test: true do
+  context 'when registering a type with an `read_only` attribute', :agent_test do
     let(:definition) do
       {
         name: 'read_only_behaviour',
@@ -1022,7 +1022,7 @@ def set(_context, _changes); end
       end
     end
 
-    describe 'a provider that does not return the namevar', agent_test: true do
+    describe 'a provider that does not return the namevar', :agent_test do
       subject(:instance) { Puppet::Type.type(:not_name_namevar) }
 
       let(:provider_class) do
@@ -1048,7 +1048,7 @@ def set(_context, changes) end
     end
   end
 
-  context 'when registering a type with multiple namevars', agent_test: true do
+  context 'when registering a type with multiple namevars', :agent_test do
     let(:name) { 'multiple' }
     let(:title_patterns) { nil }
     let(:definition) do
@@ -1332,7 +1332,7 @@ def set(_context, _changes); end
     end
   end
 
-  context 'when registering a type with a mandatory boolean value', agent_test: true do
+  context 'when registering a type with a mandatory boolean value', :agent_test do
     let(:provider_class) do
       Class.new do
         def get(_context)
@@ -1418,7 +1418,7 @@ def set(_context, _changes); end
     end
   end
 
-  describe '#load_provider', agent_test: true do
+  describe '#load_provider', :agent_test do
     before { described_class.register_type(definition) }
 
     context 'when loading a non-existing provider' do
@@ -1507,7 +1507,7 @@ class OtherDevice; end
     end
   end
 
-  context 'with a provider that does canonicalization', agent_test: true do
+  context 'with a provider that does canonicalization', :agent_test do
     let(:definition) do
       {
         name: 'canonicalizer',
@@ -1714,7 +1714,7 @@ def set(_context, changes)
       end
 
       context 'with a provider that has a custom
-       generate', agent_test: true do
+       generate', :agent_test do
         let(:definition) do
           {
             name: 'generator',
@@ -1798,7 +1798,7 @@ def set(_context, changes)
     end
   end
 
-  context 'with a provider that does not need canonicalization', agent_test: true do
+  context 'with a provider that does not need canonicalization', :agent_test do
     let(:definition) do
       {
         name: 'passthrough',
@@ -1927,7 +1927,7 @@ def set(_context, changes)
     end
   end
 
-  context 'with a provider that does custom insync', agent_test: true do
+  context 'with a provider that does custom insync', :agent_test do
     let(:provider_class) do
       Class.new do
         def insync?(_context, _title, _attribute_name, _is_hash, _should_hash)
@@ -2052,7 +2052,7 @@ def set(_context, changes)
     end
   end
 
-  context 'with a `remote_resource` provider', agent_test: true do
+  context 'with a `remote_resource` provider', :agent_test do
     let(:definition) do
       {
         name: 'remoter',
@@ -2123,7 +2123,7 @@ class Wibble; end
     end
   end
 
-  context 'with a `supports_noop` provider', agent_test: true do
+  context 'with a `supports_noop` provider', :agent_test do
     let(:definition) do
       {
         name: 'test_noop_support',
@@ -2176,7 +2176,7 @@ def set(_context, _changes, noop: false); end
     end
   end
 
-  context 'with a `simple_get_filter` provider', agent_test: true do
+  context 'with a `simple_get_filter` provider', :agent_test do
     let(:definition) do
       {
         name: 'test_get_filter',
