From f1edd2715a755573d7578839a3efe8473b79b5c5 Mon Sep 17 00:00:00 2001 From: David Schmitt Date: Tue, 6 Sep 2016 11:18:48 +0100 Subject: (MODULES-3737) validate_legacy: refactoring * validate_legacy now accepts undef values * update the TypeMismatch message to include the original validate function name * only notice, not warn, on newly allowed values * changed previous_validation to function_name to avoid confusion with the function of the same name * use deprecation() instead of warn(), when hitting a deprecated value * prepare the tests and function for MODULES-3735 * rewrite validate_legacy tests to use new rspec-puppet * move validate_re deprecation to puppet4 only * adapt validate_re_spec --- Gemfile | 2 +- lib/puppet/functions/validate_legacy.rb | 31 +++++------ spec/classes/validate_legacy_spec.rb | 39 -------------- spec/fixtures/test/manifests/validate_legacy.pp | 18 ------- spec/functions/validate_legacy_spec.rb | 68 +++++++++++++++++++++++++ spec/functions/validate_re_spec.rb | 16 ------ 6 files changed, 85 insertions(+), 89 deletions(-) delete mode 100644 spec/classes/validate_legacy_spec.rb delete mode 100644 spec/fixtures/test/manifests/validate_legacy.pp create mode 100644 spec/functions/validate_legacy_spec.rb diff --git a/Gemfile b/Gemfile index c97275b..ecf3d06 100644 --- a/Gemfile +++ b/Gemfile @@ -21,7 +21,7 @@ group :development, :unit_tests do gem 'puppet_facts' gem 'puppet-blacksmith', '>= 3.4.0' gem 'puppetlabs_spec_helper', '>= 1.2.1' - gem 'rspec-puppet', '>= 2.3.2' + gem 'rspec-puppet', '>= 2.3.2', :git => 'https://github.com/rodjek/rspec-puppet.git', :branch => 'fb27c533e2664057fba4b73d0bd902a946abfce0' gem 'rspec-puppet-facts' gem 'simplecov' gem 'parallel_tests' diff --git a/lib/puppet/functions/validate_legacy.rb b/lib/puppet/functions/validate_legacy.rb index b1066e2..0ba6dd8 100644 --- a/lib/puppet/functions/validate_legacy.rb +++ b/lib/puppet/functions/validate_legacy.rb @@ -2,48 +2,49 @@ Puppet::Functions.create_function(:validate_legacy, Puppet::Functions::InternalF # The function checks a value against both the target_type (new) and the previous_validation function (old). dispatch :validate_legacy do + scope_param param 'Type', :target_type - param 'String', :previous_validation - param 'NotUndef', :value - optional_param 'Any', :args + param 'String', :function_name + param 'Any', :value + optional_repeated_param 'Any', :args end + dispatch :validate_legacy_s do scope_param param 'String', :type_string - param 'String', :previous_validation - param 'NotUndef', :value + param 'String', :function_name + param 'Any', :value optional_repeated_param 'Any', :args end def validate_legacy_s(scope, type_string, *args) t = Puppet::Pops::Types::TypeParser.new.parse(type_string, scope) - validate_legacy(t, *args) + validate_legacy(scope, t, *args) end - def validate_legacy(target_type, previous_validation, value, *prev_args) + def validate_legacy(scope, target_type, function_name, value, *prev_args) if assert_type(target_type, value) - if previous_validation(previous_validation, value, *prev_args) + if previous_validation(scope, function_name, value, *prev_args) # Silently passes else - Puppet.warn("Accepting previously invalid value for target_type '#{target_type}'") + Puppet.notice("Accepting previously invalid value for target type '#{target_type}'") end else caller_infos = caller.first.split(":") inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value) - message = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch(previous_validation, target_type, inferred_type) - error_msg = "#{message} : #{caller_infos[0]} : #{caller_infos[1]}" - if previous_validation(previous_validation, value, *prev_args) - Puppet.warn(error_msg) + error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("validate_legacy(#{function_name}) [#{caller_infos[0]}:#{caller_infos[1]}]", target_type, inferred_type) + if previous_validation(scope, function_name, value, *prev_args) + call_function('deprecation', 'validate_legacy', error_msg) else call_function('fail', error_msg) end end end - def previous_validation(previous_validation, value, *prev_args) + def previous_validation(scope, function_name, value, *prev_args) # Call the previous validation function and catch any errors. Return true if no errors are thrown. begin - call_function(previous_validation, value, *prev_args) + scope.send("function_#{function_name}".to_s, [value, *prev_args]) true rescue Puppet::ParseError false diff --git a/spec/classes/validate_legacy_spec.rb b/spec/classes/validate_legacy_spec.rb deleted file mode 100644 index ded6890..0000000 --- a/spec/classes/validate_legacy_spec.rb +++ /dev/null @@ -1,39 +0,0 @@ -require 'spec_helper' - -if Puppet.version.to_f >= 4.0 - # validate_legacy requires a proper scope to run, so we have to trigger a true compilation here, - # instead of being able to leverage the function test group. - describe 'test::validate_legacy', type: :class do - - describe 'validate_legacy passes assertion of type but not previous validation' do - let(:params) {{ type: "Optional[Integer]", prev_validation: "validate_re", value: 5, previous_arg1: ["^\\d+$", ""] }} - it { - Puppet.expects(:warn).with(includes('Accepting previously invalid value for target_type')) - is_expected.to compile - } - end - - describe 'validate_legacy passes assertion of type and previous validation' do - let(:params) {{ type: "Optional[String]", prev_validation: "validate_re", value: "5", previous_arg1: ["."] }} - it { is_expected.to compile } - end - - describe 'validate_legacy fails assertion of type and passes previous validation' do - let(:params) {{ type: "Optional[Integer]", prev_validation: "validate_re", value: "5", previous_arg1: ["."] }} - it { - Puppet.expects(:warn).with(includes('expected')) - is_expected.to compile - } - end - - describe 'validate_legacy fails assertion and fails previous validation' do - let(:params) {{ type: "Optional[Integer]", prev_validation: "validate_re", value: "5", previous_arg1: ["thisisnotright"] }} - it { is_expected.to compile.and_raise_error(/Error while evaluating a Function Call, \w* expected an \w* value, got \w*/) } - end - - describe 'validate_legacy works with multi-argument validate_ functions' do - let(:params) {{ type: "Integer", prev_validation: "validate_integer", value: 10, previous_arg1: 100, previous_arg2: 0 }} - it { is_expected.to compile } - end - end -end diff --git a/spec/fixtures/test/manifests/validate_legacy.pp b/spec/fixtures/test/manifests/validate_legacy.pp deleted file mode 100644 index 706df88..0000000 --- a/spec/fixtures/test/manifests/validate_legacy.pp +++ /dev/null @@ -1,18 +0,0 @@ -# Class to test stdlib validate_legacy function - -class test::validate_legacy( - $type, - $prev_validation, - $value, - $previous_arg1, - $previous_arg2 = undef, - ) { - - if $previous_arg2 == undef { - validate_legacy( $type, $prev_validation, $value, $previous_arg1 ) - } else { - validate_legacy( $type, $prev_validation, $value, $previous_arg1, $previous_arg2 ) - } - notice("Success") - -} diff --git a/spec/functions/validate_legacy_spec.rb b/spec/functions/validate_legacy_spec.rb new file mode 100644 index 0000000..45089ef --- /dev/null +++ b/spec/functions/validate_legacy_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +if Puppet.version.to_f >= 4.0 + describe 'validate_legacy' do + it { is_expected.not_to eq(nil) } + it { is_expected.to run.with_params.and_raise_error(ArgumentError) } + + describe 'when passing the type assertion and passing the previous validation' do + before do + scope.expects(:function_validate_foo).with([5]).once + Puppet.expects(:notice).never + end + it 'passes without notice' do + is_expected.to run.with_params('Integer', 'validate_foo', 5) + end + end + + describe 'when passing the type assertion and failing the previous validation' do + before do + scope.expects(:function_validate_foo).with([5]).raises(Puppet::ParseError, 'foo').once + Puppet.expects(:notice).with(includes('Accepting previously invalid value for target type')) + end + it 'passes with a notice about newly accepted value' do + is_expected.to run.with_params('Integer', 'validate_foo', 5) + end + end + + describe 'when failing the type assertion and passing the previous validation' do + before do + scope.expects(:function_validate_foo).with(['5']).once + subject.func.expects(:call_function).with('deprecation', 'validate_legacy', includes('expected an Integer value')).once + end + it 'passes with a deprecation message' do + is_expected.to run.with_params('Integer', 'validate_foo', '5') + end + end + + describe 'when failing the type assertion and failing the previous validation' do + before do + scope.expects(:function_validate_foo).with(['5']).raises(Puppet::ParseError, 'foo').once + subject.func.expects(:call_function).with('fail', includes('expected an Integer value')).once + end + it 'fails with a helpful message' do + is_expected.to run.with_params('Integer', 'validate_foo', '5') + end + end + + describe 'when passing in undef' do + before do + scope.expects(:function_validate_foo).with([:undef]).once + Puppet.expects(:notice).never + end + it 'works' do + is_expected.to run.with_params('Optional[Integer]', 'validate_foo', :undef) + end + end + + describe 'when passing in multiple arguments' do + before do + scope.expects(:function_validate_foo).with([:undef, 1, 'foo']).once + Puppet.expects(:notice).never + end + it 'passes with a deprecation message' do + is_expected.to run.with_params('Optional[Integer]', 'validate_foo', :undef, 1, 'foo') + end + end + end +end diff --git a/spec/functions/validate_re_spec.rb b/spec/functions/validate_re_spec.rb index 2ee3f47..2fa9ddd 100755 --- a/spec/functions/validate_re_spec.rb +++ b/spec/functions/validate_re_spec.rb @@ -28,22 +28,6 @@ describe 'validate_re' do end describe 'invalid inputs' do - it { - pending('should implement stricter type checking') - is_expected.to run.with_params([], '').and_raise_error(Puppet::ParseError, /is not a String/) - } - it { - pending('should implement stricter type checking') - is_expected.to run.with_params('', {}).and_raise_error(Puppet::ParseError, /is not an Array/) - } - it { - pending('should implement stricter type checking') - is_expected.to run.with_params('', '', []).and_raise_error(Puppet::ParseError, /is not a String/) - } - it { - pending('should implement stricter type checking') - is_expected.to run.with_params(nil, nil).and_raise_error(Puppet::ParseError, /is not a String/) - } it { is_expected.to run.with_params('', []).and_raise_error(Puppet::ParseError, /does not match/) } it { is_expected.to run.with_params('one', 'two').and_raise_error(Puppet::ParseError, /does not match/) } it { is_expected.to run.with_params('', 'two').and_raise_error(Puppet::ParseError, /does not match/) } -- cgit v1.2.3