Merge remote-tracking branch 'zaphod42/bug/2.7.x/8174-incorrect-warni…

…ng-about-deprecated-scoping' into 2.7.x

* zaphod42/bug/2.7.x/8174-incorrect-warning-about-deprecated-scoping:
  Removed duplication of compiling a catalog
  Cleaner test for create_resources doing nothing
  Fixing problem caused by activerecord

lib/puppet/parser/scope.rb

@@ -241,7 +241,7 @@ def twoscope_lookupvar(name, options = {})
     table = ephemeral?(name) ? @ephemeral.last : @symtable
     if name =~ /^(.*)::(.+)$/
       begin
-        qualified_scope($1).twoscope_lookupvar($2,options.merge({:origin => nil}))
+        qualified_scope($1).twoscope_lookupvar($2, options.merge({:origin => nil}))
       rescue RuntimeError => e
         location = (options[:file] && options[:line]) ? " at #{options[:file]}:#{options[:line]}" : ''
         warning "Could not look up qualified variable '#{name}'; #{e.message}#{location}"
@@ -250,10 +250,10 @@ def twoscope_lookupvar(name, options = {})
     # If the value is present and either we are top/node scope or originating scope...
     elsif (ephemeral_include?(name) or table.include?(name)) and (compiler and self == compiler.topscope or (self.resource and self.resource.type == "Node") or self == options[:origin])
       table[name]
-    elsif resource and resource.resource_type and resource.resource_type.respond_to?("parent") and parent_type = resource.resource_type.parent
+    elsif resource and resource.type == "Class" and parent_type = resource.resource_type.parent
       class_scope(parent_type).twoscope_lookupvar(name,options.merge({:origin => nil}))
     elsif parent
-      parent.twoscope_lookupvar(name,options)
+      parent.twoscope_lookupvar(name, options)
     else
       :undefined
     end

spec/lib/puppet_spec/compiler.rb

@@ -0,0 +1,6 @@
+module PuppetSpec::Compiler
+  def compile_to_catalog(string)
+    Puppet[:code] = string
+    Puppet::Parser::Compiler.compile(Puppet::Node.new('foonode'))
+  end
+end

spec/unit/parser/functions/create_resources_spec.rb

@@ -1,19 +1,16 @@
 require 'puppet'
 require 'spec_helper'
+require 'puppet_spec/compiler'
 
 describe 'function for dynamically creating resources' do
+  include PuppetSpec::Compiler
 
-  def get_scope
-    @topscope = Puppet::Parser::Scope.new
-    # This is necessary so we don't try to use the compiler to discover our parent.
-    @topscope.parent = nil
+  before :each do
     @scope = Puppet::Parser::Scope.new
     @scope.compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("floppy", :environment => 'production'))
-    @scope.parent = @topscope
+    @topscope = @scope.compiler.topscope
     @compiler = @scope.compiler
-  end
-  before :each do
-    get_scope
+    @scope.parent = @topscope
     Puppet::Parser::Functions.function(:create_resources)
   end
 
@@ -32,6 +29,7 @@ def get_scope
       @scope.function_create_resources(['notify', {'test'=>{}}])
     end
   end
+
   describe 'when the caller supplies a name parameter' do
     it 'should set the resource name to the value provided' do
       Puppet::Parser::Resource.any_instance.expects(:set_parameter).with(:name, 'user_supplied').once
@@ -41,131 +39,165 @@ def get_scope
   end
 
   describe 'when creating native types' do
-    before :each do
-      Puppet[:code]='notify{test:}'
-      get_scope
-      @scope.resource=Puppet::Parser::Resource.new('class', 't', :scope => @scope)
-    end
-
     it 'empty hash should not cause resources to be added' do
-      @scope.function_create_resources(['file', {}])
-      @compiler.catalog.resources.size == 1
+      noop_catalog = compile_to_catalog("create_resources('file', {})")
+      empty_catalog = compile_to_catalog("")
+      noop_catalog.resources.size.should == empty_catalog.resources.size
     end
+
     it 'should be able to add' do
