Merge pull request #1378 from alexjfisher/strict_deprecation

Author: chelnak

lib/puppet/functions/deprecation.rb

@@ -1,38 +1,38 @@
 # frozen_string_literal: true
 
-#   Function to print deprecation warnings, Logs a warning once for a given key.
-#
-# The uniqueness key - can appear once.
-# The msg is the message text including any positional information that is formatted by the
-# user/caller of the method.
-# It is affected by the puppet setting 'strict', which can be set to :error
-# (outputs as an error message), :off (no message / error is displayed) and :warning
-# (default, outputs a warning)  *Type*: String, String.
-#
+# @summary Function to print deprecation warnings, Logs a warning once for a given key.
 Puppet::Functions.create_function(:deprecation) do
   # @param key
-  # @param  message
-  # @return deprecated warnings
+  #   The uniqueness key.  This function logs once for any given key.
+  # @param message
+  #   Is the message text including any positional information that is formatted by the user/caller of the function.
+  # @param use_strict_setting
+  #   When `true`, (the default), the function is affected by the puppet setting 'strict', which can be set to :error
+  #   (outputs as an error message), :off (no message / error is displayed) and :warning
+  #   (default, outputs a warning).
   dispatch :deprecation do
     param 'String', :key
     param 'String', :message
+    optional_param 'Boolean', :use_strict_setting
   end
 
-  def deprecation(key, message)
+  def deprecation(key, message, use_strict_setting = true) # rubocop:disable Style/OptionalBooleanParameter
     if defined? Puppet::Pops::PuppetStack.stacktrace
       stacktrace = Puppet::Pops::PuppetStack.stacktrace
       file = stacktrace[0]
       line = stacktrace[1]
       message = "#{message} at #{file}:#{line}"
     end
-    # depending on configuration setting of strict
-    case Puppet.settings[:strict]
-    when :off
-      # do nothing
-    when :error
-      raise("deprecation. #{key}. #{message}")
-    else
-      Puppet.deprecation_warning(message, key) unless ENV['STDLIB_LOG_DEPRECATIONS'] == 'false'
-    end
+
+    # Do nothing if using strict setting and strict is set to `off`
+    return if use_strict_setting && Puppet.settings[:strict] == :off
+
+    # Fail hard if using strict setting and strict is set to `error`
+    raise("deprecation. #{key}. #{message}") if use_strict_setting && Puppet.settings[:strict] == :error
+
+    # Otherwise raise a soft warning
+    # (unless the STDLIB_LOG_DEPRECATIONS has been set to `false`.  This is mainly for use in rspec-puppet testing to suppress noise in logs)
+    Puppet.deprecation_warning(message, key) unless ENV['STDLIB_LOG_DEPRECATIONS'] == 'false'
+    nil
   end
 end

spec/functions/deprecation_spec.rb

@@ -2,76 +2,75 @@
 
 require 'spec_helper'
 
-if Puppet::Util::Package.versioncmp(Puppet.version, '4.5.0') >= 0
-  describe 'deprecation' do
-    before(:each) do
-      # this is to reset the strict variable to default
-      Puppet.settings[:strict] = :warning
-    end
+describe 'deprecation' do
+  before(:each) do
+    # this is to reset the strict variable to default
+    Puppet.settings[:strict] = :warning
+  end
 
-    after(:each) do
-      # this is to reset the strict variable to default
-      Puppet.settings[:strict] = :warning
-    end
+  after(:each) do
+    # this is to reset the strict variable to default
+    Puppet.settings[:strict] = :warning
+  end
 
-    it { is_expected.not_to be_nil }
-    it { is_expected.to run.with_params.and_raise_error(ArgumentError) }
+  it { is_expected.not_to be_nil }
+  it { is_expected.to run.with_params.and_raise_error(ArgumentError) }
 
-    it 'displays a single warning' do
-      if Puppet::Util::Package.versioncmp(Puppet.version, '5.0.0') >= 0 && Puppet::Util::Package.versioncmp(Puppet.version, '5.5.7') < 0
-        expect(Puppet).to receive(:deprecation_warning).with('heelo at :', 'key')
-        expect(Puppet).to receive(:deprecation_warning).with("Modifying 'autosign' as a setting is deprecated.")
-      else
-        expect(Puppet).to receive(:warning).with(include('heelo')).once
-      end
-      expect(subject).to run.with_params('key', 'heelo')
+  it 'displays a single warning' do
+    if Puppet::Util::Package.versioncmp(Puppet.version, '5.0.0') >= 0 && Puppet::Util::Package.versioncmp(Puppet.version, '5.5.7') < 0
+      expect(Puppet).to receive(:deprecation_warning).with('heelo at :', 'key')
+      expect(Puppet).to receive(:deprecation_warning).with("Modifying 'autosign' as a setting is deprecated.")
+    else
+      expect(Puppet).to receive(:warning).with(include('heelo')).once
     end
