Merge pull request #289 from puppetlabs/maint-refactor_retry_method

(maint) - refactor retry_invoke_dsc_resource

lib/puppet/provider/dsc_base_provider/dsc_base_provider.rb

@@ -310,24 +310,16 @@ def retry_invoke_dsc_resource(context, max_retry_count, retry_wait_interval_secs
       # notify and retry
       context.notice("Retrying: attempt #{try} of #{max_retry_count}.")
       data = JSON.parse(yield)
-      # if no error, break
-      if data['errormessage'].nil?
-        break
-      # check if error matches error matcher supplied
-      elsif data['errormessage'].match?(error_matcher)
-        # if last attempt, return error
-        if try == max_retry_count
-          context.notice("Attempt #{try} of #{max_retry_count} failed. No more retries.")
-          # all attempts failed, raise error
-          return context.err(data['errormessage'])
-        end
-        # if not last attempt, notify, continue and retry
-        context.notice("Attempt #{try} of #{max_retry_count} failed.")
-        next
-      else
-        # if we get an unexpected error, return
-        return context.err(data['errormessage'])
-      end
+      # if no error, assume successful invocation and break
+      break if data['errormessage'].nil?
+
+      # notify of failed retry
+      context.notice("Attempt #{try} of #{max_retry_count} failed.")
+      # return if error does not match expceted error, or all retries exhausted
+      return context.err(data['errormessage']) unless data['errormessage'].match?(error_matcher) && try < max_retry_count
+
+      # else, retry
+      next
     end
     data
   end

spec/unit/puppet/provider/dsc_base_provider/dsc_base_provider_spec.rb

@@ -814,9 +814,9 @@
                                                .exactly(5).times
         expect(context).to receive(:notice).with(/Invoke-DscResource collision detected: Please stagger the timing of your Puppet runs as this can lead to unexpected behaviour./).once
         expect(context).to receive(:notice).with('Sleeping for 60 seconds.').exactly(5).times
-        expect(context).to receive(:notice).with(/Retrying: attempt [1-6] of 5/).exactly(5).times
+        expect(context).to receive(:notice).with(/Retrying: attempt [1-5] of 5/).exactly(5).times
         expect(ps_manager).to receive(:execute).and_return({ stdout: '{"errormessage": "The Invoke-DscResource cmdlet is in progress and must return before Invoke-DscResource can be invoked"}' })
-        expect(context).to receive(:notice).with(/Attempt [1-6] of 5 failed/).exactly(5).times
+        expect(context).to receive(:notice).with(/Attempt [1-5] of 5 failed/).exactly(5).times
         expect(context).to receive(:err).with(/The Invoke-DscResource cmdlet is in progress and must return before Invoke-DscResource can be invoked/)
         allow(provider).to receive(:sleep)
         expect(result).to be_nil
@@ -829,6 +829,7 @@
         expect(context).to receive(:notice).with(/Retrying: attempt 1 of 5/).once
         allow(provider).to receive(:sleep)
         expect(ps_manager).to receive(:execute).and_return({ stdout: '{"errormessage": "Some unexpected error"}' }).once
+        expect(context).to receive(:notice).with(/Attempt 1 of 5 failed/).once
         expect(context).to receive(:err).with(/Some unexpected error/)
         expect(result).to be_nil
       end