-      @scope.function_create_resources(['file', {'/etc/foo'=>{'ensure'=>'present'}}])
-      @compiler.catalog.resource(:file, "/etc/foo")['ensure'].should == 'present'
+      catalog = compile_to_catalog("create_resources('file', {'/etc/foo'=>{'ensure'=>'present'}})")
+      catalog.resource(:file, "/etc/foo")['ensure'].should == 'present'
     end
+
     it 'should accept multiple types' do
-      type_hash = {}
-      type_hash['foo'] = {'message' => 'one'}
-      type_hash['bar'] = {'message' => 'two'}
-      @scope.function_create_resources(['notify', type_hash])
-      @compiler.catalog.resource(:notify, "foo")['message'].should == 'one'
-      @compiler.catalog.resource(:notify, "bar")['message'].should == 'two'
+      catalog = compile_to_catalog("create_resources('notify', {'foo'=>{'message'=>'one'}, 'bar'=>{'message'=>'two'}})")
+      catalog.resource(:notify, "foo")['message'].should == 'one'
+      catalog.resource(:notify, "bar")['message'].should == 'two'
     end
+
     it 'should fail to add non-existing type' do
       expect { @scope.function_create_resources(['create-resource-foo', {}]) }.should raise_error(ArgumentError, 'could not create resource of unknown type create-resource-foo')
     end
+
     it 'should be able to add edges' do
-      @scope.function_create_resources(['notify', {'foo'=>{'require' => 'Notify[test]'}}])
-      @scope.compiler.compile
-      rg = @scope.compiler.catalog.to_ral.relationship_graph
+      catalog = compile_to_catalog("notify { test: }\n create_resources('notify', {'foo'=>{'require'=>'Notify[test]'}})")
+      rg = catalog.to_ral.relationship_graph
       test  = rg.vertices.find { |v| v.title == 'test' }
       foo   = rg.vertices.find { |v| v.title == 'foo' }
       test.should be
       foo.should be
       rg.path_between(test,foo).should be
     end
+
     it 'should account for default values' do
-      @scope.function_create_resources(['file', {'/etc/foo'=>{'ensure'=>'present'}, '/etc/baz'=>{'group'=>'food'}}, {'group' => 'bar'}])
-      @compiler.catalog.resource(:file, "/etc/foo")['group'].should == 'bar'
-      @compiler.catalog.resource(:file, "/etc/baz")['group'].should == 'food'
+      catalog = compile_to_catalog("create_resources('file', {'/etc/foo'=>{'ensure'=>'present'}, '/etc/baz'=>{'group'=>'food'}}, {'group' => 'bar'})")
+      catalog.resource(:file, "/etc/foo")['group'].should == 'bar'
+      catalog.resource(:file, "/etc/baz")['group'].should == 'food'
     end
   end
   describe 'when dynamically creating resource types' do
-    before :each do 
-      Puppet[:code]=
-'define foocreateresource($one){notify{$name: message => $one}}
-notify{test:}
-'
-      get_scope
-      @scope.resource=Puppet::Parser::Resource.new('class', 't', :scope => @scope)
-      Puppet::Parser::Functions.function(:create_resources)
-    end
     it 'should be able to create defined resoure types' do
-      @scope.function_create_resources(['foocreateresource', {'blah'=>{'one'=>'two'}}])
-      # still have to compile for this to work...
-      # I am not sure if this constraint ruins the tests
-      @scope.compiler.compile
-      @compiler.catalog.resource(:notify, "blah")['message'].should == 'two'
+      catalog = compile_to_catalog(<<-MANIFEST)
+        define foocreateresource($one) {
+          notify { $name: message => $one }
+        }
+        
+        create_resources('foocreateresource', {'blah'=>{'one'=>'two'}})
+      MANIFEST
+      catalog.resource(:notify, "blah")['message'].should == 'two'
     end
+
     it 'should fail if defines are missing params' do
-      @scope.function_create_resources(['foocreateresource', {'blah'=>{}}])
-      expect { @scope.compiler.compile }.should raise_error(Puppet::ParseError, 'Must pass one to Foocreateresource[blah] at line 1')
+      expect { 
+        compile_to_catalog(<<-MANIFEST)
+          define foocreateresource($one) {
+            notify { $name: message => $one }
+          }
+          
+          create_resources('foocreateresource', {'blah'=>{}})
+        MANIFEST
+      }.should raise_error(Puppet::Error, 'Must pass one to Foocreateresource[blah] at line 1 on node foonode')
     end
