Fixing most failing test/ tests.

This is mostly just adjusting existing tests to
meet new APIs, but it's a small amount of fixing the
code to meet new standards and an even smaller amount
of porting code over.

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

lib/puppet/type/exec.rb

@@ -70,12 +70,16 @@ module Puppet
             @checks.keys
         end
 
-        newproperty(:returns, :array_matching => :all) do |property|
+        newproperty(:returns, :array_matching => :all, :event => :executed_command) do |property|
             include Puppet::Util::Execution
             munge do |value|
                 value.to_s
             end
 
+            def event_name
+                :executed_command
+            end
+
             defaultto "0"
 
             attr_reader :output

lib/puppet/type/file/ensure.rb

@@ -40,7 +40,7 @@ module Puppet
 
         aliasvalue(:false, :absent)
 
-        newvalue(:file) do
+        newvalue(:file, :event => :file_created) do
             # Make sure we're not managing the content some other way
             if property = (@resource.property(:content) || @resource.property(:source))
                 property.sync
@@ -48,17 +48,16 @@ module Puppet
                 @resource.write("", :ensure)
                 mode = @resource.should(:mode)
             end
-            return :file_created
         end
 
         #aliasvalue(:present, :file)
-        newvalue(:present) do
+        newvalue(:present, :event => :file_created) do
             # Make a file if they want something, but this will match almost
             # anything.
             set_file
         end
 
-        newvalue(:directory) do
+        newvalue(:directory, :event => :directory_created) do
             mode = @resource.should(:mode)
             parent = File.dirname(@resource[:path])
             unless FileTest.exists? parent
@@ -79,7 +78,7 @@ module Puppet
         end
 
 
-        newvalue(:link) do
+        newvalue(:link, :event => :link_created) do
             fail "Cannot create a symlink without a target" unless property = resource.property(:target)
             property.retrieve
             property.mklink

lib/puppet/type/host.rb

@@ -19,7 +19,7 @@ module Puppet
                 the "alias" metaparam; use this property to add aliases to a host
                 on disk, and "alias" to aliases for use in your Puppet scripts.'
 
-           def insync?(is)
+            def insync?(is)
                 is == @should
             end
 

spec/unit/type/exec.rb

@@ -28,6 +28,13 @@ module ExecModuleTesting
     end
 end
 
+describe Puppet::Type.type(:exec) do
+    it "should return :executed_command as its event" do
+        resource = Puppet::Type.type(:exec).new :command => "/bin/true"
+        resource.parameter(:returns).event.name.should == :executed_command
+    end
+end
+
 describe Puppet::Type.type(:exec), " when execing" do
     include ExecModuleTesting
 

test/lib/puppettest/support/assertions.rb

@@ -36,10 +36,6 @@ module PuppetTest
         FileUtils.rm(filename)
     end
 
-    def assert_rollback_events(trans, events, msg = nil)
-        run_events(:rollback, trans, events, msg)
-    end
-
     def assert_events(events, *resources)
         trans = nil
         comp = nil

test/ral/providers/parsedfile.rb

@@ -635,8 +635,7 @@ class TestParsedFile < Test::Unit::TestCase
             current_value = bill.retrieve
         end
 
-        assert(bill.insync?(current_value),
-            "An invalid field marked the record out of sync")
+        assert_events([], bill)
     end
 
     # Make sure we call the prefetch hook at the right place.

test/ral/type/cron.rb

@@ -138,12 +138,7 @@ class TestCron < Test::Unit::TestCase
             cron.provider.month = ["4"]
             cron.provider.class.prefetch
             currentvalue = cron.retrieve
-
-            currentvalue.each do |prop, value|
-                # We're only interested in comparing the command.
-                next unless prop.name.to_s == "command"
-                assert(prop.insync?(value), "Property %s is not considered in sync with value %s" % [prop.name, value.inspect])
-            end
+            assert(cron.parameter(:command).insync?(currentvalue[:command]), "Property :command is not considered in sync with value #{currentvalue[:command]}")
         end
     end
 
@@ -239,10 +234,10 @@ class TestCron < Test::Unit::TestCase
             cron.provider.class.prefetch
             currentvalue = cron.retrieve
 
-            currentvalue.each do |prop, value|
+            currentvalue.each do |name, value|
                 # We're only interested in comparing minutes.
-                next unless prop.name.to_s == "minute"
-                assert(prop.insync?(value), "Property %s is not considered in sync with value %s" % [prop.name, value.inspect])
+                next unless name.to_s == "minute"
+                assert(cron.parameter(name).insync?(value), "Property #{name} is not considered in sync with value #{value.inspect}")
             end
         end
     end
