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.
