(CAT-1644) - Fix RSpec/StubbedMock

Author: jordanbreen28

.rubocop_todo.yml

@@ -23,7 +23,7 @@ RSpec/InstanceVariable:
     - 'spec/acceptance/namevar_spec.rb'
     - 'spec/puppet/resource_api/data_type_handling_spec.rb'
 
-# Offense count: 15
+# # Offense count: 15
 RSpec/LeakyConstantDeclaration:
   Exclude:
     - 'spec/puppet/resource_api/base_context_spec.rb'
@@ -45,14 +45,6 @@ RSpec/MultipleMemoizedHelpers:
 RSpec/NestedGroups:
   Max: 7
 
-# Offense count: 25
-RSpec/StubbedMock:
-  Exclude:
-    - 'spec/puppet/resource_api/property_spec.rb'
-    - 'spec/puppet/resource_api/transport/wrapper_spec.rb'
-    - 'spec/puppet/resource_api/transport_spec.rb'
-    - 'spec/puppet/resource_api_spec.rb'
-
 # Offense count: 47
 RSpec/SubjectStub:
   Exclude:

spec/puppet/resource_api/property_spec.rb

@@ -113,7 +113,8 @@
 
         context 'when insync? is not defined in the provider' do
           it 'raises an error' do
-            expect(referrable_type_custom_insync).to receive(:my_provider).and_return(test_provider_without_insync)
+            allow(referrable_type_custom_insync).to receive(:my_provider).and_return(test_provider_without_insync)
+            expect(referrable_type_custom_insync).to receive(:my_provider)
             expect { custom_insync_property.insync?('Foo') }.to raise_error Puppet::DevError, /No insync\? method defined in the provider/
           end
         end
@@ -159,33 +160,38 @@
 
           context 'when custom insync from the provider returns a boolean for the result' do
             it 'returns true if the result was true' do
-              expect(test_provider_with_insync).to receive(:insync?).and_return(true)
+              allow(test_provider_with_insync).to receive(:insync?).and_return(true)
+              expect(test_provider_with_insync).to receive(:insync?)
               expect(custom_insync_property.insync?('Foo')).to be true
             end
 
             it 'returns false if result was false' do
-              expect(test_provider_with_insync).to receive(:insync?).and_return(false)
+              allow(test_provider_with_insync).to receive(:insync?).and_return(false)
+              expect(test_provider_with_insync).to receive(:insync?)
               expect(custom_insync_property.insync?('Foo')).to be false
             end
           end
 
           context 'when custom insync from the provider returns a string for the result' do
             it 'raises an explanatory DevError' do
-              expect(test_provider_with_insync).to receive(:insync?).and_return('true')
+              allow(test_provider_with_insync).to receive(:insync?).and_return('true')
+              expect(test_provider_with_insync).to receive(:insync?)
               expect { custom_insync_property.insync?('foo') }.to raise_error(Puppet::DevError, %r{returned a String with a value of "true" instead of true/false})
             end
           end
 
           context 'when custom insync from the provider returns a symbol for the result' do
             it 'raises an explanatory DevError' do
-              expect(test_provider_with_insync).to receive(:insync?).and_return(:true) # rubocop:disable Lint/BooleanSymbol
+              allow(test_provider_with_insync).to receive(:insync?).and_return(:true) # rubocop:disable Lint/BooleanSymbol
+              expect(test_provider_with_insync).to receive(:insync?)
               expect { custom_insync_property.insync?('foo') }.to raise_error(Puppet::DevError, %r{returned a Symbol with a value of :true instead of true/false})
             end
           end
 
           context 'when insync? returned an unexpected result class' do
             it 'raises an explanatory DevError' do
-              expect(test_provider_with_insync).to receive(:insync?).and_return(foo: 1)
+              allow(test_provider_with_insync).to receive(:insync?).and_return(foo: 1)
+              expect(test_provider_with_insync).to receive(:insync?)
               expect { custom_insync_property.insync?('foo') }.to raise_error(Puppet::DevError, %r{returned a Hash with a value of \{:foo=>1\} instead of true/false})
             end
           end
@@ -208,7 +214,8 @@
 
           context 'when insync? returns nil for the result' do
             it 'relies on Puppet::Property.change_to_s for change reporting' do
-              expect(test_provider_with_insync).to receive(:insync?).and_return([nil, 'custom change message'])
+              allow(test_provider_with_insync).to receive(:insync?).and_return([nil, 'custom change message'])
+              expect(test_provider_with_insync).to receive(:insync?)
               expect(custom_insync_property.insync?('Foo')).to be(false)
               expect(custom_insync_property.change_to_s('Foo', 'foo')).to match(/changed 'Foo' to 'foo'/)
             end
@@ -217,23 +224,26 @@
           context 'when insync? returns a change message' do
             context 'when the message is empty' do
               it 'relies on Puppet::Property.change_to_s for change reporting' do
