On Feb 2, 2011, at 9:37 AM, Daniel Pittman wrote: > Thanks. We (mostly Matt, actually) raised a sweat for that bug fix. :) > > Getting that refactored would be good; we could use the oportunity to better > merge the Win32 and POSIX parts of the code along the way. > Yeah, the existing providers were not really done in according to normal provider best practices. Any refactor of the file parameters would definitely need to fix that.
> -- > Puppet Labs Developer –http://puppetlabs.com > Daniel Pittman <[email protected]> > Contact me via gtalk, email, or phone: +1 (877) 575-9775 > Sent from a mobile device. Please forgive me if this is briefer than usual. > > On Feb 1, 2011 10:44 PM, "Luke Kanies" <[email protected]> wrote: > > Nice catch. > > > > And yeah, the whole file type needs to be refactored. If we just moved it > > to having providers, that would probably fix the majority of it. > > > > On Feb 1, 2011, at 4:02 PM, Matt Robinson wrote: > > > >> The manifest: > >> > >> file { "/tmp/foo" : > >> ensure => present, > >> audit => content, > >> } > >> > >> produced the error: > >> > >> err: /Stage[main]//File[/tmp/foo]/ensure: change from absent to present > >> failed: Could not retrieve content for from filebucket: private method > >> `sub' called for nil:NilClass at > >> /Users/matthewrobinson/work/puppet/test.pp:4 > >> > >> This was due to logic in content assuming that if you didn't specify > >> content while you were auditing it you must have specified a source. > >> > >> The code paths in the file type badly need a cleanup so that these sorts > >> of errors aren't so difficult to track down and things are easier to > >> test. > >> > >> Paired-with: Daniel Pittman > >> > >> Signed-off-by: Matt Robinson <[email protected]> > >> --- > >> lib/puppet/type/file/content.rb | 2 + > >> spec/unit/type/file/content_spec.rb | 49 > >> +++++++++++++++++++++++++++++++++++ > >> spec/unit/type/file_spec.rb | 12 ++++++++ > >> 3 files changed, 63 insertions(+), 0 deletions(-) > >> > >> diff --git a/lib/puppet/type/file/content.rb > >> b/lib/puppet/type/file/content.rb > >> index 04b917a..cf924f3 100755 > >> --- a/lib/puppet/type/file/content.rb > >> +++ b/lib/puppet/type/file/content.rb > >> @@ -167,6 +167,8 @@ module Puppet > >> def each_chunk_from(source_or_content) > >> if source_or_content.is_a?(String) > >> yield source_or_content > >> + elsif source_or_content.nil? && resource.parameter(:ensure) && > >> [:present, :file].include?(resource.parameter(:ensure).value) > >> + yield '' > >> elsif source_or_content.nil? > >> yield read_file_from_filebucket > >> elsif self.class.standalone? > >> diff --git a/spec/unit/type/file/content_spec.rb > >> b/spec/unit/type/file/content_spec.rb > >> index 3ec57f0..9178c94 100755 > >> --- a/spec/unit/type/file/content_spec.rb > >> +++ b/spec/unit/type/file/content_spec.rb > >> @@ -461,5 +461,54 @@ describe content do > >> > >> describe "from a filebucket" do > >> end > >> + > >> + # These are testing the implementation rather than the desired > >> behaviour; while that bites, there are a whole > >> + # pile of other methods in the File type that depend on intimate details > >> of this implementation and vice-versa. > >> + # If these blow up, you are gonna have to review the callers to make > >> sure they don't explode! --daniel 2011-02-01 > >> + describe "each_chunk_from should work" do > >> + before do > >> + @content = content.new(:resource => @resource) > >> + end > >> + > >> + it "when content is a string" do > >> + @content.each_chunk_from('i_am_a_string') { |chunk| chunk.should == > >> 'i_am_a_string' } > >> + end > >> + > >> + it "when no content, source, but ensure present" do > >> + @resource[:ensure] = :present > >> + @content.each_chunk_from(nil) { |chunk| chunk.should == '' } > >> + end > >> + > >> + it "when no content, source, but ensure file" do > >> + @resource[:ensure] = :file > >> + @content.each_chunk_from(nil) { |chunk| chunk.should == '' } > >> + end > >> + > >> + it "when no content or source" do > >> + > >> @content.expects(:read_file_from_filebucket).once.returns('im_a_filebucket') > >> + @content.each_chunk_from(nil) { |chunk| chunk.should == > >> 'im_a_filebucket' } > >> + end > >> + > >> + it "when running as puppet apply" do > >> + @content.class.expects(:standalone?).returns true > >> + source_or_content = stubs('source_or_content') > >> + source_or_content.expects(:content).once.returns :whoo > >> + @content.each_chunk_from(source_or_content) { |chunk| chunk.should == > >> :whoo } > >> + end > >> + > >> + it "when running from source with a local file" do > >> + source_or_content = stubs('source_or_content') > >> + source_or_content.expects(:local?).returns true > >> + > >> @content.expects(:chunk_file_from_disk).with(source_or_content).once.yields > >> 'woot' > >> + @content.each_chunk_from(source_or_content) { |chunk| chunk.should == > >> 'woot' } > >> + end > >> + > >> + it "when running from source with a remote file" do > >> + source_or_content = stubs('source_or_content') > >> + source_or_content.expects(:local?).returns false > >> + > >> @content.expects(:chunk_file_from_source).with(source_or_content).once.yields > >> 'woot' > >> + @content.each_chunk_from(source_or_content) { |chunk| chunk.should == > >> 'woot' } > >> + end > >> + end > >> end > >> end > >> diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb > >> index db0fa9f..d5cde5f 100755 > >> --- a/spec/unit/type/file_spec.rb > >> +++ b/spec/unit/type/file_spec.rb > >> @@ -1095,5 +1095,17 @@ describe Puppet::Type.type(:file) do > >> transaction.report.resource_statuses["File[#{@path}]"].failed.should == > >> false > >> File.exists?(@path).should == true > >> end > >> + > >> + it "should not log errors if creating a new file with ensure present and > >> no content" do > >> + File.exists?(@path).should == false > >> + file = Puppet::Type::File.new(:name => @path, :audit => "content", > >> :ensure => "present") > >> + catalog = Puppet::Resource::Catalog.new > >> + catalog.add_resource(file) > >> + > >> + Puppet::Util::Storage.stubs(:store) # to prevent the catalog from trying > >> to write state.yaml > >> + > >> + catalog.apply > >> + @logs.reject {|l| l.level == :notice }.should be_empty > >> + end > >> end > >> end > >> -- > >> 1.7.3.1 > >> > >> -- > >> 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. > >> > > > > > > -- > > Honest criticism is hard to take, particularly from a relative, a > > friend, an acquaintance, or a stranger. -- Franklin P. Jones > > --------------------------------------------------------------------- > > 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. > > > > -- > 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. -- Learning is not attained by chance, it must be sought for with ardor and attended to with diligence. -- Abigail Adams --------------------------------------------------------------------- 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.
