Fixing default parameter value assignment

The method for adding class resources to the
catalog was only working in cases where the default
values weren't AST objects.  This commit fixes
this, along with the tests that were failing
and drew out the problem.

Signed-off-by: Luke Kanies <luke@puppetlabs.com>
Reviewed-by: Nick Lewis <nick@puppetlabs.com>

2 files changed, 54 insertions, 30 deletions

diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
index ca6e8b53b..8b154ce95 100644
--- a/lib/puppet/resource/type.rb
+++ b/lib/puppet/resource/type.rb
@@ -187,8 +187,12 @@ class Puppet::Resource::Type
   def assign_parameter_values(parameters, resource)
     return unless parameters
     scope = resource.scope || {}
-    arguments.merge(parameters).each do |name, default|
-      resource.set_parameter name, default
+
+    # It'd be nice to assign default parameter values here,
+    # but we can't because they often rely on local variables
+    # created during set_resource_parameters.
+    parameters.each do |name, value|
+      resource.set_parameter name, value
     end
   end
 
diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb
index 06f8044e3..e6f481114 100755
--- a/spec/unit/parser/compiler_spec.rb
+++ b/spec/unit/parser/compiler_spec.rb
@@ -571,42 +571,62 @@ describe Puppet::Parser::Compiler do
       @compiler.evaluate_classes(%w{myclass}, @scope)
     end
 
-    it "should ensure each node class hash is in catalog and have appropriate parameters", :'fails_on_ruby_1.9.2' => true do
-      klasses = {'foo'=>{'1'=>'one'}, 'bar::foo'=>{'2'=>'two'}, 'bar'=>{'1'=> [1,2,3], '2'=>{'foo'=>'bar'}}}
-      @node.classes = klasses
-      ast_obj = Puppet::Parser::AST::String.new(:value => 'foo')
-      klasses.each do |name, params|
-        klass = Puppet::Resource::Type.new(:hostclass, name, :arguments => {'1' => ast_obj, '2' => ast_obj})
+    describe "and the classes are specified as a hash with parameters" do
+      before do
+        @node.classes = {}
+        @ast_obj = Puppet::Parser::AST::String.new(:value => 'foo')
+      end
+
+      # Define the given class with default parameters
+      def define_class(name, parameters)
+        @node.classes[name] = parameters
+        klass = Puppet::Resource::Type.new(:hostclass, name, :arguments => {'1' => @ast_obj, '2' => @ast_obj})
         @compiler.topscope.known_resource_types.add klass
       end
-      catalog = @compiler.compile
-      catalog.classes.should =~ ['foo', 'bar::foo', 'settings', 'bar']
 
-      r1 = catalog.resources.detect {|r| r.title == 'Foo' }
-      r1.to_hash.should == {:'1' => 'one', :'2' => 'foo'}
-      r1.tags. should =~ ['class', 'foo']
+      def compile
+        @catalog = @compiler.compile
+      end
 
-      r2 = catalog.resources.detect {|r| r.title == 'Bar::Foo' }
-      r2.to_hash.should == {:'1' => 'foo', :'2' => 'two'}
-      r2.tags.should =~ ['bar::foo', 'class', 'bar', 'foo']
+      it "should record which classes are evaluated" do
+        classes = {'foo'=>{}, 'bar::foo'=>{}, 'bar'=>{}}
+        classes.each { |c, params| define_class(c, params) }
+        compile()
+        classes.each { |name, p| @catalog.classes.should include(name) }
+      end
 
-      r2 = catalog.resources.detect {|r| r.title == 'Bar' }
-      r2.to_hash.should == {:'1' => [1,2,3], :'2' => {'foo'=>'bar'}}
-      r2.tags.should =~ ['class', 'bar']
-    end
+      it "should provide default values for parameters that have no values specified" do
+        define_class('foo', {})
+        compile()
+        @catalog.resource(:class, 'foo')['1'].should == "foo"
+      end
 
-    it "should ensure each node class is in catalog and has appropriate tags", :'fails_on_ruby_1.9.2' => true do
-      klasses = ['bar::foo']
-      @node.classes = klasses
-      ast_obj = Puppet::Parser::AST::String.new(:value => 'foo')
-      klasses.each do |name|
-        klass = Puppet::Resource::Type.new(:hostclass, name, :arguments => {'1' => ast_obj, '2' => ast_obj})
-        @compiler.topscope.known_resource_types.add klass
+      it "should use any provided values" do
+        define_class('foo', {'1' => 'real_value'})
+        compile()
+        @catalog.resource(:class, 'foo')['1'].should == "real_value"
       end
-      catalog = @compiler.compile
 
-      r2 = catalog.resources.detect {|r| r.title == 'Bar::Foo' }
-      r2.tags.should =~ ['bar::foo', 'class', 'bar', 'foo']
+      it "should support providing some but not all values" do
+        define_class('foo', {'1' => 'real_value'})
+        compile()
+        @catalog.resource(:class, 'Foo')['1'].should == "real_value"
+        @catalog.resource(:class, 'Foo')['2'].should == "foo"
+      end
+
+      it "should ensure each node class is in catalog and has appropriate tags", :'fails_on_ruby_1.9.2' => true do
+        klasses = ['bar::foo']
+        @node.classes = klasses
+        ast_obj = Puppet::Parser::AST::String.new(:value => 'foo')
+        klasses.each do |name|
+          klass = Puppet::Resource::Type.new(:hostclass, name, :arguments => {'1' => ast_obj, '2' => ast_obj})
+          @compiler.topscope.known_resource_types.add klass
+        end
+        catalog = @compiler.compile
+
+        r2 = catalog.resources.detect {|r| r.title == 'Bar::Foo' }
+        r2.tags.should =~ ['bar::foo', 'class', 'bar', 'foo']
+      end
     end
 
     it "should fail if required parameters are missing" do