+    expect(subject).to run.with_params('key', 'heelo')
+  end
 
-    it 'displays a single warning, despite multiple calls' do
-      if Puppet::Util::Package.versioncmp(Puppet.version, '5.0.0') >= 0 && Puppet::Util::Package.versioncmp(Puppet.version, '5.5.7') < 0
-        expect(Puppet).to receive(:deprecation_warning).with('heelo at :', 'key').twice
-        expect(Puppet).to receive(:deprecation_warning).with("Modifying 'autosign' as a setting is deprecated.")
-      else
-        expect(Puppet).to receive(:warning).with(include('heelo')).once
-      end
-      2.times do |_i|
-        expect(subject).to run.with_params('key', 'heelo')
-      end
+  it 'displays a single warning, despite multiple calls' do
+    if Puppet::Util::Package.versioncmp(Puppet.version, '5.0.0') >= 0 && Puppet::Util::Package.versioncmp(Puppet.version, '5.5.7') < 0
+      expect(Puppet).to receive(:deprecation_warning).with('heelo at :', 'key').twice
+      expect(Puppet).to receive(:deprecation_warning).with("Modifying 'autosign' as a setting is deprecated.")
+    else
+      expect(Puppet).to receive(:warning).with(include('heelo')).once
     end
-
-    it 'fails twice with message, with multiple calls. when strict= :error' do
-      Puppet.settings[:strict] = :error
-      expect(Puppet).not_to receive(:warning).with(include('heelo'))
-      2.times do |_i|
-        expect(subject).to run.with_params('key', 'heelo').and_raise_error(RuntimeError, %r{deprecation. key. heelo})
-      end
+    2.times do |_i|
+      expect(subject).to run.with_params('key', 'heelo')
     end
+  end
 
-    it 'displays nothing, despite multiple calls. strict= :off' do
-      Puppet.settings[:strict] = :off
-      expect(Puppet).not_to receive(:warning).with(include('heelo'))
-      2.times do |_i|
-        expect(subject).to run.with_params('key', 'heelo')
-      end
+  it 'fails twice with message, with multiple calls. when strict= :error' do
+    Puppet.settings[:strict] = :error
+    expect(Puppet).not_to receive(:warning).with(include('heelo'))
+    2.times do |_i|
+      expect(subject).to run.with_params('key', 'heelo').and_raise_error(RuntimeError, %r{deprecation. key. heelo})
     end
   end
-elsif Puppet.version.to_f < 4.0
-  # Puppet version < 4 will use these tests.
-  describe 'deprecation' do
-    after(:each) do
-      ENV.delete('STDLIB_LOG_DEPRECATIONS')
-    end
 
-    before(:each) do
-      ENV['STDLIB_LOG_DEPRECATIONS'] = 'true'
+  it 'displays nothing, despite multiple calls. strict= :off' do
+    Puppet.settings[:strict] = :off
+    expect(Puppet).not_to receive(:warning).with(include('heelo'))
+    2.times do |_i|
+      expect(subject).to run.with_params('key', 'heelo')
     end
+  end
 
-    it { is_expected.not_to be_nil }
-    it { is_expected.to run.with_params.and_raise_error(Puppet::ParseError, %r{wrong number of arguments}i) }
+  context 'with `use_strict_setting` `false`' do
+    let(:params) { ['key', 'heelo', false] }
 
-    it 'displays a single warning' do
-      expect(scope).to receive(:warning).with(include('heelo'))
-      expect(subject).to run.with_params('key', 'heelo')
+    context 'and `strict` setting set to `error`' do
+      it 'displays a warning' do
+        Puppet.settings[:strict] = :error
+        expect(Puppet).to receive(:warning).with(include('heelo')).once
+        expect(subject).to run.with_params(*params)
+      end
+    end
+
+    context 'and `strict` setting set to `off`' do
+      it 'displays a warning' do
+        Puppet.settings[:strict] = :off
+        expect(Puppet).to receive(:warning).with(include('heelo')).once
+        expect(subject).to run.with_params(*params)
+      end
     end
   end
 end