-                expect(test_provider_with_insync).to receive(:insync?).and_return([false, ''])
+                allow(test_provider_with_insync).to receive(:insync?).and_return([false, ''])
+                expect(test_provider_with_insync).to receive(:insync?)
                 expect(custom_insync_property.insync?('Foo')).to be(false)
                 expect(custom_insync_property.change_to_s('Foo', 'foo')).to match(/changed 'Foo' to 'foo'/)
               end
             end
 
             context 'when the result is nil' do
               it 'relies on Puppet::Property.change_to_s for change reporting' do
-                expect(test_provider_with_insync).to receive(:insync?).and_return(nil)
+                allow(test_provider_with_insync).to receive(:insync?).and_return(nil)
+                expect(test_provider_with_insync).to receive(:insync?)
                 expect(custom_insync_property.insync?('Foo')).to be(false)
                 expect(custom_insync_property.change_to_s('Foo', 'foo')).to match(/changed 'Foo' to 'foo'/)
               end
             end
 
             context 'when the result is not nil and the message is not empty' do
               it 'passes the message for change_to_s' do
-                expect(test_provider_with_insync).to receive(:insync?).and_return([false, 'custom change log'])
+                allow(test_provider_with_insync).to receive(:insync?).and_return([false, 'custom change log'])
+                expect(test_provider_with_insync).to receive(:insync?)
                 expect(custom_insync_property.insync?('Foo')).to be(false)
                 expect(custom_insync_property.change_to_s('Foo', 'foo')).to match(/custom change log/)
               end
@@ -251,13 +261,15 @@
           end
 
           it 'passes the default message for change reporting if insync? did not return a string' do
-            expect(test_provider_with_insync).to receive(:insync?).and_return(false)
+            allow(test_provider_with_insync).to receive(:insync?).and_return(false)
+            expect(test_provider_with_insync).to receive(:insync?)
             custom_insync_property.insync?('Foo')
             expect(custom_insync_property.change_to_s(false, true)).to match(/Custom insync logic determined that this resource is out of sync/)
           end
 
           it 'passes the string returned by insync? for change reporting' do
-            expect(test_provider_with_insync).to receive(:insync?).and_return(insync_result)
+            allow(test_provider_with_insync).to receive(:insync?).and_return(insync_result)
+            expect(test_provider_with_insync).to receive(:insync?)
             custom_insync_property.insync?('Foo')
             expect(custom_insync_property.change_to_s(false, true)).to be insync_result[1]
           end

spec/puppet/resource_api/transport/wrapper_spec.rb

@@ -22,7 +22,8 @@ class SomethingSomethingDarkside; end
           allow(File).to receive(:exist?).and_return(true)
           allow(Hocon).to receive(:load).and_call_original
           expect(Puppet::ResourceApi::Transport).to receive(:connect)
-          expect(Hocon).to receive(:load).with('/etc/credentials', any_args).and_return('foo' => %w[a b], 'bar' => 2)
+          allow(Hocon).to receive(:load).with('/etc/credentials', any_args).and_return('foo' => %w[a b], 'bar' => 2)
+          expect(Hocon).to receive(:load).with('/etc/credentials', any_args)
           expect { described_class.new('wibble', url) }.not_to raise_error
         end
       end

spec/puppet/resource_api/transport_spec.rb

@@ -222,7 +222,8 @@ def change_environment(name = nil)
           allow(described_class).to receive(:require).with('puppet/resource_api/puppet_context').and_call_original
           expect(described_class).to receive(:require).with('puppet/transport/test_target')
           expect(described_class).to receive(:validate).with(name, { host: 'example.com' })
-          expect(Puppet::ResourceApi::PuppetContext).to receive(:new).with(kind_of(Puppet::ResourceApi::TransportSchemaDef)).and_return(context)
+          allow(Puppet::ResourceApi::PuppetContext).to receive(:new).with(kind_of(Puppet::ResourceApi::TransportSchemaDef)).and_return(context)
+          expect(Puppet::ResourceApi::PuppetContext).to receive(:new).with(kind_of(Puppet::ResourceApi::TransportSchemaDef))
 
           stub_const('Puppet::Transport::TestTarget', test_target)
           expect(test_target).to receive(:new).with(context, { host: 'example.com' })
@@ -250,7 +251,8 @@ class Wibble; end
 
     context 'when puppet has set_device' do
       it 'wraps the transport and calls set_device within NetworkDevice' do
-        expect(Puppet::ResourceApi::Transport::Wrapper).to receive(:new).with(device_name, transport).and_return(wrapper)
+        allow(Puppet::ResourceApi::Transport::Wrapper).to receive(:new).with(device_name, transport).and_return(wrapper)
+        expect(Puppet::ResourceApi::Transport::Wrapper).to receive(:new).with(device_name, transport)
         allow(Puppet::Util::NetworkDevice).to receive(:respond_to?).with(:set_device).and_return(true)
         expect(Puppet::Util::NetworkDevice).to receive(:set_device).with(device_name, wrapper)
 
