On Tue, 2010-02-09 at 16:56 -0800, Luke Kanies wrote: > +0.9, with one comment.
Woh, we're using floating point scores now :-P I need to add a patch with a +0.1 and we're good :-D > On Feb 9, 2010, at 1:26 PM, Brice Figureau wrote: > > > Signed-off-by: Brice Figureau <[email protected]> > > --- > > lib/puppet/parser/functions/require.rb | 7 +++++++ > > spec/unit/parser/functions/require.rb | 11 +++++++++++ > > 2 files changed, 18 insertions(+), 0 deletions(-) > > > > diff --git a/lib/puppet/parser/functions/require.rb b/lib/puppet/ > > parser/functions/require.rb > > index d72169a..3e79619 100644 > > --- a/lib/puppet/parser/functions/require.rb > > +++ b/lib/puppet/parser/functions/require.rb > > @@ -37,6 +37,13 @@ fail if used with earlier clients. > > vals = [vals] unless vals.is_a?(Array) > > > > vals.each do |klass| > > + # lookup the class in the scopes > > + if classobj = find_hostclass(klass) > > + klass = classobj.classname > > + else > > + raise Puppet::ParseError, "Could not find class %s" > > % klass > > + end > > + > > # This is a bit hackish, in some ways, but it's the only > > way > > # to configure a dependency that will make it to the > > client. > > # The 'obvious' way is just to add an edge in the catalog, > > diff --git a/spec/unit/parser/functions/require.rb b/spec/unit/ > > parser/functions/require.rb > > index 577a52a..532c069 100755 > > --- a/spec/unit/parser/functions/require.rb > > +++ b/spec/unit/parser/functions/require.rb > > @@ -13,6 +13,8 @@ describe "the require function" do > > @scope.stubs(:resource).returns @resource > > @scope.stubs(:findresource) > > @scope.stubs(:compiler).returns(@compiler) > > + @klass = stub 'class', :classname => "myclass" > > + @scope.stubs(:find_hostclass).returns(@klass) > > end > > > > it "should exist" do > > @@ -45,4 +47,13 @@ describe "the require function" do > > > > @scope.function_require("myclass") > > end > > + > > + it "should lookup the absolute class path" do > > + @scope.stubs(:function_include) > > + > > + > > @scope.expects(:find_hostclass).with("myclass").returns(@klass) > > + @klass.expects(:classname).returns("myclass") > > + > > + @scope.function_require("myclass") > > + end > > end > > I've been trying to use real classes in these tests, rather than > mocks, because it's often actually easier and does a better job of > testing. > > In this case, you should be able to do something like: > > @parser.newclass "my::class" > @scope.search "my" > @scope.expects(:function_include).with("my::class") > @scope.function_require("my::class") I'm not sure this test is equivalent to the stubbed one. You seem to check that we indeed include the correct class, where I was trying to make sure we build the correct resource reference (for the require). What is failing is not the include, but the validation of the resource reference of the require parameter when applying the catalog, because it can't find the said class in the catalog. It can't find it because it tries to find: Class["::one"] which is not a valid resource reference: titles are absolute path to resources so don't need the "::" prefix (it is arguable that it should work). Note, using: file {... ... require => Class["::one"] } Works fine because the AST resource reference does the look-up of the ::one class in the scope and gets its "absolute path" (which is what I added to the require function). > That shows you're actually looking up qualified variables, in a way > that's more resilient to internal changes. That's almost what I tried to do in an integration test that I never could make work, so I abandoned it. I'll try again :-) -- Brice Figureau Follow the latest Puppet Community evolutions on www.planetpuppet.org! -- 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.
