Renamed some variables to be clearer, made tests use less stubbing, added some additional tests and got rid of some unecessary logic.
Paired-with: Dan Bode Signed-off-by: Matt Robinson <[email protected]> --- lib/puppet/parser/compiler.rb | 2 +- lib/puppet/resource/type.rb | 12 +++--- spec/unit/parser/compiler_spec.rb | 84 +++++++++++++++++++++++++------------ spec/unit/resource/type_spec.rb | 11 ++--- 4 files changed, 69 insertions(+), 40 deletions(-) diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb index 3134e03..fdabd05 100644 --- a/lib/puppet/parser/compiler.rb +++ b/lib/puppet/parser/compiler.rb @@ -153,7 +153,7 @@ class Puppet::Parser::Compiler if param_classes resource = klass.ensure_in_catalog(scope, param_classes[name] || {}) else - found << name and next if scope.class_scope(klass) and !param_classes + found << name and next if scope.class_scope(klass) resource = klass.ensure_in_catalog(scope) end diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb index a31795a..92e1ef7 100644 --- a/lib/puppet/resource/type.rb +++ b/lib/puppet/resource/type.rb @@ -143,23 +143,23 @@ class Puppet::Resource::Type # classes and nodes. No parameters are be supplied--if this is a # parameterized class, then all parameters take on their default # values. - def ensure_in_catalog(scope, attributes=nil) + def ensure_in_catalog(scope, parameters=nil) type == :definition and raise ArgumentError, "Cannot create resources for defined resource types" resource_type = type == :hostclass ? :class : :node # Do nothing if the resource already exists; this makes sure we don't # get multiple copies of the class resource, which helps provide the # singleton nature of classes. - # we should not do this for classes with attributes - # if attributes are passed, we should still try to create the resource + # we should not do this for classes with parameters + # if parameters are passed, we should still try to create the resource # even if it exists so that we can fail # this prevents us from being able to combine param classes with include - if resource = scope.catalog.resource(resource_type, name) and !attributes + if resource = scope.catalog.resource(resource_type, name) and !parameters return resource end resource = Puppet::Parser::Resource.new(resource_type, name, :scope => scope, :source => self) - if attributes - attributes.each do |k,v| + if parameters + parameters.each do |k,v| resource.set_parameter(k,v) end end diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb index c1e7c48..687f2ec 100755 --- a/spec/unit/parser/compiler_spec.rb +++ b/spec/unit/parser/compiler_spec.rb @@ -105,8 +105,7 @@ describe Puppet::Parser::Compiler do node.classes = %w{foo bar} compiler = Puppet::Parser::Compiler.new(node) - compiler.classlist.should include("foo") - compiler.classlist.should include("bar") + compiler.classlist.should =~ ['foo', 'bar'] end it "should transform node class hashes into a class list" do @@ -114,8 +113,7 @@ describe Puppet::Parser::Compiler do node.classes = {'foo'=>{'one'=>'1'}, 'bar'=>{'two'=>'2'}} compiler = Puppet::Parser::Compiler.new(node) - compiler.classlist.should include("foo") - compiler.classlist.should include("bar") + compiler.classlist.should =~ ['foo', 'bar'] end it "should add a 'main' stage to the catalog" do @@ -549,7 +547,7 @@ describe Puppet::Parser::Compiler do @compiler.add_collection(coll) - lambda { @compiler.compile }.should raise_error(Puppet::ParseError) + lambda { @compiler.compile }.should raise_error Puppet::ParseError, 'Failed to realize virtual resources something' end it "should fail when there are unevaluated resource collections that refer to multiple specific resources" do @@ -558,7 +556,7 @@ describe Puppet::Parser::Compiler do @compiler.add_collection(coll) - lambda { @compiler.compile }.should raise_error(Puppet::ParseError) + lambda { @compiler.compile }.should raise_error Puppet::ParseError, 'Failed to realize virtual resources one, two' end end @@ -611,28 +609,68 @@ describe Puppet::Parser::Compiler do @compiler.evaluate_classes(%w{myclass}, @scope) end - # test that ensure_in_catalog is called for existing classes - it "should ensure each node class hash is in catalog" do - klasses = {'foo'=>{'1'=>'one'}, 'bar'=>{'2'=>'two'}} + it "should ensure each node class hash is in catalog and have appropriate parameters" 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) - @compiler.topscope.stubs(:find_hostclass).with(name).returns(klass) - klass.expects(:ensure_in_catalog).with(@compiler.topscope, params) + klass = Puppet::Resource::Type.new(:hostclass, name, :arguments => {'1' => ast_obj, '2' => ast_obj}) + @compiler.topscope.known_resource_types.add klass end - @compiler.compile + 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'] + + 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'] + + 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 ensure each node class is in catalog and has appropriate tags" 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 + + it "should fail if required parameters are missing" do + klass = {'foo'=>{'1'=>'one'}} + @node.classes = klass + klass = Puppet::Resource::Type.new(:hostclass, 'foo', :arguments => {'1' => nil, '2' => nil}) + @compiler.topscope.known_resource_types.add klass + lambda { @compiler.compile }.should raise_error Puppet::ParseError, "Must pass 2 to Class[Foo]" + end + + it "should fail if invalid parameters are passed" do + klass = {'foo'=>{'3'=>'one'}} + @node.classes = klass + klass = Puppet::Resource::Type.new(:hostclass, 'foo', :arguments => {'1' => nil, '2' => nil}) + @compiler.topscope.known_resource_types.add klass + lambda { @compiler.compile }.should raise_error Puppet::ParseError, "Invalid parameter 3" end it "should ensure class is in catalog without params" do @node.classes = klasses = {'foo'=>nil} foo = Puppet::Resource::Type.new(:hostclass, 'foo') - @compiler.topscope.stubs(:find_hostclass).with('foo').returns(foo) - foo.expects(:ensure_in_catalog).with(@compiler.topscope, {}) - @compiler.compile + @compiler.topscope.known_resource_types.add foo + catalog = @compiler.compile + catalog.classes.should include 'foo' end - - it "should not evaluate the resources created for found classes unless asked" do @compiler.catalog.stubs(:tag) @@ -667,14 +705,6 @@ describe Puppet::Parser::Compiler do @compiler.evaluate_classes(%w{myclass}, @scope, false) end - it "should not skip param classes that have already been evaluated" do - @scope.stubs(:class_scope).with(@class).returns("something") - @node.classes = klasses = {'foo'=>nil} - foo = Puppet::Resource::Type.new(:hostclass, 'foo') - @compiler.topscope.stubs(:find_hostclass).with('foo').returns(foo) - foo.expects(:ensure_in_catalog).with(@compiler.topscope, {}) - @compiler.compile - end it "should skip classes previously evaluated with different capitalization" do @compiler.catalog.stubs(:tag) @scope.stubs(:find_hostclass).with("MyClass").returns(@class) @@ -814,7 +844,7 @@ describe Puppet::Parser::Compiler do it "should fail if the compile is finished and resource overrides have not been applied" do @compiler.add_override(@override) - lambda { @compiler.compile }.should raise_error(Puppet::ParseError) + lambda { @compiler.compile }.should raise_error Puppet::ParseError, 'Could not find resource(s) File[/foo] for overriding' end end end diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb index 11288f1..206616e 100755 --- a/spec/unit/resource/type_spec.rb +++ b/spec/unit/resource/type_spec.rb @@ -496,7 +496,7 @@ describe Puppet::Resource::Type do it "should evaluate the parent's resource" do @type.parent_type(@scope) - + @type.evaluate_code(@resource) @scope.class_scope(@parent_type).should_not be_nil @@ -504,7 +504,7 @@ describe Puppet::Resource::Type do it "should not evaluate the parent's resource if it has already been evaluated" do @parent_resource.evaluate - + @type.parent_type(@scope) @parent_resource.expects(:evaluate).never @@ -545,7 +545,7 @@ describe Puppet::Resource::Type do it "should not evaluate the parent's resource if it has already been evaluated" do @parent_resource.evaluate - + @type.parent_type(@scope) @parent_resource.expects(:evaluate).never @@ -575,7 +575,7 @@ describe Puppet::Resource::Type do @code = Puppet::Resource::TypeCollection.new("env") @code.add @top @code.add @middle - + @node.environment.stubs(:known_resource_types).returns(@code) end @@ -601,7 +601,7 @@ describe Puppet::Resource::Type do @compiler.catalog.resource(:class, "top").should be_instance_of(Puppet::Parser::Resource) end - it "should add specified attributes to the resource" do + it "should add specified parameters to the resource" do @top.ensure_in_catalog(@scope, {'one'=>'1', 'two'=>'2'}) @compiler.catalog.resource(:class, "top")['one'].should == '1' @compiler.catalog.resource(:class, "top")['two'].should == '2' @@ -628,7 +628,6 @@ describe Puppet::Resource::Type do othertop = Puppet::Parser::Resource.new(:class, 'top',:source => @source, :scope => @scope ) # add the same class resource to the catalog @compiler.catalog.add_resource(othertop) - @compiler.catalog.expects(:resource).with(:class, 'top').returns true lambda { @top.ensure_in_catalog(@scope, {}) }.should raise_error(Puppet::Resource::Catalog::DuplicateResourceError) end -- 1.7.3.1 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
