summaryrefslogtreecommitdiffstats
path: root/spec/integration
diff options
context:
space:
mode:
authorMarkus Roberts <Markus@reality.com>2010-07-09 18:05:04 -0700
committerMarkus Roberts <Markus@reality.com>2010-07-09 18:05:04 -0700
commit9ee56f2e67be973da49b1d3f21de1bf87de35e6f (patch)
treeddab8c01509f47664c52c8a6b165bb5a974f138f /spec/integration
parent051bd98751d9d4bc97f93f66723d9b7a00c0cfb4 (diff)
Code smell: Inconsistent indentation and related formatting issues
* Replaced 163 occurances of defined\? +([@a-zA-Z_.0-9?=]+) with defined?(\1) This makes detecting subsequent patterns easier. 3 Examples: The code: if ! defined? @parse_config becomes: if ! defined?(@parse_config) The code: return @option_parser if defined? @option_parser becomes: return @option_parser if defined?(@option_parser) The code: if defined? @local and @local becomes: if defined?(@local) and @local * Eliminate trailing spaces. Replaced 428 occurances of ^(.*?) +$ with \1 1 file was skipped. test/ral/providers/host/parsed.rb because 0 * Replace leading tabs with an appropriate number of spaces. Replaced 306 occurances of ^(\t+)(.*) with Tabs are not consistently expanded in all environments. * Don't arbitrarily wrap on sprintf (%) operator. Replaced 143 occurances of (.*['"] *%) +(.*) with Splitting the line does nothing to aid clarity and hinders further refactorings. 3 Examples: The code: raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)] becomes: raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)] The code: Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig] becomes: Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig] The code: $stderr.puts "Could not find host for PID %s with status %s" % [pid, $?.exitstatus] becomes: $stderr.puts "Could not find host for PID %s with status %s" % [pid, $?.exitstatus] * Don't break short arrays/parameter list in two. Replaced 228 occurances of (.*) +(.*) with 3 Examples: The code: puts @format.wrap(type.provider(prov).doc, :indent => 4, :scrub => true) becomes: puts @format.wrap(type.provider(prov).doc, :indent => 4, :scrub => true) The code: assert(FileTest.exists?(daily), "Did not make daily graph for %s" % type) becomes: assert(FileTest.exists?(daily), "Did not make daily graph for %s" % type) The code: assert(prov.target_object(:first).read !~ /^notdisk/, "Did not remove thing from disk") becomes: assert(prov.target_object(:first).read !~ /^notdisk/, "Did not remove thing from disk") * If arguments must wrap, treat them all equally Replaced 510 occurances of lines ending in things like ...(foo, or ...(bar(1,3), with \1 \2 3 Examples: The code: midscope.to_hash(false), becomes: assert_equal( The code: botscope.to_hash(true), becomes: # bottomscope, then checking that we see the right stuff. The code: :path => link, becomes: * Replaced 4516 occurances of ^( *)(.*) with The present code base is supposed to use four-space indentation. In some places we failed to maintain that standard. These should be fixed regardless of the 2 vs. 4 space question. 15 Examples: The code: def run_comp(cmd) puts cmd results = [] old_sync = $stdout.sync $stdout.sync = true line = [] begin open("| #{cmd}", "r") do |f| until f.eof? do c = f.getc becomes: def run_comp(cmd) puts cmd results = [] old_sync = $stdout.sync $stdout.sync = true line = [] begin open("| #{cmd}", "r") do |f| until f.eof? do c = f.getc The code: s.gsub!(/.{4}/n, '\\\\u\&') } string.force_encoding(Encoding::UTF_8) string rescue Iconv::Failure => e raise GeneratorError, "Caught #{e.class}: #{e}" end else def utf8_to_pson(string) # :nodoc: string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&] } string.gsub!(/( becomes: s.gsub!(/.{4}/n, '\\\\u\&') } string.force_encoding(Encoding::UTF_8) string rescue Iconv::Failure => e raise GeneratorError, "Caught #{e.class}: #{e}" end else def utf8_to_pson(string) # :nodoc: string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&] } string.gsub!(/( The code: end } rvalues: rvalue | rvalues comma rvalue { if val[0].instance_of?(AST::ASTArray) result = val[0].push(val[2]) else result = ast AST::ASTArray, :children => [val[0],val[2]] end } becomes: end } rvalues: rvalue | rvalues comma rvalue { if val[0].instance_of?(AST::ASTArray) result = val[0].push(val[2]) else result = ast AST::ASTArray, :children => [val[0],val[2]] end } The code: #passwdproc = proc { @password } keytext = @key.export( OpenSSL::Cipher::DES.new(:EDE3, :CBC), @password ) File.open(@keyfile, "w", 0400) { |f| f << keytext } becomes: # passwdproc = proc { @password } keytext = @key.export( OpenSSL::Cipher::DES.new(:EDE3, :CBC), @password ) File.open(@keyfile, "w", 0400) { |f| f << keytext } The code: end def to_manifest "%s { '%s':\n%s\n}" % [self.type.to_s, self.name, @params.collect { |p, v| if v.is_a? Array " #{p} => [\'#{v.join("','")}\']" else " #{p} => \'#{v}\'" end }.join(",\n") becomes: end def to_manifest "%s { '%s':\n%s\n}" % [self.type.to_s, self.name, @params.collect { |p, v| if v.is_a? Array " #{p} => [\'#{v.join("','")}\']" else " #{p} => \'#{v}\'" end }.join(",\n") The code: via the augeas tool. Requires: - augeas to be installed (http://www.augeas.net) - ruby-augeas bindings Sample usage with a string:: augeas{\"test1\" : context => \"/files/etc/sysconfig/firstboot\", changes => \"set RUN_FIRSTBOOT YES\", becomes: via the augeas tool. Requires: - augeas to be installed (http://www.augeas.net) - ruby-augeas bindings Sample usage with a string:: augeas{\"test1\" : context => \"/files/etc/sysconfig/firstboot\", changes => \"set RUN_FIRSTBOOT YES\", The code: names.should_not be_include("root") end describe "when generating a purgeable resource" do it "should be included in the generated resources" do Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource] @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref) end end describe "when the instance's do not have an ensure property" do becomes: names.should_not be_include("root") end describe "when generating a purgeable resource" do it "should be included in the generated resources" do Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource] @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref) end end describe "when the instance's do not have an ensure property" do The code: describe "when the instance's do not have an ensure property" do it "should not be included in the generated resources" do @no_ensure_resource = Puppet::Type.type(:exec).new(:name => '/usr/bin/env echo') Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource] @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref) end end describe "when the instance's ensure property does not accept absent" do it "should not be included in the generated resources" do @no_absent_resource = Puppet::Type.type(:service).new(:name => 'foobar') becomes: describe "when the instance's do not have an ensure property" do it "should not be included in the generated resources" do @no_ensure_resource = Puppet::Type.type(:exec).new(:name => '/usr/bin/env echo') Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource] @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref) end end describe "when the instance's ensure property does not accept absent" do it "should not be included in the generated resources" do @no_absent_resource = Puppet::Type.type(:service).new(:name => 'foobar') The code: func = nil assert_nothing_raised do func = Puppet::Parser::AST::Function.new( :name => "template", :ftype => :rvalue, :arguments => AST::ASTArray.new( :children => [stringobj(template)] ) becomes: func = nil assert_nothing_raised do func = Puppet::Parser::AST::Function.new( :name => "template", :ftype => :rvalue, :arguments => AST::ASTArray.new( :children => [stringobj(template)] ) The code: assert( @store.allowed?("hostname.madstop.com", "192.168.1.50"), "hostname not allowed") assert( ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"), "subname name allowed") becomes: assert( @store.allowed?("hostname.madstop.com", "192.168.1.50"), "hostname not allowed") assert( ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"), "subname name allowed") The code: assert_nothing_raised { server = Puppet::Network::Handler.fileserver.new( :Local => true, :Config => false ) } becomes: assert_nothing_raised { server = Puppet::Network::Handler.fileserver.new( :Local => true, :Config => false ) } The code: 'yay', { :failonfail => false, :uid => @user.uid, :gid => @user.gid } ).returns('output') output = Puppet::Util::SUIDManager.run_and_capture 'yay', @user.uid, @user.gid becomes: 'yay', { :failonfail => false, :uid => @user.uid, :gid => @user.gid } ).returns('output') output = Puppet::Util::SUIDManager.run_and_capture 'yay', @user.uid, @user.gid The code: ).times(1) pkg.provider.expects( :aptget ).with( '-y', '-q', 'remove', 'faff' becomes: ).times(1) pkg.provider.expects( :aptget ).with( '-y', '-q', 'remove', 'faff' The code: johnny one two billy three four\n" # Just parse and generate, to make sure it's isomorphic. assert_nothing_raised do assert_equal(text, @parser.to_file(@parser.parse(text)), "parsing was not isomorphic") end end def test_valid_attrs becomes: johnny one two billy three four\n" # Just parse and generate, to make sure it's isomorphic. assert_nothing_raised do assert_equal(text, @parser.to_file(@parser.parse(text)), "parsing was not isomorphic") end end def test_valid_attrs The code: "testing", :onboolean => [true, "An on bool"], :string => ["a string", "A string arg"] ) result = [] should = [] assert_nothing_raised("Add args failed") do @config.addargs(result) end @config.each do |name, element| becomes: "testing", :onboolean => [true, "An on bool"], :string => ["a string", "A string arg"] ) result = [] should = [] assert_nothing_raised("Add args failed") do @config.addargs(result) end @config.each do |name, element|
Diffstat (limited to 'spec/integration')
-rwxr-xr-xspec/integration/defaults_spec.rb10
-rw-r--r--spec/integration/indirector/report/rest_spec.rb5
-rwxr-xr-xspec/integration/transaction_spec.rb55
-rwxr-xr-xspec/integration/type/file_spec.rb25
4 files changed, 73 insertions, 22 deletions
diff --git a/spec/integration/defaults_spec.rb b/spec/integration/defaults_spec.rb
index 3f66a0e8b..3fa753823 100755
--- a/spec/integration/defaults_spec.rb
+++ b/spec/integration/defaults_spec.rb
@@ -195,13 +195,13 @@ describe "Puppet defaults" do
it "should have a setting for determining the configuration version and should default to an empty string" do
Puppet.settings[:config_version].should == ""
end
-
+
describe "when enabling reports" do
it "should use the default server value when report server is unspecified" do
Puppet.settings[:server] = "server"
Puppet.settings[:report_server].should == "server"
end
-
+
it "should use the default masterport value when report port is unspecified" do
Puppet.settings[:masterport] = "1234"
Puppet.settings[:report_port].should == "1234"
@@ -211,20 +211,20 @@ describe "Puppet defaults" do
Puppet.settings[:reportserver] = "reportserver"
Puppet.settings[:report_server].should == "reportserver"
end
-
+
it "should use report_port when set" do
Puppet.settings[:masterport] = "1234"
Puppet.settings[:report_port] = "5678"
Puppet.settings[:report_port].should == "5678"
end
-
+
it "should prefer report_server over reportserver" do
Puppet.settings[:reportserver] = "reportserver"
Puppet.settings[:report_server] = "report_server"
Puppet.settings[:report_server].should == "report_server"
end
end
-
+
it "should have a :caname setting that defaults to the cert name" do
Puppet.settings[:certname] = "foo"
Puppet.settings[:ca_name].should == "foo"
diff --git a/spec/integration/indirector/report/rest_spec.rb b/spec/integration/indirector/report/rest_spec.rb
index 76b3b9003..1ddced9f9 100644
--- a/spec/integration/indirector/report/rest_spec.rb
+++ b/spec/integration/indirector/report/rest_spec.rb
@@ -84,7 +84,10 @@ describe "Report REST Terminus" do
}
report.add_metric(:times, timemetrics)
- report.add_metric(:changes,
+
+ report.add_metric(
+ :changes,
+
:total => 20
)
diff --git a/spec/integration/transaction_spec.rb b/spec/integration/transaction_spec.rb
index c93de1761..57dd490dd 100755
--- a/spec/integration/transaction_spec.rb
+++ b/spec/integration/transaction_spec.rb
@@ -106,20 +106,29 @@ describe Puppet::Transaction do
path = tmpfile("path")
file1 = tmpfile("file1")
file2 = tmpfile("file2")
- file = Puppet::Type.type(:file).new(
+
+ file = Puppet::Type.type(:file).new(
+
:path => path,
+
:ensure => "file"
)
- exec1 = Puppet::Type.type(:exec).new(
+
+ exec1 = Puppet::Type.type(:exec).new(
+
:path => ENV["PATH"],
:command => "touch %s" % file1,
:refreshonly => true,
+
:subscribe => Puppet::Resource.new(:file, path)
)
- exec2 = Puppet::Type.type(:exec).new(
+
+ exec2 = Puppet::Type.type(:exec).new(
+
:path => ENV["PATH"],
:command => "touch %s" % file2,
:refreshonly => true,
+
:subscribe => Puppet::Resource.new(:file, path)
)
@@ -132,24 +141,33 @@ describe Puppet::Transaction do
it "should not let one failed refresh result in other refreshes failing" do
path = tmpfile("path")
newfile = tmpfile("file")
- file = Puppet::Type.type(:file).new(
+
+ file = Puppet::Type.type(:file).new(
+
:path => path,
+
:ensure => "file"
)
- exec1 = Puppet::Type.type(:exec).new(
+
+ exec1 = Puppet::Type.type(:exec).new(
+
:path => ENV["PATH"],
:command => "touch /this/cannot/possibly/exist",
:logoutput => true,
:refreshonly => true,
:subscribe => file,
+
:title => "one"
)
- exec2 = Puppet::Type.type(:exec).new(
+
+ exec2 = Puppet::Type.type(:exec).new(
+
:path => ENV["PATH"],
:command => "touch %s" % newfile,
:logoutput => true,
:refreshonly => true,
:subscribe => [file, exec1],
+
:title => "two"
)
@@ -165,17 +183,23 @@ describe Puppet::Transaction do
catalog.add_resource(*Puppet::Type.type(:schedule).mkdefaultschedules)
Puppet[:ignoreschedules] = false
- file = Puppet::Type.type(:file).new(
+
+ file = Puppet::Type.type(:file).new(
+
:name => tmpfile("file"),
+
:ensure => "file",
:backup => false
)
fname = tmpfile("exec")
- exec = Puppet::Type.type(:exec).new(
+
+ exec = Puppet::Type.type(:exec).new(
+
:name => "touch #{fname}",
:path => "/usr/bin:/bin",
:schedule => "monthly",
+
:subscribe => Puppet::Resource.new("file", file.name)
)
@@ -211,21 +235,30 @@ describe Puppet::Transaction do
end
it "should not attempt to evaluate resources with failed dependencies" do
- exec = Puppet::Type.type(:exec).new(
+
+ exec = Puppet::Type.type(:exec).new(
+
:command => "/bin/mkdir /this/path/cannot/possibly/exit",
+
:title => "mkdir"
)
- file1 = Puppet::Type.type(:file).new(
+
+ file1 = Puppet::Type.type(:file).new(
+
:title => "file1",
:path => tmpfile("file1"),
+
:require => exec,
:ensure => :file
)
- file2 = Puppet::Type.type(:file).new(
+
+ file2 = Puppet::Type.type(:file).new(
+
:title => "file2",
:path => tmpfile("file2"),
+
:require => file1,
:ensure => :file
)
diff --git a/spec/integration/type/file_spec.rb b/spec/integration/type/file_spec.rb
index f215f0b08..e16da67cc 100755
--- a/spec/integration/type/file_spec.rb
+++ b/spec/integration/type/file_spec.rb
@@ -392,8 +392,11 @@ describe Puppet::Type.type(:file) do
it "should be able to create files when 'content' is specified but 'ensure' is not" do
dest = tmpfile("files_with_content")
- file = Puppet::Type.type(:file).new(
+
+ file = Puppet::Type.type(:file).new(
+
:name => dest,
+
:content => "this is some content, yo"
)
@@ -407,9 +410,12 @@ describe Puppet::Type.type(:file) do
it "should create files with content if both 'content' and 'ensure' are set" do
dest = tmpfile("files_with_content")
- file = Puppet::Type.type(:file).new(
+
+ file = Puppet::Type.type(:file).new(
+
:name => dest,
:ensure => "file",
+
:content => "this is some content, yo"
)
@@ -426,10 +432,13 @@ describe Puppet::Type.type(:file) do
File.open(source, "w") { |f| f.puts "yay" }
File.open(dest, "w") { |f| f.puts "boo" }
- file = Puppet::Type.type(:file).new(
+
+ file = Puppet::Type.type(:file).new(
+
:name => dest,
:ensure => :absent,
:source => source,
+
:backup => false
)
@@ -455,18 +464,24 @@ describe Puppet::Type.type(:file) do
# this file should get removed
File.open(@purgee, "w") { |f| f.puts "footest" }
- @lfobj = Puppet::Type.newfile(
+
+ @lfobj = Puppet::Type.newfile(
+
:title => "localfile",
:path => @localfile,
:content => "rahtest\n",
:ensure => :file,
+
:backup => false
)
- @destobj = Puppet::Type.newfile(:title => "destdir", :path => @destdir,
+
+ @destobj = Puppet::Type.newfile(
+ :title => "destdir", :path => @destdir,
:source => @sourcedir,
:backup => false,
:purge => true,
+
:recurse => true)
@catalog = Puppet::Resource::Catalog.new