@@ -262,7 +257,7 @@ class TestCron < Test::Unit::TestCase
         cron.provider.class.prefetch
 
         cron[:minute] = :absent
-        assert_events([:cron_changed], cron)
+        assert_events([:minute_changed], cron)
 
         current_values = nil
         assert_nothing_raised {
@@ -483,9 +478,8 @@ class TestCron < Test::Unit::TestCase
         # Now make sure that it's correctly in sync
         cron.provider.class.prefetch("testing" => cron)
         properties = cron.retrieve
-        command, result = properties.find { |prop, value| prop.name == :command }
-        assert_equal(string, result, "Cron did not pick up extra spaces in command")
-        assert(command.insync?(string), "Command changed with multiple spaces")
+        assert_equal(string, properties[:command], "Cron did not pick up extra spaces in command")
+        assert(cron.parameter(:command).insync?(properties[:command]), "Command changed with multiple spaces")
     end
 end
 

test/ral/type/exec.rb

@@ -6,20 +6,6 @@ require 'puppettest'
 
 class TestExec < Test::Unit::TestCase
     include PuppetTest
-    def test_execution
-        command = nil
-        output = nil
-        assert_nothing_raised {
-            command = Puppet::Type.type(:exec).new(
-                :command => "/bin/echo"
-            )
-        }
-        assert_nothing_raised {
-            command.evaluate
-        }
-        assert_events([:executed_command], command)
-    end
-
     def test_numvsstring
         [0, "0"].each { |val|
             command = nil
@@ -138,7 +124,7 @@ class TestExec < Test::Unit::TestCase
 
         # Now change our content, so we throw a refresh
         file[:content] = "yayness"
-        assert_events([:file_changed, :triggered], file, cmd)
+        assert_events([:content_changed, :restarted], file, cmd)
         assert(FileTest.exists?(maker), "file was not made in refresh")
     end
 
@@ -200,7 +186,7 @@ class TestExec < Test::Unit::TestCase
 
         comp = mk_catalog("Testing", file, exec)
 
-        assert_events([:file_created, :executed_command], comp)
+        assert_events([:file_changed, :executed_command], comp)
     end
 
     # Verify that we auto-require any managed scripts.
@@ -424,7 +410,7 @@ class TestExec < Test::Unit::TestCase
 
         comp = mk_catalog(file, exec)
         comp.finalize
-        assert_events([:executed_command, :file_changed], comp)
+        assert_events([:executed_command, :mode_changed], comp)
 
         assert(FileTest.exists?(path), "Exec ran first")
         assert(File.stat(path).mode & 007777 == 0755)

test/ral/type/file.rb

@@ -231,11 +231,6 @@ class TestFile < Test::Unit::TestCase
                 assert_nothing_raised() {
                     file[:owner] = name
                 }
-                changes = []
-                assert_nothing_raised() {
-                    changes << file.evaluate
-                }
-                assert(changes.length > 0)
                 assert_apply(file)
                 currentvalue = file.retrieve
                 assert(file.insync?(currentvalue))
@@ -311,7 +306,6 @@ class TestFile < Test::Unit::TestCase
             assert_events([:file_created], file)
             assert_events([], file)
             assert(FileTest.file?(path), "File does not exist")
-            assert(file.insync?(file.retrieve))
             @@tmpfiles.push path
         }
     end
@@ -331,7 +325,6 @@ class TestFile < Test::Unit::TestCase
                 [path])
             assert_events([:directory_created], file)
             assert_events([], file)
-            assert(file.insync?(file.retrieve))
             assert(FileTest.directory?(path))
             @@tmpfiles.push path
         }
@@ -345,11 +338,9 @@ class TestFile < Test::Unit::TestCase
             assert_nothing_raised() {
                 file[:mode] = mode
             }
-            assert_events([:file_changed], file)
+            assert_events([:mode_changed], file)
             assert_events([], file)
 
-            assert(file.insync?(file.retrieve))
-
             assert_nothing_raised() {
                 file.delete(:mode)
             }
@@ -461,10 +452,6 @@ class TestFile < Test::Unit::TestCase
             )
         }
 
-        assert_nothing_raised {
-            file.evaluate
-        }
-
         assert_equal("directory", file.property(:type).retrieve)
 
         # And then check files
@@ -480,15 +467,6 @@ class TestFile < Test::Unit::TestCase
         assert_apply(file)
 
         assert_equal("file", file.property(:type).retrieve)
-
-        file[:type] = "directory"
-
-        currentvalues = {}
-        assert_nothing_raised { currentvalues = file.retrieve }
-
-        # The 'retrieve' method sets @should to @is, so they're never
-        # out of sync.  It's a read-only class.
-        assert(file.insync?(currentvalues))
     end
 
     def test_path
