]> review.fuel-infra Code Review - puppet-modules/puppetlabs-apt.git/commitdiff
Modify apt::source release parameter test
authorReid Vandewiele <marut@cat.pdx.edu>
Wed, 7 Mar 2012 18:10:46 +0000 (10:10 -0800)
committerReid Vandewiele <marut@cat.pdx.edu>
Wed, 7 Mar 2012 18:10:46 +0000 (10:10 -0800)
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
spec/defines/source_spec.rb

index 4c03412c5fb8f81979710fda460008a669fe5da1..42a21e9e49c3293a80886a75c7dc67e4cb43e1d1 100644 (file)
@@ -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 {
index 903468e5b66cc35f98e0c52389118930672836b2..e93137e8e4e32a1ece8895f63a38e933b6445272 100644 (file)
@@ -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
-