My general "ouch" sense for design that I am going to mess up using
some time has gone off for exactly this reason here, too. I would
much rather avoid this sort of magic wrapper or whatever if it was
even remotely possible.
Daniel
On Tue, Jan 25, 2011 at 15:35, Luke Kanies <[email protected]> wrote:
> Why even call this method if there's no @should value?
>
> The reason I removed the code that did exactly this was because I (thought I)
> moved the check for that value into the resource_harness class, which made
> the check in the insync? method redundant.
>
> One comment below.
>
> On Jan 25, 2011, at 3:16 PM, Paul Berry wrote:
>
>> Created a method safe_insync? which first checks whether the property
>> has a "should" value of nil, and if so returns true. Most insync?
>> methods were already doing this, but a few were not, leading to bugs
>> if a property was being audited but not set.
>>
>> Types should continue to override the insync? method, but callers of
>> insync? should call safe_insync? instead.
>>
>> Paired-with: Jesse Wolfe <[email protected]>
>> Signed-off-by: Paul Berry <[email protected]>
>> ---
>> lib/puppet/property.rb | 17 ++++++++++++++++-
>> lib/puppet/property/keyvalue.rb | 2 --
>> lib/puppet/property/list.rb | 2 --
>> lib/puppet/provider/file/posix.rb | 2 --
>> lib/puppet/provider/file/win32.rb | 2 --
>> lib/puppet/provider/zone/solaris.rb | 2 +-
>> lib/puppet/transaction/resource_harness.rb | 4 ++--
>> lib/puppet/type.rb | 4 ++--
>> lib/puppet/type/cron.rb | 6 +-----
>> lib/puppet/type/file.rb | 2 +-
>> lib/puppet/type/file/content.rb | 1 -
>> lib/puppet/type/file/target.rb | 2 +-
>> lib/puppet/type/mount.rb | 2 +-
>> lib/puppet/type/package.rb | 2 --
>> lib/puppet/type/service.rb | 2 +-
>> lib/puppet/type/user.rb | 2 --
>> lib/puppet/type/yumrepo.rb | 4 ++--
>> lib/puppet/type/zpool.rb | 4 ----
>> spec/unit/property/keyvalue_spec.rb | 10 +++++-----
>> spec/unit/property/list_spec.rb | 14 +++++++-------
>> spec/unit/type/file/content_spec.rb | 24 ++++++++++++++----------
>> spec/unit/type/file/ensure_spec.rb | 15 ++++++++-------
>> spec/unit/type/file/group_spec.rb | 10 +++++-----
>> spec/unit/type/file/owner_spec.rb | 14 +++++++-------
>> spec/unit/type/file/selinux_spec.rb | 4 ++--
>> spec/unit/type/file_spec.rb | 14 ++++++++++++++
>> spec/unit/type/mount_spec.rb | 8 ++++----
>> spec/unit/type/ssh_authorized_key_spec.rb | 2 +-
>> spec/unit/type/user_spec.rb | 6 +++---
>> spec/unit/type/zpool_spec.rb | 24 ++++++++++++------------
>> 30 files changed, 110 insertions(+), 97 deletions(-)
>>
>> diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
>> index 84e1a03..12f496a 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 quite frightens me; have we done this anywhere else? Why the
> significant concern on whether this particular method gets overridden?
>
>> + # 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 0181708..57d0ea2 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 dcee85d..b86dc87 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 415a5af..f7b8c97 100644
>> --- a/lib/puppet/provider/file/posix.rb
>> +++ b/lib/puppet/provider/file/posix.rb
>> @@ -28,8 +28,6 @@ Puppet::Type.type(:file).provide :posix do
>> end
>>
>> def is_owner_insync?(current, should)
>> - return true unless 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 23aa491..21e7ca9 100644
>> --- a/lib/puppet/provider/file/win32.rb
>> +++ b/lib/puppet/provider/file/win32.rb
>> @@ -15,8 +15,6 @@ Puppet::Type.type(:file).provide :microsoft_windows do
>> end
>>
>> def is_owner_insync?(current, should)
>> - return true unless 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 c114449..f46337b 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 0abab10..4a3d35e 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
>> diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
>> index ea3944b..e03650b 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 76399d6..4b18e71 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 eee948c..0d69446 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 b8f30a9..04b917a 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/target.rb b/lib/puppet/type/file/target.rb
>> index 9e7229d..b9fe921 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 d048c90..36fb553 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 51a8663..d73d90d 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 c00f027..973a76c 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 761d5d7..5de73e3 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 160b216..9b4c794 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 49cce55..df06522 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 7666def..a44d891 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 3e8cc54..c6c5db1 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 cde643f..3ec57f0 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 ec53ed8..dbb3a10 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 2283b57..956cd57 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 8e136a1..bcb8e07 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 1ca59e9..043471d 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 22921d8..51c27c0 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 ce82cb5..0d74042 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 a0b435f..666616c 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 ccea9ee..0c3e2ab 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 db12459..be8cb12 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
>> --
>> 1.7.2
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Puppet Developers" group.
>> To post to this group, send email to [email protected].
>> To unsubscribe from this group, send email to
>> [email protected].
>> For more options, visit this group at
>> http://groups.google.com/group/puppet-dev?hl=en.
>>
>
>
> --
> Dawkins's Law of Adversarial Debate:
> When two incompatible beliefs are advocated with equal intensity,
> the truth does not lie half way between them.
> ---------------------------------------------------------------------
> Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199
>
>
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "Puppet Developers" group.
> To post to this group, send email to [email protected].
> To unsubscribe from this group, send email to
> [email protected].
> For more options, visit this group at
> http://groups.google.com/group/puppet-dev?hl=en.
>
>
--
⎋ Puppet Labs Developer – http://puppetlabs.com
✉ Daniel Pittman <[email protected]>
✆ Contact me via gtalk, email, or phone: +1 (877) 575-9775
♲ Made with 100 percent post-consumer electrons
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
