Merge pull request #649 from DavidS/modules-3737

(MODULES-3737) refactor validate_legacy and tests

5 files changed, 80 insertions, 91 deletions

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/assert_private_spec.rb b/spec/functions/assert_private_spec.rb
index 98f2598..355e0dd 100755
--- a/spec/functions/assert_private_spec.rb
+++ b/spec/functions/assert_private_spec.rb
@@ -5,31 +5,26 @@ describe 'assert_private' do
     it "should not fail" do
       scope.expects(:lookupvar).with('module_name').returns('foo')
       scope.expects(:lookupvar).with('caller_module_name').returns('foo')
-      expect {
-        subject.call []
-      }.not_to raise_error
+
+      is_expected.to run.with_params()
     end
   end
 
-  context "with an explicit failure message" do
-    it "prints the failure message on error" do
+  context "when called from private class" do
+    before :each do
       scope.expects(:lookupvar).with('module_name').returns('foo')
       scope.expects(:lookupvar).with('caller_module_name').returns('bar')
-      expect {
-        subject.call ['failure message!']
-      }.to raise_error Puppet::ParseError, /failure message!/
     end
-  end
 
-  context "when called from private class" do
     it "should fail with a class error message" do
-      scope.expects(:lookupvar).with('module_name').returns('foo')
-      scope.expects(:lookupvar).with('caller_module_name').returns('bar')
       scope.source.expects(:name).returns('foo::baz')
       scope.source.expects(:type).returns('hostclass')
-      expect {
-        subject.call []
-      }.to raise_error Puppet::ParseError, /Class foo::baz is private/
+
+      is_expected.to run.with_params().and_raise_error(Puppet::ParseError, /Class foo::baz is private/)
+    end
+
+    context "with an explicit failure message" do
+      it { is_expected.to run.with_params('failure message!').and_raise_error(Puppet::ParseError, /failure message!/) }
     end
   end
 
@@ -39,9 +34,8 @@ describe 'assert_private' do
       scope.expects(:lookupvar).with('caller_module_name').returns('bar')
       scope.source.expects(:name).returns('foo::baz')
       scope.source.expects(:type).returns('definition')
-      expect {
-        subject.call []
-      }.to raise_error Puppet::ParseError, /Definition foo::baz is private/
+
+      is_expected.to run.with_params().and_raise_error(Puppet::ParseError, /Definition foo::baz is private/)
     end
   end
 end
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/) }