+
     it 'should be able to add multiple defines' do
-      hash = {}
-      hash['blah'] = {'one' => 'two'}
-      hash['blaz'] = {'one' => 'three'}
-      @scope.function_create_resources(['foocreateresource', hash])
-      # still have to compile for this to work...
-      # I am not sure if this constraint ruins the tests
-      @scope.compiler.compile
-      @compiler.catalog.resource(:notify, "blah")['message'].should == 'two'
-      @compiler.catalog.resource(:notify, "blaz")['message'].should == 'three'
+      catalog = compile_to_catalog(<<-MANIFEST)
+        define foocreateresource($one) {
+          notify { $name: message => $one }
+        }
+        
+        create_resources('foocreateresource', {'blah'=>{'one'=>'two'}, 'blaz'=>{'one'=>'three'}})
+      MANIFEST
+
+      catalog.resource(:notify, "blah")['message'].should == 'two'
+      catalog.resource(:notify, "blaz")['message'].should == 'three'
     end
+
     it 'should be able to add edges' do
-      @scope.function_create_resources(['foocreateresource', {'blah'=>{'one'=>'two', 'require' => 'Notify[test]'}}])
-      @scope.compiler.compile
-      rg = @scope.compiler.catalog.to_ral.relationship_graph
+      catalog = compile_to_catalog(<<-MANIFEST)
+        define foocreateresource($one) {
+          notify { $name: message => $one }
+        }
+
+        notify { test: }
+        
+        create_resources('foocreateresource', {'blah'=>{'one'=>'two', 'require' => 'Notify[test]'}})
+      MANIFEST
+
+      rg = catalog.to_ral.relationship_graph
       test = rg.vertices.find { |v| v.title == 'test' }
       blah = rg.vertices.find { |v| v.title == 'blah' }
       test.should be
       blah.should be
-      # (Yoda speak like we do)
       rg.path_between(test,blah).should be
-      @compiler.catalog.resource(:notify, "blah")['message'].should == 'two'
+      catalog.resource(:notify, "blah")['message'].should == 'two'
     end
+
     it 'should account for default values' do
-      @scope.function_create_resources(['foocreateresource', {'blah'=>{}}, {'one' => 'two'}])
-      @scope.compiler.compile
-      @compiler.catalog.resource(:notify, "blah")['message'].should == 'two'
+      catalog = compile_to_catalog(<<-MANIFEST)
+        define foocreateresource($one) {
+          notify { $name: message => $one }
+        }
+
+        create_resources('foocreateresource', {'blah'=>{}}, {'one' => 'two'})
+      MANIFEST
+
+      catalog.resource(:notify, "blah")['message'].should == 'two'
     end
   end
+
   describe 'when creating classes' do
-    before :each do
-      Puppet[:code]=
-'class bar($one){notify{test: message => $one}}
-notify{tester:}
-'
-      get_scope
-      @scope.resource=Puppet::Parser::Resource.new('class', 't', :scope => @scope)
-      Puppet::Parser::Functions.function(:create_resources)
-    end
     it 'should be able to create classes' do
-      @scope.function_create_resources(['class', {'bar'=>{'one'=>'two'}}])
-      @scope.compiler.compile
-      @compiler.catalog.resource(:notify, "test")['message'].should == 'two'
-      @compiler.catalog.resource(:class, "bar").should_not be_nil
+      catalog = compile_to_catalog(<<-MANIFEST)
+        class bar($one) {
+          notify { test: message => $one }
+        }
+
+        create_resources('class', {'bar'=>{'one'=>'two'}})
+      MANIFEST
+
+      catalog.resource(:notify, "test")['message'].should == 'two'
+      catalog.resource(:class, "bar").should_not be_nil
     end
+
     it 'should fail to create non-existing classes' do
-      expect { @scope.function_create_resources(['class', {'blah'=>{'one'=>'two'}}]) }.should raise_error(ArgumentError ,'could not find hostclass blah')
+      expect {
+        compile_to_catalog(<<-MANIFEST)
+          create_resources('class', {'blah'=>{'one'=>'two'}})
+        MANIFEST
+      }.should raise_error(Puppet::Error ,'could not find hostclass blah at line 1 on node foonode')
     end