@@ -260,8 +262,10 @@ class Wibble; end
 
     context 'when puppet does not have set_device' do
       it 'wraps the transport and sets it as current in NetworkDevice' do
-        expect(Puppet::ResourceApi::Transport::Wrapper).to receive(:new).with(device_name, transport).and_return(wrapper)
-        expect(Puppet::Util::NetworkDevice).to receive(:respond_to?).with(:set_device).and_return(false)
+        allow(Puppet::ResourceApi::Transport::Wrapper).to receive(:new).with(device_name, transport).and_return(wrapper)
+        expect(Puppet::ResourceApi::Transport::Wrapper).to receive(:new).with(device_name, transport)
+        allow(Puppet::Util::NetworkDevice).to receive(:respond_to?).with(:set_device).and_return(false)
+        expect(Puppet::Util::NetworkDevice).to receive(:respond_to?).with(:set_device)
 
         described_class.inject_device(device_name, transport)
 
@@ -299,8 +303,10 @@ class Wibble; end
 
       it 'validates the connection_info' do
         expect(described_class).not_to receive(:require).with('puppet/transport/schema/validate')
-        expect(schema_def).to receive(:check_schema).with({}, kind_of(String)).and_return(nil)
-        expect(schema_def).to receive(:validate).with({}).and_return(nil)
+        allow(schema_def).to receive(:check_schema).with({}, kind_of(String)).and_return(nil)
+        expect(schema_def).to receive(:check_schema).with({}, kind_of(String))
+        allow(schema_def).to receive(:validate).with({}).and_return(nil)
+        expect(schema_def).to receive(:validate).with({})
 
         described_class.send :validate, 'validate', {}
       end
@@ -309,8 +315,10 @@ class Wibble; end
         let(:attributes) { { host: {}, foo: {} } }
 
         it 'cleans the connection_info' do
-          expect(schema_def).to receive(:check_schema).with({ host: 'host value', foo: 'foo value' }, kind_of(String)).and_return(nil)
-          expect(schema_def).to receive(:validate).with({ host: 'host value', foo: 'foo value' }).and_return(nil)
+          allow(schema_def).to receive(:check_schema).with({ host: 'host value', foo: 'foo value' }, kind_of(String)).and_return(nil)
+          expect(schema_def).to receive(:check_schema).with({ host: 'host value', foo: 'foo value' }, kind_of(String))
+          allow(schema_def).to receive(:validate).with({ host: 'host value', foo: 'foo value' }).and_return(nil)
+          expect(schema_def).to receive(:validate).with({ host: 'host value', foo: 'foo value' })
 
           expect(context).to receive(:debug).with('Discarding bolt metaparameter: query')
           expect(context).to receive(:debug).with('Discarding bolt metaparameter: remote-transport')
@@ -331,8 +339,10 @@ class Wibble; end
         let(:attributes) { { host: { default: 'example.com' }, port: { default: 443 } } }
 
         it 'sets defaults in the connection info' do
-          expect(schema_def).to receive(:check_schema).with({ host: 'host value', port: 443 }, kind_of(String)).and_return(nil)
-          expect(schema_def).to receive(:validate).with({ host: 'host value', port: 443 }).and_return(nil)
+          allow(schema_def).to receive(:check_schema).with({ host: 'host value', port: 443 }, kind_of(String)).and_return(nil)
+          expect(schema_def).to receive(:check_schema).with({ host: 'host value', port: 443 }, kind_of(String))
+          allow(schema_def).to receive(:validate).with({ host: 'host value', port: 443 }).and_return(nil)
+          expect(schema_def).to receive(:validate).with({ host: 'host value', port: 443 })
 
           expect(context).to receive(:debug).with('Using default value for attribute: port, value: 443')
           described_class.send :validate, 'validate', host: 'host value'

spec/puppet/resource_api_spec.rb

@@ -2229,13 +2229,15 @@ def set(_context, changes) end
       end
 
       it 'passes through the an empty array to `get`' do
-        expect(provider).to receive(:get).with(anything, nil).and_return([])
+        allow(provider).to receive(:get).with(anything, nil).and_return([])
+        expect(provider).to receive(:get).with(anything, nil)
         type.instances
       end
 
       it 'passes through the resource title to `get`' do
         instance = type.new(name: 'bar', ensure: 'present')
-        expect(provider).to receive(:get).with(anything, ['bar']).and_return([])
+        allow(provider).to receive(:get).with(anything, ['bar']).and_return([])
+        expect(provider).to receive(:get).with(anything, ['bar'])
         instance.retrieve
       end
     end