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.
