From d4fec561f3eb7a61570dcdca1c3b10f309b3f4d5 Mon Sep 17 00:00:00 2001 From: Reid Vandewiele Date: Wed, 7 Mar 2012 10:10:46 -0800 Subject: [PATCH] Modify apt::source release parameter test This commit modifies the release parameter test in apt::source to work correctly within puppet-rspec for edge-case resource definitions. Previously, the test for the $release parameter was written as `if ! $release { fail() }` This commit updates the test to be written as `if $release == undef { fail() }` Additionally, the tests for correct behavior in the presence or absence of a $release parameter have been beefed up. The reason for making this change relates to examples such as the following resource definition: apt::source { "jenkins": location => "http://pkg.jenkins-ci.org/debian", release => "", repos => "binary/", key => "D50582E6", key_source => "http://pkg.jenkins-ci.org/debian/jenkins-ci.org.key", include_src => false, } Note that the $release parameter is given as the empty string. In practice, this is perfectly valid and everything will work great. However, it seems that the empty string gets interpreted by something in puppet-rspec as something equivalent to "False", and thus when testing, the above resource definition would fail with "Puppet::Error: lsbdistcodename fact not available: release parameter required" even though the $release parameter has been explicitely specified (as the empty string). See also: https://github.com/rtyler/puppet-jenkins/issues/9 --- manifests/source.pp | 3 +-- spec/defines/source_spec.rb | 11 +++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/manifests/source.pp b/manifests/source.pp index 4c03412..42a21e9 100644 --- a/manifests/source.pp +++ b/manifests/source.pp @@ -16,7 +16,7 @@ define apt::source( include apt::params - if ! $release { + if $release == undef { fail("lsbdistcodename fact not available: release parameter required") } @@ -27,7 +27,6 @@ define apt::source( group => root, mode => 644, content => template("apt/source.list.erb"), - } if $pin != false { diff --git a/spec/defines/source_spec.rb b/spec/defines/source_spec.rb index 903468e..e93137e 100644 --- a/spec/defines/source_spec.rb +++ b/spec/defines/source_spec.rb @@ -136,8 +136,11 @@ describe 'apt::source', :type => :define do } end end - describe "without release should raise a Puppet::Error" do - it { expect { should contain_apt__source(:release) }.to raise_error(Puppet::Error) } - end + describe "without release should raise a Puppet::Error" do + let(:default_params) { Hash.new } + let(:facts) { Hash.new } + it { expect { should raise_error(Puppet::Error) } } + let(:facts) { { :lsbdistcodename => 'lucid' } } + it { should contain_apt__source(title) } + end end -