These changes are all about making sure file data is expired when appropriate.

All file tests now pass.

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

7 files changed, 92 insertions, 153 deletions

diff --git a/lib/puppet/node/catalog.rb b/lib/puppet/node/catalog.rb
index e82e63444..f6c0a1959 100644
--- a/lib/puppet/node/catalog.rb
+++ b/lib/puppet/node/catalog.rb
@@ -118,6 +118,10 @@ class Puppet::Node::Catalog < Puppet::SimpleGraph
     def apply(options = {})
         @applying = true
 
+        # Expire all of the resource data -- this ensures that all
+        # data we're operating against is entirely current.
+        expire()
+
         Puppet::Util::Storage.load if host_config?
         transaction = Puppet::Transaction.new(self)
 
diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb
index 169ba9078..635cdc809 100755
--- a/lib/puppet/type/file/content.rb
+++ b/lib/puppet/type/file/content.rb
@@ -58,6 +58,7 @@ module Puppet
             if self.should
                 return super
             elsif source = resource.parameter(:source)
+                fail "Got a remote source with no checksum" unless source.checksum
                 unless sum_method = sumtype(source.checksum)
                     fail "Could not extract checksum type from source checksum '%s'" % source.checksum
                 end
diff --git a/lib/puppet/type/file/source.rb b/lib/puppet/type/file/source.rb
index 4e67b1a8b..a6ab5daaf 100755
--- a/lib/puppet/type/file/source.rb
+++ b/lib/puppet/type/file/source.rb
@@ -93,8 +93,8 @@ module Puppet
         end
         
         def checksum
-            if defined?(@metadata)
-                @metadata.checksum
+            if metadata
+                metadata.checksum
             else
                 nil
             end
diff --git a/spec/unit/node/catalog.rb b/spec/unit/node/catalog.rb
index e23c53d3a..e3661d995 100755
--- a/spec/unit/node/catalog.rb
+++ b/spec/unit/node/catalog.rb
@@ -700,8 +700,8 @@ describe Puppet::Node::Catalog do
             @catalog.resource("File[/yay]").should be_nil
         end
 
-        it "should expire cached data in the resources" do
-            @catalog.expects(:expire)
+        it "should expire cached data in the resources both before and after the transaction" do
+            @catalog.expects(:expire).times(2)
             @catalog.apply
         end
     end
diff --git a/spec/unit/type/file/ensure.rb b/spec/unit/type/file/ensure.rb
new file mode 100755
index 000000000..be1e65110
--- /dev/null
+++ b/spec/unit/type/file/ensure.rb
@@ -0,0 +1,75 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+property = Puppet::Type.type(:file).attrclass(:ensure)
+describe property do
+    before do
+        # Wow that's a messy interface to the resource.
+        @resource = stub 'resource', :[] => nil, :[]= => nil, :property => nil, :newattr => nil, :parameter => nil
+    end
+
+    it "should be a subclass of Ensure" do
+        property.superclass.must == Puppet::Property::Ensure
+    end
+
+    describe "when retrieving the current state" do
+        it "should return :absent if the file does not exist" do
+            @ensure = property.new(:resource => @resource)
+            @resource.expects(:stat).returns nil
+
+            @ensure.retrieve.should == :absent
+        end
+
+        it "should return the current file type if the file exists" do
+            @ensure = property.new(:resource => @resource)
+            stat = mock 'stat', :ftype => "directory"
+            @resource.expects(:stat).returns stat
+
+            @ensure.retrieve.should == :directory
+        end
+    end
+
+    describe "when testing whether :ensure is in sync" do
+        before do
+            @ensure = property.new(:resource => @resource)
+            @stat = stub 'stat', :ftype => "file"
+        end
+
+        it "should be in sync if :ensure is set to :absent and the file does not exist" do
+            @ensure.should = :absent
+
+            @ensure.must be_insync(:absent)
+        end
+
+        it "should not be in sync if :ensure is set to :absent and the file exists" do
+            @ensure.should = :absent
+
+            @ensure.should_not be_insync(:file)
+        end
+
+        it "should be in sync if a normal file exists and :ensure is set to :present" do
+            @ensure.should = :present
+
+            @ensure.must be_insync(:file)
+        end
+
+        it "should be in sync if a directory exists and :ensure is set to :present" do
+            @ensure.should = :present
+
+            @ensure.must be_insync(:directory)
+        end
+
+        it "should be in sync if a symlink exists and :ensure is set to :present" do
+            @ensure.should = :present
+
+            @ensure.must be_insync(:link)
+        end
+
+        it "should not be in sync if :ensure is set to :file and a directory exists" do
+            @ensure.should = :file
+
+            @ensure.should_not be_insync(:directory)
+        end
+    end
+end
diff --git a/test/ral/type/file.rb b/test/ral/type/file.rb
index 56ae8a285..5d3a9b60b 100755
--- a/test/ral/type/file.rb
+++ b/test/ral/type/file.rb
@@ -355,102 +355,6 @@ class TestFile < Test::Unit::TestCase
         }
     end
 
