Fixing cron support (I hope). It now uses providers, and seems to work, at least on my os x box.

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

21 files changed, 1055 insertions, 910 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 037cd7499..8a7ac586b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,13 @@
 0.22.2 (grover)
+    Refactored cron support entirely.  Cron now uses providers, and there
+    is a single 'crontab' provider that handles user crontabs.  While this
+    refactor does not include providers for /etc/crontab or cron.d, it should
+    now be straightforward to write those providers.
+
+    Changed the parameter sorting so that the provider parameter comes
+    right after name, so the provider is available when the other parameters
+    and properties are being created.
+
     Redid some of the internals of the ParsedFile provider base class.
     It now passes a FileRecord around instead of a hash.
 
diff --git a/lib/puppet/metatype/attributes.rb b/lib/puppet/metatype/attributes.rb
index ad964b7ed..a96533964 100644
--- a/lib/puppet/metatype/attributes.rb
+++ b/lib/puppet/metatype/attributes.rb
@@ -22,8 +22,11 @@ class Puppet::Type
         namevar = self.namevar
 
         order = [namevar]
+        if self.parameters.include?(:provider)
+            order << :provider
+        end
         order << [self.properties.collect { |property| property.name },
-            self.parameters,
+            self.parameters - [:provider],
             self.metaparams].flatten.reject { |param|
                 # we don't want our namevar in there multiple times
                 param == namevar
@@ -34,6 +37,19 @@ class Puppet::Type
         return order
     end
 
+    # Retrieve an attribute alias, if there is one.
+    def self.attr_alias(param)
+        @attr_aliases[symbolize(param)]
+    end
+
+    # Create an alias to an existing attribute.  This will cause the aliased
+    # attribute to be valid when setting and retrieving values on the instance.
+    def self.set_attr_alias(hash)
+        hash.each do |new, old|
+            @attr_aliases[symbolize(new)] = symbolize(old)
+        end
+    end
+
     # Find the class associated with any given attribute.
     def self.attrclass(name)
         @attrclasses ||= {}
@@ -409,6 +425,16 @@ class Puppet::Type
 
         return hash
     end
+
+    # Return either the attribute alias or the attribute.
+    def attr_alias(name)
+        name = symbolize(name)
+        if synonym = self.class.attr_alias(name)
+            return synonym
+        else
+            return name
+        end
+    end
     
     # Are we deleting this resource?
     def deleting?
@@ -420,9 +446,7 @@ class Puppet::Type
     # Most classes won't use this.
     def is=(ary)
         param, value = ary
-        if param.is_a?(String)
-            param = param.intern
-        end
+        param = attr_alias(param)
         if self.class.validproperty?(param)
             unless prop = @parameters[param]
                 prop = self.newattr(param)
@@ -439,18 +463,16 @@ class Puppet::Type
     # value, but you can also specifically return 'is' and 'should'
     # values using 'object.is(:property)' or 'object.should(:property)'.
     def [](name)
-        if name.is_a?(String)
-            name = name.intern
+        name = attr_alias(name)
+
+        unless self.class.validattr?(name)
+            raise TypeError.new("Invalid parameter %s(%s)" % [name, name.inspect])
         end
 
         if name == :name
             name = self.class.namevar
         end
 
-        unless self.class.validattr?(name)
-            raise TypeError.new("Invalid parameter %s(%s)" % [name, name.inspect])
-        end
-
         if obj = @parameters[name]
             if obj.is_a?(Puppet::Type::Property)
                 return obj.is
@@ -466,10 +488,11 @@ class Puppet::Type
     # access to always be symbols, not strings.  This sets the 'should'
     # value on properties, and otherwise just sets the appropriate parameter.
     def []=(name,value)
+        name = attr_alias(name)
+
         unless self.class.validattr?(name)
             raise TypeError.new("Invalid parameter %s" % [name])
         end
-        name = symbolize(name)
 
         if name == :name
             name = self.class.namevar
@@ -516,7 +539,8 @@ class Puppet::Type
 
     # retrieve the 'is' value for a specified property
     def is(name)
-        if prop = @parameters[symbolize(name)] and prop.is_a?(Puppet::Type::Property)
+        name = attr_alias(name)
+        if prop = @parameters[name] and prop.is_a?(Puppet::Type::Property)
             return prop.is
         else
             return nil
@@ -525,7 +549,8 @@ class Puppet::Type
 
     # retrieve the 'should' value for a specified property
     def should(name)
-        if prop = @parameters[symbolize(name)] and prop.is_a?(Puppet::Type::Property)
+        name = attr_alias(name)
+        if prop = @parameters[name] and prop.is_a?(Puppet::Type::Property)
             return prop.should
         else
             return nil
@@ -634,7 +659,8 @@ class Puppet::Type
 
     # Return a specific value for an attribute.
     def value(name)
-        name = symbolize(name)
+        name = attr_alias(name)
+
         if obj = @parameters[name] and obj.respond_to?(:value)
             return obj.value
         else
diff --git a/lib/puppet/provider/cron/crontab.rb b/lib/puppet/provider/cron/crontab.rb
index c50bc2680..b7caa4a21 100755
--- a/lib/puppet/provider/cron/crontab.rb
+++ b/lib/puppet/provider/cron/crontab.rb
@@ -6,49 +6,130 @@ tab = case Facter.value(:operatingsystem)
         :crontab
     end
 
+
 Puppet::Type.type(:cron).provide(:crontab,
     :parent => Puppet::Provider::ParsedFile,
+    :default_target => ENV["USER"],
     :filetype => tab
 ) do
     commands :crontab => "crontab"
 
-    attr_accessor :target
-
     text_line :comment, :match => %r{^#}, :post_parse => proc { |hash|
         if hash[:line] =~ /Puppet Name: (.+)\s*$/
             hash[:name] = $1
         end
     }
 
-    text_line :blank, :match => /^\s+/
+    text_line :blank, :match => %r{^\s+}
+
+    text_line :environment, :match => %r{^\w+=}
 
-    text_line :environment, :match => %r{/^\w+=/}
+    record_line :freebsd_special, :fields => %w{special command},
+        :match => %r{^@(\w+)\s+(.+)$}, :pre_gen => proc { |hash|
+            hash[:special] = "@" + hash[:special]
+        }
 
-    record_line :crontab, :fields => %w{minute hour weekday month monthday command},
+    crontab = record_line :crontab, :fields => %w{minute hour monthday month weekday command},
         :optional => %w{minute hour weekday month monthday}, :absent => "*"
 
-    #record_line :freebsd_special, :fields => %w{special command},
-    #    :match => %r{^@(\w+)\s+(.+)}
-    # Apparently Freebsd will "helpfully" add a new TZ line to every
-    # single cron line, but not in all cases (e.g., it doesn't do it
-    # on my machine.  This is my attempt to fix it so the TZ lines don't
-    # multiply.
-    #if tab =~ /^TZ=.+$/
-    #    return tab.sub(/\n/, "\n" + self.header)
-    #else
-    #    return self.header() + tab
-    #end
+    class << crontab
+        def numeric_fields
+            fields - [:command]
+        end
+        # Do some post-processing of the parsed record.  Basically just
+        # split the numeric fields on ','.
+        def post_parse(details)
+            numeric_fields.each do |field|
+                if val = details[field] and val != :absent
+                    details[field] = details[field].split(",")
+                end
+            end
+        end
+
+        # Join the fields back up based on ','.
+        def pre_gen(details)
+            numeric_fields.each do |field|
+                if vals = details[field] and vals.is_a?(Array)
+                    details[field] = vals.join(",")
+                end
+            end
+        end
+
+
+        # Add name and environments as necessary.
+        def to_line(details)
+            str = ""
+            if details[:name]
+                str = "# Puppet Name: %s\n" % details[:name]
+            end
+            if details[:environment] and details[:environment] != :absent
+                details[:environment].each do |env|
+                    str += env + "\n"
+                end
+            end
+
+            str += join(details)
+            str
+        end
+    end
 
 
     # Return the header placed at the top of each generated file, warning
     # users that modifying this file manually is probably a bad idea.
     def self.header
-%{# HEADER This file was autogenerated at #{Time.now} by puppet.  While it
-# HEADER can still be managed manually, it is definitely not recommended.
+%{# HEADER This file was autogenerated at #{Time.now} by puppet.
+# HEADER While it can still be managed manually, it is definitely notrecommended.
 # HEADER Note particularly that the comments starting with 'Puppet Name' should
 # HEADER not be deleted, as doing so could cause duplicate cron jobs.\n}
     end
 
+    # See if we can match the hash against an existing cron job.
+    def self.match(hash)
+        model.find_all { |obj|
+            obj.value(:user) == hash[:user] and obj.value(:command) == hash[:command]
+        }.each do |obj|
+            # we now have a cron job whose command exactly matches
+            # let's see if the other fields match
+
+            # First check the @special stuff
+            if hash[:special]
+                next unless obj.value(:special) == hash[:special]
+            end
+
+            # Then the normal fields.
+            matched = true
+            record_type(hash[:record_type]).fields().each do |field|
+                next if field == :command
+                if hash[field] and ! obj.value(field)
+                    Puppet.info "Cron is missing %s: %s and %s" %
+                        [field, hash[field].inspect, obj.value(field).inspect]
+                    matched = false
+                    break
+                end
+
+                if ! hash[field] and obj.value(field)
+                    Puppet.info "Hash is missing %s: %s and %s" %
+                        [field, obj.value(field).inspect, hash[field].inspect]
+                    matched = false
+                    break
+                end
+
+                # FIXME It'd be great if I could somehow reuse how the
+                # fields are turned into text, but....
+                next if (hash[field] == :absent and obj.value(field) == "*")
+                next if (hash[field].join(",") == obj.value(field))
+                Puppet.info "Did not match %s: %s vs %s" %
+                    [field, obj.value(field).inspect, hash[field].inspect]
+                matched = false 
+                break
+            end
+            next unless matched
+            return obj
+        end
+
+        return false
+    end
+
     # Collapse name and env records.
     def self.prefetch_hook(records)
         name = nil
@@ -78,12 +159,27 @@ Puppet::Type.type(:cron).provide(:crontab,
         }.reject { |record| record[:skip] }
     end
 
+    def self.to_file(records)
+        text = super
+        # Apparently Freebsd will "helpfully" add a new TZ line to every
+        # single cron line, but not in all cases (e.g., it doesn't do it
+        # on my machine).  This is my attempt to fix it so the TZ lines don't
+        # multiply.
+        if text =~ /(^TZ=.+\n)/
+            tz = $1 
+            text.sub!(tz, '')
+            text = tz + text
+        end
+        return text
+    end
+
     def user=(user)
-        self.target = target
+        @property_hash[:user] = user
+        @property_hash[:target] = user
     end
 
     def user
-        self.target
+        @property_hash[:user] || @property_hash[:target]
     end
 end
 
diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb
index 56474e273..bb6546095 100755
--- a/lib/puppet/provider/parsedfile.rb
+++ b/lib/puppet/provider/parsedfile.rb
@@ -80,9 +80,10 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
 
     # Flush all of the records relating to a specific target.
     def self.flush_target(target)
-        target_object(target).write(to_file(target_records(target).reject { |r|
+        records = target_records(target).reject { |r|
             r[:ensure] == :absent
-        }))
+        }
+        target_object(target).write(to_file(records))
     end
 
     # Return the header placed at the top of each generated file, warning
@@ -101,7 +102,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         @target = nil
 
         # Default to flat files
-        @filetype = Puppet::Util::FileType.filetype(:flat)
+        @filetype ||= Puppet::Util::FileType.filetype(:flat)
         super
     end
 
@@ -191,7 +192,7 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         end
 
         if respond_to?(:prefetch_hook)
-            prefetch_hook(target_records)
+            target_records = prefetch_hook(target_records)
         end
 
         @records += target_records
@@ -202,8 +203,8 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
             if instance = self.model[record[:name]]
                 next unless instance.provider.is_a?(self)
                 instance.provider.property_hash = record
-            elsif self.respond_to?(:match)
-                if instance = self.match(record)
+            elsif respond_to?(:match)
+                if instance = match(record)
                     record[:name] = instance[:name]
                     instance.provider.property_hash = record
                 end
@@ -269,6 +270,12 @@ class Puppet::Provider::ParsedFile < Puppet::Provider
         targets.uniq.reject { |t| t.nil? }
     end
 
+    def self.to_file(records)
+        text = super
+        header + text
+    end
+
+
     def create
         @model.class.validproperties.each do |property|
             if value = @model.should(property)
diff --git a/lib/puppet/provider/service/smf.rb b/lib/puppet/provider/service/smf.rb
index 08f2bd731..e2e5c989a 100755
--- a/lib/puppet/provider/service/smf.rb
+++ b/lib/puppet/provider/service/smf.rb
@@ -54,6 +54,7 @@ Puppet::Type.type(:service).provide :smf, :parent => :base do
                 value = $2
             else
                 Puppet.err "Could not match %s" % line.inspect
+                next
             end
             case var
             when "state":
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 023f85ab3..bf2060018 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -73,6 +73,8 @@ class Type < Puppet::Element
         @parameters = []
         @paramhash = {}
 
+        @attr_aliases = {}
+
         @paramdoc = Hash.new { |hash,key|
           if key.is_a?(String)
             key = key.intern
diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb
index d71654586..dd9ed3ab5 100755
--- a/lib/puppet/type/cron.rb
+++ b/lib/puppet/type/cron.rb
@@ -4,453 +4,378 @@ require 'puppet/type/property'
 require 'puppet/util/filetype'
 require 'puppet/type/parsedtype'
 
-module Puppet
-    # Model the actual cron jobs.  Supports all of the normal cron job fields
-    # as parameters, with the 'command' as the single property.  Also requires a
-    # completely symbolic 'name' paremeter, which gets written to the file
-    # and is used to manage the job.
-    newtype(:cron) do
-
-        # A base class for all of the Cron parameters, since they all have
-        # similar argument checking going on.  We're stealing the base class
-        # from parsedtype, and we should probably subclass Cron from there,
-        # but it was just too annoying to do.
-        class CronParam < Puppet::Property
-            class << self
-                attr_accessor :boundaries, :default
-            end
-
-            # We have to override the parent method, because we consume the entire
-            # "should" array
-            def insync?
-                if defined? @should and @should
-                    self.is_to_s == self.should_to_s
-                else
-                    true
-                end
-            end
+Puppet::Type.newtype(:cron) do
+    @doc = "Installs and manages cron jobs.  All fields except the command 
+        and the user are optional, although specifying no periodic
+        fields would result in the command being executed every
+        minute.  While the name of the cron job is not part of the actual
+        job, it is used by Puppet to store and retrieve it.
+        
+        If you specify a cron job that matches an existing job in every way
+        except name, then the jobs will be considered equivalent and the
+        new name will be permanently associated with that job.  Once this
+        association is made and synced to disk, you can then manage the job
+        normally (e.g., change the schedule of the job).
+        
+        Example:
+            
+            cron { logrotate:
+                command => \"/usr/sbin/logrotate\",
+                user => root,
+                hour => 2,
+                minute => 0
+            }
+        "
+    ensurable
+
+    # A base class for all of the Cron parameters, since they all have
+    # similar argument checking going on.  We're stealing the base class
+    # from parsedtype, and we should probably subclass Cron from there,
+    # but it was just too annoying to do.
+    class CronParam < Puppet::Property
+        class << self
+            attr_accessor :boundaries, :default
+        end
 
-            # A method used to do parameter input handling.  Converts integers
-            # in string form to actual integers, and returns the value if it's
-            # an integer or false if it's just a normal string.
-            def numfix(num)
-                if num =~ /^\d+$/
-                    return num.to_i
-                elsif num.is_a?(Integer)
-                    return num
-                else
-                    return false
-                end
+        # We have to override the parent method, because we consume the entire
+        # "should" array
+        def insync?
+            if defined? @should and @should
+                self.is_to_s == self.should_to_s
+            else
+                true
             end
+        end
 
-            # Verify that a number is within the specified limits.  Return the
-            # number if it is, or false if it is not.
-            def limitcheck(num, lower, upper)
-                if num >= lower and num <= upper
-                    return num
-                else
-                    return false
-                end
+        # A method used to do parameter input handling.  Converts integers
+        # in string form to actual integers, and returns the value if it's
+        # an integer or false if it's just a normal string.
+        def numfix(num)
+            if num =~ /^\d+$/
+                return num.to_i
+            elsif num.is_a?(Integer)
+                return num
+            else
+                return false
             end
+        end
 
-            # Verify that a value falls within the specified array.  Does case
-            # insensitive matching, and supports matching either the entire word
-            # or the first three letters of the word.
-            def alphacheck(value, ary)
-                tmp = value.downcase
-                
-                # If they specified a shortened version of the name, then see
-                # if we can lengthen it (e.g., mon => monday).
-                if tmp.length == 3
-                    ary.each_with_index { |name, index|
-                        if name =~ /#{tmp}/i
-                            return index
-                        end
-                    }
-                else
-                    if ary.include?(tmp)
-                        return ary.index(tmp)
-                    end
-                end
-
+        # Verify that a number is within the specified limits.  Return the
+        # number if it is, or false if it is not.
+        def limitcheck(num, lower, upper)
+            if num >= lower and num <= upper
+                return num
+            else
                 return false
             end
+        end
 
-            def should_to_s
-                if @should
-                    if self.name == :command or @should[0].is_a? Symbol
-                        @should[0]
-                    else
-                        @should.join(",")
+        # Verify that a value falls within the specified array.  Does case
+        # insensitive matching, and supports matching either the entire word
+        # or the first three letters of the word.
+        def alphacheck(value, ary)
+            tmp = value.downcase
+            
+            # If they specified a shortened version of the name, then see
+            # if we can lengthen it (e.g., mon => monday).
+            if tmp.length == 3
+                ary.each_with_index { |name, index|
+                    if name =~ /#{tmp}/i
+                        return index
                     end
-                else
-                    nil
+                }
+            else
+                if ary.include?(tmp)
+                    return ary.index(tmp)
                 end
             end
 
-            def is_to_s
-                if @is
-                    unless @is.is_a?(Array)
-                        return @is
-                    end
+            return false
+        end
 
-                    if self.name == :command or @is[0].is_a? Symbol
-                        @is[0]
-                    else
-                        @is.join(",")
-                    end
+        def should_to_s
+            if @should
+                if self.name == :command or @should[0].is_a? Symbol
+                    @should[0]
                 else
-                    nil
+                    @should.join(",")
                 end
+            else
+                nil
             end
+        end
 
-            # The method that does all of the actual parameter value
-            # checking; called by all of the +param<name>=+ methods.
-            # Requires the value, type, and bounds, and optionally supports
-            # a boolean of whether to do alpha checking, and if so requires
-            # the ary against which to do the checking.
-            munge do |value|
-                # Support 'absent' as a value, so that they can remove
-                # a value
-                if value == "absent" or value == :absent
-                    return :absent
-                end
-
-                # Allow the */2 syntax
-                if value =~ /^\*\/[0-9]+$/
-                    return value
-                end
-
-                # Allow ranges
-                if value =~ /^[0-9]+-[0-9]+$/
-                    return value
-                end
-
-                if value == "*"
-                    return value
-                end
-
-                return value unless self.class.boundaries
-                lower, upper = self.class.boundaries
-                retval = nil
-                if num = numfix(value)
-                    retval = limitcheck(num, lower, upper)
-                elsif respond_to?(:alpha)
-                    # If it has an alpha method defined, then we check
-                    # to see if our value is in that list and if so we turn
-                    # it into a number
-                    retval = alphacheck(value, alpha())
+        def is_to_s
+            if @is
+                unless @is.is_a?(Array)
+                    return @is
                 end
 
-                if retval
-                    return retval.to_s
+                if self.name == :command or @is[0].is_a? Symbol
+                    @is[0]
                 else
-                    self.fail "%s is not a valid %s" %
-                        [value, self.class.name]
+                    @is.join(",")
                 end
+            else
+                nil
             end
         end
 
-        # Somewhat uniquely, this property does not actually change anything -- it
-        # just calls +@parent.sync+, which writes out the whole cron tab for
-        # the user in question.  There is no real way to change individual cron
-        # jobs without rewriting the entire cron file.
-        #
-        # Note that this means that managing many cron jobs for a given user
-        # could currently result in multiple write sessions for that user.
-        newproperty(:command, :parent => CronParam) do
-            desc "The command to execute in the cron job.  The environment
-                provided to the command varies by local system rules, and it is
-                best to always provide a fully qualified command.  The user's
-                profile is not sourced when the command is run, so if the
-                user's environment is desired it should be sourced manually.
-                
-                All cron parameters support ``absent`` as a value; this will
-                remove any existing values for that field."
-
-            def should
-                if @should
-                    if @should.is_a? Array
-                        @should[0]
-                    else
-                        devfail "command is not an array"
-                    end
-                else
-                    nil
-                end
-            end
+        def should
+            @should
         end
 
-        newproperty(:special) do
-            desc "Special schedules only supported on FreeBSD."
-
-            def specials
-                %w{reboot yearly annually monthly weekly daily midnight hourly}
+        # The method that does all of the actual parameter value
+        # checking; called by all of the +param<name>=+ methods.
+        # Requires the value, type, and bounds, and optionally supports
+        # a boolean of whether to do alpha checking, and if so requires
+        # the ary against which to do the checking.
+        munge do |value|
+            # Support 'absent' as a value, so that they can remove
+            # a value
+            if value == "absent" or value == :absent
+                return :absent
             end
 
-            validate do |value|
-                unless specials().include?(value)
-                    raise ArgumentError, "Invalid special schedule %s" %
-                        value.inspect
-                end
+            # Allow the */2 syntax
+            if value =~ /^\*\/[0-9]+$/
+                return value
             end
-        end
 
-        newproperty(:minute, :parent => CronParam) do
-            self.boundaries = [0, 59]
-            desc "The minute at which to run the cron job.
-                Optional; if specified, must be between 0 and 59, inclusive."
-        end
-
-        newproperty(:hour, :parent => CronParam) do
-            self.boundaries = [0, 23]
-            desc "The hour at which to run the cron job. Optional;
-                if specified, must be between 0 and 23, inclusive."
-        end
+            # Allow ranges
+            if value =~ /^[0-9]+-[0-9]+$/
+                return value
+            end
 
-        newproperty(:weekday, :parent => CronParam) do
-            def alpha
-                %w{sunday monday tuesday wednesday thursday friday saturday}
+            if value == "*"
+                return value
             end
-            self.boundaries = [0, 6]
-            desc "The weekday on which to run the command.
-                Optional; if specified, must be between 0 and 6, inclusive, with
-                0 being Sunday, or must be the name of the day (e.g., Tuesday)."
-        end
 
-        newproperty(:month, :parent => CronParam) do
-            def alpha
-                %w{january february march april may june july
-                    august september october november december}
+            return value unless self.class.boundaries
+            lower, upper = self.class.boundaries
+            retval = nil
+            if num = numfix(value)
+                retval = limitcheck(num, lower, upper)
+            elsif respond_to?(:alpha)
+                # If it has an alpha method defined, then we check
+                # to see if our value is in that list and if so we turn
+                # it into a number
+                retval = alphacheck(value, alpha())
             end
-            self.boundaries = [1, 12]
-            desc "The month of the year.  Optional; if specified
-                must be between 1 and 12 or the month name (e.g., December)."
-        end
 
-        newproperty(:monthday, :parent => CronParam) do
-            self.boundaries = [1, 31]
-            desc "The day of the month on which to run the
-                command.  Optional; if specified, must be between 1 and 31."
+            if retval
+                return retval.to_s
+            else
+                self.fail "%s is not a valid %s" %
+                    [value, self.class.name]
+            end
         end
+    end
 
-        newproperty(:environment) do
-            desc "Any environment settings associated with this cron job.  They
-                will be stored between the header and the job in the crontab.  There
-                can be no guarantees that other, earlier settings will not also
-                affect a given cron job.
-
-                Also, Puppet cannot automatically determine whether an existing,
-                unmanaged environment setting is associated with a given cron
-                job.  If you already have cron jobs with environment settings,
-                then Puppet will keep those settings in the same place in the file,
-                but will not associate them with a specific job.
-                
-                Settings should be specified exactly as they should appear in
-                the crontab, e.g., 'PATH=/bin:/usr/bin:/usr/sbin'.  Multiple
-                settings should be specified as an array."
-
-            validate do |value|
-                unless value =~ /^\s*(\w+)\s*=\s*(.+)\s*$/
-                    raise ArgumentError, "Invalid environment setting %s" %
-                        value.inspect
-                end
-            end
+    # Somewhat uniquely, this property does not actually change anything -- it
+    # just calls +@parent.sync+, which writes out the whole cron tab for
+    # the user in question.  There is no real way to change individual cron
+    # jobs without rewriting the entire cron file.
+    #
+    # Note that this means that managing many cron jobs for a given user
+    # could currently result in multiple write sessions for that user.
+    newproperty(:command, :parent => CronParam) do
+        desc "The command to execute in the cron job.  The environment
+            provided to the command varies by local system rules, and it is
+            best to always provide a fully qualified command.  The user's
+            profile is not sourced when the command is run, so if the
+            user's environment is desired it should be sourced manually.
+            
+            All cron parameters support ``absent`` as a value; this will
+            remove any existing values for that field."
 
-            def insync?
-                if @is.is_a? Array
-                    return @is.sort == @should.sort
+        def should
+            if @should
+                if @should.is_a? Array
+                    @should[0]
                 else
-                    return @is == @should
+                    devfail "command is not an array"
                 end
+            else
+                nil
             end
+        end
+    end
 
-            def should
-                @should
-            end
+    newproperty(:special) do
+        desc "Special schedules only supported on FreeBSD."
+
+        def specials
+            %w{reboot yearly annually monthly weekly daily midnight hourly}
         end
 
-        newparam(:name) do
-            desc "The symbolic name of the cron job.  This name
-                is used for human reference only and is generated automatically
-                for cron jobs found on the system.  This generally won't
-                matter, as Puppet will do its best to match existing cron jobs
-                against specified jobs (and Puppet adds a comment to cron jobs it
-                adds), but it is at least possible that converting from
-                unmanaged jobs to managed jobs might require manual
-                intervention.
-                
-                The names can only have alphanumeric characters plus the '-'
-                character."
-
-            isnamevar
-
-            validate do |value|
-                unless value =~ /^[-\w]+$/
-                    raise ArgumentError, "Invalid name format '%s'" % value
-                end
+        validate do |value|
+            unless specials().include?(value)
+                raise ArgumentError, "Invalid special schedule %s" %
+                    value.inspect
             end
         end
+    end
 
-        newproperty(:user) do
-            desc "The user to run the command as.  This user must
-                be allowed to run cron jobs, which is not currently checked by
-                Puppet.
-                
-                The user defaults to whomever Puppet is running as."
+    newproperty(:minute, :parent => CronParam) do
+        self.boundaries = [0, 59]
+        desc "The minute at which to run the cron job.
+            Optional; if specified, must be between 0 and 59, inclusive."
+    end
 
-            defaultto { ENV["USER"] }
+    newproperty(:hour, :parent => CronParam) do
+        self.boundaries = [0, 23]
+        desc "The hour at which to run the cron job. Optional;
+            if specified, must be between 0 and 23, inclusive."
+    end
+
+    newproperty(:weekday, :parent => CronParam) do
+        def alpha
+            %w{sunday monday tuesday wednesday thursday friday saturday}
         end
+        self.boundaries = [0, 6]
+        desc "The weekday on which to run the command.
+            Optional; if specified, must be between 0 and 6, inclusive, with
+            0 being Sunday, or must be the name of the day (e.g., Tuesday)."
+    end
 
-        @doc = "Installs and manages cron jobs.  All fields except the command 
-            and the user are optional, although specifying no periodic
-            fields would result in the command being executed every
-            minute.  While the name of the cron job is not part of the actual
-            job, it is used by Puppet to store and retrieve it.
-            
-            If you specify a cron job that matches an existing job in every way
-            except name, then the jobs will be considered equivalent and the
-            new name will be permanently associated with that job.  Once this
-            association is made and synced to disk, you can then manage the job
-            normally (e.g., change the schedule of the job).
-            
-            Example:
-                
-                cron { logrotate:
-                    command => \"/usr/sbin/logrotate\",
-                    user => root,
-                    hour => 2,
-                    minute => 0
-                }
-            "
+    newproperty(:month, :parent => CronParam) do
+        def alpha
+            %w{january february march april may june july
+                august september october november december}
+        end
+        self.boundaries = [1, 12]
+        desc "The month of the year.  Optional; if specified
+            must be between 1 and 12 or the month name (e.g., December)."
+    end
 
-        attr_accessor :uid
+    newproperty(:monthday, :parent => CronParam) do
+        self.boundaries = [1, 31]
+        desc "The day of the month on which to run the
+            command.  Optional; if specified, must be between 1 and 31."
+    end
 
-        # Convert our hash to an object
-        def self.hash2obj(hash)
-            obj = nil
-            namevar = self.namevar
-            unless hash.include?(namevar) and hash[namevar]
-                Puppet.info "Autogenerating name for %s" % hash[:command]
-                hash[:name] = "autocron-%s" % hash.object_id
+    newproperty(:environment) do
+        desc "Any environment settings associated with this cron job.  They
+            will be stored between the header and the job in the crontab.  There
+            can be no guarantees that other, earlier settings will not also
+            affect a given cron job.
+
+            Also, Puppet cannot automatically determine whether an existing,
+            unmanaged environment setting is associated with a given cron
+            job.  If you already have cron jobs with environment settings,
+            then Puppet will keep those settings in the same place in the file,
+            but will not associate them with a specific job.
+            
+            Settings should be specified exactly as they should appear in
+            the crontab, e.g., 'PATH=/bin:/usr/bin:/usr/sbin'.  Multiple
+            settings should be specified as an array."
+
+        validate do |value|
+            unless value =~ /^\s*(\w+)\s*=\s*(.+)\s*$/
+                raise ArgumentError, "Invalid environment setting %s" %
+                    value.inspect
             end
+        end
 
-            unless hash.include?(:command)
-                raise Puppet::DevError, "No command for %s" % name
-            end
-            # if the cron already exists with that name...
-            if obj = (self[hash[:name]] || match(hash))
-                # Mark the cron job as present
-                obj.is = [:ensure, :present]
-
-                # Mark all of the values appropriately
-                hash.each { |param, value|
-                    if property = obj.property(param)
-                        property.is = value
-                    elsif val = obj[param]
-                        obj[param] = val
-                    else    
-                        # There is a value on disk, but it should go away
-                        obj.is = [param, value]
-                        obj[param] = :absent
-                    end
-                }
+        def insync?
+            if @is.is_a? Array
+                return @is.sort == @should.sort
             else
-                # create a new cron job, since no existing one
-                # seems to match
-                obj = self.create(
-                    :name => hash[namevar]
-                )
+                return @is == @should
+            end
+        end
 
-                obj.is = [:ensure, :present]
+        def should
+            @should
+        end
+    end
 
-                obj.notice "created"
+    newparam(:name) do
+        desc "The symbolic name of the cron job.  This name
+            is used for human reference only and is generated automatically
+            for cron jobs found on the system.  This generally won't
+            matter, as Puppet will do its best to match existing cron jobs
+            against specified jobs (and Puppet adds a comment to cron jobs it
+            adds), but it is at least possible that converting from
+            unmanaged jobs to managed jobs might require manual
+            intervention.
+            
+            The names can only have alphanumeric characters plus the '-'
+            character."
 
-                hash.delete(namevar)
-                hash.each { |param, value|
-                    obj.is = [param, value]
-                }
-            end
+        isnamevar
 
-            instance(obj)
+        validate do |value|
+            unless value =~ /^[-\w]+$/
+                raise ArgumentError, "Invalid name format '%s'" % value
+            end
         end
+    end
 
-        # See if we can match the hash against an existing cron job.
-        def self.match(hash)
-            self.find_all { |obj|
-                obj[:user] == hash[:user] and obj.value(:command) == hash[:command][0]
-            }.each do |obj|
-                # we now have a cron job whose command exactly matches
-                # let's see if the other fields match
-
-                # First check the @special stuff
-                if hash[:special]
-                    next unless obj.value(:special) == hash[:special]
-                end
+    newproperty(:user) do
+        desc "The user to run the command as.  This user must
+            be allowed to run cron jobs, which is not currently checked by
+            Puppet.
+            
+            The user defaults to whomever Puppet is running as."
 
-                # Then the normal fields.
-                matched = true
-                fields().each do |field|
-                    next if field == :command
-                    if hash[field] and ! obj.value(field)
-                        #Puppet.info "Cron is missing %s: %s and %s" %
-                        #    [field, hash[field].inspect, obj.value(field).inspect]
-                        matched = false
-                        break
-                    end
+        defaultto { ENV["USER"] }
+    end
 
-                    if ! hash[field] and obj.value(field)
-                        #Puppet.info "Hash is missing %s: %s and %s" %
-                        #    [field, obj.value(field).inspect, hash[field].inspect]
-                        matched = false
-                        break
-                    end
+    newproperty(:target) do
+        desc "Where the cron job should be stored.  For crontab-style
+            entries this is the same as the user and defaults that way.
+            Other providers default accordingly."
 
-                    # FIXME It'd be great if I could somehow reuse how the
-                    # fields are turned into text, but....
-                    next if (hash[field] == [:absent] and obj.value(field) == "*")
-                    next if (hash[field].join(",") == obj.value(field))
-                    #Puppet.info "Did not match %s: %s vs %s" %
-                    #    [field, obj.value(field).inspect, hash[field].inspect]
-                    matched = false 
-                    break
+        defaultto {
+            if provider.is_a?(@parent.class.provider(:crontab))
+                if val = @parent.should(:user)
+                    val
+                else
+                    raise ArgumentError,
+                        "You must provide a user with crontab entries"
                 end
-                next unless matched
-                return obj
+            elsif provider.class.ancestors.include?(Puppet::Provider::ParsedFile)
+                provider.class.default_target
+            else
+                nil
             end
+        }
+    end
 
-            return false
-        end
+    # We have to reorder things so that :provide is before :target
 
-        def value(name)
-            name = symbolize(name)
-            ret = nil
-            if obj = @parameters[name]
-                ret = obj.should_to_s
+    attr_accessor :uid
 
-                if ret.nil?
-                    ret = obj.is_to_s
-                end
+    def value(name)
+        name = symbolize(name)
+        ret = nil
+        if obj = @parameters[name]
+            ret = obj.should_to_s
 
-                if ret == :absent
-                    ret = nil
-                end
+            if ret.nil?
+                ret = obj.is_to_s
             end
 
-            unless ret
-                case name
-                when :command
-                    devfail "No command, somehow"
-                when :special
-                    # nothing
-                else
-                    #ret = (self.class.validproperty?(name).default || "*").to_s
-                    ret = "*"
-                end
+            if ret == :absent
+                ret = nil
             end
+        end
 
-            ret
+        unless ret
+            case name
+            when :command
+                devfail "No command, somehow"
+            when :special
+                # nothing
+            else
+                #ret = (self.class.validproperty?(name).default || "*").to_s
+                ret = "*"
+            end
         end
+
+        ret
     end
 end
 
diff --git a/lib/puppet/type/property.rb b/lib/puppet/type/property.rb
index b4dcdfae0..2cc6beb03 100644
--- a/lib/puppet/type/property.rb
+++ b/lib/puppet/type/property.rb
@@ -201,11 +201,15 @@ class Property < Puppet::Parameter
                     return event
                 end
             else
-                event = case self.should
-                when :present: (@parent.class.name.to_s + "_created").intern
-                when :absent: (@parent.class.name.to_s + "_removed").intern
+                if self.class.name == :ensure
+                    event = case self.should
+                    when :present: (@parent.class.name.to_s + "_created").intern
+                    when :absent: (@parent.class.name.to_s + "_removed").intern
+                    else
+                        (@parent.class.name.to_s + "_changed").intern
+                    end
                 else
-                    (@parent.class.name.to_s + "_changed").intern
+                    event = (@parent.class.name.to_s + "_changed").intern
                 end
             end
         end
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 3bd7d277c..fa4d0fae4 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -324,9 +324,9 @@ module Util
                         Process.uid = uid unless @@os == "Darwin"
                     end
                     if command.is_a?(Array)
-                        system(*command)
+                        Kernel.exec(*command)
                     else
-                        system(command)
+                        Kernel.exec(command)
                     end
                     exit!($?.exitstatus)
                 rescue => detail
diff --git a/lib/puppet/util/fileparsing.rb b/lib/puppet/util/fileparsing.rb
index 08f1c79d9..ed92c398d 100644
--- a/lib/puppet/util/fileparsing.rb
+++ b/lib/puppet/util/fileparsing.rb
@@ -75,6 +75,25 @@ module Puppet::Util::FileParsing
             end
         end
 
+        # Convert a record into a line by joining the fields together appropriately.
+        # This is pulled into a separate method so it can be called by the hooks.
+        def join(details)
+            joinchar = self.joiner
+
+            fields.collect { |field|
+                # If the field is marked absent, use the appropriate replacement
+                if details[field] == :absent or details[field].nil?
+                    if self.optional.include?(field)
+                        self.absent
+                    else
+                        raise ArgumentError, "Field %s is required" % field
+                    end
+                else
+                    details[field].to_s
+                end
+            }.reject { |c| c.nil?}.join(joinchar)
+        end
+
         # Customize this so we can do a bit of validation.
         def optional=(optional)
             @optional = optional.collect do |field|
@@ -91,6 +110,10 @@ module Puppet::Util::FileParsing
         def pre_gen=(block)
             meta_def(:pre_gen, &block)
         end
+
+        def to_line=(block)
+            meta_def(:to_line, &block)
+        end
     end
 
     # Clear all existing record definitions.  Only used for testing.
@@ -118,28 +141,25 @@ module Puppet::Util::FileParsing
 
     # Try to match a record.
     def handle_record_line(line, record)
+        ret = nil
         if record.respond_to?(:process)
             if ret = record.send(:process, line.dup)
                 unless ret.is_a?(Hash)
                     raise Puppet::DevError,
                         "Process record type %s returned non-hash" % record.name
                 end
-                ret[:record_type] = record.name
-                return ret
             else
                 return nil
             end
         elsif regex = record.match
-            raise "Cannot use matches to handle records yet"
             # In this case, we try to match the whole line and then use the
             # match captures to get our fields.
             if match = regex.match(line)
                 fields = []
-                ignore = record.ignore || []
-                match.captures.each_with_index do |value, i|
-                    fields << value unless ignore.include? i
+                ret = {}
+                record.fields.zip(match.captures).each do |f, v|
+                    ret[f] = v
                 end
-                nil
             else
                 Puppet.info "Did not match %s" % line
                 nil
@@ -168,8 +188,13 @@ module Puppet::Util::FileParsing
                 val = ([ret[last_field]] + line_fields).join(record.joiner)
                 ret[last_field] = val
             end
+        end
+
+        if ret
             ret[:record_type] = record.name
             return ret
+        else
+            return nil
         end
     end
 
@@ -200,13 +225,7 @@ module Puppet::Util::FileParsing
             raise Puppet::DevError, "No record types defined; cannot parse lines"
         end
 
-        @record_order.each do |name|
-            record = record_type(name)
-            unless record
-                raise Puppet::DevError, "Did not get record type for %s: %s" %
-                    [name, @record_types.inspect]
-            end
-
+        @record_order.each do |record|
             # These are basically either text or record lines.
             method = "handle_%s_line" % record.type
             if respond_to?(method)
@@ -290,20 +309,11 @@ module Puppet::Util::FileParsing
         case record.type
         when :text: return details[:line]
         else
-            joinchar = record.joiner
+            if record.respond_to?(:to_line)
+                return record.to_line(details)
+            end
 
-            line = record.fields.collect { |field|
-                # If the field is marked absent, use the appropriate replacement
-                if details[field] == :absent or details[field].nil?
-                    if record.optional.include?(field)
-                        record.absent
-                    else
-                        raise ArgumentError, "Field %s is required" % field
-                    end
-                else
-                    details[field].to_s
-                end
-            }.reject { |c| c.nil?}.join(joinchar)
+            line = record.join(details)
 
             if regex = record.rts
                 # If they say true, then use whitespace; else, use their regex.
@@ -340,6 +350,7 @@ module Puppet::Util::FileParsing
     end
 
     private
+
     # Define a new type of record.
     def new_line_type(record)
         @record_types ||= {}
@@ -350,7 +361,7 @@ module Puppet::Util::FileParsing
         end
 
         @record_types[record.name] = record
-        @record_order << record.name
+        @record_order << record
 
         return record
     end
diff --git a/test/data/providers/cron/crontab.allthree b/test/data/providers/cron/crontab.allthree
new file mode 100644
index 000000000..dd2a40466
--- /dev/null
+++ b/test/data/providers/cron/crontab.allthree
@@ -0,0 +1,17 @@
+--- 
+- |
+  # comment 1
+  # Puppet Name: name2
+  env3=val
+  * * * * * command4
+  # comment 5
+
+- - :record_type: :comment
+    :line: "# comment 1"
+  - :command: command4
+    :environment: 
+    - env3=val
+    :name: name2
+    :record_type: :crontab
+  - :record_type: :comment
+    :line: "# comment 5"
diff --git a/test/data/providers/cron/crontab.envNcomment b/test/data/providers/cron/crontab.envNcomment
new file mode 100644
index 000000000..63effe076
--- /dev/null
+++ b/test/data/providers/cron/crontab.envNcomment
@@ -0,0 +1,12 @@
+--- 
+- |
+  # comment 9
+  env10=val
+  * * * * * command11
+
+- - :record_type: :comment
+    :line: "# comment 9"
+  - :record_type: :environment
+    :line: env10=val
+  - :command: command11
+    :record_type: :crontab
diff --git a/test/data/providers/cron/crontab.envNname b/test/data/providers/cron/crontab.envNname
new file mode 100644
index 000000000..cc058248e
--- /dev/null
+++ b/test/data/providers/cron/crontab.envNname
@@ -0,0 +1,11 @@
+--- 
+- |
+  env6=val
+  # Puppet Name: name7
+  * * * * * command8
+
+- - :record_type: :environment
+    :line: env6=val
+  - :command: command8
+    :record_type: :crontab
+    :name: name7
diff --git a/test/data/providers/cron/crontab.multirecords b/test/data/providers/cron/crontab.multirecords
new file mode 100644
index 000000000..63effe076
--- /dev/null
+++ b/test/data/providers/cron/crontab.multirecords
@@ -0,0 +1,12 @@
+--- 
+- |
+  # comment 9
+  env10=val
+  * * * * * command11
+
+- - :record_type: :comment
+    :line: "# comment 9"
+  - :record_type: :environment
+    :line: env10=val
+  - :command: command11
+    :record_type: :crontab
diff --git a/test/data/types/cron/freebsd b/test/data/providers/cron/examples/freebsd
index fba1e310b..fba1e310b 100644
--- a/test/data/types/cron/freebsd
+++ b/test/data/providers/cron/examples/freebsd
diff --git a/test/data/types/cron/one b/test/data/providers/cron/examples/one
index 9cddf97e7..9cddf97e7 100644
--- a/test/data/types/cron/one
+++ b/test/data/providers/cron/examples/one
diff --git a/test/ral/manager/attributes.rb b/test/ral/manager/attributes.rb
index 6fd132aef..f27b0513a 100755
--- a/test/ral/manager/attributes.rb
+++ b/test/ral/manager/attributes.rb
@@ -182,6 +182,51 @@ class TestTypeAttributes < Test::Unit::TestCase
         end
         assert_equal(false, inst[:falsetest], "false default was not set")
     end
+
+    def test_alias_parameter
+        type = mktype
+        type.newparam(:name) {}
+        type.newparam(:one) {}
+        type.newproperty(:two) {}
+        
+        aliases = {
+            :three => :one,
+            :four => :two
+        }
+        aliases.each do |new, old|
+            assert_nothing_raised("Could not create alias parameter %s" % new) do
+                type.set_attr_alias new => old
+            end
+        end
+
+        aliases.each do |new, old|
+            assert_equal(old, type.attr_alias(new), "did not return alias info for %s" %
+                new)
+        end
+
+        assert_nil(type.attr_alias(:name), "got invalid alias info for name")
+
+        inst = type.create(:name => "my name")
+        assert(inst, "could not create instance")
+
+        aliases.each do |new, old|
+            val = "value %s" % new
+            assert_nothing_raised do
+                inst[new] = val
+            end
+
+            case old
+            when :one: # param
+                assert_equal(val, inst[new],
+                    "Incorrect alias value for %s in []" % new)
+            else
+                assert_equal(val, inst.should(new),
+                    "Incorrect alias value for %s in should" % new)
+            end
+            assert_equal(val, inst.value(new), "Incorrect alias value for %s" % new)
+            assert_equal(val, inst.value(old), "Incorrect orig value for %s" % old)
+        end
+    end
 end
 
 # $Id$
diff --git a/test/ral/manager/provider.rb b/test/ral/manager/provider.rb
new file mode 100755
index 000000000..6075697d6
--- /dev/null
+++ b/test/ral/manager/provider.rb
@@ -0,0 +1,52 @@
+#!/usr/bin/env ruby
+
+$:.unshift("../../lib") if __FILE__ =~ /\.rb$/
+
+require 'puppettest'
+require 'mocha'
+
+class TestTypeProviders < Test::Unit::TestCase
+	include PuppetTest
+
+    # Make sure default providers behave correctly
+    def test_defaultproviders
+        # Make a fake type
+        type = Puppet::Type.newtype(:defaultprovidertest) do
+            newparam(:name) do end
+        end
+
+        basic = type.provide(:basic) do
+            defaultfor :operatingsystem => :somethingelse,
+                :operatingsystemrelease => :yayness
+        end
+
+        assert_equal(basic, type.defaultprovider)
+        type.defaultprovider = nil
+
+        greater = type.provide(:greater) do
+            defaultfor :operatingsystem => Facter.value("operatingsystem")
+        end
+
+        assert_equal(greater, type.defaultprovider)
+    end
+
+    # Make sure the provider is always the first parameter created.
+    def test_provider_sorting
+        type = Puppet::Type.newtype(:sorttest) do
+            newparam(:name) {}
+            ensurable
+        end
+        cleanup { Puppet::Type.rmtype(:sorttest) }
+
+        should = [:name, :ensure]
+        assert_equal(should, type.allattrs.reject { |p| ! should.include?(p) },
+            "Got wrong order of parameters")
+
+        type.provide(:yay) { }
+        should = [:name, :provider, :ensure]
+        assert_equal(should, type.allattrs.reject { |p| ! should.include?(p) },
+            "Providify did not reorder parameters")
+    end
+end
+
+# $Id$
diff --git a/test/ral/providers/cron/crontab.rb b/test/ral/providers/cron/crontab.rb
index c4f1031cd..6d0d1e8a9 100755
--- a/test/ral/providers/cron/crontab.rb
+++ b/test/ral/providers/cron/crontab.rb
@@ -14,7 +14,9 @@ class TestCronParsedProvider < Test::Unit::TestCase
 
     def setup
         super
-        @provider = Puppet::Type.type(:cron).provider(:crontab)
+        @type = Puppet::Type.type(:cron)
+        @provider = @type.provider(:crontab)
+        @provider.initvars
 
         @oldfiletype = @provider.filetype
     end
@@ -30,7 +32,7 @@ class TestCronParsedProvider < Test::Unit::TestCase
         fields = [:month, :weekday, :monthday, :hour, :command, :minute]
         {
             "* * * * * /bin/echo" => {:command => "/bin/echo"},
-            "10 * * * * /bin/echo test" => {:minute => "10",
+            "10 * * * * /bin/echo test" => {:minute => ["10"],
                 :command => "/bin/echo test"}
         }.each do |line, should|
             result = nil
@@ -50,113 +52,44 @@ class TestCronParsedProvider < Test::Unit::TestCase
         end
     end
 
-    def test_prefetch_hook
-        count = 0
-        env = Proc.new do
-            count += 1
-            {:record_type => :environment, :line => "env%s=val" % count}
-        end
-        comment = Proc.new do |name|
-            count += 1
-            hash = {:record_type => :comment, :line => "comment %s" % count}
-            if name
-                hash[:name] = name
-            end
-            hash
-        end
-        record = Proc.new do
-            count += 1
-            {:record_type => :crontab, :command => "command%s" % count}
-        end
+    def test_parse_and_generate
+        @provider.filetype = :ram
+        crondir = datadir(File.join(%w{providers cron}))
+        files = Dir.glob("%s/crontab.*" % crondir)
 
-        result = nil
-        args = []
-        # First try it with all three
-        args << comm = comment.call(false)
-        args << name = comment.call(true)
-        args << var = env.call()
-        args << line = record.call
-        args << sec = comment.call(false)
-        assert_nothing_raised do
-            result = @provider.prefetch_hook(args)
-        end
-        assert_equal([comm, line, sec], result,
-            "Did not remove name and var records")
-
-        assert_equal(name[:name], line[:name],
-            "did not set name in hook")
-        assert_equal([var[:line]], line[:environment],
-            "did not set env")
-
-        # Now try it with an env, a name, and a record
-        args.clear
-        args << var = env.call()
-        args << name = comment.call(true)
-        args << line = record.call
-        assert_nothing_raised do
-            result = @provider.prefetch_hook(args)
-        end
+        setme
+        target = @provider.target_object(@me)
+        files.each do |file|
+            str = args = nil
+            assert_nothing_raised("could not load %s" % file) do
+                str, args = YAML.load(File.read(file))
+            end
+            target.write(str)
+            assert_nothing_raised("could not parse %s" % file) do
+                @provider.prefetch_target(@me)
+            end
+            records = @provider.send(:instance_variable_get, "@records")
 
-        assert_equal([var, line], result,
-            "Removed var record")
-        assert_equal(name[:name], line[:name],
-            "did not set name in hook")
-        assert_nil(line[:environment], "incorrectly set env")
-
-        # try it with a comment, an env, and a record
-        args.clear
-        args << comm = comment.call(false)
-        args << var = env.call()
-        args << line = record.call
-        assert_nothing_raised do
-            result = @provider.prefetch_hook(args)
-        end
-        assert_equal([comm, var, line], result,
-            "Removed var record")
-        assert_nil(line[:name], "name got set somehow")
-        assert_nil(line[:environment], "env got set somehow")
-
-        # Try it with multiple records
-        args = []
-        should = []
-        args << startcom = comment.call(false)
-        should << startcom
-        args << startenv = env.call
-        should << startenv
-
-        args << name1 = comment.call(true)
-        args << env1s = env.call
-        args << env1m = env.call
-        args << line1 = record.call
-        should << line1
-
-        args << midcom = comment.call(false)
-        args << midenv = env.call
-        should << midcom << midenv
-
-        args << name2 = comment.call(true)
-        args << env2s = env.call
-        args << env2m = env.call
-        args << line2 = record.call
-        should << line2
-
-        args << endcom = comment.call(false)
-        args << endenv = env.call
-        should << endcom << endenv
-
-        assert_nothing_raised do
-            result = @provider.prefetch_hook(args)
-        end
-        assert_equal(should, result,
-            "Did not handle records correctly")
+            args.zip(records) do |should, sis|
+                # Make the values a bit more equal.
+                should[:target] = @me
+                should[:ensure] = :present
+                should[:on_disk] = true
+                is = sis.dup
+                sis.dup.each do |p,v|
+                    is.delete(p) if v == :absent
+                end
+                assert_equal(should, is,
+                    "Did not parse %s correctly" % file)
+            end
 
-        assert_equal(line1[:name], line1[:name], "incorrectly set first name")
-        assert_equal(line1[:environment], line1[:environment],
-            "incorrectly set first env")
+            assert_nothing_raised("could not generate %s" % file) do
+                @provider.flush_target(@me)
+            end
 
-        assert_equal(line2[:name], line2[:name], "incorrectly set second name")
-        assert_equal(line2[:environment], line2[:environment],
-            "incorrectly set second env")
+            assert_equal(str, target.read, "%s changed" % file)
+            @provider.clear
+        end
     end
 
     # A simple test to see if we can load the cron from disk.
@@ -168,6 +101,227 @@ class TestCronParsedProvider < Test::Unit::TestCase
         }
         assert_instance_of(Array, records, "did not get correct response")
     end
+
+    # Test that a cron job turns out as expected, by creating one and generating
+    # it directly
+    def test_simple_to_cron
+        # make the cron
+        setme()
+
+        name = "yaytest"
+        args = {:name => name,
+            :command => "date > /dev/null",
+            :minute => "30",
+            :user => @me,
+            :record_type => :crontab
+        }
+        # generate the text
+        str = nil
+        assert_nothing_raised {
+            str = @provider.to_line(args)
+        }
+
+        assert_equal("# Puppet Name: #{name}\n30 * * * * date > /dev/null", str,
+            "Cron did not generate correctly")
+    end
+    
+    # Test that comments are correctly retained
+    def test_retain_comments
+        str = "# this is a comment\n#and another comment\n"
+        user = "fakeuser"
+        records = nil
+        target = @provider.filetype = :ram
+        target = @provider.target_object(user)
+        target.write(str)
+        assert_nothing_raised {
+            @provider.prefetch_target(user)
+        }
+
+        assert_nothing_raised {
+            newstr = @provider.flush_target(user)
+            assert(target.read.include?(str), "Comments were lost")
+        }
+    end
+
+    def test_simpleparsing
+        @provider.filetype = :ram
+        text = "5 1,2 * 1 0 /bin/echo funtest"
+
+        records = nil
+        assert_nothing_raised {
+            records = @provider.parse(text)
+        }
+
+        should = {
+            :minute => %w{5},
+            :hour => %w{1 2},
+            :monthday => :absent,
+            :month => %w{1},
+            :weekday => %w{0},
+            :command => "/bin/echo funtest"
+        }
+
+        is = records.shift
+        assert(is, "Did not get record")
+
+        should.each do |p, v|
+            assert_equal(v, is[p], "did not parse %s correctly" % p)
+        end
+    end
+
+    # Make sure we can create a cron in an empty tab
+    def test_mkcron_if_empty
+        setme
+        @provider.filetype = @oldfiletype
+
+        records = @provider.retrieve(@me)
+
+        target = @provider.target_object(@me)
+
+        cleanup do
+            if records.length == 0
+                target.remove
+            else
+                target.write(@provider.to_file(records))
+            end
+        end
+
+        # Now get rid of it
+        assert_nothing_raised("Could not remove cron tab") do
+            target.remove
+        end
+
+        @provider.flush :target => @me, :command => "/do/something",
+            :record_type => :crontab
+        created = @provider.retrieve(@me)
+        assert(created.detect { |r| r[:command] == "/do/something" },
+            "Did not create cron tab")
+    end
+
+    # Make sure we correctly bidirectionally parse things.
+    def test_records_and_strings
+        @provider.filetype = :ram
+        setme
+
+        target = @provider.target_object(@me)
+        
+        [
+            "* * * * * /some/command",
+            "0,30 * * * * /some/command",
+            "0-30 * * * * /some/command",
+            "# Puppet Name: name\n0-30 * * * * /some/command",
+            "# Puppet Name: name\nVAR=VALUE\n0-30 * * * * /some/command",
+            "# Puppet Name: name\nVAR=VALUE\nC=D\n0-30 * * * * /some/command",
+            "0 * * * * /some/command"
+        ].each do |str|
+            @provider.initvars
+            str += "\n"
+            target.write(str)
+            assert_equal(str, target.read,
+                "Did not write correctly")
+            assert_nothing_raised("Could not prefetch with %s" % str.inspect) do
+                @provider.prefetch_target(@me)
+            end
+            assert_nothing_raised("Could not flush with %s" % str.inspect) do
+                @provider.flush_target(@me)
+            end
+
+            assert_equal(str, target.read,
+                "Changed in read/write")
+
+            @provider.clear
+        end
+    end
+
+    # Test that a specified cron job will be matched against an existing job
+    # with no name, as long as all fields match
+    def test_matchcron
+        str = "0,30 * * * * date\n"
+        setme
+        @provider.filetype = :ram
+
+        cron = nil
+        assert_nothing_raised {
+            cron = @type.create(
+                :name => "yaycron",
+                :minute => [0, 30],
+                :command => "date",
+                :user => @me
+            )
+        }
+
+        hash = @provider.parse_line(str)
+        hash[:user] = @me
+
+        instance = @provider.match(hash)
+        assert(instance, "did not match cron")
+        assert_equal(cron, instance,
+            "Did not match cron job")
+    end
+
+    def test_data
+        setme
+        @provider.filetype = :ram
+        target = @provider.target_object(@me)
+        fakedata("data/providers/cron/examples").each do |file|
+            text = File.read(file)
+            target.write(text)
+
+            assert_nothing_raised("Could not parse %s" % file) do
+                @provider.prefetch_target(@me)
+            end
+            # mark the provider modified
+            @provider.modified(@me)
+
+            # and zero the text
+            target.write("")
+
+            result = nil
+            assert_nothing_raised("Could not generate %s" % file) do
+                @provider.flush_target(@me)
+            end
+
+            assert_equal(text, target.read,
+                "File was not rewritten the same")
+
+            @provider.clear
+        end
+    end
+
+    # Match freebsd's annoying @daily stuff.
+    def test_match_freebsd_special
+        @provider.filetype = :ram
+        setme
+
+        target = @provider.target_object(@me)
+        
+        [
+            "@daily /some/command",
+            "@daily /some/command more"
+        ].each do |str|
+            @provider.initvars
+            str += "\n"
+            target.write(str)
+            assert_equal(str, target.read,
+                "Did not write correctly")
+            assert_nothing_raised("Could not prefetch with %s" % str.inspect) do
+                @provider.prefetch_target(@me)
+            end
+            records = @provider.send(:instance_variable_get, "@records")
+            records.each do |r|
+                assert_equal(:freebsd_special, r[:record_type],
+                    "Did not create lines as freebsd lines")
+            end
+            assert_nothing_raised("Could not flush with %s" % str.inspect) do
+                @provider.flush_target(@me)
+            end
+
+            assert_equal(str, target.read,
+                "Changed in read/write")
+
+            @provider.clear
+        end
+    end
 end
 
 # $Id$
diff --git a/test/ral/types/cron.rb b/test/ral/types/cron.rb
index f4b9719ea..96d7cb3f6 100755
--- a/test/ral/types/cron.rb
+++ b/test/ral/types/cron.rb
@@ -3,18 +3,38 @@
 $:.unshift("../../lib") if __FILE__ =~ /\.rb$/
 
 require 'puppettest'
+require 'spec'
 
 # Test cron job creation, modification, and destruction
 
 class TestCron < Test::Unit::TestCase
 	include PuppetTest
+
     def setup
         super
 
         setme()
 
-        # god i'm lazy
-        @crontype = Puppet.type(:cron)
+        @crontype = Puppet::Type.type(:cron)
+        @provider = @crontype.defaultprovider
+        if @provider.respond_to?(:filetype=)
+            @oldfiletype = @provider.filetype
+            @provider.filetype = :ram
+        end
+        @crontype = Puppet::Type.type(:cron)
+    end
+
+    def teardown
+        @crontype.defaultprovider = nil
+        if defined? @oldfiletype
+            @provider.filetype = @oldfiletype
+        end
+    end
+
+    def eachprovider
+        @crontype.suitableprovider.each do |provider|
+            yield provider
+        end
     end
 
     # Back up the user's existing cron tab if they have one.
@@ -76,6 +96,7 @@ class TestCron < Test::Unit::TestCase
         comp = newcomp(name, cron)
 
         assert_events([:cron_created], comp)
+        cron.provider.class.prefetch
         cron.retrieve
 
         assert(cron.insync?, "Cron is not in sync")
@@ -92,187 +113,47 @@ class TestCron < Test::Unit::TestCase
 
         assert_events([:cron_removed], comp)
 
+        cron.provider.class.prefetch
         cron.retrieve
 
         assert(cron.insync?, "Cron is not in sync")
         assert_events([], comp)
     end
 
-    # Test that a cron job turns out as expected, by creating one and generating
-    # it directly
-    def test_simple_to_cron
-        cron = nil
-        # make the cron
-        name = "yaytest"
-        assert_nothing_raised {
-            cron = @crontype.create(
-                :name => name,
-                :command => "date > /dev/null",
-                :user => @me
-            )
-        }
-        str = nil
-        # generate the text
-        assert_nothing_raised {
-            str = cron.to_record
-        }
-
-        assert_equal(str, "# Puppet Name: #{name}\n* * * * * date > /dev/null",
-            "Cron did not generate correctly")
-    end
-
-    def test_simpleparsing
-        @fakefiletype = Puppet::Util::FileType.filetype(:ram)
-        @crontype.filetype = @fakefiletype
-
-        @crontype.retrieve(@me)
-        obj = Puppet::Type::Cron.cronobj(@me)
-
-        text = "5 1,2 * 1 0 /bin/echo funtest"
-
-        assert_nothing_raised {
-            @crontype.parse(@me, text)
-        }
-
-        @crontype.each do |obj|
-            assert_equal(["5"], obj.is(:minute), "Minute was not parsed correctly")
-            assert_equal(["1", "2"], obj.is(:hour), "Hour was not parsed correctly")
-            assert_equal([:absent], obj.is(:monthday), "Monthday was not parsed correctly")
-            assert_equal(["1"], obj.is(:month), "Month was not parsed correctly")
-            assert_equal(["0"], obj.is(:weekday), "Weekday was not parsed correctly")
-            assert_equal(["/bin/echo funtest"], obj.is(:command), "Command was not parsed correctly")
-        end
-    end
-
-    # Test that changing any field results in the cron tab being rewritten.
-    # it directly
-    def test_any_field_changes
-        cron = nil
-        # make the cron
-        name = "yaytest"
-        assert_nothing_raised {
-            cron = @crontype.create(
-                :name => name,
-                :command => "date > /dev/null",
-                :month => "May",
-                :user => @me
-            )
-        }
-        assert(cron, "Cron did not get created")
-        comp = newcomp(cron)
-        assert_events([:cron_created], comp)
-
-        assert_nothing_raised {
-            cron[:month] = "June"
-        }
-
-        cron.retrieve
-
-        assert_events([:cron_changed], comp)
-    end
-
     # Test that a cron job with spaces at the end doesn't get rewritten
     def test_trailingspaces
-        cron = nil
-        # make the cron
-        name = "yaytest"
-        assert_nothing_raised {
-            cron = @crontype.create(
-                :name => name,
-                :command => "date > /dev/null ",
-                :month => "May",
-                :user => @me
-            )
-        }
-        comp = newcomp(cron)
-
-        assert_events([:cron_created], comp, "did not create cron job")
-        cron.retrieve
-        assert_events([], comp, "cron job got rewritten")
-    end
-    
-    # Test that comments are correctly retained
-    def test_retain_comments
-        str = "# this is a comment\n#and another comment\n"
-        user = "fakeuser"
-        @crontype.retrieve(@me)
-        assert_nothing_raised {
-            @crontype.parse(@me, str)
-        }
-
-        assert_nothing_raised {
-            newstr = @crontype.tab(@me)
-            assert(newstr.include?(str), "Comments were lost")
-        }
-    end
-
-    # Test that a specified cron job will be matched against an existing job
-    # with no name, as long as all fields match
-    def test_matchcron
-        str = "0,30 * * * * date\n"
-
-        assert_nothing_raised {
-            cron = @crontype.create(
-                :name => "yaycron",
-                :minute => [0, 30],
-                :command => "date",
-                :user => @me
-            )
-        }
-
-        assert_nothing_raised {
-            @crontype.parse(@me, str)
-        }
+        eachprovider do |provider|
+            cron = nil
+            # make the cron
+            name = "yaytest"
+            command = "date > /dev/null "
+            assert_nothing_raised {
+                cron = @crontype.create(
+                    :name => name,
+                    :command => "date > /dev/null ",
+                    :month => "May",
+                    :user => @me
+                )
+            }
+            property = cron.send(:property, :command)
+            cron.provider.command = command
+            cron.provider.class.prefetch
+            cron.retrieve
 
-        count = @crontype.inject(0) do |c, obj|
-            c + 1
+            assert(property.insync?, "command parsing removes trailing whitespace")
+            @crontype.clear
         end
-
-        assert_equal(1, count, "Did not match cron job")
-
-        modstr = "# Puppet Name: yaycron\n%s" % str
-
-        assert_nothing_raised {
-            newstr = @crontype.tab(@me)
-            assert(newstr.include?(modstr),
-                "Cron was not correctly matched")
-        }
-    end
-
-    # Test adding a cron when there is currently no file.
-    def test_mkcronwithnotab
-        tab = @fakefiletype.new(@me)
-        tab.remove
-
-        @crontype.retrieve(@me)
-        cron = mkcron("testwithnotab")
-        cyclecron(cron)
-    end
-
-    def test_mkcronwithtab
-        @crontype.retrieve(@me)
-        obj = Puppet::Type::Cron.cronobj(@me)
-        obj.write(
-"1 1 1 1 * date > %s/crontesting\n" % tstdir()
-        )
-
-        cron = mkcron("testwithtab")
-        cyclecron(cron)
     end
 
     def test_makeandretrievecron
-        tab = @fakefiletype.new(@me)
-        tab.remove
-
         %w{storeandretrieve a-name another-name more_naming SomeName}.each do |name|
             cron = mkcron(name)
             comp = newcomp(name, cron)
             trans = assert_events([:cron_created], comp, name)
             
+            cron.provider.class.prefetch
             cron = nil
 
-            Puppet.type(:cron).retrieve(@me)
-
             assert(cron = Puppet.type(:cron)[name], "Could not retrieve named cron")
             assert_instance_of(Puppet.type(:cron), cron)
         end
@@ -336,78 +217,80 @@ class TestCron < Test::Unit::TestCase
         }
     end
 
-    # Test that we can read and write cron tabs
-    def test_crontab
-        Puppet.type(:cron).filetype = Puppet.type(:cron).defaulttype
-        type = nil
-        unless type = Puppet.type(:cron).filetype
-            $stderr.puts "No crontab type; skipping test"
-        end
-
-        obj = nil
-        assert_nothing_raised {
-            obj = type.new(Puppet::Util::SUIDManager.uid)
-        }
-
-        txt = nil
-        assert_nothing_raised {
-            txt = obj.read
-        }
-
-        assert_nothing_raised {
-            obj.write(txt)
-        }
-    end
-
     # Verify that comma-separated numbers are not resulting in rewrites
-    def test_norewrite
-        cron = nil
-        assert_nothing_raised {
-            cron = Puppet.type(:cron).create(
-                :command => "/bin/date > /dev/null",
-                :minute => [0, 30],
-                :name => "crontest"
-            )
-        }
+    def test_comma_separated_vals_work
+        eachprovider do |provider|
+            cron = nil
+            assert_nothing_raised {
+                cron = @crontype.create(
+                    :command => "/bin/date > /dev/null",
+                    :minute => [0, 30],
+                    :name => "crontest",
+                    :provider => provider.name
+                )
+            }
 
-        assert_events([:cron_created], cron)
-        cron.retrieve
-        assert_events([], cron)
+            minute = cron.send(:property, :minute)
+            cron.provider.minute = %w{0 30}
+            cron.provider.class.prefetch
+            cron.retrieve
+
+            assert(minute.insync?, "minute is out of sync with %s" % provider.name)
+            @crontype.clear
+        end
     end
 
     def test_fieldremoval
         cron = nil
         assert_nothing_raised {
-            cron = Puppet.type(:cron).create(
+            cron = @crontype.create(
                 :command => "/bin/date > /dev/null",
                 :minute => [0, 30],
-                :name => "crontest"
+                :name => "crontest",
+                :provider => :crontab
             )
         }
 
         assert_events([:cron_created], cron)
+        cron.provider.class.prefetch
 
         cron[:minute] = :absent
         assert_events([:cron_changed], cron)
         assert_nothing_raised {
+            cron.provider.class.prefetch
             cron.retrieve
         }
         assert_equal(:absent, cron.is(:minute))
     end
 
     def test_listing
-        @crontype.filetype = @oldfiletype
+        # Make a crontab cron for testing
+        provider = @crontype.provider(:crontab)
+        return unless provider.suitable?
+
+        ft = provider.filetype
+        provider.filetype = :ram
+        cleanup { provider.filetype = ft }
+
+        setme
+        cron = @crontype.create(:name => "testing",
+            :minute => [0, 30],
+            :command => "/bin/testing",
+            :user => @me
+        )
+        # Write it to our file
+        assert_apply(cron)
 
         crons = []
         assert_nothing_raised {
-            Puppet::Type.type(:cron).list.each do |cron|
+            @crontype.list.each do |cron|
                 crons << cron
             end
         }
 
         crons.each do |cron|
-            assert(cron, "Did not receive a real cron object")
-            assert_instance_of(String, cron[:user],
+            assert_instance_of(@crontype, cron, "Did not receive a real cron object")
+            assert_instance_of(String, cron.value(:user),
                 "Cron user is not a string")
         end
     end
@@ -434,61 +317,13 @@ class TestCron < Test::Unit::TestCase
         end
     end
 
-    # Make sure we don't puke on env settings
-    def test_envsettings
-        cron = mkcron("envtst")
-
-        assert_apply(cron)
-
-        obj = Puppet::Type::Cron.cronobj(@me)
-
-        assert(obj)
-
-        text = obj.read
-
-        text = "SHELL = /path/to/some/thing\n" + text
-
-        obj.write(text)
-
-        assert_nothing_raised {
-            cron.retrieve
-        }
-
-        cron[:command] = "/some/other/command"
-
-        assert_apply(cron)
-
-        assert(obj.read =~ /SHELL/, "lost env setting")
-
-        env1 = "TEST = /bin/true"
-        env2 = "YAY = fooness"
-        assert_nothing_raised {
-            cron[:environment] = [env1, env2]
-        }
-
-        assert_apply(cron)
-
-        cron.retrieve
-
-        vals = cron.is(:environment)
-        assert(vals, "Did not get environment settings")
-        assert(vals != :absent, "Env is incorrectly absent")
-        assert_instance_of(Array, vals)
-
-        assert(vals.include?(env1), "Missing first env setting")
-        assert(vals.include?(env2), "Missing second env setting")
-
-        # Now do it again and make sure there are no changes
-        assert_events([], cron)
-
-    end
-
     def test_divisionnumbers
         cron = mkcron("divtest")
         cron[:minute] = "*/5"
 
         assert_apply(cron)
 
+        cron.provider.class.prefetch
         cron.retrieve
 
         assert_equal(["*/5"], cron.is(:minute))
@@ -500,49 +335,12 @@ class TestCron < Test::Unit::TestCase
 
         assert_apply(cron)
 
+        cron.provider.class.prefetch
         cron.retrieve
 
         assert_equal(["2-4"], cron.is(:minute))
     end
 
-    def test_data
-        @fakefiletype = Puppet::Util::FileType.filetype(:ram)
-        @crontype.filetype = @fakefiletype
-
-        @crontype.retrieve(@me)
-        obj = Puppet::Type::Cron.cronobj(@me)
-
-        fakedata("data/types/cron").each do |file|
-            names = []
-            text = File.read(file)
-            obj.write(File.read(file))
-
-            @crontype.retrieve(@me)
-
-            @crontype.each do |cron|
-                names << cron.name
-            end
-
-            name = File.basename(file)
-            cron = mkcron("filetest-#{name}")
-
-            assert_apply(cron)
-
-            @crontype.retrieve(@me)
-
-            names.each do |name|
-                assert(@crontype[name], "Could not retrieve %s" % name)
-            end
-
-            tablines = @crontype.tab(@me).split("\n")
-
-            text.split("\n").each do |line|
-                assert(tablines.include?(line),
-                    "Did not get %s back out" % line.inspect)
-            end
-        end
-    end
-
     def test_value
         cron = mkcron("valuetesting", false)
 
@@ -591,6 +389,8 @@ class TestCron < Test::Unit::TestCase
             assert_equal("4,5", cron.value(param))
         end
 
+        Puppet[:trace] = false
+
         # Now make sure that :command works correctly
         cron.delete(:command)
         cron.newattr(:command)
@@ -632,71 +432,6 @@ class TestCron < Test::Unit::TestCase
         assert_equal("4", cron.value(param))
     end
 
-    # Make sure we can successfully list all cron jobs on all users
-    def test_cron_listing
-        crons = []
-        %w{fake1 fake2 fake3 fake4 fake5}.each do |user|
-            crons << @crontype.create(
-                :name => "#{user}-1",
-                :command => "/usr/bin/#{user}",
-                :minute => "0",
-                :user => user,
-                :hour => user.sub("fake",'')
-            )
-
-            crons << @crontype.create(
-                :name => "#{user}-2",
-                :command => "/usr/sbin/#{user}",
-                :minute => "0",
-                :user => user,
-                :weekday => user.sub("fake",'')
-            )
-
-            assert_apply(*crons)
-        end
-
-        list = @crontype.list.collect { |c| c.name }
-
-        crons.each do |cron|
-            assert(list.include?(cron.name), "Did not match cron %s" % name)
-        end
-    end
-
-    # Make sure we can create a cron in an empty tab
-    def test_mkcron_if_empty
-        @crontype.filetype = @oldfiletype
-
-        @crontype.retrieve(@me)
-
-        # Backup our tab
-        text = @crontype.tabobj(@me).read
-
-        cleanup do
-            if text == ""
-                @crontype.tabobj(@me).remove
-            else
-                @crontype.tabobj(@me).write(text)
-            end
-        end
-
-        # Now get rid of it
-        @crontype.tabobj(@me).remove
-        @crontype.clear
-
-        cron = mkcron("emptycron")
-
-        assert_apply(cron)
-
-        # Clear the type, but don't clear the filetype
-        @crontype.clear
-
-        # Get the stuff again
-        @crontype.retrieve(@me)
-
-        assert(@crontype["emptycron"],
-            "Did not retrieve cron")
-    end
-
     def test_multiple_users
         crons = []
         users = ["root", nonrootuser.name]
@@ -708,18 +443,43 @@ class TestCron < Test::Unit::TestCase
                 :minute => [0,30]
             )
         end
+        provider = crons[0].provider.class
 
         assert_apply(*crons)
 
         users.each do |user|
             users.each do |other|
                 next if user == other
-                assert(Puppet::Type.type(:cron).tabobj(other).read !~ /testcron-#{user}/,
+                text = provider.target_object(other).read
+
+                assert(text !~ /testcron-#{user}/,
                        "%s's cron job is in %s's tab" %
                        [user, other])
             end
         end
     end
+
+    # Make sure the user stuff defaults correctly.
+    def test_default_user
+        crontab = @crontype.provider(:crontab)
+        if crontab.suitable?
+            inst = @crontype.create(
+                :name => "something", :command => "/some/thing",
+                :provider => :crontab)
+            assert_equal(ENV["USER"], inst.should(:user),
+                "user did not default to current user with crontab")
+            assert_equal(ENV["USER"], inst.should(:target),
+                "target did not default to current user with crontab")
+
+            # Now make a new cron with a user, and make sure it gets copied
+            # over
+            inst = @crontype.create(:name => "yay", :command => "/some/thing",
+                :user => "bin", :provider => :crontab)
+            assert_equal("bin", inst.should(:target),
+                "target did not default to user with crontab")
+        end
+    end
 end
 
+
 # $Id$
diff --git a/test/util/fileparsing.rb b/test/util/fileparsing.rb
index 12343c38b..c81c9e006 100755
--- a/test/util/fileparsing.rb
+++ b/test/util/fileparsing.rb
@@ -564,7 +564,8 @@ billy three four\n"
         assert_nothing_raised("Could not set hooks") do
             record = @parser.record_line :yay, :fields => %w{one two},
                 :post_parse => proc { |hash| hash[:posted] = true },
-                :pre_gen => proc { |hash| hash[:one] = hash[:one].upcase }
+                :pre_gen => proc { |hash| hash[:one] = hash[:one].upcase },
+                :to_line => proc { |hash| "# Line\n" + join(hash) }
         end
 
         assert(record.respond_to?(:post_parse), "did not create method for post-hook")
@@ -582,7 +583,7 @@ billy three four\n"
         assert_nothing_raised("Could not generate line with hooks") do
             result = @parser.to_line(result)
         end
-        assert_equal("ONE two", result, "did not call pre-gen hook")
+        assert_equal("# Line\nONE two", result, "did not call pre-gen hook")
         assert_equal("one", old_result[:one], "passed original hash to pre hook")
     end
 end