Fix backwards compatibility from #511

Maintain the old behavior in the case where the optional second
parameter isn't passed. Also, adding arity is backwards incompatible since
stdlib still supports 2.7, so remove that.

diff --git a/lib/puppet/parser/functions/parsejson.rb b/lib/puppet/parser/functions/parsejson.rb
index f822fc473744f0d34856a3bbf3614eae1b4c6012..b4af40e7e6c6a51bdf6bbb00b379a37ed9544e3f 100644 (file)
--- a/lib/puppet/parser/functions/parsejson.rb
+++ b/lib/puppet/parser/functions/parsejson.rb
@@ -3,7 +3,7 @@
 #
 
 module Puppet::Parser::Functions
-  newfunction(:parsejson, :type => :rvalue, :arity => -2, :doc => <<-EOS
+  newfunction(:parsejson, :type => :rvalue, :doc => <<-EOS
 This function accepts JSON as a string and converts it into the correct
 Puppet structure.
 
@@ -15,8 +15,12 @@ be returned if the parsing of YAML string have failed.
 
     begin
       PSON::load(arguments[0]) || arguments[1]
-    rescue Exception
-      arguments[1]
+    rescue Exception => e
+      if arguments[1]
+        arguments[1]
+      else
+        raise e
+      end
     end
 
   end

diff --git a/lib/puppet/parser/functions/parseyaml.rb b/lib/puppet/parser/functions/parseyaml.rb
index d38b3ef84c38ec3d8021847bd2bf4889336428a7..66d04131e5eed08455e86d977b8537d8000c800a 100644 (file)
--- a/lib/puppet/parser/functions/parseyaml.rb
+++ b/lib/puppet/parser/functions/parseyaml.rb
@@ -3,7 +3,7 @@
 #
 
 module Puppet::Parser::Functions
-  newfunction(:parseyaml, :type => :rvalue, :arity => -2, :doc => <<-EOS
+  newfunction(:parseyaml, :type => :rvalue, :doc => <<-EOS
 This function accepts YAML as a string and converts it into the correct
 Puppet structure.
 
@@ -16,8 +16,12 @@ be returned if the parsing of YAML string have failed.
 
     begin
       YAML::load(arguments[0]) || arguments[1]
-    rescue Exception
-      arguments[1]
+    rescue Exception => e
+      if arguments[1]
+        arguments[1]
+      else
+        raise e
+      end
     end
 
   end

diff --git a/spec/acceptance/parsejson_spec.rb b/spec/acceptance/parsejson_spec.rb
index d0feabd37baa4f9429f72f7035a6e95c20627523..d0e3de847714aa15864a9e5eedb7c60a54e5ccb7 100755 (executable)
--- a/spec/acceptance/parsejson_spec.rb
+++ b/spec/acceptance/parsejson_spec.rb
@@ -21,12 +21,24 @@ describe 'parsejson function', :unless => UNSUPPORTED_PLATFORMS.include?(fact('o
     it 'raises error on incorrect json' do
       pp = <<-EOS
       $a = '{"hunter": "washere", "tests": "passing",}'
-      $ao = parsejson($a, {'tests' => 'using the default value'})
+      $ao = parsejson($a, 'tests are using the default value')
       notice(inline_template('a is <%= @ao.inspect %>'))
       EOS
 
       apply_manifest(pp, :catch_failures => true) do |r|
-        expect(r.stdout).to match(/tests are "using the default value"/)
+        expect(r.stdout).to match(/tests are using the default value/)
+      end
+    end
+
+    it 'raises error on incorrect json' do
+      pp = <<-EOS
+      $a = '{"hunter": "washere", "tests": "passing",}'
+      $ao = parsejson($a)
+      notice(inline_template('a is <%= @ao.inspect %>'))
+      EOS
+
+      apply_manifest(pp, :expect_failures => true) do |r|
+        expect(r.stderr).to match(/expected next name/)
       end
     end
 

diff --git a/spec/acceptance/parseyaml_spec.rb b/spec/acceptance/parseyaml_spec.rb
index 7946de09d04a28557dc34a6b1712a802c1f110da..64511f13e6cf8ac7e1750fa27e35508d518ba6a5 100755 (executable)
--- a/spec/acceptance/parseyaml_spec.rb
+++ b/spec/acceptance/parseyaml_spec.rb
@@ -31,6 +31,20 @@ describe 'parseyaml function', :unless => UNSUPPORTED_PLATFORMS.include?(fact('o
       end
     end
 
+    it 'raises error on incorrect yaml' do
+      pp = <<-EOS
+      $a = "---\nhunter: washere\ntests: passing\n:"
+      $o = parseyaml($a)
+      $tests = $o['tests']
+      notice(inline_template('tests are <%= @tests.inspect %>'))
+      EOS
+
+      apply_manifest(pp, :expect_failures => true) do |r|
+        expect(r.stderr).to match(/(syntax error|did not find expected key)/)
+      end
+    end
+
+
     it 'raises error on incorrect number of arguments' do
       pp = <<-EOS
       $o = parseyaml()

diff --git a/spec/functions/parsejson_spec.rb b/spec/functions/parsejson_spec.rb
index 5bea8af7fbcb2cdb809bad3318a6354049785638..a01f1f67b9bda4e0e01e045789062d41b6280841 100755 (executable)
--- a/spec/functions/parsejson_spec.rb
+++ b/spec/functions/parsejson_spec.rb
@@ -41,10 +41,10 @@ describe 'parsejson' do
 
   end
 
-  context 'with incorrect YAML data' do
-    it 'should return "nil" if a default value should be returned but is not provided' do
+  context 'with incorrect JSON data' do
+    it 'should raise an error with invalid JSON and no default' do
       is_expected.to run.with_params('').
-                         and_return(nil)
+                         and_raise_error(PSON::ParserError)
     end
 
     it 'should support a structure for a default value' do

diff --git a/spec/functions/parseyaml_spec.rb b/spec/functions/parseyaml_spec.rb
index 492a1c92119779fd34180fc666f1179a376840a5..fa947ca6a437306a60977ba18dacb60f55118723 100755 (executable)
--- a/spec/functions/parseyaml_spec.rb
+++ b/spec/functions/parseyaml_spec.rb
@@ -40,12 +40,21 @@ describe 'parseyaml' do
 
   end
 
-  context 'with incorrect YAML data' do
-    it 'should return "nil" if a default value should be returned but is not provided' do
-      is_expected.to run.with_params('').
-                         and_return(nil)
+  context 'on a modern ruby', :unless => RUBY_VERSION == '1.8.7' do
+    it 'should raise an error with invalid YAML and no default' do
+      is_expected.to run.with_params('["one"').
+                         and_raise_error(Psych::SyntaxError)
+    end
+  end
+
+    context 'when running on ruby 1.8.7, which does not have Psych', :if => RUBY_VERSION == '1.8.7' do
+      it 'should raise an error with invalid YAML and no default' do
+        is_expected.to run.with_params('["one"').
+          and_raise_error(ArgumentError)
+      end
     end
 
+  context 'with incorrect YAML data' do
     it 'should support a structure for a default value' do
       is_expected.to run.with_params('', {'a' => '1'}).
                          and_return({'a' => '1'})