-    def test_checksums
-        types = %w{md5 md5lite timestamp time}
-        exists = "/tmp/sumtest-exists"
-        nonexists = "/tmp/sumtest-nonexists"
-
-        @@tmpfiles << exists
-        @@tmpfiles << nonexists
-
-        # try it both with files that exist and ones that don't
-        files = [exists, nonexists]
-        initstorage
-        File.open(exists,File::CREAT|File::TRUNC|File::WRONLY) { |of|
-            of.puts "initial text"
-        }
-        types.each { |type|
-            files.each { |path|
-                if Puppet[:debug]
-                    Puppet.warning "Testing %s on %s" % [type,path]
-                end
-                file = nil
-                events = nil
-                # okay, we now know that we have a file...
-                assert_nothing_raised() {
-                    file = Puppet.type(:file).create(
-                        :name => path,
-                        :ensure => "file",
-                        :checksum => type
-                    )
-                }
-                trans = nil
-
-                currentvalues = file.retrieve
-
-                if file.title !~ /nonexists/
-                    sum = file.property(:checksum)
-                    assert(sum.insync?(currentvalues[sum]), "file is not in sync")
-                end
-
-                events = assert_apply(file)
-
-                assert(events)
-
-                assert(! events.include?(:file_changed), "File incorrectly changed")
-                assert_events([], file)
-
-                # We have to sleep because the time resolution of the time-based
-                # mechanisms is greater than one second
-                sleep 1 if type =~ /time/
-
-                assert_nothing_raised() {
-                    File.open(path,File::CREAT|File::TRUNC|File::WRONLY) { |of|
-                        of.puts "some more text, yo"
-                    }
-                }
-
-                # now recreate the file
-                assert_nothing_raised() {
-                    file = Puppet.type(:file).create(
-                        :name => path,
-                        :checksum => type
-                    )
-                }
-                trans = nil
-
-                assert_events([:file_changed], file)
-
-                # Run it a few times to make sure we aren't getting
-                # spurious changes.
-                sum = nil
-                assert_nothing_raised do
-                    sum = file.property(:checksum).retrieve
-                end
-                assert(file.property(:checksum).insync?(sum),
-                    "checksum is not in sync")
-
-                sleep 1.1 if type =~ /time/
-                assert_nothing_raised() {
-                    File.unlink(path)
-                    File.open(path,File::CREAT|File::TRUNC|File::WRONLY) { |of|
-                        # We have to put a certain amount of text in here or
-                        # the md5-lite test fails
-                        2.times {
-                            of.puts rand(100)
-                        }
-                        of.flush
-                    }
-                }
-                assert_events([:file_changed], file)
-
-                if path =~ /nonexists/
-                    File.unlink(path)
-                end
-            }
-        }
-    end
-
     def cyclefile(path)
         # i had problems with using :name instead of :path
         [:name,:path].each { |param|
@@ -820,21 +724,20 @@ class TestFile < Test::Unit::TestCase
             500.times { |i| f.puts "line %s" % i }
         }
 
