Fixing #673, but I have not written a test case for it. I moved all rails-related unit tests into the rails/ dir, because they keep getting missed.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@2596 980ebf18-57e1-0310-9a29-db15c13687c0

5 files changed, 336 insertions, 285 deletions

diff --git a/lib/puppet/parser/resource/param.rb b/lib/puppet/parser/resource/param.rb
index 4bd169bdd..c719a7fd8 100644
--- a/lib/puppet/parser/resource/param.rb
+++ b/lib/puppet/parser/resource/param.rb
@@ -56,7 +56,7 @@ class Puppet::Parser::Resource::Param
     
     def values_to_remove(db_values)
         values = value.is_a?(Array) ? value : [value]
-        values.map! { |v| v.to_s }
+        values = values.map { |v| v.to_s }
         line_number = line_to_i()
         db_values.collect do |db|
             db unless (db.line == line_number && 
@@ -68,7 +68,7 @@ class Puppet::Parser::Resource::Param
 
     def values_to_add(db_values)
         values = value.is_a?(Array) ? value : [value]
-        values.map! { |v| v.to_s }
+        values = values.map { |v| v.to_s }
         line_number = line_to_i()
         values.collect do |v|
             v unless db_values.find { |db| (v == db.value && 
diff --git a/test/language/collector.rb b/test/language/collector.rb
index 549ed7c9a..e3a2db98c 100755
--- a/test/language/collector.rb
+++ b/test/language/collector.rb
@@ -142,193 +142,6 @@ class TestCollector < Test::Unit::TestCase
         assert_equal(false, ret)
     end
 
-    if Puppet.features.rails?
-    def test_collect_exported
-        railsinit
-
-        # Set a hostname
-        @scope.host = Facter.value(:hostname)
-
-        # make an exported resource
-        exported = mkresource(:type => "file", :title => "/tmp/exported",
-            :exported => true, :params => {:owner => "root"})
-        @scope.setresource exported
-
-        assert(exported.exported?, "Object was not marked exported")
-        assert(exported.virtual?, "Object was not marked virtual")
-
-        # And a non-exported
-        real = mkresource(:type => "file", :title => "/tmp/real",
-            :params => {:owner => "root"})
-        @scope.setresource real
-
-        # Now make a collector
-        coll = nil
-        assert_nothing_raised do
-            coll = Puppet::Parser::Collector.new(@scope, "file", nil, nil, :exported)
-        end
-
-        # Set it in our scope
-        @scope.newcollection(coll)
-
-        # Make sure it's in the collections
-        assert_equal([coll], @scope.collections)
-
-        # And try to collect the virtual resources.
-        ret = nil
-        assert_nothing_raised do
-            ret = coll.collect_exported
-        end
-
-        assert_equal([exported], ret)
-
-        # Now make sure evaluate does the right thing.
-        assert_nothing_raised do
-            ret = coll.evaluate
-        end
-
-        # Make sure that the collection does not find the resource on the
-        # next run.
-        ret = nil
-        assert_nothing_raised do
-            ret = coll.collect_exported
-        end
-
-        assert(ret.empty?, "Exported resource was collected on the second run")
-
-
-        # And make sure our exported object is no longer exported
-        assert(! exported.virtual?, "Virtual object did not get realized")
-
-        # But it should still be marked exported.
-        assert(exported.exported?, "Resource got un-exported")
-
-        # Now make a new collector of a different type and make sure it
-        # finds nothing.
-        assert_nothing_raised do
-            coll = Puppet::Parser::Collector.new(@scope, "exec", nil, nil, :exported)
-        end
-
-        # Remark this as virtual
-        exported.virtual = true
-
-        assert_nothing_raised do
-            ret = coll.evaluate
-        end
-
-        assert(! ret, "got resources back")
-
-        # Now create a whole new scope and make sure we can actually retrieve
-        # the resource from the database, not just from the scope.
-        # First create a host object and store our resource in it.
-
-        # Now collect our facts
-        facts = {}
-        Facter.each do |fact, value| facts[fact] = value end 
-
-        # Now try storing our crap
-        # Remark this as exported
-        exported.exported = true
-        host = Puppet::Rails::Host.store(
-            :resources => [exported],
-            :facts => facts,
-            :name => facts["hostname"]
-        )
-        assert(host, "did not get rails host")
-        host.save
-
-        # And make sure it's in there
-        newres = host.resources.find_by_restype_and_title_and_exported("file", "/tmp/exported", true)
-        assert(newres, "Did not find resource in db")
-        interp, scope, source = mkclassframing
-        scope.host = "two"
-
-        # Now make a collector
-        coll = nil
-        assert_nothing_raised do
-            coll = Puppet::Parser::Collector.new(scope, "file", nil, nil, :exported)
-        end
-
-        # Set it in our scope
-        scope.newcollection(coll)
-
-        # Make sure it's in the collections
-        assert_equal([coll], scope.collections)
-
-        # And try to collect the virtual resources.
-        ret = nil
-        assert_nothing_raised do
-            ret = coll.collect_exported
-        end
-
-        assert_equal(["/tmp/exported"], ret.collect { |f| f.title })
-
-        # Make sure we can evaluate the same collection multiple times and
-        # that later collections do nothing
-        assert_nothing_raised("Collection found same resource twice") do
-            ret = coll.evaluate
-        end
-    end
-
-    def test_collection_conflicts
-        railsinit
-
-        # First make a railshost we can conflict with
-        host = Puppet::Rails::Host.new(:name => "myhost")
-
-        host.resources.build(:title => "/tmp/conflicttest", :restype => "file",
-            :exported => true)
-
-        host.save
-
-        # Now make a normal resource
-        normal = mkresource(:type => "file", :title => "/tmp/conflicttest",
-            :params => {:owner => "root"})
-        @scope.setresource normal
-        @scope.host = "otherhost"
-
-        # Now make a collector
-        coll = nil
-        assert_nothing_raised do
-            coll = Puppet::Parser::Collector.new(@scope, "file", nil, nil, :exported)
-        end
-
-        # And try to collect the virtual resources.
-        assert_raise(Puppet::ParseError) do
-            ret = coll.collect_exported
-        end
-    end
-
-    # Make sure we do not collect resources from the host we're on
-    def test_no_resources_from_me
-        railsinit
-
-        # Make our configuration
-        host = Puppet::Rails::Host.new(:name => "myhost")
-
-        host.resources.build(:title => "/tmp/hosttest", :type => "file",
-            :exported => true)
-
-        host.save
-
-        @scope.host = "myhost"
-
-        # Now make a collector
-        coll = nil
-        assert_nothing_raised do
-            coll = Puppet::Parser::Collector.new(@scope, "file", nil, nil, :exported)
-        end
-
-        # And make sure we get nada back
-        ret = nil
-        assert_nothing_raised do
-            ret = coll.collect_exported
-        end
-
-        assert(ret.empty?, "Found exports from our own host")
-    end
-    end
-
     # Collections that specify resources should be deleted when they succeed,
     # but others should remain until the very end.
     def test_normal_collections_remain
diff --git a/test/language/resource.rb b/test/language/resource.rb
index 9f4c61999..47978eda9 100755
--- a/test/language/resource.rb
+++ b/test/language/resource.rb
@@ -13,6 +13,7 @@ class TestResource < PuppetTest::TestCase
     include PuppetTest::RailsTesting
     Parser = Puppet::Parser
     AST = Parser::AST
+    Reference = Puppet::Parser::Resource::Reference
 
     def setup
         super
@@ -490,100 +491,4 @@ class TestResource < PuppetTest::TestCase
     end
 end
 
-# A separate class for testing rails integration
-class TestExportedResources < TestResource
-    confine "Missing rails support" => Puppet.features.rails?
-
-    # Compare a parser resource to a rails resource.
-    def compare_resources(host, res, updating, options = {})
-        # to_rails now expects to be passed a resource, else it will create a new one
-        newobj = host.resources.find_by_restype_and_title(res.type, res.title)
-        assert_nothing_raised do
-            newobj = res.to_rails(host, newobj)
-        end
-
-        assert_instance_of(Puppet::Rails::Resource, newobj)
-        newobj.save
-
-        if updating
-            tail = "on update"
-        else
-            tail = ""
-        end
-
-        # Make sure we find our object and only our object
-        count = 0
-        obj = nil
-        Puppet::Rails::Resource.find(:all).each do |obj|
-            assert_equal(newobj.id, obj.id, "Found object has a different id than generated object %s" % tail)
-            count += 1
-            [:title, :restype, :line, :exported].each do |param|
-                if param == :restype
-                    method = :type
-                else
-                    method = param
-                end
-                assert_equal(res.send(method), obj[param], 
-                    "attribute %s was not set correctly in rails %s" % [param, tail])
-            end
-        end
-        assert_equal(1, count, "Got too many resources %s" % tail)
-        # Now make sure we can find it again
-        assert_nothing_raised do
-            obj = Puppet::Rails::Resource.find_by_restype_and_title(
-                res.type, res.title, :include => :param_names
-            )
-        end
-        assert_instance_of(Puppet::Rails::Resource, obj)
-
-        # Make sure we get the parameters back
-        params = options[:params] || [obj.param_names.collect { |p| p.name },
-            res.to_hash.keys].flatten.collect { |n| n.to_s }.uniq
-
-        params.each do |name|
-            param = obj.param_names.find_by_name(name)
-            if res[name]
-                assert(param, "resource did not keep %s %s" % [name, tail])
-            else
-                assert(! param, "resource did not delete %s %s" % [name, tail])
-            end
-            if param
-                values = param.param_values.collect { |pv| pv.value }
-                should = res[param.name]
-                should = [should] unless should.is_a?(Array)
-                assert_equal(should, values,
-                    "%s was different %s" % [param.name, tail])
-            end
-        end
-    end
-
-    def test_to_rails
-        railsteardown
-        railsinit
-        res = mkresource :type => "file", :title => "/tmp/testing",
-            :source => @source, :scope => @scope,
-            :params => {:owner => "root", :source => ["/tmp/A", "/tmp/B"],
-                :mode => "755"}
-
-        res.line = 50
-
-        # We also need a Rails Host to store under
-        host = Puppet::Rails::Host.new(:name => Facter.hostname)
-
-        compare_resources(host, res, false, :params => %w{owner source mode})
-
-        # Now make some changes to our resource.  We're removing the mode,
-        # changing the source, and adding 'check'.
-        res = mkresource :type => "file", :title => "/tmp/testing",
-            :source => @source, :scope => @scope,
-            :params => {:owner => "bin", :source => ["/tmp/A", "/tmp/C"],
-            :check => "checksum"}
-
-        res.line = 75
-        res.exported = true
-
-        compare_resources(host, res, true, :params => %w{owner source mode check})
-    end
-end
-
 # $Id$
diff --git a/test/rails/collection.rb b/test/rails/collection.rb
new file mode 100755
index 000000000..05baf41cd
--- /dev/null
+++ b/test/rails/collection.rb
@@ -0,0 +1,214 @@
+#!/usr/bin/env ruby
+
+$:.unshift("../lib").unshift("../../lib") if __FILE__ =~ /\.rb$/
+
+require 'puppet'
+require 'puppet/rails'
+require 'puppettest'
+require 'puppettest/railstesting'
+require 'puppettest/resourcetesting'
+
+
+# A separate class for testing rails integration
+class TestRailsCollection < PuppetTest::TestCase
+    confine "Missing rails support" => Puppet.features.rails?
+	include PuppetTest
+    include PuppetTest::ParserTesting
+    include PuppetTest::ResourceTesting
+    include PuppetTest::RailsTesting
+    Parser = Puppet::Parser
+    AST = Parser::AST
+
+    def setup
+        super
+        Puppet[:trace] = false
+        @interp, @scope, @source = mkclassframing
+    end
+
+    def test_collect_exported
+        railsinit
+
+        # Set a hostname
+        @scope.host = Facter.value(:hostname)
+
+        # make an exported resource
+        exported = mkresource(:type => "file", :title => "/tmp/exported",
+            :exported => true, :params => {:owner => "root"})
+        @scope.setresource exported
+
+        assert(exported.exported?, "Object was not marked exported")
+        assert(exported.virtual?, "Object was not marked virtual")
+
+        # And a non-exported
+        real = mkresource(:type => "file", :title => "/tmp/real",
+            :params => {:owner => "root"})
+        @scope.setresource real
+
+        # Now make a collector
+        coll = nil
+        assert_nothing_raised do
+            coll = Puppet::Parser::Collector.new(@scope, "file", nil, nil, :exported)
+        end
+
+        # Set it in our scope
+        @scope.newcollection(coll)
+
+        # Make sure it's in the collections
+        assert_equal([coll], @scope.collections)
+
+        # And try to collect the virtual resources.
+        ret = nil
+        assert_nothing_raised do
+            ret = coll.collect_exported
+        end
+
+        assert_equal([exported], ret)
+
+        # Now make sure evaluate does the right thing.
+        assert_nothing_raised do
+            ret = coll.evaluate
+        end
+
+        # Make sure that the collection does not find the resource on the
+        # next run.
+        ret = nil
+        assert_nothing_raised do
+            ret = coll.collect_exported
+        end
+
+        assert(ret.empty?, "Exported resource was collected on the second run")
+
+
+        # And make sure our exported object is no longer exported
+        assert(! exported.virtual?, "Virtual object did not get realized")
+
+        # But it should still be marked exported.
+        assert(exported.exported?, "Resource got un-exported")
+
+        # Now make a new collector of a different type and make sure it
+        # finds nothing.
+        assert_nothing_raised do
+            coll = Puppet::Parser::Collector.new(@scope, "exec", nil, nil, :exported)
+        end
+
+        # Remark this as virtual
+        exported.virtual = true
+
+        assert_nothing_raised do
+            ret = coll.evaluate
+        end
+
+        assert(! ret, "got resources back")
+
+        # Now create a whole new scope and make sure we can actually retrieve
+        # the resource from the database, not just from the scope.
+        # First create a host object and store our resource in it.
+
+        # Now collect our facts
+        facts = {}
+        Facter.each do |fact, value| facts[fact] = value end 
+
+        # Now try storing our crap
+        # Remark this as exported
+        exported.exported = true
+        host = Puppet::Rails::Host.store(
+            :resources => [exported],
+            :facts => facts,
+            :name => facts["hostname"]
+        )
+        assert(host, "did not get rails host")
+        host.save
+
+        # And make sure it's in there
+        newres = host.resources.find_by_restype_and_title_and_exported("file", "/tmp/exported", true)
+        assert(newres, "Did not find resource in db")
+        interp, scope, source = mkclassframing
+        scope.host = "two"
+
+        # Now make a collector
+        coll = nil
+        assert_nothing_raised do
+            coll = Puppet::Parser::Collector.new(scope, "file", nil, nil, :exported)
+        end
+
+        # Set it in our scope
+        scope.newcollection(coll)
+
+        # Make sure it's in the collections
+        assert_equal([coll], scope.collections)
+
+        # And try to collect the virtual resources.
+        ret = nil
+        assert_nothing_raised do
+            ret = coll.collect_exported
+        end
+
+        assert_equal(["/tmp/exported"], ret.collect { |f| f.title })
+
+        # Make sure we can evaluate the same collection multiple times and
+        # that later collections do nothing
+        assert_nothing_raised("Collection found same resource twice") do
+            ret = coll.evaluate
+        end
+    end
+
+    def test_collection_conflicts
+        railsinit
+
+        # First make a railshost we can conflict with
+        host = Puppet::Rails::Host.new(:name => "myhost")
+
+        host.resources.build(:title => "/tmp/conflicttest", :restype => "file",
+            :exported => true)
+
+        host.save
+
+        # Now make a normal resource
+        normal = mkresource(:type => "file", :title => "/tmp/conflicttest",
+            :params => {:owner => "root"})
+        @scope.setresource normal
+        @scope.host = "otherhost"
+
+        # Now make a collector
+        coll = nil
+        assert_nothing_raised do
+            coll = Puppet::Parser::Collector.new(@scope, "file", nil, nil, :exported)
+        end
+
+        # And try to collect the virtual resources.
+        assert_raise(Puppet::ParseError) do
+            ret = coll.collect_exported
+        end
+    end
+
+    # Make sure we do not collect resources from the host we're on
+    def test_no_resources_from_me
+        railsinit
+
+        # Make our configuration
+        host = Puppet::Rails::Host.new(:name => "myhost")
+
+        host.resources.build(:title => "/tmp/hosttest", :type => "file",
+            :exported => true)
+
+        host.save
+
+        @scope.host = "myhost"
+
+        # Now make a collector
+        coll = nil
+        assert_nothing_raised do
+            coll = Puppet::Parser::Collector.new(@scope, "file", nil, nil, :exported)
+        end
+
+        # And make sure we get nada back
+        ret = nil
+        assert_nothing_raised do
+            ret = coll.collect_exported
+        end
+
+        assert(ret.empty?, "Found exports from our own host")
+    end
+end
+
+# $Id$
diff --git a/test/rails/railsresource.rb b/test/rails/railsresource.rb
index 236c43594..2a790be39 100755
--- a/test/rails/railsresource.rb
+++ b/test/rails/railsresource.rb
@@ -98,5 +98,124 @@ else
     $stderr.puts "Install Rails for Rails and Caching tests"
 end
 
+# A separate class for testing rails integration
+class TestExportedResources < PuppetTest::TestCase
+	include PuppetTest
+    include PuppetTest::ParserTesting
+    include PuppetTest::ResourceTesting
+    include PuppetTest::RailsTesting
+    Parser = Puppet::Parser
+    AST = Parser::AST
+    Reference = Puppet::Parser::Resource::Reference
+
+    def setup
+        super
+        Puppet[:trace] = false
+        @interp, @scope, @source = mkclassframing
+    end
+
+    confine "Missing rails support" => Puppet.features.rails?
+
+    # Compare a parser resource to a rails resource.
+    def compare_resources(host, res, updating, options = {})
+        # to_rails now expects to be passed a resource, else it will create a new one
+        newobj = host.resources.find_by_restype_and_title(res.type, res.title)
+        assert_nothing_raised do
+            #newobj = res.to_rails(host, newobj)
+            newobj = res.to_rails(host)
+        end
+
+        assert_instance_of(Puppet::Rails::Resource, newobj)
+        newobj.save
+
+        if updating
+            tail = "on update"
+        else
+            tail = ""
+        end
+
+        # Make sure we find our object and only our object
+        count = 0
+        obj = nil
+        Puppet::Rails::Resource.find(:all).each do |obj|
+            assert_equal(newobj.id, obj.id, "Found object has a different id than generated object %s" % tail)
+            count += 1
+            [:title, :restype, :line, :exported].each do |param|
+                if param == :restype
+                    method = :type
+                else
+                    method = param
+                end
+                assert_equal(res.send(method), obj[param], 
+                    "attribute %s was not set correctly in rails %s" % [param, tail])
+            end
+        end
+        assert_equal(1, count, "Got too many resources %s" % tail)
+        # Now make sure we can find it again
+        assert_nothing_raised do
+            obj = Puppet::Rails::Resource.find_by_restype_and_title(
+                res.type, res.title, :include => :param_names
+            )
+        end
+        assert_instance_of(Puppet::Rails::Resource, obj)
+
+        # Make sure we get the parameters back
+        params = options[:params] || [obj.param_names.collect { |p| p.name },
+            res.to_hash.keys].flatten.collect { |n| n.to_s }.uniq
+
+        params.each do |name|
+            param = obj.param_names.find_by_name(name)
+            if res[name]
+                assert(param, "resource did not keep %s %s" % [name, tail])
+            else
+                assert(! param, "resource did not delete %s %s" % [name, tail])
+            end
+            if param
+                values = param.param_values.collect { |pv| pv.value }
+                should = res[param.name]
+                should = [should] unless should.is_a?(Array)
+                assert_equal(should, values,
+                    "%s was different %s" % [param.name, tail])
+            end
+        end
+    end
+
+    def test_to_rails
+        railsteardown
+        railsinit
+        ref1 = Reference.new :type => "exec", :title => "one"
+        ref2 = Reference.new :type => "exec", :title => "two"
+        res = mkresource :type => "file", :title => "/tmp/testing",
+            :source => @source, :scope => @scope,
+            :params => {:owner => "root", :source => ["/tmp/A", "/tmp/B"],
+                :mode => "755", :require => [ref1, ref2]}
+
+        res.line = 50
+
+        # We also need a Rails Host to store under
+        host = Puppet::Rails::Host.new(:name => Facter.hostname)
+
+        compare_resources(host, res, false, :params => %w{owner source mode})
+
+        # Now make some changes to our resource.  We're removing the mode,
+        # changing the source, and adding 'check'.
+        res = mkresource :type => "file", :title => "/tmp/testing",
+            :source => @source, :scope => @scope,
+            :params => {:owner => "bin", :source => ["/tmp/A", "/tmp/C"],
+            :check => "checksum"}
+
+        res.line = 75
+        res.exported = true
+
+        compare_resources(host, res, true, :params => %w{owner source mode check})
+
+        # Now make sure our parameters did not change
+        assert_instance_of(Array, res[:require], "Parameter array changed")
+        res[:require].each do |ref|
+            assert_instance_of(Reference, ref, "Resource reference changed")
+        end
+    end
+end
+
 # $Id$