Merge branch 'ticket/2.6.next/5931' into 2.6.next

* ticket/2.6.next/5931:
  (#5931) Prevent errors when calling insync? on audited properties
  Maint: Removed dead code from resource harness.
  Maint: Rename misleading insync? method in file provider

31 files changed, 113 insertions, 118 deletions

diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
index 84e1a0360..12f496a6e 100644
--- a/lib/puppet/property.rb
+++ b/lib/puppet/property.rb
@@ -152,9 +152,24 @@ class Puppet::Property < Puppet::Parameter
   # since we cannot fix it.  Otherwise, we expect our should value
   # to be an array, and if @is matches any of those values, then
   # we consider it to be in-sync.
-  def insync?(is)
+  #
+  # Don't override this method.
+  def safe_insync?(is)
+    # If there is no @should value, consider the property to be in sync.
     return true unless @should
 
+    # Otherwise delegate to the (possibly derived) insync? method.
+    insync?(is)
+  end
+
+  def self.method_added(sym)
+    raise "Puppet::Property#safe_insync? shouldn't be overridden; please override insync? instead" if sym == :safe_insync?
+  end
+
+  # This method should be overridden by derived classes if necessary
+  # to provide extra logic to determine whether the property is in
+  # sync.
+  def insync?(is)
     self.devfail "#{self.class.name}'s should is not array" unless @should.is_a?(Array)
 
     # an empty array is analogous to no should values
diff --git a/lib/puppet/property/keyvalue.rb b/lib/puppet/property/keyvalue.rb
index 0181708f9..57d0ea2d9 100644
--- a/lib/puppet/property/keyvalue.rb
+++ b/lib/puppet/property/keyvalue.rb
@@ -77,8 +77,6 @@ module Puppet
       end
 
       def insync?(is)
-        return true unless @should
-
         return true unless is
 
         (is == self.should)
diff --git a/lib/puppet/property/list.rb b/lib/puppet/property/list.rb
index dcee85db7..b86dc87f2 100644
--- a/lib/puppet/property/list.rb
+++ b/lib/puppet/property/list.rb
@@ -66,8 +66,6 @@ module Puppet
       end
 
       def insync?(is)
-        return true unless @should
-
         return true unless is
 
         (prepare_is_for_comparison(is) == self.should)
diff --git a/lib/puppet/provider/file/posix.rb b/lib/puppet/provider/file/posix.rb
index 6cbf98e9a..f7b8c9797 100644
--- a/lib/puppet/provider/file/posix.rb
+++ b/lib/puppet/provider/file/posix.rb
@@ -27,9 +27,7 @@ Puppet::Type.type(:file).provide :posix do
     end
   end
 
-  def insync?(current, should)
-    return true unless should
-
+  def is_owner_insync?(current, should)
     should.each do |value|
       if value =~ /^\d+$/
         uid = Integer(value)
diff --git a/lib/puppet/provider/file/win32.rb b/lib/puppet/provider/file/win32.rb
index 8ead69a89..21e7ca974 100644
--- a/lib/puppet/provider/file/win32.rb
+++ b/lib/puppet/provider/file/win32.rb
@@ -14,9 +14,7 @@ Puppet::Type.type(:file).provide :microsoft_windows do
     id
   end
 
-  def insync?(current, should)
-    return true unless should
-
+  def is_owner_insync?(current, should)
     should.each do |value|
       if value =~ /^\d+$/
         uid = Integer(value)
diff --git a/lib/puppet/provider/zone/solaris.rb b/lib/puppet/provider/zone/solaris.rb
index c11444993..f46337b14 100644
--- a/lib/puppet/provider/zone/solaris.rb
+++ b/lib/puppet/provider/zone/solaris.rb
@@ -40,7 +40,7 @@ Puppet::Type.type(:zone).provide(:solaris) do
     # Then perform all of our configuration steps.  It's annoying
     # that we need this much internal info on the resource.
     @resource.send(:properties).each do |property|
-      str += property.configtext + "\n" if property.is_a? ZoneConfigProperty and ! property.insync?(properties[property.name])
+      str += property.configtext + "\n" if property.is_a? ZoneConfigProperty and ! property.safe_insync?(properties[property.name])
     end
 
     str += "commit\n"
diff --git a/lib/puppet/transaction/resource_harness.rb b/lib/puppet/transaction/resource_harness.rb
index c1b980632..4a3d35e0d 100644
--- a/lib/puppet/transaction/resource_harness.rb
+++ b/lib/puppet/transaction/resource_harness.rb
@@ -52,13 +52,13 @@ class Puppet::Transaction::ResourceHarness
     # Update the machine state & create logs/events
     events = []
     ensure_param = resource.parameter(:ensure)
-    if desired_values[:ensure] && !ensure_param.insync?(current_values[:ensure])
+    if desired_values[:ensure] && !ensure_param.safe_insync?(current_values[:ensure])
       events << apply_parameter(ensure_param, current_values[:ensure], audited_params.include?(:ensure), historical_values[:ensure])
       synced_params << :ensure
     elsif current_values[:ensure] != :absent
       work_order = resource.properties # Note: only the resource knows what order to apply changes in
       work_order.each do |param|
-        if desired_values[param.name] && !param.insync?(current_values[param.name])
+        if desired_values[param.name] && !param.safe_insync?(current_values[param.name])
           events << apply_parameter(param, current_values[param.name], audited_params.include?(param.name), historical_values[param.name])
           synced_params << param.name
         end
@@ -175,22 +175,4 @@ class Puppet::Transaction::ResourceHarness
     return nil unless name = resource[:schedule]
     resource.catalog.resource(:schedule, name) || resource.fail("Could not find schedule #{name}")
   end
-
-  private
-
-  def absent_and_not_being_created?(current, param)
-    current[:ensure] == :absent and param.should.nil?
-  end
-
-  def ensure_is_insync?(current, param)
-    param.insync?(current[:ensure])
-  end
-
-  def ensure_should_be_absent?(current, param)
-    param.should == :absent
-  end
-
-  def param_is_insync?(current, param)
-    param.insync?(current[param.name])
-  end
 end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index ea3944b4e..e03650b54 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -648,7 +648,7 @@ class Type
           "The is value is not in the is array for '#{property.name}'"
       end
       ensureis = is[property]
-      if property.insync?(ensureis) and property.should == :absent
+      if property.safe_insync?(ensureis) and property.should == :absent
         return true
       end
     end
@@ -660,7 +660,7 @@ class Type
       end
 
       propis = is[property]
-      unless property.insync?(propis)
+      unless property.safe_insync?(propis)
         property.debug("Not in sync: #{propis.inspect} vs #{property.should.inspect}")
         insync = false
       #else
diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb
index 76399d693..4b18e71f9 100755
--- a/lib/puppet/type/cron.rb
+++ b/lib/puppet/type/cron.rb
@@ -54,11 +54,7 @@ Puppet::Type.newtype(:cron) do
     # We have to override the parent method, because we consume the entire
     # "should" array
     def insync?(is)
-      if @should
-        self.is_to_s(is) == self.should_to_s
-      else
-        true
-      end
+      self.is_to_s(is) == self.should_to_s
     end
 
     # A method used to do parameter input handling.  Converts integers
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index eee948cd5..0d69446b4 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -779,7 +779,7 @@ Puppet::Type.newtype(:file) do
       # Make sure we get a new stat objct
       expire
       currentvalue = thing.retrieve
-      thing.sync unless thing.insync?(currentvalue)
+      thing.sync unless thing.safe_insync?(currentvalue)
     end
   end
 end
diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb
index b8f30a9c7..04b917a7e 100755
--- a/lib/puppet/type/file/content.rb
+++ b/lib/puppet/type/file/content.rb
@@ -96,7 +96,6 @@ module Puppet
       end
 
       return true if ! @resource.replace?
-      return true unless self.should
 
       result = super
 
diff --git a/lib/puppet/type/file/owner.rb b/lib/puppet/type/file/owner.rb
index d473da20e..483cc7fce 100755
--- a/lib/puppet/type/file/owner.rb
+++ b/lib/puppet/type/file/owner.rb
@@ -6,7 +6,7 @@ module Puppet
     @event = :file_changed
 
     def insync?(current)
-      provider.insync?(current, @should)
+      provider.is_owner_insync?(current, @should)
     end
 
     # We want to print names, not numbers
diff --git a/lib/puppet/type/file/target.rb b/lib/puppet/type/file/target.rb
index 9e7229dda..b9fe9213b 100644
--- a/lib/puppet/type/file/target.rb
+++ b/lib/puppet/type/file/target.rb
@@ -14,7 +14,7 @@ module Puppet
 
       # Only call mklink if ensure didn't call us in the first place.
       currentensure  = @resource.property(:ensure).retrieve
-      mklink if @resource.property(:ensure).insync?(currentensure)
+      mklink if @resource.property(:ensure).safe_insync?(currentensure)
     end
 
     # Create our link.
diff --git a/lib/puppet/type/mount.rb b/lib/puppet/type/mount.rb
index d048c90f1..36fb553f5 100755
--- a/lib/puppet/type/mount.rb
+++ b/lib/puppet/type/mount.rb
@@ -89,7 +89,7 @@ module Puppet
           if prop.name == :ensure
             false
           else
-            ! prop.insync?(currentvalues[prop])
+            ! prop.safe_insync?(currentvalues[prop])
           end
         end.each { |prop| prop.sync }.length
         @resource.flush if oos > 0
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index 51a866332..d73d90dff 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -109,8 +109,6 @@ module Puppet
       # Override the parent method, because we've got all kinds of
       # funky definitions of 'in sync'.
       def insync?(is)
-        @should ||= []
-
         @latest ||= nil
         @lateststamp ||= (Time.now.to_i - 1000)
         # Iterate across all of the should values, and see how they
diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb
index 2801f3a78..0d09c3d5d 100644
--- a/lib/puppet/type/service.rb
+++ b/lib/puppet/type/service.rb
@@ -73,7 +73,7 @@ module Puppet
 
         if property = @resource.property(:enable)
           val = property.retrieve
-          property.sync unless property.insync?(val)
+          property.sync unless property.safe_insync?(val)
         end
 
         event
diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb
index 761d5d71b..5de73e3dd 100755
--- a/lib/puppet/type/user.rb
+++ b/lib/puppet/type/user.rb
@@ -112,8 +112,6 @@ module Puppet
       end
 
       def insync?(is)
-        return true unless self.should
-
         # We know the 'is' is a number, so we need to convert the 'should' to a number,
         # too.
         @should.each do |value|
diff --git a/lib/puppet/type/yumrepo.rb b/lib/puppet/type/yumrepo.rb
index 160b2164d..9b4c79428 100644
--- a/lib/puppet/type/yumrepo.rb
+++ b/lib/puppet/type/yumrepo.rb
@@ -7,14 +7,14 @@ module Puppet
   class IniProperty < Puppet::Property
     def insync?(is)
       # A should property of :absent is the same as nil
-      if is.nil? && (should.nil? || should == :absent)
+      if is.nil? && should == :absent
         return true
       end
       super(is)
     end
 
     def sync
-      if insync?(retrieve)
+      if safe_insync?(retrieve)
         result = nil
       else
         result = set(self.should)
diff --git a/lib/puppet/type/zpool.rb b/lib/puppet/type/zpool.rb
index 49cce552a..df06522e8 100755
--- a/lib/puppet/type/zpool.rb
+++ b/lib/puppet/type/zpool.rb
@@ -8,8 +8,6 @@ module Puppet
       end
 
       def insync?(is)
-        return true unless self.should
-
         return @should == [:absent] if is == :absent
 
         flatten_and_sort(is) == flatten_and_sort(@should)
@@ -18,8 +16,6 @@ module Puppet
 
     class MultiVDev < VDev
       def insync?(is)
-        return true unless self.should
-
         return @should == [:absent] if is == :absent
 
         return false unless is.length == @should.length
diff --git a/spec/unit/property/keyvalue_spec.rb b/spec/unit/property/keyvalue_spec.rb
index 7666def56..a44d891d7 100644
--- a/spec/unit/property/keyvalue_spec.rb
+++ b/spec/unit/property/keyvalue_spec.rb
@@ -134,7 +134,7 @@ describe klass do
       end
     end
 
-    describe "when calling insync?" do
+    describe "when calling safe_insync?" do
       before do
         @provider = mock("provider")
         @property.stubs(:provider).returns(@provider)
@@ -142,26 +142,26 @@ describe klass do
       end
 
       it "should return true unless @should is defined and not nil" do
-        @property.insync?("foo") == true
+        @property.safe_insync?("foo") == true
       end
 
       it "should return true if the passed in values is nil" do
         @property.should = "foo"
-        @property.insync?(nil) == true
+        @property.safe_insync?(nil) == true
       end
 
       it "should return true if hashified should value == (retrieved) value passed in" do
         @provider.stubs(:prop_name).returns({ :foo => "baz", :bar => "boo" })
         @property.should = ["foo=baz", "bar=boo"]
         @property.expects(:inclusive?).returns(true)
-        @property.insync?({ :foo => "baz", :bar => "boo" }).must == true
+        @property.safe_insync?({ :foo => "baz", :bar => "boo" }).must == true
       end
 
       it "should return false if prepared value != should value" do
         @provider.stubs(:prop_name).returns({ "foo" => "bee", "bar" => "boo" })
         @property.should = ["foo=baz", "bar=boo"]
         @property.expects(:inclusive?).returns(true)
-        @property.insync?({ "foo" => "bee", "bar" => "boo" }).must == false
+        @property.safe_insync?({ "foo" => "bee", "bar" => "boo" }).must == false
       end
     end
   end
diff --git a/spec/unit/property/list_spec.rb b/spec/unit/property/list_spec.rb
index 3e8cc5402..c6c5db10e 100644
--- a/spec/unit/property/list_spec.rb
+++ b/spec/unit/property/list_spec.rb
@@ -118,39 +118,39 @@ describe list_class do
       end
     end
 
-    describe "when calling insync?" do
+    describe "when calling safe_insync?" do
       it "should return true unless @should is defined and not nil" do
-        @property.must be_insync("foo")
+        @property.must be_safe_insync("foo")
       end
 
       it "should return true unless the passed in values is not nil" do
         @property.should = "foo"
-        @property.must be_insync(nil)
+        @property.must be_safe_insync(nil)
       end
 
       it "should call prepare_is_for_comparison with value passed in and should" do
         @property.should = "foo"
         @property.expects(:prepare_is_for_comparison).with("bar")
         @property.expects(:should)
-        @property.insync?("bar")
+        @property.safe_insync?("bar")
       end
 
       it "should return true if 'is' value is array of comma delimited should values" do
         @property.should = "bar,foo"
         @property.expects(:inclusive?).returns(true)
-        @property.must be_insync(["bar","foo"])
+        @property.must be_safe_insync(["bar","foo"])
       end
 
       it "should return true if 'is' value is :absent and should value is empty string" do
         @property.should = ""
         @property.expects(:inclusive?).returns(true)
-        @property.must be_insync([])
+        @property.must be_safe_insync([])
       end
 
       it "should return false if prepared value != should value" do
         @property.should = "bar,baz,foo"
         @property.expects(:inclusive?).returns(true)
-        @property.must_not be_insync(["bar","foo"])
+        @property.must_not be_safe_insync(["bar","foo"])
       end
     end
 
diff --git a/spec/unit/type/file/content_spec.rb b/spec/unit/type/file/content_spec.rb
index cde643fc8..3ec57f00e 100755
--- a/spec/unit/type/file/content_spec.rb
+++ b/spec/unit/type/file/content_spec.rb
@@ -141,17 +141,20 @@ describe content do
 
     it "should return true if the resource shouldn't be a regular file" do
       @resource.expects(:should_be_file?).returns false
-      @content.must be_insync("whatever")
+      @content.should = "foo"
+      @content.must be_safe_insync("whatever")
     end
 
     it "should return false if the current content is :absent" do
-      @content.should_not be_insync(:absent)
+      @content.should = "foo"
+      @content.should_not be_safe_insync(:absent)
     end
 
     it "should return false if the file should be a file but is not present" do
       @resource.expects(:should_be_file?).returns true
+      @content.should = "foo"
 
-      @content.should_not be_insync(:absent)
+      @content.should_not be_safe_insync(:absent)
     end
 
     describe "and the file exists" do
@@ -161,12 +164,12 @@ describe content do
 
       it "should return false if the current contents are different from the desired content" do
         @content.should = "some content"
-        @content.should_not be_insync("other content")
+        @content.should_not be_safe_insync("other content")
       end
 
       it "should return true if the sum for the current contents is the same as the sum for the desired content" do
         @content.should = "some content"
-        @content.must be_insync("{md5}" + Digest::MD5.hexdigest("some content"))
+        @content.must be_safe_insync("{md5}" + Digest::MD5.hexdigest("some content"))
       end
 
       describe "and Puppet[:show_diff] is set" do
@@ -179,14 +182,14 @@ describe content do
           @content.expects(:diff).returns("my diff").once
           @content.expects(:print).with("my diff").once
 
-          @content.insync?("other content")
+          @content.safe_insync?("other content")
         end
 
         it "should not display a diff if the sum for the current contents is the same as the sum for the desired content" do
           @content.should = "some content"
           @content.expects(:diff).never
 
-          @content.insync?("{md5}" + Digest::MD5.hexdigest("some content"))
+          @content.safe_insync?("{md5}" + Digest::MD5.hexdigest("some content"))
         end
       end
     end
@@ -199,17 +202,18 @@ describe content do
       it "should be insync if the file exists and the content is different" do
         @resource.stubs(:stat).returns mock('stat')
 
-        @content.must be_insync("whatever")
+        @content.must be_safe_insync("whatever")
       end
 
       it "should be insync if the file exists and the content is right" do
         @resource.stubs(:stat).returns mock('stat')
 
-        @content.must be_insync("something")
+        @content.must be_safe_insync("something")
       end
 
       it "should not be insync if the file does not exist" do
-        @content.should_not be_insync(:absent)
+        @content.should = "foo"
+        @content.should_not be_safe_insync(:absent)
       end
     end
   end
diff --git a/spec/unit/type/file/ensure_spec.rb b/spec/unit/type/file/ensure_spec.rb
index ec53ed85a..dbb3a1053 100755
--- a/spec/unit/type/file/ensure_spec.rb
+++ b/spec/unit/type/file/ensure_spec.rb
@@ -41,44 +41,45 @@ describe property do
     end
 
     it "should always be in sync if replace is 'false' unless the file is missing" do
+      @ensure.should = :file
       @resource.expects(:replace?).returns false
-      @ensure.insync?(:link).should be_true
+      @ensure.safe_insync?(:link).should be_true
     end
 
     it "should be in sync if :ensure is set to :absent and the file does not exist" do
       @ensure.should = :absent
 
-      @ensure.must be_insync(:absent)
+      @ensure.must be_safe_insync(:absent)
     end
 
     it "should not be in sync if :ensure is set to :absent and the file exists" do
       @ensure.should = :absent
 
-      @ensure.should_not be_insync(:file)
+      @ensure.should_not be_safe_insync(:file)
     end
 
     it "should be in sync if a normal file exists and :ensure is set to :present" do
       @ensure.should = :present
 
-      @ensure.must be_insync(:file)
+      @ensure.must be_safe_insync(:file)
     end
 
     it "should be in sync if a directory exists and :ensure is set to :present" do
       @ensure.should = :present
 
-      @ensure.must be_insync(:directory)
+      @ensure.must be_safe_insync(:directory)
     end
 
     it "should be in sync if a symlink exists and :ensure is set to :present" do
       @ensure.should = :present
 
-      @ensure.must be_insync(:link)
+      @ensure.must be_safe_insync(:link)
     end
 
     it "should not be in sync if :ensure is set to :file and a directory exists" do
       @ensure.should = :file
 
-      @ensure.should_not be_insync(:directory)
+      @ensure.should_not be_safe_insync(:directory)
     end
   end
 end
diff --git a/spec/unit/type/file/group_spec.rb b/spec/unit/type/file/group_spec.rb
index 2283b57fa..956cd57e7 100755
--- a/spec/unit/type/file/group_spec.rb
+++ b/spec/unit/type/file/group_spec.rb
@@ -56,19 +56,19 @@ describe property do
   describe "when determining if the file is in sync" do
     it "should directly compare the group values if the desired group is an integer" do
       @group.should = [10]
-      @group.must be_insync(10)
+      @group.must be_safe_insync(10)
     end
 
     it "should treat numeric strings as integers" do
       @group.should = ["10"]
-      @group.must be_insync(10)
+      @group.must be_safe_insync(10)
     end
 
     it "should convert the group name to an integer if the desired group is a string" do
       @group.expects(:gid).with("foo").returns 10
       @group.should = %w{foo}
 
-      @group.must be_insync(10)
+      @group.must be_safe_insync(10)
     end
 
     it "should not validate that groups exist when a group is specified as an integer" do
@@ -80,12 +80,12 @@ describe property do
       @group.expects(:gid).with("foo").returns nil
       @group.should = %w{foo}
 
-      lambda { @group.insync?(10) }.should raise_error(Puppet::Error)
+      lambda { @group.safe_insync?(10) }.should raise_error(Puppet::Error)
     end
 
     it "should return false if the groups are not equal" do
       @group.should = [10]
-      @group.should_not be_insync(20)
+      @group.should_not be_safe_insync(20)
     end
   end
 
diff --git a/spec/unit/type/file/owner_spec.rb b/spec/unit/type/file/owner_spec.rb
index 8e136a187..bcb8e07d6 100755
--- a/spec/unit/type/file/owner_spec.rb
+++ b/spec/unit/type/file/owner_spec.rb
@@ -68,7 +68,7 @@ describe property do
         @provider.expects(:warnonce)
 
         @owner.should = [10]
-        @owner.must be_insync(20)
+        @owner.must be_safe_insync(20)
       end
     end
 
@@ -77,24 +77,24 @@ describe property do
     end
 
     it "should be in sync if 'should' is not provided" do
-      @owner.must be_insync(10)
+      @owner.must be_safe_insync(10)
     end
 
     it "should directly compare the owner values if the desired owner is an integer" do
       @owner.should = [10]
-      @owner.must be_insync(10)
+      @owner.must be_safe_insync(10)
     end
 
     it "should treat numeric strings as integers" do
       @owner.should = ["10"]
-      @owner.must be_insync(10)
+      @owner.must be_safe_insync(10)
     end
 
     it "should convert the owner name to an integer if the desired owner is a string" do
       @provider.expects(:uid).with("foo").returns 10
       @owner.should = %w{foo}
 
-      @owner.must be_insync(10)
+      @owner.must be_safe_insync(10)
     end
 
     it "should not validate that users exist when a user is specified as an integer" do
@@ -106,12 +106,12 @@ describe property do
       @provider.expects(:uid).with("foo").returns nil
       @owner.should = %w{foo}
 
-      lambda { @owner.insync?(10) }.should raise_error(Puppet::Error)
+      lambda { @owner.safe_insync?(10) }.should raise_error(Puppet::Error)
     end
 
     it "should return false if the owners are not equal" do
       @owner.should = [10]
-      @owner.should_not be_insync(20)
+      @owner.should_not be_safe_insync(20)
     end
   end
 
diff --git a/spec/unit/type/file/selinux_spec.rb b/spec/unit/type/file/selinux_spec.rb
index 1ca59e9e7..043471dec 100644
--- a/spec/unit/type/file/selinux_spec.rb
+++ b/spec/unit/type/file/selinux_spec.rb
@@ -73,10 +73,10 @@ Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f
       @sel.sync
     end
 
-    it "should do nothing for insync? if no SELinux support" do
+    it "should do nothing for safe_insync? if no SELinux support" do
       @sel.should = %{newcontext}
       @sel.expects(:selinux_support?).returns false
-      @sel.insync?("oldcontext").should == true
+      @sel.safe_insync?("oldcontext").should == true
     end
   end
 end
diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb
index 22921d85a..51c27c0e6 100755
--- a/spec/unit/type/file_spec.rb
+++ b/spec/unit/type/file_spec.rb
@@ -1065,4 +1065,18 @@ describe Puppet::Type.type(:file) do
     end
   end
 
+  describe "when auditing" do
+    it "should not fail if creating a new file if group is not set" do
+      File.exists?(@path).should == false
+      file = Puppet::Type::File.new(:name => @path, :audit => "all", :content => "content")
+      catalog = Puppet::Resource::Catalog.new
+      catalog.add_resource(file)
+
+      Puppet::Util::Storage.stubs(:store) # to prevent the catalog from trying to write state.yaml
+      transaction = catalog.apply
+
+      transaction.report.resource_statuses["File[#{@path}]"].failed.should == false
+      File.exists?(@path).should == true
+    end
+  end
 end
diff --git a/spec/unit/type/mount_spec.rb b/spec/unit/type/mount_spec.rb
index ce82cb516..0d74042e3 100755
--- a/spec/unit/type/mount_spec.rb
+++ b/spec/unit/type/mount_spec.rb
@@ -184,22 +184,22 @@ describe Puppet::Type.type(:mount)::Ensure do
 
     it "should be insync if it is mounted and should be defined" do
       @ensure.should = :defined
-      @ensure.insync?(:mounted).should == true
+      @ensure.safe_insync?(:mounted).should == true
     end
 
     it "should be insync if it is unmounted and should be defined" do
       @ensure.should = :defined
-      @ensure.insync?(:unmounted).should == true
+      @ensure.safe_insync?(:unmounted).should == true
     end
 
     it "should be insync if it is mounted and should be present" do
       @ensure.should = :present
-      @ensure.insync?(:mounted).should == true
+      @ensure.safe_insync?(:mounted).should == true
     end
 
     it "should be insync if it is unmounted and should be present" do
       @ensure.should = :present
-      @ensure.insync?(:unmounted).should == true
+      @ensure.safe_insync?(:unmounted).should == true
     end
   end
 
diff --git a/spec/unit/type/ssh_authorized_key_spec.rb b/spec/unit/type/ssh_authorized_key_spec.rb
index a0b435f80..666616c03 100755
--- a/spec/unit/type/ssh_authorized_key_spec.rb
+++ b/spec/unit/type/ssh_authorized_key_spec.rb
@@ -134,7 +134,7 @@ describe ssh_authorized_key do
     it "should not raise spurious change events" do
       resource = @class.new(:name => "Test", :user => "root")
       target = File.expand_path("~root/.ssh/authorized_keys")
-      resource.property(:target).insync?(target).should == true
+      resource.property(:target).safe_insync?(target).should == true
     end
   end
 
diff --git a/spec/unit/type/user_spec.rb b/spec/unit/type/user_spec.rb
index ccea9ee4c..0c3e2ab2d 100755
--- a/spec/unit/type/user_spec.rb
+++ b/spec/unit/type/user_spec.rb
@@ -201,21 +201,21 @@ describe user do
       it "should return true if no 'should' values are set" do
         @gid = user.attrclass(:gid).new(:resource => @resource)
 
-        @gid.must be_insync(500)
+        @gid.must be_safe_insync(500)
       end
 
       it "should return true if any of the specified groups are equal to the current integer" do
         Puppet::Util.expects(:gid).with("foo").returns 300
         Puppet::Util.expects(:gid).with("bar").returns 500
 
-        @gid.must be_insync(500)
+        @gid.must be_safe_insync(500)
       end
 
       it "should return false if none of the specified groups are equal to the current integer" do
         Puppet::Util.expects(:gid).with("foo").returns 300
         Puppet::Util.expects(:gid).with("bar").returns 500
 
-        @gid.should_not be_insync(700)
+        @gid.should_not be_safe_insync(700)
       end
     end
 
diff --git a/spec/unit/type/zpool_spec.rb b/spec/unit/type/zpool_spec.rb
index db12459ab..be8cb12ba 100755
--- a/spec/unit/type/zpool_spec.rb
+++ b/spec/unit/type/zpool_spec.rb
@@ -38,27 +38,27 @@ describe vdev_property do
 
   it "should be insync if the devices are the same" do
     @property.should = ["dev1 dev2"]
-    @property.insync?(["dev2 dev1"]).must be_true
+    @property.safe_insync?(["dev2 dev1"]).must be_true
   end
 
   it "should be out of sync if the devices are not the same" do
     @property.should = ["dev1 dev3"]
-    @property.insync?(["dev2 dev1"]).must be_false
+    @property.safe_insync?(["dev2 dev1"]).must be_false
   end
 
   it "should be insync if the devices are the same and the should values are comma seperated" do
     @property.should = ["dev1", "dev2"]
-    @property.insync?(["dev2 dev1"]).must be_true
+    @property.safe_insync?(["dev2 dev1"]).must be_true
   end
 
   it "should be out of sync if the device is absent and should has a value" do
     @property.should = ["dev1", "dev2"]
-    @property.insync?(:absent).must be_false
+    @property.safe_insync?(:absent).must be_false
   end
 
   it "should be insync if the device is absent and should is absent" do
     @property.should = [:absent]
-    @property.insync?(:absent).must be_true
+    @property.safe_insync?(:absent).must be_true
   end
 end
 
@@ -73,38 +73,38 @@ describe multi_vdev_property do
 
   it "should be insync if the devices are the same" do
     @property.should = ["dev1 dev2"]
-    @property.insync?(["dev2 dev1"]).must be_true
+    @property.safe_insync?(["dev2 dev1"]).must be_true
   end
 
   it "should be out of sync if the devices are not the same" do
     @property.should = ["dev1 dev3"]
-    @property.insync?(["dev2 dev1"]).must be_false
+    @property.safe_insync?(["dev2 dev1"]).must be_false
   end
 
   it "should be out of sync if the device is absent and should has a value" do
     @property.should = ["dev1", "dev2"]
-    @property.insync?(:absent).must be_false
+    @property.safe_insync?(:absent).must be_false
   end
 
   it "should be insync if the device is absent and should is absent" do
     @property.should = [:absent]
-    @property.insync?(:absent).must be_true
+    @property.safe_insync?(:absent).must be_true
   end
 
   describe "when there are multiple lists of devices" do
     it "should be in sync if each group has the same devices" do
       @property.should = ["dev1 dev2", "dev3 dev4"]
-      @property.insync?(["dev2 dev1", "dev3 dev4"]).must be_true
+      @property.safe_insync?(["dev2 dev1", "dev3 dev4"]).must be_true
     end
 
     it "should be out of sync if any group has the different devices" do
       @property.should = ["dev1 devX", "dev3 dev4"]
-      @property.insync?(["dev2 dev1", "dev3 dev4"]).must be_false
+      @property.safe_insync?(["dev2 dev1", "dev3 dev4"]).must be_false
     end
 
     it "should be out of sync if devices are in the wrong group" do
       @property.should = ["dev1 dev2", "dev3 dev4"]
-      @property.insync?(["dev2 dev3", "dev1 dev4"]).must be_false
+      @property.safe_insync?(["dev2 dev3", "dev1 dev4"]).must be_false
     end
   end
 end