-        obj = Puppet::Type.type(:file).create(
+        resource = Puppet::Type.type(:file).create(
             :title => dest, :source => source
         )
 
-        assert_events([:file_created], obj)
+        assert_events([:file_created], resource)
 
         File.open(source, File::APPEND|File::WRONLY) { |f| f.puts "another line" }
 
-        assert_events([:file_changed], obj)
+        assert_events([:file_changed], resource)
 
         # Now modify the dest file
         File.open(dest, File::APPEND|File::WRONLY) { |f| f.puts "one more line" }
 
-        assert_events([:file_changed, :file_changed], obj)
-
+        assert_events([:file_changed, :file_changed], resource)
     end
 
     def test_replacefilewithlink
@@ -877,48 +780,6 @@ class TestFile < Test::Unit::TestCase
         assert(FileTest.exists?(dest), "File did not get created")
     end
 
-    def test_present_matches_anything
-        path = tempfile()
-
-        file = Puppet::Type.newfile(:path => path, :ensure => :present)
-
-        currentvalues = file.retrieve
-        assert(! file.insync?(currentvalues), "File incorrectly in sync")
-
-        # Now make a file
-        File.open(path, "w") { |f| f.puts "yay" }
-
-        currentvalues = file.retrieve
-        assert(file.insync?(currentvalues), "File not in sync")
-
-        # Now make a directory
-        File.unlink(path)
-        Dir.mkdir(path)
-
-        currentvalues = file.retrieve
-        assert(file.insync?(currentvalues), "Directory not considered 'present'")
-
-        Dir.rmdir(path)
-
-        # Now make a link
-        file[:links] = :manage
-
-        otherfile = tempfile()
-        File.symlink(otherfile, path)
-
-        currentvalues = file.retrieve
-        assert(file.insync?(currentvalues), "Symlink not considered 'present'")
-        File.unlink(path)
-        file.flush
-
-        # Now set some content, and make sure it works
-        file[:content] = "yayness"
-
-        assert_apply(file)
-
-        assert_equal("yayness", File.read(path), "Content did not get set correctly")
-    end
-
     # Testing #274.  Make sure target can be used without 'ensure'.
     def test_target_without_ensure
         source = tempfile()
@@ -985,7 +846,7 @@ class TestFile < Test::Unit::TestCase
         dir = tempfile()
         Dir.mkdir(dir)
 
-        bucket = Puppet::Type.newfilebucket :name => "main"
+        bucket = Puppet::Type.type(:filebucket).create :name => "main"
         File.symlink(dir, link)
         File.open(file, "w") { |f| f.puts "" }
         assert_equal(dir, File.readlink(link))
@@ -1036,7 +897,7 @@ class TestFile < Test::Unit::TestCase
             :link => proc { File.symlink(linkdest, path) }
         }
 
-        bucket = Puppet::Type.newfilebucket :name => "main", :path => tempfile()
+        bucket = Puppet::Type.type(:filebucket).create :name => "main", :path => tempfile()
 
         obj = Puppet::Type.newfile :path => path, :force => true,
             :links => :manage
diff --git a/test/ral/type/filesources.rb b/test/ral/type/filesources.rb
index 2479eb491..5b66f0e6f 100755
--- a/test/ral/type/filesources.rb
+++ b/test/ral/type/filesources.rb
@@ -433,12 +433,10 @@ class TestFileSources < Test::Unit::TestCase
             "File got replaced when :replace was false")
 
         # Now set it to true and make sure it does change.
-        assert_nothing_raised {
-            file[:replace] = true
-        }
+        file[:replace] = true
         assert_apply(file)
 
-        # Make sure it doesn't change.
+        # Make sure it changes.
         assert_equal("funtest\n", File.read(dest),
             "File was not replaced when :replace was true")
     end