From d522877cdde21ffca44c5af7a0ba6a3312c4af8a Mon Sep 17 00:00:00 2001 From: Matthaus Litteken Date: Fri, 3 Feb 2012 17:24:09 -0800 Subject: [PATCH] (#12094) Replace chained .with_* with a hash The hash passing to the with method is cleaner and closer to puppet code, so all of the with_$param have been replaced with with($hash). This also includes two minor whitspace changes to unstable.pp and source.pp. This also replaces the ternary switch on param_set with a hash merge, which is cleaner and will support more use cases. --- manifests/debian/unstable.pp | 2 +- manifests/source.pp | 2 +- spec/classes/apt_spec.rb | 49 +++++++------ spec/classes/debian_testing_spec.rb | 17 ++--- spec/classes/debian_unstable_spec.rb | 17 ++--- spec/classes/params_spec.rb | 2 +- spec/classes/release_spec.rb | 11 +-- spec/defines/builddep_spec.rb | 13 +++- spec/defines/force_spec.rb | 2 +- spec/defines/pin_spec.rb | 17 ++--- spec/defines/ppa_spec.rb | 4 +- spec/defines/source_spec.rb | 100 ++++++++++++++++----------- 12 files changed, 134 insertions(+), 102 deletions(-) diff --git a/manifests/debian/unstable.pp b/manifests/debian/unstable.pp index 89e5d9a..782440f 100644 --- a/manifests/debian/unstable.pp +++ b/manifests/debian/unstable.pp @@ -7,7 +7,7 @@ class apt::debian::unstable { # Key: 55BE302B Server: subkeys.pgp.net # debian-keyring # debian-archive-keyring - + apt::source { "debian_unstable": location => "http://debian.mirror.iweb.ca/debian/", release => "unstable", diff --git a/manifests/source.pp b/manifests/source.pp index 9f040b7..ddf753a 100644 --- a/manifests/source.pp +++ b/manifests/source.pp @@ -48,7 +48,7 @@ define apt::source( command => "/bin/echo '${key_content}' | /usr/bin/apt-key add -", unless => "/usr/bin/apt-key list | /bin/grep '${key}'", before => File["${name}.list"], - } + } } else { exec { "/usr/bin/apt-key adv --keyserver ${key_server} --recv-keys ${key}": unless => "/usr/bin/apt-key list | /bin/grep ${key}", diff --git a/spec/classes/apt_spec.rb b/spec/classes/apt_spec.rb index a858180..5ac64f2 100644 --- a/spec/classes/apt_spec.rb +++ b/spec/classes/apt_spec.rb @@ -15,7 +15,7 @@ describe 'apt', :type => :class do ].each do |param_set| describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do let :param_hash do - param_set == {} ? default_params : params + default_params.merge(param_set) end let :params do @@ -35,38 +35,43 @@ describe 'apt', :type => :class do it { should contain_package("python-software-properties") } it { - should create_file("sources.list")\ - .with_path("/etc/apt/sources.list")\ - .with_ensure("present")\ - .with_owner("root")\ - .with_group("root")\ - .with_mode(644) + should contain_file("sources.list").with({ + 'path' => "/etc/apt/sources.list", + 'ensure' => "present", + 'owner' => "root", + 'group' => "root", + 'mode' => 644 + }) } it { - should create_file("sources.list.d")\ - .with_path("/etc/apt/sources.list.d")\ - .with_ensure("directory")\ - .with_owner("root")\ - .with_group("root") + should create_file("sources.list.d").with({ + "path" => "/etc/apt/sources.list.d", + "ensure" => "directory", + "owner" => "root", + "group" => "root" + }) } it { - should create_exec("apt_update")\ - .with_command("/usr/bin/apt-get update")\ - .with_subscribe(["File[sources.list]", "File[sources.list.d]"])\ - .with_refreshonly(refresh_only_apt_update) + should contain_exec("apt_update").with({ + 'command' => "/usr/bin/apt-get update", + 'subscribe' => ["File[sources.list]", "File[sources.list.d]"], + 'refreshonly' => refresh_only_apt_update + }) } it { if param_hash[:disable_keys] - should create_exec("make-apt-insecure")\ - .with_command('/bin/echo "APT::Get::AllowUnauthenticated 1;" >> /etc/apt/apt.conf.d/99unauth')\ - .with_creates('/etc/apt/apt.conf.d/99unauth') + should contain_exec("make-apt-insecure").with({ + 'command' => '/bin/echo "APT::Get::AllowUnauthenticated 1;" >> /etc/apt/apt.conf.d/99unauth', + 'creates' => '/etc/apt/apt.conf.d/99unauth' + }) else - should_not create_exec("make-apt-insecure")\ - .with_command('/bin/echo "APT::Get::AllowUnauthenticated 1;" >> /etc/apt/apt.conf.d/99unauth')\ - .with_creates('/etc/apt/apt.conf.d/99unauth') + should_not contain_exec("make-apt-insecure").with({ + 'command' => '/bin/echo "APT::Get::AllowUnauthenticated 1;" >> /etc/apt/apt.conf.d/99unauth', + 'creates' => '/etc/apt/apt.conf.d/99unauth' + }) end } end diff --git a/spec/classes/debian_testing_spec.rb b/spec/classes/debian_testing_spec.rb index 0ee1b2e..6006afb 100644 --- a/spec/classes/debian_testing_spec.rb +++ b/spec/classes/debian_testing_spec.rb @@ -1,13 +1,14 @@ require 'spec_helper' describe 'apt::debian::testing', :type => :class do it { - should create_resource("Apt::source", "debian_testing")\ - .with_param("location", "http://debian.mirror.iweb.ca/debian/")\ - .with_param("release", "testing")\ - .with_param("repos", "main contrib non-free")\ - .with_param("required_packages", "debian-keyring debian-archive-keyring")\ - .with_param("key", "55BE302B")\ - .with_param("key_server", "subkeys.pgp.net")\ - .with_param("pin", "-10") + should contain_apt__source("debian_testing").with({ + "location" => "http://debian.mirror.iweb.ca/debian/", + "release" => "testing", + "repos" => "main contrib non-free", + "required_packages" => "debian-keyring debian-archive-keyring", + "key" => "55BE302B", + "key_server" => "subkeys.pgp.net", + "pin" => "-10" + }) } end diff --git a/spec/classes/debian_unstable_spec.rb b/spec/classes/debian_unstable_spec.rb index 5673b78..411182d 100644 --- a/spec/classes/debian_unstable_spec.rb +++ b/spec/classes/debian_unstable_spec.rb @@ -1,13 +1,14 @@ require 'spec_helper' describe 'apt::debian::unstable', :type => :class do it { - should create_resource("Apt::source", "debian_unstable")\ - .with_param("location", "http://debian.mirror.iweb.ca/debian/")\ - .with_param("release", "unstable")\ - .with_param("repos", "main contrib non-free")\ - .with_param("required_packages", "debian-keyring debian-archive-keyring")\ - .with_param("key", "55BE302B")\ - .with_param("key_server", "subkeys.pgp.net")\ - .with_param("pin", "-10") + should contain_apt__source("debian_unstable").with({ + "location" => "http://debian.mirror.iweb.ca/debian/", + "release" => "unstable", + "repos" => "main contrib non-free", + "required_packages" => "debian-keyring debian-archive-keyring", + "key" => "55BE302B", + "key_server" => "subkeys.pgp.net", + "pin" => "-10" + }) } end diff --git a/spec/classes/params_spec.rb b/spec/classes/params_spec.rb index a0ec08c..f2790b0 100644 --- a/spec/classes/params_spec.rb +++ b/spec/classes/params_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe 'apt::params', :type => :class do let (:title) { 'my_package' } - it { should create_class("apt::params") } + it { should contain_apt__params } # There are 4 resources in this class currently # there should not be any more resources because it is a params class diff --git a/spec/classes/release_spec.rb b/spec/classes/release_spec.rb index a8b1448..221df03 100644 --- a/spec/classes/release_spec.rb +++ b/spec/classes/release_spec.rb @@ -11,11 +11,12 @@ describe 'apt::release', :type => :class do it { should include_class("apt::params") } it { - should contain_file("/etc/apt/apt.conf.d/01release")\ - .with_owner("root")\ - .with_group("root")\ - .with_mode(644)\ - .with_content("APT::Default-Release \"#{param_set[:release_id]}\";") + should contain_file("/etc/apt/apt.conf.d/01release").with({ + "owner" => "root", + "group" => "root", + "mode" => 644, + "content" => "APT::Default-Release \"#{param_set[:release_id]}\";" + }) } end diff --git a/spec/defines/builddep_spec.rb b/spec/defines/builddep_spec.rb index d60a330..52a0704 100644 --- a/spec/defines/builddep_spec.rb +++ b/spec/defines/builddep_spec.rb @@ -6,12 +6,19 @@ describe 'apt::builddep', :type => :define do describe "should succeed with a Class['apt']" do let(:pre_condition) { 'class {"apt": } ' } - it { should contain_exec("apt-update-#{title}").with_command("/usr/bin/apt-get update").with_refreshonly(true) } + it { should contain_exec("apt-update-#{title}").with({ + 'command' => "/usr/bin/apt-get update", + 'refreshonly' => true + }) + } end describe "should fail without Class['apt']" do - it { expect {should contain_exec("apt-update-#{title}").with_command("/usr/bin/apt-get update").with_refreshonly(true) }\ - .to raise_error(Puppet::Error) + it { expect {should contain_exec("apt-update-#{title}").with({ + 'command' => "/usr/bin/apt-get update", + 'refreshonly' => true + }).to raise_error(Puppet::Error) + } } end diff --git a/spec/defines/force_spec.rb b/spec/defines/force_spec.rb index 11b82e3..eeea7ff 100644 --- a/spec/defines/force_spec.rb +++ b/spec/defines/force_spec.rb @@ -19,7 +19,7 @@ describe 'apt::force', :type => :define do ].each do |param_set| describe "when #{param_set == {} ? "using default" : "specifying"} define parameters" do let :param_hash do - param_set == {} ? default_params : params + default_params.merge(param_set) end let :params do diff --git a/spec/defines/pin_spec.rb b/spec/defines/pin_spec.rb index ac61864..52c6bfa 100644 --- a/spec/defines/pin_spec.rb +++ b/spec/defines/pin_spec.rb @@ -17,7 +17,7 @@ describe 'apt::pin', :type => :define do ].each do |param_set| describe "when #{param_set == {} ? "using default" : "specifying"} define parameters" do let :param_hash do - param_set == {} ? default_params : params + default_params.merge(param_set) end let :params do @@ -26,13 +26,14 @@ describe 'apt::pin', :type => :define do it { should include_class("apt::params") } - it { should create_file("#{title}.pref")\ - .with_path("/etc/apt/preferences.d/#{title}")\ - .with_ensure("file")\ - .with_owner("root")\ - .with_group("root")\ - .with_mode("644")\ - .with_content("# #{title}\nPackage: #{param_hash[:packages]}\nPin: release a=#{title}\nPin-Priority: #{param_hash[:priority]}") + it { should contain_file("#{title}.pref").with({ + 'path' => "/etc/apt/preferences.d/#{title}", + 'ensure' => "file", + 'owner' => "root", + 'group' => "root", + 'mode' => "644", + 'content' => "# #{title}\nPackage: #{param_hash[:packages]}\nPin: release a=#{title}\nPin-Priority: #{param_hash[:priority]}" + }) } end end diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb index 3d786e1..c2ce264 100644 --- a/spec/defines/ppa_spec.rb +++ b/spec/defines/ppa_spec.rb @@ -20,7 +20,7 @@ describe 'apt::ppa', :type => :define do 'notify' => "Exec[apt-update-#{t}]" ) } - it { should contain_exec("add-apt-repository-#{t}").with_unless(unless_statement) } + it { should contain_exec("add-apt-repository-#{t}").with({"unless" => unless_statement}) } it { should contain_exec("apt-update-#{t}").with( 'command' => '/usr/bin/aptitude update', 'refreshonly' => true @@ -32,6 +32,6 @@ describe 'apt::ppa', :type => :define do describe "without Class[apt] should raise a Puppet::Error" do let(:title) { "ppa" } - it { expect { should create_resource("apt::ppa", title) }.to raise_error(Puppet::Error) } + it { expect { should contain_apt__ppa(title) }.to raise_error(Puppet::Error) } end end diff --git a/spec/defines/source_spec.rb b/spec/defines/source_spec.rb index 333d534..01949b6 100644 --- a/spec/defines/source_spec.rb +++ b/spec/defines/source_spec.rb @@ -33,7 +33,7 @@ describe 'apt::source', :type => :define do ].each do |param_set| describe "when #{param_set == {} ? "using default" : "specifying"} class parameters" do let :param_hash do - param_set == {} ? default_params : params + default_params.merge(param_set) end let :params do @@ -53,72 +53,88 @@ describe 'apt::source', :type => :define do content end - it { should contain_class("apt::params") } + it { should contain_apt__params } - it { should contain_file("#{title}.list")\ - .with_path(filename)\ - .with_ensure("file")\ - .with_owner("root")\ - .with_group("root")\ - .with_mode(644)\ - .with_content(content) + it { should contain_file("#{title}.list").with({ + 'path' => filename, + 'ensure' => "file", + 'owner' => "root", + 'group' => "root", + 'mode' => 644, + 'content' => content + }) } it { if param_hash[:pin] - should create_resource("apt::pin", param_hash[:release]).with_param("priority", param_hash[:pin]).with_param("before", "File[#{title}.list]") + should contain_apt__pin(param_hash[:release]).with({ + "priority" => param_hash[:pin], + "before" => "File[#{title}.list]" + }) else - should_not create_resource("apt::pin", param_hash[:release]).with_param("priority", param_hash[:pin]).with_param("before", "File[#{title}.list]") + should_not contain_apt__pin(param_hash[:release]).with({ + "priority" => param_hash[:pin], + "before" => "File[#{title}.list]" + }) end } it { - should contain_exec("#{title} apt update")\ - .with_command("/usr/bin/apt-get update")\ - .with_subscribe("File[#{title}.list]")\ - .with_refreshonly(true) + should contain_exec("#{title} apt update").with({ + "command" => "/usr/bin/apt-get update", + "subscribe" => "File[#{title}.list]", + "refreshonly" => true + }) } it { if param_hash[:required_packages] - should contain_exec("/usr/bin/apt-get -y install #{param_hash[:required_packages]}")\ - .with_subscribe("File[#{title}.list]")\ - .with_refreshonly(true) + should contain_exec("/usr/bin/apt-get -y install #{param_hash[:required_packages]}").with({ + "subscribe" => "File[#{title}.list]", + "refreshonly" => true + }) else - should_not contain_exec("/usr/bin/apt-get -y install #{param_hash[:required_packages]}")\ - .with_subscribe("File[#{title}.list]")\ - .with_refreshonly(true) + should_not contain_exec("/usr/bin/apt-get -y install #{param_hash[:required_packages]}").with({ + "subscribe" => "File[#{title}.list]", + "refreshonly" => true + }) end } it { if param_hash[:key] if param_hash[:key_content] - should contain_exec("Add key: #{param_hash[:key]} from content")\ - .with_command("/bin/echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -")\ - .with_unless("/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'")\ - .with_before("File[#{title}.list]") - should_not contain_exec("/usr/bin/apt-key adv --keyserver #{param_hash[:key_server]} --recv-keys #{param_hash[:key]}")\ - .with_unless("/usr/bin/apt-key list | /bin/grep #{param_hash[:key]}")\ - .with_before("File[#{title}.list]") + should contain_exec("Add key: #{param_hash[:key]} from content").with({ + "command" => "/bin/echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -", + "unless" => "/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'", + "before" => "File[#{title}.list]" + }) + should_not contain_exec("/usr/bin/apt-key adv --keyserver #{param_hash[:key_server]} --recv-keys #{param_hash[:key]}").with({ + "unless" => "/usr/bin/apt-key list | /bin/grep #{param_hash[:key]}", + "before" => "File[#{title}.list]" + }) else - should contain_exec("/usr/bin/apt-key adv --keyserver #{param_hash[:key_server]} --recv-keys #{param_hash[:key]}")\ - .with_unless("/usr/bin/apt-key list | /bin/grep #{param_hash[:key]}")\ - .with_before("File[#{title}.list]") - should_not contain_exec("Add key: #{param_hash[:key]} from content")\ - .with_command("/bin/echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -")\ - .with_unless("/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'")\ - .with_before("File[#{title}.list]") + should contain_exec("/usr/bin/apt-key adv --keyserver #{param_hash[:key_server]} --recv-keys #{param_hash[:key]}").with({ + "unless" => "/usr/bin/apt-key list | /bin/grep #{param_hash[:key]}", + "before" => "File[#{title}.list]" + }) + should_not contain_exec("Add key: #{param_hash[:key]} from content").with({ + "command" => "/bin/echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -", + "unless" => "/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'", + "before" => "File[#{title}.list]" + }) end else - should_not contain_exec("Add key: #{param_hash[:key]} from content")\ - .with_command("/bin/echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -")\ - .with_unless("/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'")\ - .with_before("File[#{title}.list]") - should_not contain_exec("/usr/bin/apt-key adv --keyserver #{param_hash[:key_server]} --recv-keys #{param_hash[:key]}")\ - .with_unless("/usr/bin/apt-key list | /bin/grep #{param_hash[:key]}")\ - .with_before("File[#{title}.list]") + should_not contain_exec("Add key: #{param_hash[:key]} from content").with({ + "command" => "/bin/echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -", + "unless" => "/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'", + "before" => "File[#{title}.list]" + }) + should_not contain_exec("/usr/bin/apt-key adv --keyserver #{param_hash[:key_server]} --recv-keys #{param_hash[:key]}").with({ + "unless" => "/usr/bin/apt-key list | /bin/grep #{param_hash[:key]}", + "before" => "File[#{title}.list]" + }) end }