@@ -585,7 +563,7 @@ class TestFile < Test::Unit::TestCase
 
         file.retrieve
 
-        assert_events([:file_changed], file)
+        assert_events([:content_changed], file)
         file.retrieve
         assert_events([], file)
     end
@@ -964,15 +942,6 @@ class TestFile < Test::Unit::TestCase
     end
     end
 
-    # #567
-    def test_missing_files_are_in_sync
-        file = tempfile
-        obj = Puppet::Type.newfile(:path => file, :mode => 0755)
-
-        changes = obj.evaluate
-        assert(changes.empty?, "Missing file with no ensure resulted in changes")
-    end
-
     def test_root_dir_is_named_correctly
         obj = Puppet::Type.newfile(:path => '/', :mode => 0755)
         assert_equal("/", obj.title, "/ directory was changed to empty string")

test/ral/type/filesources.rb

@@ -279,32 +279,6 @@ class TestFileSources < Test::Unit::TestCase
         assert(!FileTest.exists?(name), "File with no source exists anyway")
     end
 
-    def test_alwayschecksum
-        from = tempfile()
-        to = tempfile()
-
-        File.open(from, "w") { |f| f.puts "yayness" }
-        File.open(to, "w") { |f| f.puts "yayness" }
-
-        file = nil
-
-        # Now the files should be exactly the same, so we should not see attempts
-        # at copying
-        assert_nothing_raised {
-            file = Puppet::Type.type(:file).new(
-                :path => to,
-                :source => from
-            )
-        }
-
-        currentvalue = file.retrieve
-
-        assert(currentvalue[file.property(:checksum)],
-               "File does not have a checksum property")
-
-        assert_equal(0, file.evaluate.length, "File produced changes")
-    end
-
     def test_sourcepaths
         files = []
         3.times {
@@ -334,28 +308,6 @@ class TestFileSources < Test::Unit::TestCase
         assert_equal("yee-haw", txt, "Contents do not match")
     end
 
-    # Make sure that source-copying updates the checksum on the same run
-    def test_checksumchange
-        source = tempfile()
-        dest = tempfile()
-        File.open(dest, "w") { |f| f.puts "boo" }
-        File.open(source, "w") { |f| f.puts "yay" }
-
-        file = nil
-        assert_nothing_raised {
-            file = Puppet::Type.type(:file).new(
-                :name => dest,
-                :source => source
-            )
-        }
-
-        file.retrieve
-
-        assert_events([:file_changed], file)
-        file.retrieve
-        assert_events([], file)
-    end
-
     # Make sure that source-copying updates the checksum on the same run
     def test_sourcebeatsensure
         source = tempfile()
@@ -404,40 +356,31 @@ class TestFileSources < Test::Unit::TestCase
     # Make sure files aren't replaced when replace is false, but otherwise
     # are.
     def test_replace
-        source = tempfile()
-        File.open(source, "w") { |f| f.puts "yayness" }
-
         dest = tempfile()
         file = Puppet::Type.newfile(
             :path => dest,
-            :source => source,
+            :content => "foobar",
             :recurse => true
         )
 
 
         assert_apply(file)
 
-        assert(FileTest.exists?(dest), "Did not create file")
-        assert_equal("yayness\n", File.read(dest))
+        File.open(dest, "w") { |f| f.puts "yayness" }
 
-        # Now set :replace
-        assert_nothing_raised {
-            file[:replace] = false
-        }
+        file[:replace] = false
 
-        File.open(source, "w") { |f| f.puts "funtest" }
         assert_apply(file)
 
         # Make sure it doesn't change.
         assert_equal("yayness\n", File.read(dest),
             "File got replaced when :replace was false")
 
-        # Now set it to true and make sure it does change.
         file[:replace] = true
         assert_apply(file)
 
         # Make sure it changes.
-        assert_equal("funtest\n", File.read(dest),
+        assert_equal("foobar", File.read(dest),
             "File was not replaced when :replace was true")
     end
 

test/ral/type/group.rb

@@ -73,9 +73,6 @@ class TestGroup < Test::Unit::TestCase
         group[:ensure] = :absent
         trans = assert_events([:group_removed], comp)
         assert_equal(:absent, group.provider.ensure,  "Group is present")
-
-        assert_rollback_events(trans, [:group_created], "group")
-        assert_equal(:present, group.provider.ensure,  "Group is absent")
     end
 
     # This is a bit odd, since we're not actually doing anything on the machine.

test/ral/type/host.rb

@@ -110,7 +110,7 @@ class TestHost < Test::Unit::TestCase
 
         host[:host_aliases] = %w{madstop kirby yayness}
 
-        assert_events([:host_changed], host)
+        assert_events([:host_aliases_changed], host)
 
         assert_nothing_raised {
             current_values = host.retrieve

test/ral/type/mailalias.rb

@@ -34,10 +34,7 @@ class TestMailAlias < Test::Unit::TestCase
     # This isn't much of a test, but then, it's not much of a type.
     def test_recipient_arrays
         resource = @type.new(:name => "luke", :recipient => "yay", :target => tempfile)
-        values = nil
-        assert_nothing_raised("Could not retrieve mailalias") do
-            values = resource.retrieve.inject({}) { |hash, a| hash[a[0].name] = a[1]; hash }
-        end
+        values = resource.retrieve
         assert_equal(:absent, values[:recipient])
         resource.property(:recipient).expects(:set).with(%w{yay})
         assert_nothing_raised("Could not sync mailalias") do

test/ral/type/user.rb

@@ -91,8 +91,6 @@ class TestUser < Test::Unit::TestCase
         user[:ensure] = :absent
         trans = assert_events([:user_removed], comp)
 
-        assert_rollback_events(trans, [:user_created], "user")
-
         user[:ensure] = old
         assert_apply(user)
     end
@@ -104,13 +102,13 @@ class TestUser < Test::Unit::TestCase
 
         comp = mk_catalog("commenttest", user)
 
-        trans = assert_events([:user_changed], comp, "user")
+        assert_apply user
 
         assert_equal("A different comment", user.provider.comment,
             "Comment was not changed")
 
-        assert_rollback_events(trans, [:user_changed], "user")
-
+        user[:comment] = old
+        assert_apply user
         assert_equal(old, user.provider.comment,
             "Comment was not reverted")
     end
@@ -120,17 +118,15 @@ class TestUser < Test::Unit::TestCase
         comp = mk_catalog("hometest", user)
 
         old = user.provider.home
-        user[:home] = old
-
-        trans = assert_events([], comp, "user")
+        assert_apply user
 
         user[:home] = "/tmp"
 
-        trans = assert_events([:user_changed], comp, "user")
+        assert_apply user
 
         assert_equal("/tmp", user.provider.home, "Home was not changed")
-
-        assert_rollback_events(trans, [:user_changed], "user")
+        user[:home] = old
+        assert_apply user
 
         assert_equal(old, user.provider.home, "Home was not reverted")
     end
@@ -141,7 +137,7 @@ class TestUser < Test::Unit::TestCase
 
         user[:shell] = old
 
-        trans = assert_events([], comp, "user")
+        assert_apply(user)
 
         newshell = findshell(old)
 
@@ -152,14 +148,15 @@ class TestUser < Test::Unit::TestCase
 
         user[:shell] = newshell
 
-        trans = assert_events([:user_changed], comp, "user")
+        assert_apply(user)
 
         user.retrieve
         assert_equal(newshell, user.provider.shell,
             "Shell was not changed")
 
-        assert_rollback_events(trans, [:user_changed], "user")
         user.retrieve
+        user[:shell] = old
+        assert_apply user
 
         assert_equal(old, user.provider.shell, "Shell was not reverted")
     end
@@ -190,11 +187,11 @@ class TestUser < Test::Unit::TestCase
             user[:uid] = newuid
         }
 
-        trans = assert_events([:user_changed], comp, "user")
+        assert_apply user
 
         assert_equal(newuid, user.provider.uid, "UID was not changed")
-
-        assert_rollback_events(trans, [:user_changed], "user")
+        user[:uid] = old
+        assert_apply user
 
         assert_equal(old, user.provider.uid, "UID was not reverted")
     end
@@ -296,7 +293,7 @@ class TestUser < Test::Unit::TestCase
 
         assert(!user.insync?(currentvalue), "User is incorrectly in sync")
 
-        assert_events([:user_changed], user)
+        assert_events([:user_created], user)
         assert_nothing_raised {
             currentvalue = user.retrieve
         }
@@ -367,7 +364,8 @@ class TestUser < Test::Unit::TestCase
         assert_equal(user.should(:comment), user.provider.comment,
             "Comment was not set correctly")
 
-        assert_rollback_events(trans, [:user_removed], "user")
+        user[:ensure] = :absent
+        assert_events([:user_removed], user)
 
         assert(! user.provider.exists?, "User did not get deleted")
     end
@@ -413,11 +411,7 @@ class TestUser < Test::Unit::TestCase
 
         user[:ensure] = :absent
 
-        assert_nothing_raised do
-            user.evaluate
-        end
-
-        assert(user.send(:property, :groups).insync?(nil),
+        assert(user.property(:groups).insync?(nil),
             "Groups state considered out of sync with no :should value")
     end