+
     it 'should be able to add edges' do
-      @scope.function_create_resources(['class', {'bar'=>{'one'=>'two', 'require' => 'Notify[tester]'}}])
-      @scope.compiler.compile
-      rg = @scope.compiler.catalog.to_ral.relationship_graph
+      catalog = compile_to_catalog(<<-MANIFEST)
+        class bar($one) {
+          notify { test: message => $one }
+        }
+
+        notify { tester: }
+
+        create_resources('class', {'bar'=>{'one'=>'two', 'require' => 'Notify[tester]'}})
+      MANIFEST
+
+      rg = catalog.to_ral.relationship_graph
       test   = rg.vertices.find { |v| v.title == 'test' }
       tester = rg.vertices.find { |v| v.title == 'tester' }
       test.should be
       tester.should be
       rg.path_between(tester,test).should be
     end
+
     it 'should account for default values' do
-      @scope.function_create_resources(['class', {'bar'=>{}}, {'one' => 'two'}])
-      @scope.compiler.compile
-      @compiler.catalog.resource(:notify, "test")['message'].should == 'two'
-      @compiler.catalog.resource(:class, "bar").should_not be_nil
+      catalog = compile_to_catalog(<<-MANIFEST)
+        class bar($one) {
+          notify { test: message => $one }
+        }
+
+        create_resources('class', {'bar'=>{}}, {'one' => 'two'})
+      MANIFEST
+
+      catalog.resource(:notify, "test")['message'].should == 'two'
+      catalog.resource(:class, "bar").should_not be_nil
     end
   end
 end

spec/unit/parser/scope_spec.rb

@@ -1,9 +1,9 @@
 #!/usr/bin/env rspec
 require 'spec_helper'
+require 'puppet_spec/compiler'
 
 describe Puppet::Parser::Scope do
   before :each do
-    # This is necessary so we don't try to use the compiler to discover our parent.
     @scope = Puppet::Parser::Scope.new
     @scope.compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("foo"))
     @scope.source = Puppet::Resource::Type.new(:node, :foo)
@@ -215,11 +215,10 @@ def create_class_scope(name)
   end
 
   describe "when mixing inheritence and inclusion" do
-    let(:catalog) { Puppet::Parser::Compiler.compile(Puppet::Node.new('foonode')) }
+    include PuppetSpec::Compiler
 
     def expect_the_message_to_be(message) 
-      Puppet[:code] = yield
-      catalog = Puppet::Parser::Compiler.compile(Puppet::Node.new('foonode'))
+      catalog = compile_to_catalog(yield)
       catalog.resource('Notify', 'something')[:message].should == message
     end
 
@@ -290,6 +289,55 @@ class baz {
         Puppet.expects(:deprecation_warning).never
       end
 
+      it "finds value define in the inherited node" do
+        expect_the_message_to_be('parent_msg') do <<-MANIFEST
+            $var = "top_msg"
+            node parent {
+              $var = "parent_msg"
+            }
+            node default inherits parent {
+              include foo
+            }
+            class foo {
+              notify { 'something': message => $var, }
+            }
+          MANIFEST
+        end
+      end
+
+      it "finds top scope when the class is included before the node defines the var" do
+        expect_the_message_to_be('top_msg') do <<-MANIFEST
+            $var = "top_msg"
+            node parent {
+              include foo
+            }
+            node default inherits parent {
+              $var = "default_msg"
+            }
+            class foo {
+              notify { 'something': message => $var, }
+            }
+          MANIFEST
+        end
+      end
+
+      it "finds top scope when the class is included before the node defines the var" do
+        expect_the_message_to_be('top_msg') do <<-MANIFEST
+            $var = "top_msg"
+            node parent {
+              include foo
+            }
+            node default inherits parent {
+              $var = "default_msg"
+            }
+            class foo {
+              notify { 'something': message => $var, }
+            }
+          MANIFEST
+        end
+      end
+
+
       it "should find values in its local scope" do
         expect_the_message_to_be('local_msg') do <<-MANIFEST
             node default {