Fixed #997 -- virtual defined types are no longer evaluated.

NOTE: This introduces a behaviour change, in that you previously
could realize a resource within a virtual defined resource, and now
you must realize the entire defined resource, rather than just
the contained resource.

CHANGELOG

@@ -1,3 +1,9 @@
+    Fixed #997 -- virtual defined types are no longer evaluated.
+    NOTE: This introduces a behaviour change, in that you previously
+    could realize a resource within a virtual defined resource, and now
+    you must realize the entire defined resource, rather than just
+    the contained resource.
+
     Fixed #1030 - class and definition evaluation has been significantly
     refactored, fixing this problem and making the whole interplay
     between the classes, definitions, and nodes, and the Compile class much

spec/unit/parser/compiler.rb

@@ -169,7 +169,7 @@ def compile_stub(*except)
     end
 
     it "should evaluate unevaluated resources" do
-        resource = stub 'notevaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => false
+        resource = stub 'notevaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => false, :virtual? => false
         @compiler.add_resource(@scope, resource)
 
         # We have to now mark the resource as evaluated
@@ -179,18 +179,18 @@ def compile_stub(*except)
     end
 
     it "should not evaluate already-evaluated resources" do
-        resource = stub 'already_evaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => true
+        resource = stub 'already_evaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => true, :virtual? => false
         @compiler.add_resource(@scope, resource)
         resource.expects(:evaluate).never
 
         @compiler.compile
     end
 
     it "should evaluate unevaluated resources created by evaluating other resources" do
-        resource = stub 'notevaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => false
+        resource = stub 'notevaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => false, :virtual? => false
         @compiler.add_resource(@scope, resource)
 
-        resource2 = stub 'created', :ref => "File[other]", :builtin? => false, :evaluated? => false
+        resource2 = stub 'created', :ref => "File[other]", :builtin? => false, :evaluated? => false, :virtual? => false
 
         # We have to now mark the resource as evaluated
         resource.expects(:evaluate).with { |*whatever| resource.stubs(:evaluated?).returns(true); @compiler.add_resource(@scope, resource2) }
@@ -244,6 +244,15 @@ def compile_stub(*except)
         @compiler.add_resource(@scope, resource)
         @compiler.findresource("Yay", "foo").should equal(resource)
     end
+
+    it "should not evaluate virtual defined resources" do
+        resource = stub 'notevaluated', :ref => "File[testing]", :builtin? => false, :evaluated? => false, :virtual? => true
+        @compiler.add_resource(@scope, resource)
+
+        resource.expects(:evaluate).never
+
+        @compiler.compile
+    end
 end
 
 describe Puppet::Parser::Compiler, " when evaluating collections" do

test/data/snippets/collection_within_virtual_definitions.pp

@@ -0,0 +1,20 @@
+define test($name) {
+    file {"/tmp/collection_within_virtual_definitions1_$name.txt":
+        content => "File name $name\n"
+    }
+    Test2 <||>
+}
+
+define test2() {
+    file {"/tmp/collection_within_virtual_definitions2_$name.txt":
+        content => "This is a test\n"
+    }
+}
+
+node default {
+    @test {"foo":
+        name => "foo"
+    }
+    @test2 {"foo2": }
+    Test <||>
+}

test/language/scope.rb

@@ -328,10 +328,9 @@ def test_truth
             "undef considered true")
     end
 
-    if defined? ActiveRecord
     # Verify that we recursively mark as exported the results of collectable
     # components.
-    def test_exportedcomponents
+    def test_virtual_definitions_do_not_get_evaluated
         config = mkcompiler
         parser = config.parser
 
@@ -348,35 +347,19 @@ def test_exportedcomponents
             :children => [nameobj("arg")]
         )
 
-        # Create a top-level component
+        # Create a top-level define
         parser.newdefine "one", :arguments => [%w{arg}],
             :code => AST::ASTArray.new(
                 :children => [
                     resourcedef("file", "/tmp", {"owner" => varref("arg")})
                 ]
             )
 
-        # And a component that calls it
-        parser.newdefine "two", :arguments => [%w{arg}],
-            :code => AST::ASTArray.new(
-                :children => [
-                    resourcedef("one", "ptest", {"arg" => varref("arg")})
-                ]
-            )
-
-        # And then a third component that calls the second
-        parser.newdefine "three", :arguments => [%w{arg}],
-            :code => AST::ASTArray.new(
-                :children => [
-                    resourcedef("two", "yay", {"arg" => varref("arg")})
-                ]
-            )
-
-        # lastly, create an object that calls our third component
-        obj = resourcedef("three", "boo", {"arg" => "parentfoo"})
+        # create a resource that calls our third define
+        obj = resourcedef("one", "boo", {"arg" => "parentfoo"})
 
-        # And mark it as exported
-        obj.exported = true
+        # And mark it as virtual
+        obj.virtual = true
 
         # And then evaluate it
         obj.evaluate config.topscope
@@ -385,12 +368,13 @@ def test_exportedcomponents
         config.send(:evaluate_generators)
 
         %w{File}.each do |type|
-            objects = config.resources.find_all { |r| r.type == type and r.exported }
+            objects = config.resources.find_all { |r| r.type == type and r.virtual }
 
-            assert(!objects.empty?, "Did not get an exported %s" % type)
+            assert(objects.empty?, "Virtual define got evaluated")
         end
     end
 
+    if defined? ActiveRecord
     # Verify that we can both store and collect an object in the same
     # run, whether it's in the same scope as a collection or a different
     # scope.

test/language/snippets.rb

@@ -424,6 +424,11 @@ def snippet_realize_defined_types
         assert_file("/tmp/realize_defined_test2")
     end
 
+    def snippet_collection_within_virtual_definitions
+        assert_file("/tmp/collection_within_virtual_definitions1_foo.txt")
+        assert_file("/tmp/collection_within_virtual_definitions2_foo2.txt")
+    end
+
     def snippet_fqparents
         assert_file("/tmp/fqparent1", "Did not make file from parent class")
         assert_file("/tmp/fqparent2", "Did not make file from subclass")