On Aug 9, 2009, at 9:49 PM, Andrew Shafer wrote: > > This change will effect all the properties implemented with list.rb > (groups, roles, auths, profiles). The change will match [] values for > should as insync when none exist. (so no more log message)
It'd be nice to have the commit title match the purpose of the commit, rather than just what it does. In this case, you're fixing a bug, so it'd be good to at least refer to the actual bug, rather than the implementation that fixes it. And please keep that title below 50 or so chars, as mentioned here: http://www.tpope.net/node/106 > > Signed-off-by: Andrew Shafer <[email protected]> > --- > lib/puppet/property/list.rb | 6 +++--- > spec/unit/property/list.rb | 19 +++++++++++++++++-- > 2 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/lib/puppet/property/list.rb b/lib/puppet/property/list.rb > index b7db8b4..31af76b 100644 > --- a/lib/puppet/property/list.rb > +++ b/lib/puppet/property/list.rb > @@ -65,10 +65,10 @@ module Puppet > end > > def prepare_is_for_comparison(is) > - if is.is_a? Array > - is = dearrayify(is) > + if is == :absent > + is = [] > end > - is > + dearrayify(is) > end > > def insync?(is) > diff --git a/spec/unit/property/list.rb b/spec/unit/property/list.rb > index 854ab48..a5400ef 100644 > --- a/spec/unit/property/list.rb > +++ b/spec/unit/property/list.rb > @@ -118,14 +118,29 @@ describe list_class do > end > end > > + describe "when calling prepare_is_for_comparison" do > + it "should return '' when called with :absent" do > + @property.prepare_is_for_comparison(:absent).must > == '' > + end > + > + it "should call dearrayify" do > + @property.expects(:dearrayify) > + @property.prepare_is_for_comparison(:absent) > + end > + > + it "should return a sorted, comma delimited string when > called with Array" do > + > @property.prepare_is_for_comparison(["foo","bar"]).must == "bar,foo" > + end > + end > + It seems like this is essentially a private method and shouldn't actually be tested. Shouldn't you be able to test it thoroughly with just inputs to 'insync?'? > > describe "when calling insync?" do > it "should return true unless @should is defined and not > nil" do > - @property.insync?("foo") == true > + @property.insync?("foo").must == true > end These would produce better output as: @property.must be_insync("foo") > > it "should return true unless the passed in values is > not nil" do > @property.should = "foo" > - @property.insync?(nil) == true > + @property.insync?(nil).must == true > end > > it "should call prepare_is_for_comparison with value > passed in and should" do > -- > 1.6.2 > > > > -- A classic is something that everybody wants to have read and nobody wants to read. -- Mark Twain --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
