Recursion using REST seems to almost work.

I think this is the bulk of the work, I just
need to write some integration tests to hunt down
a couple of small issues.

Signed-off-by: Luke Kanies <luke@madstop.com>

2 files changed, 348 insertions, 252 deletions

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 2e3a585c1..2ae1e61b7 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -155,8 +155,6 @@ module Puppet
                 engine, so shell metacharacters are fully supported, e.g. ``[a-z]*``.
                 Matches that would descend into the directory structure are ignored,
                 e.g., ``*/*``."
-       
-            defaultto false
 
             validate do |value|
                 unless value.is_a?(Array) or value.is_a?(String) or value == false
@@ -277,11 +275,6 @@ module Puppet
 
         @depthfirst = false
 
-
-        def argument?(arg)
-            @arghash.include?(arg)
-        end
-
         # Determine the user to write files as.
         def asuser
             if self.should(:owner) and ! self.should(:owner).is_a?(Symbol)
@@ -329,7 +322,12 @@ module Puppet
         
         # Create any children via recursion or whatever.
         def eval_generate
-            recurse() if self.recurse?
+            raise(Puppet::DevError, "File recursion cannot happen without a catalog") unless catalog
+
+            return nil unless self.recurse?
+            recurse.reject { |resource| catalog.resource(:file, resource[:path]) }.each do |child|
+                catalog.relationship_graph.add_edge self, child
+            end
         end
 
         def flush
@@ -462,17 +460,6 @@ module Puppet
             
             @title.sub!(/\/$/, "") unless @title == "/"
 
-            # Clean out as many references to any file paths as possible.
-            # This was the source of many, many bugs.
-            @arghash = tmphash
-            @arghash.delete(self.class.namevar)
-
-            [:source, :parent].each do |param|
-                if @arghash.include?(param)
-                    @arghash.delete(param)
-                end
-            end
-
             @stat = nil
         end
 
@@ -568,88 +555,15 @@ module Puppet
         
         # Create a new file or directory object as a child to the current
         # object.
-        def newchild(path, local, hash = {})
-            raise(Puppet::DevError, "File recursion cannot happen without a catalog") unless catalog
-
-            # make local copy of arguments
-            args = symbolize_options(@arghash)
-
-            # There's probably a better way to do this, but we don't want
-            # to pass this info on.
-            if v = args[:ensure]
-                v = symbolize(v)
-                args.delete(:ensure)
-            end
-
-            if path =~ %r{^#{File::SEPARATOR}}
-                self.devfail(
-                    "Must pass relative paths to PFile#newchild()"
-                )
-            else
-                path = File.join(self[:path], path)
-            end
-
-            args[:path] = path
-
-            unless hash.include?(:recurse)
-                if args.include?(:recurse)
-                    if args[:recurse].is_a?(Integer)
-                        args[:recurse] -= 1 # reduce the level of recursion
-                    end
-                end
-
-            end
-
-            hash.each { |key,value|
-                args[key] = value
-            }
+        def newchild(path)
+            full_path = File.join(self[:path], path)
 
-            child = nil
-
-            # The child might already exist because 'localrecurse' runs
-            # before 'sourcerecurse'.  I could push the override stuff into
-            # a separate method or something, but the work is the same other
-            # than this last bit, so it doesn't really make sense.
-            if child = catalog.resource(:file, path)
-                unless child.parent.object_id == self.object_id
-                    self.debug "Not managing more explicit file %s" %
-                        path
-                    return nil
-                end
+            # the right-side hash wins in the merge.
+            options = to_hash.merge(:path => full_path)
+            options.delete(:parent) if options.include?(:parent)
+            options.delete(:recurse) if options.include?(:recurse)
 
-                # This is only necessary for sourcerecurse, because we might have
-                # created the object with different 'should' values than are
-                # set remotely.
-                unless local
-                    args.each { |var,value|
-                        next if var == :path
-                        next if var == :name
-
-                        # behave idempotently
-                        unless child.should(var) == value
-                            child[var] = value
-                        end
-                    }
-                end
-                return nil
-            else # create it anew
-                #notice "Creating new file with args %s" % args.inspect
-                args[:parent] = self
-                begin
-                    # This method is used by subclasses of :file, so use the class name rather than hard-coding
-                    # :file.
-                    return nil unless child = catalog.create_implicit_resource(self.class.name, args)
-                rescue => detail
-                    self.notice "Cannot manage: %s" % [detail]
-                    return nil
-                end
-            end
-
-            # LAK:FIXME This shouldn't be necessary, but as long as we're
-            # modeling the relationship graph specifically, it is.
-            catalog.relationship_graph.add_edge self, child
-
-            return child
+            return catalog.create_implicit_resource(self.class.name, options)
         end
 
         # Files handle paths specially, because they just lengthen their
@@ -679,97 +593,103 @@ module Puppet
             @parameters.include?(:purge) and (self[:purge] == :true or self[:purge] == "true")
         end
 
+        def make_children(metadata)
+            metadata.collect { |meta| newchild(meta.relative_path) }
+        end
+
+        # Recursively generate a list of file resources, which will
+        # be used to copy remote files, manage local files, and/or make links
+        # to map to another directory.
+        def recurse
+            children = recurse_local
+
+            if self[:target]
+                recurse_link(children)
+            elsif self[:source]
+                recurse_remote(children)
+            end
+
+            return children.values.sort { |a, b| a[:path] <=> b[:path] }
+        end
+
+        # A simple method for determining whether we should be recursing.
+        def recurse?
+            return false unless @parameters.include?(:recurse)
+
+            val = @parameters[:recurse].value
+
+            if val and (val == true or val > 0)
+                return true
+            else
+                return false
+            end
+        end
+
         # Recurse the target of the link.
-        def recurse_link
-            perform_recursion(self[:target])
+        def recurse_link(children)
+            perform_recursion(self[:target]).each do |meta|
+                children[meta.relative_path] ||= newchild(meta.relative_path)
+                if meta.ftype == "directory"
+                    children[meta.relative_path][:ensure] = :directory
+                else
+                    children[meta.relative_path][:ensure] = :link
+                    children[meta.relative_path][:target] = meta.full_path
+                end
+            end
+            children
         end
 
         # Recurse the file itself, returning a Metadata instance for every found file.
         def recurse_local
-            perform_recursion(self[:path])
+            perform_recursion(self[:path]).inject({}) { |hash, meta| hash[meta.relative_path] = newchild(meta.relative_path); hash }
         end
 
         # Recurse against our remote file.
-        def recurse_remote
+        def recurse_remote(children)
+            sourceselect = self[:sourceselect]
+
             total = self[:source].collect do |source|
                 next unless result = perform_recursion(source)
                 result.each { |data| data.source = "%s/%s" % [source, data.relative_path] }
-                return result if result and ! result.empty? and self[:sourceselect] == :first
+                break result if result and ! result.empty? and sourceselect == :first
                 result
             end.flatten
 
             # This only happens if we have sourceselect == :all
-            found = []
-            total.find_all do |data|
-                result = ! found.include?(data.relative_path)
-                found << data.relative_path unless found.include?(data.relative_path)
-                result
-            end
-        end
-
-        def perform_recursion(path)
-            Puppet::FileServing::Metadata.search(self[:path], :links => self[:links], :recurse => self[:recurse], :ignore => self[:ignore])
-        end
-
-        # Recurse into the directory.  This basically just calls 'localrecurse'
-        # and maybe 'sourcerecurse', returning the collection of generated
-        # files.
-        def recurse
-            children = recurse_local
-
-            if self[:target]
-                children += recurse_link 
-            end
-
-            if self[:source]
-                recurse_remote 
-            end
-
-            return children.collect { |child| newchild(child.relative_path) }.reject { |child| child.nil? }
-
-            children = []
-            
-            # We want to do link-recursing before normal recursion so that all
-            # of the target stuff gets copied over correctly.
-            if @parameters.include? :target and ret = self.linkrecurse(recurse)
-                children += ret
-            end
-            if ret = self.localrecurse(recurse)
-                children += ret
+            unless sourceselect == :first
+                found = []
+                total.reject! do |data|
+                    result = found.include?(data.relative_path)
+                    found << data.relative_path unless found.include?(data.relative_path)
+                    result
+                end
             end
 
-            # These will be files pulled in by the file source
-            sourced = false
-            if @parameters.include?(:source)
-                ret, sourced = self.sourcerecurse(recurse)
-                if ret
-                    children += ret
-                end
+            total.each do |meta|
+                children[meta.relative_path] ||= newchild(meta.relative_path)
+                children[meta.relative_path][:source] = meta.source
+                children[meta.relative_path].property(:source).metadata = meta
             end
 
-            # The purge check needs to happen after all of the other recursion.
+            # If we're purging resources, then delete any resource that isn't on the
+            # remote system.
             if self.purge?
-                children.each do |child|
-                    if (sourced and ! sourced.include?(child[:path])) or ! child.managed?
+                # Make a hash of all of the resources we found remotely -- all we need is the
+                # fast lookup, the values don't matter.
+                remotes = total.inject({}) { |hash, meta| hash[meta.relative_path] = true; hash }
+
+                children.each do |name, child|
+                    unless remotes.include?(name)
                         child[:ensure] = :absent
                     end
                 end
             end
-            
+
             children
         end
 
-        # A simple method for determining whether we should be recursing.
-        def recurse?
-            return false unless @parameters.include?(:recurse)
-
-            val = @parameters[:recurse].value
-
-            if val and (val == true or val > 0)
-                return true
-            else
-                return false
-            end
+        def perform_recursion(path)
+            Puppet::FileServing::Metadata.search(path, :links => self[:links], :recurse => self[:recurse], :ignore => self[:ignore])
         end
 
         # Remove the old backup.
diff --git a/spec/unit/type/file.rb b/spec/unit/type/file.rb
index 7bbd2a4b2..27a077bed 100755
--- a/spec/unit/type/file.rb
+++ b/spec/unit/type/file.rb
@@ -8,6 +8,10 @@ describe Puppet::Type.type(:file) do
         @path.close!()
         @path = @path.path
         @file = Puppet::Type::File.create(:name => @path)
+
+        @catalog = mock 'catalog'
+        @catalog.stub_everything
+        @file.catalog = @catalog
     end
 
     describe "when used with content and replace=>false" do
@@ -89,9 +93,9 @@ describe Puppet::Type.type(:file) do
             @file.perform_recursion(@file[:path])
         end
 
-        it "should use its path as the key to the search" do
-            Puppet::FileServing::Metadata.expects(:search).with { |key, options| key = @file[:path] }
-            @file.perform_recursion(@file[:path])
+        it "should use the provided path as the key to the search" do
+            Puppet::FileServing::Metadata.expects(:search).with { |key, options| key == "/foo" }
+            @file.perform_recursion("/foo")
         end
 
         it "should return the results of the metadata search" do
@@ -122,88 +126,258 @@ describe Puppet::Type.type(:file) do
         @file.must respond_to(:recurse_local)
     end
 
-    it "should pass its path to the :perform_recursion method to do local recursion" do
-        @file.expects(:perform_recursion).with(@file[:path]).returns "foobar"
-        @file.recurse_local.should == "foobar"
+    describe "when doing local recursion" do
+        before do
+            @metadata = stub 'metadata', :relative_path => "my/file"
+        end
+
+        it "should pass its to the :perform_recursion method" do
+            @file.expects(:perform_recursion).with(@file[:path]).returns [@metadata]
+            @file.stubs(:newchild)
+            @file.recurse_local
+        end
+
+        it "should create a new child resource with each generated metadata instance's relative path" do
+            @file.expects(:perform_recursion).returns [@metadata]
+            @file.expects(:newchild).with(@metadata.relative_path).returns "fiebar"
+            @file.recurse_local
+        end
+
+        it "should return a hash of the created resources with the relative paths as the hash keys" do
+            @file.expects(:perform_recursion).returns [@metadata]
+            @file.expects(:newchild).with("my/file").returns "fiebar"
+            @file.recurse_local.should == {"my/file" => "fiebar"}
+        end
     end
 
     it "should have a method for performing link recursion" do
         @file.must respond_to(:recurse_link)
     end
 
-    it "should pass its target to the :perform_recursion method to do link recursion" do
-        @file[:target] = "mylinks"
-        @file.expects(:perform_recursion).with("mylinks").returns "foobar"
-        @file.recurse_link.should == "foobar"
+    describe "when doing link recursion" do
+        before do
+            @first = stub 'first', :relative_path => "first", :full_path => "/my/first", :ftype => "directory"
+            @second = stub 'second', :relative_path => "second", :full_path => "/my/second", :ftype => "file"
+
+            @resource = stub 'file', :[]= => nil
+        end
+
+        it "should pass its target to the :perform_recursion method" do
+            @file[:target] = "mylinks"
+            @file.expects(:perform_recursion).with("mylinks").returns [@first]
+            @file.stubs(:newchild).returns @resource
+            @file.recurse_link({})
+        end
+
+        it "should create a new child resource for each generated metadata instance's relative path that doesn't already exist in the children hash" do
+            @file.expects(:perform_recursion).returns [@first, @second]
+            @file.expects(:newchild).with(@first.relative_path).returns @resource
+            @file.recurse_link("second" => @resource)
+        end
+
+        it "should not create a new child resource for paths that already exist in the children hash" do
+            @file.expects(:perform_recursion).returns [@first]
+            @file.expects(:newchild).never
+            @file.recurse_link("first" => @resource)
+        end
+
+        it "should set the target to the full path of discovered file and set :ensure to :link if the file is not a directory" do
+            file = stub 'file'
+            file.expects(:[]=).with(:target, "/my/second")
+            file.expects(:[]=).with(:ensure, :link)
+
+            @file.stubs(:perform_recursion).returns [@first, @second]
+            @file.recurse_link("first" => @resource, "second" => file)
+        end
+
+        it "should :ensure to :directory if the file is a directory" do
+            file = stub 'file'
+            file.expects(:[]=).with(:ensure, :directory)
+
+            @file.stubs(:perform_recursion).returns [@first, @second]
+            @file.recurse_link("first" => file, "second" => @resource)
+        end
+
+        it "should return a hash with both created and existing resources with the relative paths as the hash keys" do
+            file = stub 'file', :[]= => nil
+
+            @file.expects(:perform_recursion).returns [@first, @second]
+            @file.stubs(:newchild).returns file
+            @file.recurse_link("second" => @resource).should == {"second" => @resource, "first" => file}
+        end
     end
 
     it "should have a method for performing remote recursion" do
         @file.must respond_to(:recurse_remote)
     end
 
-    it "should pass its source to the :perform_recursion method to do source recursion" do
-        data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => "foobar")
-        @file[:source] = "puppet://foo/bar"
-        @file.expects(:perform_recursion).with("puppet://foo/bar").returns [data]
-        @file.recurse_remote.should == [data]
-    end
+    describe "when doing remote recursion" do
+        before do
+            @file[:source] = "puppet://foo/bar"
 
-    it "should set the source of each returned file to the searched-for URI plus the found relative path" do
-        metadata = stub 'metadata', :relative_path => "foobar"
-        metadata.expects(:source=).with "puppet://foo/bar/foobar"
-        @file[:source] = "puppet://foo/bar"
-        @file.expects(:perform_recursion).with("puppet://foo/bar").returns [metadata]
-        @file.recurse_remote.should == [metadata]
-    end
+            @first = Puppet::FileServing::Metadata.new("/my", :relative_path => "first")
+            @second = Puppet::FileServing::Metadata.new("/my", :relative_path => "second")
 
-    describe "when multiple sources are provided" do
-        describe "and :sourceselect is set to :first" do
-            it "should return the results for the first source to return any values" do
-                data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => "foobar")
-                @file[:source] = %w{/one /two /three /four}
-                @file.expects(:perform_recursion).with("/one").returns nil
-                @file.expects(:perform_recursion).with("/two").returns []
-                @file.expects(:perform_recursion).with("/three").returns [data]
-                @file.expects(:perform_recursion).with("/four").never
-                @file.recurse_remote.should == [data]
-            end
+            @property = stub 'property', :metadata= => nil
+            @resource = stub 'file', :[]= => nil, :property => @property
         end
 
-        describe "and :sourceselect is set to :all" do
+        it "should pass its source to the :perform_recursion method" do
+            data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => "foobar")
+            @file.expects(:perform_recursion).with("puppet://foo/bar").returns [data]
+            @file.stubs(:newchild).returns @resource
+            @file.recurse_remote({})
+        end
+
+        it "should set the source of each returned file to the searched-for URI plus the found relative path" do
+            @first.expects(:source=).with File.join("puppet://foo/bar", @first.relative_path)
+            @file.expects(:perform_recursion).returns [@first]
+            @file.stubs(:newchild).returns @resource
+            @file.recurse_remote({})
+        end
+
+        it "should create a new resource for any relative file paths that do not already have a resource" do
+            @file.stubs(:perform_recursion).returns [@first]
+            @file.expects(:newchild).with("first").returns @resource
+            @file.recurse_remote({}).should == {"first" => @resource}
+        end
+
+        it "should not create a new resource for any relative file paths that do already have a resource" do
+            @file.stubs(:perform_recursion).returns [@first]
+            @file.expects(:newchild).never
+            @file.recurse_remote("first" => @resource)
+        end
+
+        it "should set the source of each resource to the source of the metadata" do
+            @file.stubs(:perform_recursion).returns [@first]
+            @resource.expects(:[]=).with(:source, File.join("puppet://foo/bar", @first.relative_path))
+            @file.recurse_remote("first" => @resource)
+        end
+
+        it "should store the metadata in the source property for each resource so the source does not have to requery the metadata" do
+            @file.stubs(:perform_recursion).returns [@first]
+            @resource.expects(:property).with(:source).returns @property
+            
+            @property.expects(:metadata=).with(@first)
+
+            @file.recurse_remote("first" => @resource)
+        end
+
+        describe "and purging is enabled" do
             before do
-                @file[:sourceselect] = :all
+                @file[:purge] = true
             end
 
-            it "should return every found file that is not in a previous source" do
-                klass = Puppet::FileServing::Metadata
-                @file[:source] = %w{/one /two /three /four}
+            it "should configure each file not on the remote system to be removed" do
+                @file.stubs(:perform_recursion).returns [@second]
 
-                one = [klass.new("/one", :relative_path => "a")]
-                @file.expects(:perform_recursion).with("/one").returns one
+                @resource.expects(:[]=).with(:ensure, :absent)
 
-                two = [klass.new("/two", :relative_path => "a"), klass.new("/two", :relative_path => "b")]
-                @file.expects(:perform_recursion).with("/two").returns two
+                @file.expects(:newchild).returns stub('secondfile', :[]= => nil, :property => @property)
 
-                three = [klass.new("/three", :relative_path => "a"), klass.new("/three", :relative_path => "c")]
-                @file.expects(:perform_recursion).with("/three").returns three
+                @file.recurse_remote("first" => @resource)
+            end
+        end
+
+        describe "and multiple sources are provided" do
+            describe "and :sourceselect is set to :first" do
+                it "should create file instances for the results for the first source to return any values" do
+                    data = Puppet::FileServing::Metadata.new("/whatever", :relative_path => "foobar")
+                    @file[:source] = %w{/one /two /three /four}
+                    @file.expects(:perform_recursion).with("/one").returns nil
+                    @file.expects(:perform_recursion).with("/two").returns []
+                    @file.expects(:perform_recursion).with("/three").returns [data]
+                    @file.expects(:perform_recursion).with("/four").never
+                    @file.expects(:newchild).with("foobar").returns @resource
+                    @file.recurse_remote({})
+                end
+            end
+
+            describe "and :sourceselect is set to :all" do
+                before do
+                    @file[:sourceselect] = :all
+                end
+
+                it "should return every found file that is not in a previous source" do
+                    klass = Puppet::FileServing::Metadata
+                    @file[:source] = %w{/one /two /three /four}
+                    @file.stubs(:newchild).returns @resource
+
+                    one = [klass.new("/one", :relative_path => "a")]
+                    @file.expects(:perform_recursion).with("/one").returns one
+                    @file.expects(:newchild).with("a").returns @resource
 
-                @file.expects(:perform_recursion).with("/four").returns []
+                    two = [klass.new("/two", :relative_path => "a"), klass.new("/two", :relative_path => "b")]
+                    @file.expects(:perform_recursion).with("/two").returns two
+                    @file.expects(:newchild).with("b").returns @resource
 
-                @file.recurse_remote.should == [one[0], two[1], three[1]]
+                    three = [klass.new("/three", :relative_path => "a"), klass.new("/three", :relative_path => "c")]
+                    @file.expects(:perform_recursion).with("/three").returns three
+                    @file.expects(:newchild).with("c").returns @resource
+
+                    @file.expects(:perform_recursion).with("/four").returns []
+
+                    @file.recurse_remote({})
+                end
             end
         end
     end
 
-    it "should recurse during eval_generate if recursion is enabled" do
-        @file.expects(:recurse?).returns true
-        @file.expects(:recurse).returns "foobar"
-        @file.eval_generate.should == "foobar"
-    end
+    describe "when returning resources with :eval_generate" do
+        before do
+            @catalog = mock 'catalog'
+            @catalog.stub_everything
+
+            @graph = stub 'graph', :add_edge => nil
+            @catalog.stubs(:relationship_graph).returns @graph
+
+            @file.catalog = @catalog
+            @file[:recurse] = true
+        end
+
+        it "should recurse if recursion is enabled" do
+            resource = stub('resource', :[] => "resource")
+            @file.expects(:recurse?).returns true
+            @file.expects(:recurse).returns [resource]
+            @file.eval_generate.should == [resource]
+        end
+
+        it "should not recurse if recursion is disabled" do
+            @file.expects(:recurse?).returns false
+            @file.expects(:recurse).never
+            @file.eval_generate.should be_nil
+        end
+
+        it "should fail if no catalog is set" do
+            @file.catalog = nil
+            lambda { @file.eval_generate }.should raise_error(Puppet::DevError)
+        end
+
+        it "should skip resources that are already in the catalog" do
+            foo = stub 'foo', :[] => "/foo"
+            bar = stub 'bar', :[] => "/bar"
+            bar2 = stub 'bar2', :[] => "/bar"
+
+            @catalog.expects(:resource).with(:file, "/foo").returns nil
+            @catalog.expects(:resource).with(:file, "/bar").returns bar2
+
+            @file.expects(:recurse).returns [foo, bar]
 
-    it "should not recurse during eval_generate if recursion is disabled" do
-        @file.expects(:recurse?).returns false
-        @file.expects(:recurse).never
-        @file.eval_generate.should be_nil
+            @file.eval_generate.should == [foo]
+        end
+
+        it "should add a relationshp edge for each returned resource" do
+            foo = stub 'foo', :[] => "/foo"
+
+            @file.expects(:recurse).returns [foo]
+
+            graph = mock 'graph'
+            @catalog.stubs(:relationship_graph).returns graph
+
+            graph.expects(:add_edge).with(@file, foo)
+
+            @file.eval_generate
+        end
     end
 
     describe "when recursing" do
@@ -215,58 +389,60 @@ describe Puppet::Type.type(:file) do
         describe "and a source is set" do
             before { @file[:source] = "/my/source" }
 
-            it "should use recurse_remote" do
-                @file.stubs(:recurse_local).returns []
-                @file.expects(:recurse_remote)
+            it "should pass the already-discovered resources to recurse_remote" do
+                @file.stubs(:recurse_local).returns(:foo => "bar")
+                @file.expects(:recurse_remote).with(:foo => "bar").returns []
                 @file.recurse
             end
-
-            it "should create a new file resource for each remote file"
-
-            it "should set the source for each new file resource"
-
-            it "should copy the metadata to the new file's source property so the file does not have to requery the remote system for metadata"
-
-            describe "and purging is enabled" do
-                it "should configure each file not on the remote system to be removed"
-            end
         end
 
         describe "and a target is set" do
             before { @file[:target] = "/link/target" }
 
             it "should use recurse_link" do
-                @file.stubs(:recurse_local).returns []
-                @file.expects(:recurse_link).returns []
+                @file.stubs(:recurse_local).returns(:foo => "bar")
+                @file.expects(:recurse_link).with(:foo => "bar").returns []
                 @file.recurse
             end
-
-            it "should return a new file resource for each link destination found"
-
-            it "should set the target for each new file resource"
         end
 
         it "should use recurse_local" do
-            @file.expects(:recurse_local).returns []
+            @file.expects(:recurse_local).returns({})
             @file.recurse
         end
 
-        it "should attempt to turn each found file into a child resource" do
-            a = @metadata.new("/foo", :relative_path => "a")
-            @file.expects(:recurse_local).returns [a]
-            @file.expects(:newchild).with("a")
-
-            @file.recurse
+        it "should return the generated resources as an array sorted by file path" do
+            one = stub 'one', :[] => "/one"
+            two = stub 'two', :[] => "/one/two"
+            three = stub 'three', :[] => "/three"
+            @file.expects(:recurse_local).returns(:one => one, :two => two, :three => three)
+            @file.recurse.should == [one, two, three]
         end
 
-        it "should not return nil for those files that could not be turned into children" do
-            a = @metadata.new("/foo", :relative_path => "a")
-            b = @metadata.new("/foo", :relative_path => "b")
-            @file.expects(:recurse_local).returns [a, b]
-            @file.expects(:newchild).with("a").returns "A"
-            @file.expects(:newchild).with("b").returns nil
+        describe "and making a new child resource" do
+            it "should create an implicit resource using the provided relative path joined with the file's path" do
+                path = File.join(@file[:path], "my/path")
+                @catalog.expects(:create_implicit_resource).with { |klass, options| options[:path] == path }
+                @file.newchild("my/path")
+            end
 
-            @file.recurse.should == ["A"]
+            it "should copy most of the parent resource's 'should' values to the new resource" do
+                @file.expects(:to_hash).returns :foo => "bar", :fee => "fum"
+                @catalog.expects(:create_implicit_resource).with { |klass, options| options[:foo] == "bar" and options[:fee] == "fum" }
+                @file.newchild("my/path")
+            end
+
+            it "should not copy the parent resource's parent" do
+                @file.expects(:to_hash).returns :parent => "foo"
+                @catalog.expects(:create_implicit_resource).with { |klass, options| ! options.include?(:parent) }
+                @file.newchild("my/path")
+            end
+
+            it "should not copy the parent resource's recurse value" do
+                @file.expects(:to_hash).returns :recurse => true
+                @catalog.expects(:create_implicit_resource).with { |klass, options| ! options.include?(:recurse) }
+                @file.newchild("my/path")
+            end
         end
     end
 end