Code smell: Use string interpolation

* Replaced 83 occurances of

      (.*)" *[+] *([$@]?[\w_0-9.:]+?)(.to_s\b)?(?! *[*(%\w_0-9.:{\[])

  with

      \1#{\2}"

  3 Examples:

      The code:
          puts "PUPPET " + status + ": " + process + ", " + state
      becomes:
          puts "PUPPET " + status + ": " + process + ", #{state}"
      The code:
          puts "PUPPET " + status + ": #{process}" + ", #{state}"
      becomes:
          puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
      The code:
          }.compact.join( "\n" ) + "\n" + t + "]\n"
      becomes:
          }.compact.join( "\n" ) + "\n#{t}" + "]\n"

* Replaced 21 occurances of (.*)" *[+] *" with \1

  3 Examples:

      The code:
          puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
      becomes:
          puts "PUPPET #{status}" + ": #{process}, #{state}"
      The code:
          puts "PUPPET #{status}" + ": #{process}, #{state}"
      becomes:
          puts "PUPPET #{status}: #{process}, #{state}"
      The code:
          res = self.class.name + ": #{@name}" + "\n"
      becomes:
          res = self.class.name + ": #{@name}\n"

* Don't use string concatenation to split lines unless they would be very long.

  Replaced 11 occurances of

      (.*)(['"]) *[+]
       *(['"])(.*)

  with

  3 Examples:

      The code:
          o.define_head "The check_puppet Nagios plug-in checks that specified " +
              "Puppet process is running and the state file is no " +
      becomes:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " +
      The code:
          o.separator   "Mandatory arguments to long options are mandatory for " +
          "short options too."
      becomes:
          o.separator   "Mandatory arguments to long options are mandatory for short options too."
      The code:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " +
              "older than specified interval."
      becomes:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no older than specified interval."

* Replaced no occurances of do (.*?) end with {\1}

* Replaced 1488 occurances of

      "([^"\n]*%s[^"\n]*)" *% *(.+?)(?=$| *\b(do|if|while|until|unless|#)\b)

  with

  20 Examples:

      The code:
          args[0].split(/\./).map do |s| "dc=%s"%[s] end.join(",")
      becomes:
          args[0].split(/\./).map do |s| "dc=#{s}" end.join(",")
      The code:
          puts "%s" % Puppet.version
      becomes:
          puts "#{Puppet.version}"
      The code:
          raise "Could not find information for %s" % node
      becomes:
          raise "Could not find information for #{node}"
      The code:
          raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)]
      becomes:
          raise Puppet::Error, "Cannot create #{dir}: basedir #{File.join(path)} is a file"
      The code:
          Puppet.err "Could not run %s: %s" % [client_class, detail]
      becomes:
          Puppet.err "Could not run #{client_class}: #{detail}"
      The code:
          raise "Could not find handler for %s" % arg
      becomes:
          raise "Could not find handler for #{arg}"
      The code:
          Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig]
      becomes:
          Puppet.err "Will not start without authorization file #{Puppet[:authconfig]}"
      The code:
          raise Puppet::Error, "Could not deserialize catalog from pson: %s" % detail
      becomes:
          raise Puppet::Error, "Could not deserialize catalog from pson: #{detail}"
      The code:
          raise "Could not find facts for %s" % Puppet[:certname]
      becomes:
          raise "Could not find facts for #{Puppet[:certname]}"
      The code:
          raise ArgumentError, "%s is not readable" % path
      becomes:
          raise ArgumentError, "#{path} is not readable"
      The code:
          raise ArgumentError, "Invalid handler %s" % name
      becomes:
          raise ArgumentError, "Invalid handler #{name}"
      The code:
          debug "Executing '%s' in zone %s with '%s'" % [command, @resource[:name], str]
      becomes:
          debug "Executing '#{command}' in zone #{@resource[:name]} with '#{str}'"
      The code:
          raise Puppet::Error, "unknown cert type '%s'" % hash[:type]
      becomes:
          raise Puppet::Error, "unknown cert type '#{hash[:type]}'"
      The code:
          Puppet.info "Creating a new certificate request for %s" % Puppet[:certname]
      becomes:
          Puppet.info "Creating a new certificate request for #{Puppet[:certname]}"
      The code:
          "Cannot create alias %s: object already exists" % [name]
      becomes:
          "Cannot create alias #{name}: object already exists"
      The code:
          return "replacing from source %s with contents %s" % [metadata.source, metadata.checksum]
      becomes:
          return "replacing from source #{metadata.source} with contents #{metadata.checksum}"
      The code:
          it "should have a %s parameter" % param do
      becomes:
          it "should have a #{param} parameter" do
      The code:
          describe "when registring '%s' messages" % log do
      becomes:
          describe "when registring '#{log}' messages" do
      The code:
          paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration%stest" % l }
      becomes:
          paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration#{l}test" }
      The code:
          assert_raise(Puppet::Error, "Check '%s' did not fail on false" % check) do
      becomes:
          assert_raise(Puppet::Error, "Check '#{check}' did not fail on false") do

11 files changed, 47 insertions, 47 deletions

diff --git a/test/lib/puppettest.rb b/test/lib/puppettest.rb
index 522112db8..9bcbff9c1 100755
--- a/test/lib/puppettest.rb
+++ b/test/lib/puppettest.rb
@@ -274,7 +274,7 @@ module PuppetTest
                         end
 
 
-            @tmpdir = File.join(@tmpdir, "puppettesting" + Process.pid.to_s)
+            @tmpdir = File.join(@tmpdir, "puppettesting#{Process.pid}")
 
             unless File.exists?(@tmpdir)
                 FileUtils.mkdir_p(@tmpdir)
@@ -287,12 +287,12 @@ module PuppetTest
     def remove_tmp_files
         @@tmpfiles.each { |file|
             unless file =~ /tmp/
-                puts "Not deleting tmpfile %s" % file
+                puts "Not deleting tmpfile #{file}"
                 next
             end
             if FileTest.exists?(file)
-                system("chmod -R 755 %s" % file)
-                system("rm -rf %s" % file)
+                system("chmod -R 755 #{file}")
+                system("rm -rf #{file}")
             end
         }
         @@tmpfiles.clear
@@ -320,7 +320,7 @@ module PuppetTest
         diff = @memoryatend - @memoryatstart
 
         if diff > 1000
-            Puppet.info "%s#%s memory growth (%s to %s): %s" % [self.class, @method_name, @memoryatstart, @memoryatend, diff]
+            Puppet.info "#{self.class}##{@method_name} memory growth (#{@memoryatstart} to #{@memoryatend}): #{diff}"
         end
 
         # reset all of the logs
diff --git a/test/lib/puppettest/certificates.rb b/test/lib/puppettest/certificates.rb
index 3ba052505..4f643b2d5 100644
--- a/test/lib/puppettest/certificates.rb
+++ b/test/lib/puppettest/certificates.rb
@@ -9,7 +9,7 @@ module PuppetTest::Certificates
         keyfile = File.join(@dir, "tmpkeyfile")
         @@tmpfiles << keyfile
         unless FileTest.exists?(@dir)
-            system("mkdir -p %s" % @dir)
+            system("mkdir -p #{@dir}")
         end
         File.open(keyfile, "w", 0600) { |f|
             f.print "as;dklj23rlkjzdflij23wr"
diff --git a/test/lib/puppettest/exetest.rb b/test/lib/puppettest/exetest.rb
index b0857d19f..c07615245 100644
--- a/test/lib/puppettest/exetest.rb
+++ b/test/lib/puppettest/exetest.rb
@@ -50,28 +50,28 @@ module PuppetTest::ExeTest
         output = nil
 
         manifest = mktestmanifest()
-        args += " --manifest %s" % manifest
-        args += " --confdir %s" % Puppet[:confdir]
-        args += " --rundir %s" % File.join(Puppet[:vardir], "run")
-        args += " --vardir %s" % Puppet[:vardir]
-        args += " --certdnsnames %s" % Puppet[:certdnsnames]
-        args += " --masterport %s" % @@port
-        args += " --user %s" % Puppet::Util::SUIDManager.uid
-        args += " --group %s" % Puppet::Util::SUIDManager.gid
+        args += " --manifest #{manifest}"
+        args += " --confdir #{Puppet[:confdir]}"
+        args += " --rundir #{File.join(Puppet[:vardir], "run")}"
+        args += " --vardir #{Puppet[:vardir]}"
+        args += " --certdnsnames #{Puppet[:certdnsnames]}"
+        args += " --masterport #{@@port}"
+        args += " --user #{Puppet::Util::SUIDManager.uid}"
+        args += " --group #{Puppet::Util::SUIDManager.gid}"
         args += " --autosign true"
 
         #if Puppet[:debug]
         #    args += " --debug"
         #end
 
-        cmd = "puppetmasterd %s" % args
+        cmd = "puppetmasterd #{args}"
 
 
         assert_nothing_raised {
             output = %x{#{cmd}}.chomp
         }
-        assert_equal("", output, "Puppetmasterd produced output %s" % output)
-        assert($CHILD_STATUS == 0, "Puppetmasterd exit status was %s" % $CHILD_STATUS)
+        assert_equal("", output, "Puppetmasterd produced output #{output}")
+        assert($CHILD_STATUS == 0, "Puppetmasterd exit status was #{$CHILD_STATUS}")
         sleep(1)
 
         cleanup do
diff --git a/test/lib/puppettest/fakes.rb b/test/lib/puppettest/fakes.rb
index d421b292f..c0e0bd899 100644
--- a/test/lib/puppettest/fakes.rb
+++ b/test/lib/puppettest/fakes.rb
@@ -22,7 +22,7 @@ module PuppetTest
         end
 
         def self.to_s
-            "Fake%s" % @name.to_s.capitalize
+            "Fake#{@name.to_s.capitalize}"
         end
 
         def [](param)
@@ -36,7 +36,7 @@ module PuppetTest
         def []=(param, value)
             param = symbolize(param)
             unless @realresource.valid_parameter?(param)
-                raise Puppet::DevError, "Invalid attribute %s for %s" % [param, @realresource.name]
+                raise Puppet::DevError, "Invalid attribute #{param} for #{@realresource.name}"
             end
             if @realresource.attrtype(param) == :property
                 @should[param] = value
@@ -55,7 +55,7 @@ module PuppetTest
         end
 
         def inspect
-            "%s(%s)" % [self.class.to_s.sub(/.+::/, ''), super()]
+            "#{self.class.to_s.sub(/.+::/, '')}(#{super()})"
         end
 
         def is(param)
@@ -174,7 +174,7 @@ module PuppetTest
             @@fakeresources[type].name = type
 
             resource = Puppet::Type.type(type)
-            raise("Could not find type %s" % type) unless resource
+            raise("Could not find type #{type}") unless resource
             @@fakeresources[type].realresource = resource
         end
 
diff --git a/test/lib/puppettest/filetesting.rb b/test/lib/puppettest/filetesting.rb
index 605de6a73..3c869f5cf 100644
--- a/test/lib/puppettest/filetesting.rb
+++ b/test/lib/puppettest/filetesting.rb
@@ -88,7 +88,7 @@ module PuppetTest::FileTesting
         tolist = file_list(todir).sort
 
         fromlist.sort.zip(tolist.sort).each { |a,b|
-            assert_equal(a, b, "Fromfile %s with length %s does not match tofile %s with length %s" % [a, fromlist.length, b, tolist.length])
+            assert_equal(a, b, "Fromfile #{a} with length #{fromlist.length} does not match tofile #{b} with length #{tolist.length}")
         }
         #assert_equal(fromlist,tolist)
 
diff --git a/test/lib/puppettest/parsertesting.rb b/test/lib/puppettest/parsertesting.rb
index f4f0eeb44..2211d17be 100644
--- a/test/lib/puppettest/parsertesting.rb
+++ b/test/lib/puppettest/parsertesting.rb
@@ -91,7 +91,7 @@ module PuppetTest::ParserTesting
             end
         end
         args[:type] = astarray(*newnames)
-        assert_nothing_raised("Could not create tag %s" % names.inspect) {
+        assert_nothing_raised("Could not create tag #{names.inspect}") {
             return AST::Tag.new(args)
         }
     end
@@ -100,7 +100,7 @@ module PuppetTest::ParserTesting
         unless title.is_a?(AST)
             title = stringobj(title)
         end
-        assert_nothing_raised("Could not create %s %s" % [type, title]) {
+        assert_nothing_raised("Could not create #{type} #{title}") {
 
             return AST::Resource.new(
 
@@ -121,7 +121,7 @@ module PuppetTest::ParserTesting
     end
 
     def resourceoverride(type, title, params)
-        assert_nothing_raised("Could not create %s %s" % [type, name]) {
+        assert_nothing_raised("Could not create #{type} #{name}") {
 
             return AST::ResourceOverride.new(
 
@@ -136,7 +136,7 @@ module PuppetTest::ParserTesting
     end
 
     def resourceref(type, title)
-        assert_nothing_raised("Could not create %s %s" % [type, title]) {
+        assert_nothing_raised("Could not create #{type} #{title}") {
 
             return AST::ResourceReference.new(
 
@@ -150,13 +150,13 @@ module PuppetTest::ParserTesting
     end
 
     def fileobj(path, hash = {"owner" => "root"})
-        assert_nothing_raised("Could not create file %s" % path) {
+        assert_nothing_raised("Could not create file #{path}") {
             return resourcedef("file", path, hash)
         }
     end
 
     def nameobj(name)
-        assert_nothing_raised("Could not create name %s" % name) {
+        assert_nothing_raised("Could not create name #{name}") {
 
             return AST::Name.new(
 
@@ -169,7 +169,7 @@ module PuppetTest::ParserTesting
     end
 
     def typeobj(name)
-        assert_nothing_raised("Could not create type %s" % name) {
+        assert_nothing_raised("Could not create type #{name}") {
 
             return AST::Type.new(
 
@@ -182,7 +182,7 @@ module PuppetTest::ParserTesting
     end
 
     def nodedef(name)
-        assert_nothing_raised("Could not create node %s" % name) {
+        assert_nothing_raised("Could not create node #{name}") {
 
             return AST::NodeDef.new(
 
@@ -194,9 +194,9 @@ module PuppetTest::ParserTesting
                     :code => AST::ASTArray.new(
 
                         :children => [
-                            varobj("%svar" % name, "%svalue" % name),
+                            varobj("#{name}var", "#{name}value"),
 
-                            fileobj("/%s" % name)
+                            fileobj("/#{name}")
                     ]
                 )
             )
@@ -224,7 +224,7 @@ module PuppetTest::ParserTesting
         if value.is_a?(String)
             value = stringobj(value)
         end
-        assert_nothing_raised("Could not create param %s" % param) {
+        assert_nothing_raised("Could not create param #{param}") {
 
             return AST::ResourceParam.new(
 
@@ -252,7 +252,7 @@ module PuppetTest::ParserTesting
         unless value.is_a? AST
             value = stringobj(value)
         end
-        assert_nothing_raised("Could not create %s code" % name) {
+        assert_nothing_raised("Could not create #{name} code") {
 
             return AST::VarDef.new(
 
@@ -266,7 +266,7 @@ module PuppetTest::ParserTesting
     end
 
     def varref(name)
-        assert_nothing_raised("Could not create %s variable" % name) {
+        assert_nothing_raised("Could not create #{name} variable") {
 
             return AST::Variable.new(
 
@@ -279,7 +279,7 @@ module PuppetTest::ParserTesting
     end
 
     def argobj(name, value)
-        assert_nothing_raised("Could not create %s compargument" % name) {
+        assert_nothing_raised("Could not create #{name} compargument") {
             return AST::CompArgument.new(
                 :children => [nameobj(name), stringobj(value)]
                     )
@@ -308,7 +308,7 @@ module PuppetTest::ParserTesting
                 :children => pary
                     )
 
-        assert_nothing_raised("Could not create defaults for %s" % type) {
+        assert_nothing_raised("Could not create defaults for #{type}") {
 
             return AST::ResourceDefaults.new(
 
@@ -352,7 +352,7 @@ module PuppetTest::ParserTesting
         catalog.apply
 
         files.each do |file|
-            assert(FileTest.exists?(file), "Did not create %s" % file)
+            assert(FileTest.exists?(file), "Did not create #{file}")
         end
     ensure
         Puppet[:manifest] = oldmanifest
diff --git a/test/lib/puppettest/reporttesting.rb b/test/lib/puppettest/reporttesting.rb
index 97e32b1c3..49520d23a 100644
--- a/test/lib/puppettest/reporttesting.rb
+++ b/test/lib/puppettest/reporttesting.rb
@@ -5,7 +5,7 @@ module PuppetTest::Reporttesting
 
         3.times { |i|
             # We have to use warning so that the logs always happen
-            log = Puppet.warning("Report test message %s" % i)
+            log = Puppet.warning("Report test message #{i}")
 
             report << log
         }
diff --git a/test/lib/puppettest/servertest.rb b/test/lib/puppettest/servertest.rb
index bda99c66a..0a7b7f01a 100644
--- a/test/lib/puppettest/servertest.rb
+++ b/test/lib/puppettest/servertest.rb
@@ -15,9 +15,9 @@ module PuppetTest::ServerTest
 
     # create a simple manifest that just creates a file
     def mktestmanifest
-        file = File.join(Puppet[:confdir], "%ssite.pp" % (self.class.to_s + "test"))
+        file = File.join(Puppet[:confdir], "#{(self.class.to_s + "test")}site.pp")
         #@createdfile = File.join(tmpdir(), self.class.to_s + "manifesttesting" +
-        #    "_" + @method_name)
+        #    "_#{@method_name}")
         @createdfile = tempfile()
 
         File.open(file, "w") { |f|
diff --git a/test/lib/puppettest/support/assertions.rb b/test/lib/puppettest/support/assertions.rb
index b4dd1338b..528c7b8db 100644
--- a/test/lib/puppettest/support/assertions.rb
+++ b/test/lib/puppettest/support/assertions.rb
@@ -20,7 +20,7 @@ module PuppetTest
             Puppet::Util::SUIDManager.gid = gid
             Puppet::Util::SUIDManager.uid = uid
             # FIXME: use the tempfile method from puppettest.rb
-            system("mkfifo "+filename)
+            system("mkfifo #{filename}")
             f = File.open(filename, "w")
             f << "#{Puppet::Util::SUIDManager.uid}\n#{Puppet::Util::SUIDManager.gid}\n"
             yield if block_given?
diff --git a/test/lib/puppettest/support/utils.rb b/test/lib/puppettest/support/utils.rb
index db850d699..06ce3ad4a 100644
--- a/test/lib/puppettest/support/utils.rb
+++ b/test/lib/puppettest/support/utils.rb
@@ -4,7 +4,7 @@ module PuppetTest::Support
 end
 module PuppetTest::Support::Utils
     def gcdebug(type)
-        Puppet.warning "%s: %s" % [type, ObjectSpace.each_object(type) { |o| }]
+        Puppet.warning "#{type}: #{ObjectSpace.each_object(type) { |o| }}"
     end
 
     #
@@ -84,7 +84,7 @@ module PuppetTest::Support::Utils
             e.name
         }
 
-        assert_equal(events, newevents, "Incorrect %s %s events" % [type, msg])
+        assert_equal(events, newevents, "Incorrect #{type} #{msg} events")
 
         return trans
     end
@@ -94,7 +94,7 @@ module PuppetTest::Support::Utils
         ary += name.split("/")
         file = File.join(ary)
         unless FileTest.exists?(file)
-            raise Puppet::DevError, "No fakedata file %s" % file
+            raise Puppet::DevError, "No fakedata file #{file}"
         end
         return file
     end
@@ -130,7 +130,7 @@ module PuppetTest::Support::Utils
         }.find_all { |file|
             FileTest.file?(file)
         }.sort.each { |file|
-            Puppet.debug "Processing %s" % file
+            Puppet.debug "Processing #{file}"
             yield file
         }
     end
diff --git a/test/lib/puppettest/testcase.rb b/test/lib/puppettest/testcase.rb
index bd566dff2..3521589bf 100644
--- a/test/lib/puppettest/testcase.rb
+++ b/test/lib/puppettest/testcase.rb
@@ -21,7 +21,7 @@ class PuppetTest::TestCase < Test::Unit::TestCase
             return super
         else
             if defined? $console
-                puts "Skipping %s: %s" % [name, @messages.join(", ")]
+                puts "Skipping #{name}: #{@messages.join(", ")}"
             end
             suite = Test::Unit::TestSuite.new(name)
             return suite