From cd03959116f7b27ab3ec7ccace98bd10be435397 Mon Sep 17 00:00:00 2001 From: Bryan Jen Date: Thu, 16 Apr 2015 15:49:03 -0700 Subject: [PATCH] re-add removed params for backwards compatibility --- manifests/fragment.pp | 6 +++ manifests/init.pp | 15 ++++-- spec/acceptance/deprecation_warnings_spec.rb | 44 +++++++++++++++ spec/acceptance/warn_header_spec.rb | 8 +-- spec/unit/defines/concat_spec.rb | 57 +++++++------------- 5 files changed, 84 insertions(+), 46 deletions(-) create mode 100644 spec/acceptance/deprecation_warnings_spec.rb diff --git a/manifests/fragment.pp b/manifests/fragment.pp index 5a3e31043..fabc3596d 100644 --- a/manifests/fragment.pp +++ b/manifests/fragment.pp @@ -16,11 +16,17 @@ # define concat::fragment( $target, + $ensure = undef, $content = undef, $source = undef, $order = '10', ) { validate_string($target) + + if $ensure != undef { + warning('The $ensure parameter to concat::fragment is deprecated and has no effect.') + } + validate_string($content) if !(is_string($source) or is_array($source)) { fail('$source is not a string or an Array.') diff --git a/manifests/init.pp b/manifests/init.pp index 9c3c21d8e..981a5c271 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -46,7 +46,8 @@ $owner = undef, $group = undef, $mode = '0644', - $warn_header = false, + $warn = false, + $force = undef, $backup = 'puppet', $replace = true, $order = 'alpha', @@ -57,8 +58,8 @@ validate_string($owner) validate_string($group) validate_string($mode) - if ! (is_string($warn_header) or $warn_header == true or $warn_header == false) { - fail('$warn_header is not a string or boolean') + if ! (is_string($warn) or $warn == true or $warn == false) { + fail('$warn is not a string or boolean') } if ! is_bool($backup) and ! is_string($backup) { fail('$backup must be string or bool!') @@ -69,10 +70,14 @@ fail('$validate_cmd must be a string') } + if $force != undef { + warning('The $force parameter to concat is deprecated and has no effect.') + } + $safe_name = regsubst($name, '[/:\n\s]', '_', 'G') $default_warn_message = "# This file is managed by Puppet. DO NOT EDIT.\n" - case $warn_header { + case $warn { true: { $warn_message = $default_warn_message $append_header = true @@ -81,7 +86,7 @@ $warn_message = '' } default: { - $warn_message = $warn_header + $warn_message = $warn $append_header = true } } diff --git a/spec/acceptance/deprecation_warnings_spec.rb b/spec/acceptance/deprecation_warnings_spec.rb new file mode 100644 index 000000000..8a689a76b --- /dev/null +++ b/spec/acceptance/deprecation_warnings_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper_acceptance' + +describe 'deprecation warnings' do + basedir = default.tmpdir('concat') + + shared_examples 'has_warning' do |pp, w| + it 'applies the manifest twice with a stderr regex' do + expect(apply_manifest(pp, :catch_failures => true).stderr).to match(/#{Regexp.escape(w)}/m) + expect(apply_manifest(pp, :catch_changes => true).stderr).to match(/#{Regexp.escape(w)}/m) + end + end + + context 'concat force parameter' do + pp = <<-EOS + concat { '#{basedir}/file': + force => false, + } + concat::fragment { 'foo': + target => '#{basedir}/file', + content => 'bar', + } + EOS + w = 'The $force parameter to concat is deprecated and has no effect.' + + it_behaves_like 'has_warning', pp, w + end + + context 'concat::fragment ensure parameter' do + context 'target file exists' do + pp = <<-EOS + concat { '#{basedir}/file': + } + concat::fragment { 'foo': + target => '#{basedir}/file', + ensure => false, + content => 'bar', + } + EOS + w = 'The $ensure parameter to concat::fragment is deprecated and has no effect.' + + it_behaves_like 'has_warning', pp, w + end + end +end diff --git a/spec/acceptance/warn_header_spec.rb b/spec/acceptance/warn_header_spec.rb index 52615e1d7..b73414e34 100644 --- a/spec/acceptance/warn_header_spec.rb +++ b/spec/acceptance/warn_header_spec.rb @@ -1,11 +1,11 @@ require 'spec_helper_acceptance' -describe 'concat warn_header =>' do +describe 'concat warn =>' do basedir = default.tmpdir('concat') context 'true should enable default warning message' do pp = <<-EOS concat { '#{basedir}/file': - warn_header => true, + warn => true, } concat::fragment { '1': @@ -38,7 +38,7 @@ context 'false should not enable default warning message' do pp = <<-EOS concat { '#{basedir}/file': - warn_header => false, + warn => false, } concat::fragment { '1': @@ -71,7 +71,7 @@ context '# foo should overide default warning message' do pp = <<-EOS concat { '#{basedir}/file': - warn_header => "# foo\n", + warn => "# foo\n", } concat::fragment { '1': diff --git a/spec/unit/defines/concat_spec.rb b/spec/unit/defines/concat_spec.rb index 176f24a37..35767aea6 100644 --- a/spec/unit/defines/concat_spec.rb +++ b/spec/unit/defines/concat_spec.rb @@ -13,8 +13,7 @@ :owner => nil, :group => nil, :mode => '0644', - :warn_header => false, - #:force => false, + :warn => false, :backup => 'puppet', :replace => true, }.merge(params) @@ -168,17 +167,17 @@ end end # mode => - context 'warn_header =>' do + context 'warn =>' do [true, false, '# foo'].each do |warn| context warn do - it_behaves_like 'concat', '/etc/foo.bar', { :warn_header => warn } + it_behaves_like 'concat', '/etc/foo.bar', { :warn => warn } end end context '(stringified boolean)' do ['true', 'yes', 'on', 'false', 'no', 'off'].each do |warn| context warn do - it_behaves_like 'concat', '/etc/foo.bar', { :warn_header => warn } + it_behaves_like 'concat', '/etc/foo.bar', { :warn => warn } it 'should create a warning' do skip('rspec-puppet support for testing warning()') @@ -189,29 +188,13 @@ context '123' do let(:title) { '/etc/foo.bar' } - let(:params) {{ :warn_header => 123 }} + let(:params) {{ :warn => 123 }} it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /is not a string or boolean/) end end end # warn => - #context 'force =>' do - # [true, false].each do |force| - # context force do - # it_behaves_like 'concat', '/etc/foo.bar', { :force => force } - # end - # end - - # context '123' do - # let(:title) { '/etc/foo.bar' } - # let(:params) {{ :force => 123 }} - # it 'should fail' do - # expect { catalogue }.to raise_error(Puppet::Error, /is not a boolean/) - # end - # end - #end # force => - context 'backup =>' do context 'reverse' do it_behaves_like 'concat', '/etc/foo.bar', { :backup => 'reverse' } @@ -250,19 +233,19 @@ end end # replace => - #context 'order =>' do - # ['alpha', 'numeric'].each do |order| - # context order do - # it_behaves_like 'concat', '/etc/foo.bar', { :order => order } - # end - # end - - # context 'invalid' do - # let(:title) { '/etc/foo.bar' } - # let(:params) {{ :order => 'invalid' }} - # it 'should fail' do - # expect { catalogue }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^alpha$|^numeric$"')}/) - # end - # end - #end # order => + context 'order =>' do + ['alpha', 'numeric'].each do |order| + context order do + it_behaves_like 'concat', '/etc/foo.bar', { :order => order } + end + end + + context 'invalid' do + let(:title) { '/etc/foo.bar' } + let(:params) {{ :order => 'invalid' }} + it 'should fail' do + expect { catalogue }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^alpha$|^numeric$"')}/) + end + end + end # order => end