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.
Regards,
Daniel
--
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]<puppet-dev%[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]<puppet-dev%[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.
