From bc380a033488653b566dbc2a9255ded0599b2df2 Mon Sep 17 00:00:00 2001
From: Conor Horan-Kates <conor.code@gmail.com>
Date: Sat, 13 May 2017 22:07:03 -0700
Subject: [PATCH 1/4] externalizing log4r configuration

---
 README.md      |  2 +-
 lib/rouster.rb | 40 ++++------------------------------------
 log4r.yaml     | 22 ++++++++++++++++++++++
 3 files changed, 27 insertions(+), 37 deletions(-)
 create mode 100644 log4r.yaml

diff --git a/README.md b/README.md
index 7b38f17..222eec0 100644
--- a/README.md
+++ b/README.md
@@ -239,7 +239,7 @@ require 'test/unit'
 class TestPuppetRun < Test::Unit::TestCase
 
   def setup
-    @ppm = Rouster.new(:name => 'ppm', :verbose => 4)
+    @ppm = Rouster.new(:name => 'ppm')
     @app = Rouster.new(:name => 'app')
   end
 
diff --git a/lib/rouster.rb b/lib/rouster.rb
index 72d5944..d26362d 100644
--- a/lib/rouster.rb
+++ b/lib/rouster.rb
@@ -1,5 +1,6 @@
 require 'rubygems'
 require 'log4r'
+require 'log4r/yamlconfigurator'
 require 'json'
 require 'net/scp'
 require 'net/ssh'
@@ -45,7 +46,6 @@ class SSHConnectionError   < StandardError; end # thrown by available_via_ssh()
   # * [verbosity]           - an integer representing console level logging, or an array of integers representing console,file level logging - DEBUG (0) < INFO (1) < WARN (2) < ERROR (3) < FATAL (4)
   def initialize(opts = nil)
     @cache_timeout       = opts[:cache_timeout].nil? ? false : opts[:cache_timeout]
-    @logfile             = opts[:logfile].nil? ? false : opts[:logfile]
     @name                = opts[:name]
     @passthrough         = opts[:passthrough].nil? ? false : opts[:passthrough]
     @retries             = opts[:retries].nil? ? 0 : opts[:retries]
@@ -56,28 +56,6 @@ def initialize(opts = nil)
     @vagrant_concurrency = opts[:vagrant_concurrency].nil? ? false : opts[:vagrant_concurrency]
     @vagrant_reboot      = opts[:vagrant_reboot].nil? ? false : opts[:vagrant_reboot]
 
-    # TODO kind of want to invert this, 0 = trace, 1 = debug, 2 = info, 3 = warning, 4 = error
-    # could do `fixed_ordering = [4, 3, 2, 1, 0]` and use user input as index instead, so an input of 4 (which should be more verbose), yields 0
-    if opts[:verbosity]
-      # TODO decide how to handle this case -- currently #2 is implemented
-      # - option 1, if passed a single integer, use that level for both loggers
-      # - option 2, if passed a single integer, use that level for stdout, and a hardcoded level (probably INFO) to logfile
-
-      # kind of want to do if opts[:verbosity].respond_to?(:[]), but for 1.87 compatability, going this way..
-      if ! opts[:verbosity].is_a?(Array) or opts[:verbosity].is_a?(Integer)
-        @verbosity_console = opts[:verbosity].to_i
-        @verbosity_logfile = 2
-      elsif opts[:verbosity].is_a?(Array)
-        # TODO more error checking here when we are sure this is the right way to go
-        @verbosity_console = opts[:verbosity][0].to_i
-        @verbosity_logfile = opts[:verbosity][1].to_i
-        @logfile = true if @logfile.eql?(false) # overriding the default setting
-      end
-    else
-      @verbosity_console = 3
-      @verbosity_logfile = 2 # this is kind of arbitrary, but won't actually be created unless opts[:logfile] is also passed
-    end
-
     @ostype    = nil
     @osversion = nil
 
@@ -90,19 +68,9 @@ def initialize(opts = nil)
     @ssh_info = nil # hash containing connection information
 
     # set up logging
-    require 'log4r/config'
-    Log4r.define_levels(*Log4r::Log4rConfig::LogLevels)
-
-    @logger            = Log4r::Logger.new(sprintf('rouster:%s', @name))
-    @logger.outputters << Log4r::Outputter.stderr
-    #@log.outputters << Log4r::Outputter.stdout
-
-    if @logfile
-      @logfile = @logfile.eql?(true) ? sprintf('/tmp/rouster-%s.%s.%s.log', @name, Time.now.to_i, $$) : @logfile
-      @logger.outputters << Log4r::FileOutputter.new(sprintf('rouster:%s', @name), :filename => @logfile, :level => @verbosity_logfile)
-    end
-
-    @logger.outputters[0].level = @verbosity_console # can't set this when instantiating a .std* logger, and want the FileOutputter at a different level
+    Log4r::YamlConfigurator.load_yaml_file(File.expand_path(sprintf('%s/../log4r.yaml', File.dirname(__FILE__))))
+    @logger = Log4r::Logger.get('rouster')
+    #@logger.name = sprintf('rouster:%s', @name) # want to use the same configs for everyone, but want to distinguish between VMs
 
     if opts.has_key?(:sudo)
       @sudo = opts[:sudo]
diff --git a/log4r.yaml b/log4r.yaml
new file mode 100644
index 0000000..4e511d9
--- /dev/null
+++ b/log4r.yaml
@@ -0,0 +1,22 @@
+---
+log4r_config:
+  loggers:
+  - name: 'rouster'
+    outputters:
+      - stdout
+      - stderr
+  outputters:
+  - type: StdoutOutputter
+    level: INFO
+    name: stdout
+    formatter:
+      type: PatternFormatter
+      date_pattern: '%Y/%m/%d %H:%M.%S'
+      pattern: '%d | %C | %l | %m'
+  - type: StderrOutputter
+    level: ERROR
+    name: stderr
+    formatter:
+      type: PatternFormatter
+      date_pattern: '%Y/%m/%d %H:%M.%S'
+      pattern: '%d | %C | %l | %m'

From 94803273ed9a3a4b12eb3136d870aea144e00a2e Mon Sep 17 00:00:00 2001
From: Conor Horan-Kates <conor.code@gmail.com>
Date: Sat, 13 May 2017 22:12:11 -0700
Subject: [PATCH 2/4] do you want ants? because this is how you get ants.

---
 lib/rouster.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rouster.rb b/lib/rouster.rb
index d26362d..d5547ea 100644
--- a/lib/rouster.rb
+++ b/lib/rouster.rb
@@ -70,7 +70,7 @@ def initialize(opts = nil)
     # set up logging
     Log4r::YamlConfigurator.load_yaml_file(File.expand_path(sprintf('%s/../log4r.yaml', File.dirname(__FILE__))))
     @logger = Log4r::Logger.get('rouster')
-    #@logger.name = sprintf('rouster:%s', @name) # want to use the same configs for everyone, but want to distinguish between VMs
+    @logger.instance_variable_set(:@fullname, sprintf('rouster:%s', @name)) # want to use the same configs for everyone, but want to distinguish between VMs
 
     if opts.has_key?(:sudo)
       @sudo = opts[:sudo]

From 699f70309f240e7aa2966e809b7bac066aac3ebe Mon Sep 17 00:00:00 2001
From: Conor Horan-Kates <conor.code@gmail.com>
Date: Sun, 14 May 2017 19:38:31 -0700
Subject: [PATCH 3/4] test 'fixes' for moving log4r configs to YAML. previous
 conor made this way harder than it should have been

---
 examples/aws.rb                        |  3 --
 examples/bootstrap.rb                  |  8 ++---
 examples/demo.rb                       |  2 +-
 examples/error.rb                      |  4 +--
 examples/openstack.rb                  |  2 --
 examples/passthrough.rb                |  6 ----
 lib/rouster.rb                         |  4 +--
 test/basic.rb                          |  8 ++---
 test/functional/test_inspect.rb        |  1 -
 test/functional/test_new.rb            | 42 +++++++++-----------------
 test/functional/test_passthroughs.rb   |  1 -
 test/functional/test_restart.rb        |  2 +-
 test/unit/test_new.rb                  |  4 ---
 test/unit/testing/test_get_services.rb |  2 +-
 14 files changed, 29 insertions(+), 60 deletions(-)

diff --git a/examples/aws.rb b/examples/aws.rb
index 8210c60..f5d10e8 100644
--- a/examples/aws.rb
+++ b/examples/aws.rb
@@ -10,7 +10,6 @@
     :instance => 'your-instance-id',
     :key      => sprintf('%s/.ssh/id_rsa-aws', ENV['HOME'])
   },
-  :verbosity => 1,
 )
 
 a = aws_already_running.run('ls -l /etc/hosts; who')
@@ -40,7 +39,6 @@
     :secret_key   => ENV['AWS_SECRET_ACCESS_KEY'],
   },
   :sshtunnel => false,
-  :verbosity => 1,
 )
 
 p "up(): #{aws.up}"
@@ -52,7 +50,6 @@
     :key      => sprintf('%s/.ssh/id_rsa-aws', ENV['HOME']),
     :instance => aws.aws_get_instance,
   },
-  :verbosity => 1,
 )
 
 [ aws, aws_already_running, aws_clone ].each do |a|
diff --git a/examples/bootstrap.rb b/examples/bootstrap.rb
index 1f1c66c..fbd0175 100644
--- a/examples/bootstrap.rb
+++ b/examples/bootstrap.rb
@@ -4,13 +4,13 @@
 require 'rouster/puppet'
 require 'rouster/tests'
 
-app = Rouster.new(:name => 'app', :verbosity => 4, :sudo => true)
-ppm = Rouster.new(:name => 'ppm', :verbosity => 1, :sudo => true)
+app = Rouster.new(:name => 'app', :sudo => true)
+ppm = Rouster.new(:name => 'ppm', :sudo => true)
 
 # passthrough boxes do not need to specify a name
 # commented out currently because passthrough is not MVP
-#lpt = Rouster.new(:passthrough => 'local', :verbosity => 4)
-#rpt = Rouster.new(:passthrough => 'remote', :verbosity => 4, :sshkey => '~/.ssh/id_dsa')
+#lpt = Rouster.new(:passthrough => 'local')
+#rpt = Rouster.new(:passthrough => 'remote', :sshkey => '~/.ssh/id_dsa')
 
 workers = [app]
 
diff --git a/examples/demo.rb b/examples/demo.rb
index 2457f46..4ce90be 100644
--- a/examples/demo.rb
+++ b/examples/demo.rb
@@ -3,7 +3,7 @@
 require 'rouster'
 require 'rouster/tests'
 
-@app = Rouster.new(:name => 'app', :sudo => true, :verbosity => 3)
+@app = Rouster.new(:name => 'app', :sudo => true)
 @app.up()
 
 # get list of packages
diff --git a/examples/error.rb b/examples/error.rb
index f2c7cce..d4f8854 100644
--- a/examples/error.rb
+++ b/examples/error.rb
@@ -2,7 +2,7 @@
 require 'rouster'
 
 begin
-  badkey = Rouster.new(:name => 'whatever', :verbosity => 4, :sshkey => __FILE__)
+  badkey = Rouster.new(:name => 'whatever', :sshkey => __FILE__)
 rescue Rouster::InternalError => e
   p "caught #{e.class}: #{e.message}"
 rescue => e
@@ -22,7 +22,7 @@
 p badvagrantfile
 
 begin
-  good = Rouster.new(:name => 'app', verbosity => 4)
+  good = Rouster.new(:name => 'app')
 rescue => e
   p "caught unexpected exception #{e.class}: #{e.message}"
 end
diff --git a/examples/openstack.rb b/examples/openstack.rb
index f2abf16..f5b6ed9 100644
--- a/examples/openstack.rb
+++ b/examples/openstack.rb
@@ -20,7 +20,6 @@
     :key                 => '/path/to/ssh_keys.pem',                      # SSH key filename
   },
   :sshtunnel => false,
-  :verbosity => 1,
 )
 
 p "UP(): #{ostack.up}"
@@ -41,7 +40,6 @@
     :instance            => ostack.ostack_get_instance_id,                # ID of a running OpenStack instance.
   },
   :sshtunnel => false,
-  :verbosity => 1,
 )
 
 [ ostack, ostack_copy ].each do |o|
diff --git a/examples/passthrough.rb b/examples/passthrough.rb
index 039da90..75afbc4 100644
--- a/examples/passthrough.rb
+++ b/examples/passthrough.rb
@@ -4,14 +4,11 @@
 require 'rouster/puppet'
 require 'rouster/tests'
 
-verbosity = ENV['VERBOSE'].nil? ? 4 : 0
-
 # .inspect of this is blank for sshkey and status, looks ugly, but is ~accurate.. fix this?
 local = Rouster.new(
   :name        => 'local',
   :sudo        => false,
   :passthrough => { :type => :local },
-  :verbosity   => verbosity,
 )
 
 remote = Rouster.new(
@@ -23,20 +20,17 @@
     :user => ENV['USER'],
     :key  => sprintf('%s/.ssh/id_dsa', ENV['HOME']),
   },
-  :verbosity => verbosity,
 )
 
 sudo = Rouster.new(
   :name        => 'sudo',
   :sudo        => true,
   :passthrough => { :type => :local },
-  :verbosity   => verbosity,
 )
 
 vagrant = Rouster.new(
   :name        => 'ppm',
   :sudo        => true,
-  :verbosity   => verbosity,
 )
 
 workers = [ local, remote, vagrant ]
diff --git a/lib/rouster.rb b/lib/rouster.rb
index d5547ea..a7e543d 100644
--- a/lib/rouster.rb
+++ b/lib/rouster.rb
@@ -43,7 +43,6 @@ class SSHConnectionError   < StandardError; end # thrown by available_via_ssh()
   # * [vagrantfile]         - the full or relative path to the Vagrantfile to use, if not specified, will look for one in 5 directories above current location
   # * [vagrant_concurrency] - boolean controlling whether Rouster will attempt to run `vagrant *` if another vagrant process is already running, default is false
   # * [vagrant_reboot]      - particularly sticky systems restart better if Vagrant does it for us, default is false
-  # * [verbosity]           - an integer representing console level logging, or an array of integers representing console,file level logging - DEBUG (0) < INFO (1) < WARN (2) < ERROR (3) < FATAL (4)
   def initialize(opts = nil)
     @cache_timeout       = opts[:cache_timeout].nil? ? false : opts[:cache_timeout]
     @name                = opts[:name]
@@ -248,8 +247,7 @@ def inspect
       sshkey[#{@sshkey}],
       status[#{s}],
       sudo[#{@sudo}],
-      vagrantfile[#{@vagrantfile}],
-      verbosity console[#{@verbosity_console}] / log[#{@verbosity_logfile} - #{@logfile}]\n"
+      vagrantfile[#{@vagrantfile}]"
   end
 
   ## internal methods
diff --git a/test/basic.rb b/test/basic.rb
index f0cc3ab..4d41577 100644
--- a/test/basic.rb
+++ b/test/basic.rb
@@ -5,9 +5,9 @@
 require 'rouster/testing'
 require 'rouster/tests'
 
-#a = Rouster.new(:name => 'app', :verbosity => 1, :vagrantfile => '../piab/Vagrantfile', :retries => 3)
-#p = Rouster.new(:name => 'ppm', :verbosity => 1, :vagrantfile => '../piab/Vagrantfile')
-#r = Rouster.new(:name => 'app', :verbosity => 1, :vagrantfile => 'Vagrantfile')
-l = Rouster.new(:name => 'local', :passthrough => { :type => :local }, :verbosity => 3)
+#a = Rouster.new(:name => 'app', :vagrantfile => '../piab/Vagrantfile', :retries => 3)
+#p = Rouster.new(:name => 'ppm', :vagrantfile => '../piab/Vagrantfile')
+#r = Rouster.new(:name => 'app', :vagrantfile => 'Vagrantfile')
+l = Rouster.new(:name => 'local', :passthrough => { :type => :local })
 
 p 'DBGZ' if nil?
diff --git a/test/functional/test_inspect.rb b/test/functional/test_inspect.rb
index 0ad18e0..3e61173 100644
--- a/test/functional/test_inspect.rb
+++ b/test/functional/test_inspect.rb
@@ -21,7 +21,6 @@ def test_happy_path
     assert_match(/status/, res)
     assert_match(/sudo\[true\]/, res)
     assert_match(/vagrantfile/, res)
-    assert_match(/verbosity console\[3\] \/ log\[2/, res)
   end
 
   def teardown
diff --git a/test/functional/test_new.rb b/test/functional/test_new.rb
index dc910de..364c91b 100644
--- a/test/functional/test_new.rb
+++ b/test/functional/test_new.rb
@@ -47,9 +47,6 @@ def test_defaults
     assert_equal(true, @app.instance_variable_get(:@sshtunnel))
     assert_equal(false, @app.instance_variable_get(:@unittest))
     assert_equal(false, @app.instance_variable_get(:@vagrant_concurrency))
-    assert_equal(3, @app.instance_variable_get(:@verbosity_console))
-    assert_equal(2, @app.instance_variable_get(:@verbosity_logfile))
-
   end
 
   def test_good_openssh_tunnel
@@ -66,7 +63,6 @@ def test_good_advanced_instantiation
         :name          => 'app',
         :passthrough   => false,
         :sudo          => false,
-        :verbosity     => [4,0],
         #:vagrantfile  => traverse_up(Dir.pwd, 'Vagrantfile'), # this is what happens anyway..
         :sshkey        =>  ENV['VAGRANT_HOME'].nil? ? sprintf('%s/.vagrant.d/insecure_private_key', ENV['HOME']) : sprintf('%s/insecure_private_key', ENV['VAGRANT_HOME']),
         :cache_timeout => 10,
@@ -78,8 +74,6 @@ def test_good_advanced_instantiation
     assert_equal('app', @app.name)
     assert_equal(false, @app.is_passthrough?())
     assert_equal(false, @app.uses_sudo?())
-    assert_equal(4, @app.instance_variable_get(:@verbosity_console))
-    assert_equal(0, @app.instance_variable_get(:@verbosity_logfile))
     assert_equal(true, File.file?(@app.vagrantfile))
     assert_equal(true, File.file?(@app.sshkey))
     assert_equal(10, @app.cache_timeout)
@@ -130,7 +124,7 @@ def test_bad_sshkey_instantiation
   def test_good_local_passthrough
 
     assert_nothing_raised do
-      @app = Rouster.new(:name => 'local', :passthrough => { :type => :local }, :verbosity => 4)
+      @app = Rouster.new(:name => 'local', :passthrough => { :type => :local })
     end
 
     assert_equal('local', @app.name)
@@ -158,8 +152,7 @@ def test_good_remote_passthrough
           :user => ENV['USER'],
           :key  => @@user_sshkey,
           :paranoid => false,
-        },
-        :verbosity => 4,
+        },s
       )
     end
 
@@ -180,16 +173,15 @@ def test_paranoia_remote_passthrough
 
     assert_nothing_raised do
       @app = Rouster.new(
-          :name => 'remote',
-          :sudo => false,
-          :passthrough => {
-              :type => :remote,
-              :host => host,
-              :user => ENV['USER'],
-              :key  => @@user_sshkey,
-              :paranoid => :very,
-          },
-          :verbosity => 4,
+        :name => 'remote',
+        :sudo => false,
+        :passthrough => {
+            :type => :remote,
+            :host => host,
+            :user => ENV['USER'],
+            :key  => @@user_sshkey,
+            :paranoid => :very,
+        },
       )
     end
 
@@ -207,19 +199,19 @@ def test_invalid_passthrough
     # missing required parameters
 
     assert_raise Rouster::ArgumentError do
-      @app = Rouster.new(:name => 'fizzy', :passthrough => {}, :verbosity => 4)
+      @app = Rouster.new(:name => 'fizzy', :passthrough => {})
     end
 
     assert_raise Rouster::ArgumentError do
-      @app = Rouster.new(:name => 'fizzy', :passthrough => { :type => 'invalid' }, :verbosity => 4)
+      @app = Rouster.new(:name => 'fizzy', :passthrough => { :type => 'invalid' })
     end
 
     assert_raise Rouster::ArgumentError do
-      @app = Rouster.new(:name => 'fizzy', :passthrough => { :type => :remote }, :verbosity => 4)
+      @app = Rouster.new(:name => 'fizzy', :passthrough => { :type => :remote })
     end
 
     assert_raise Rouster::ArgumentError do
-      @app = Rouster.new(:name => 'fizzy', :passthrough => { :type => :remote, :user => 'foo' }, :verbosity => 4)
+      @app = Rouster.new(:name => 'fizzy', :passthrough => { :type => :remote, :user => 'foo' })
     end
 
   end
@@ -236,7 +228,6 @@ def test_bad_passthrough
           :user => 'foo',
           :host => 'bar',
         },
-        :verbosity => 4,
       )
     end
 
@@ -250,7 +241,6 @@ def test_bad_passthrough
             :user => 'foo',
             :host => 'bar',
         },
-        :verbosity => 4,
       )
     end
 
@@ -270,7 +260,6 @@ def test_bad_passthrough
         },
 
         :sshtunnel => true,
-        :verbosity => 4,
       )
     end
 
@@ -289,7 +278,6 @@ def test_bad_passthrough
         },
 
         :sshtunnel => true,
-        :verbosity => 4,
       )
     end
 
diff --git a/test/functional/test_passthroughs.rb b/test/functional/test_passthroughs.rb
index afdeb9f..e936aa2 100644
--- a/test/functional/test_passthroughs.rb
+++ b/test/functional/test_passthroughs.rb
@@ -69,7 +69,6 @@ def test_functional_remote_passthrough
           :user => ENV['USER'],
           :key  => @@user_sshkey,
         },
-        :verbosity => 4,
       )
     end
 
diff --git a/test/functional/test_restart.rb b/test/functional/test_restart.rb
index 39003f0..082d7a9 100644
--- a/test/functional/test_restart.rb
+++ b/test/functional/test_restart.rb
@@ -8,7 +8,7 @@ class TestRestart < Test::Unit::TestCase
 
     def setup
       assert_nothing_raised do
-        @app = Rouster.new(:name => 'app', :verbosity => 4)
+        @app = Rouster.new(:name => 'app')
       end
     end
 
diff --git a/test/unit/test_new.rb b/test/unit/test_new.rb
index 66ffd5b..17267e9 100644
--- a/test/unit/test_new.rb
+++ b/test/unit/test_new.rb
@@ -33,7 +33,6 @@ def test_2_good_instantiation
         :cache_timeout => 10,
         :name          => 'ppm',
         :retries       => 3,
-        :verbosity     => [3,2],
         :unittest      => true,
       )
     end
@@ -42,9 +41,6 @@ def test_2_good_instantiation
     assert_equal('ppm', @app.name)
     assert_equal(3, @app.retries)
 
-    assert_equal(3, @app.instance_variable_get(:@verbosity_console))
-    assert_equal(2, @app.instance_variable_get(:@verbosity_logfile))
-
   end
 
 
diff --git a/test/unit/testing/test_get_services.rb b/test/unit/testing/test_get_services.rb
index 1a7f00e..e5baec8 100644
--- a/test/unit/testing/test_get_services.rb
+++ b/test/unit/testing/test_get_services.rb
@@ -12,7 +12,7 @@ def setup
     Rouster.send(:public, *Rouster.private_instance_methods)
     Rouster.send(:public, *Rouster.protected_instance_methods)
 
-    @app = Rouster.new(:name => 'app', :unittest => true, :verbosity => 4)
+    @app = Rouster.new(:name => 'app', :unittest => true)
 
   end
 

From 79d6d5838fe1f5c2a868cdb8abedea91f277e8dd Mon Sep 17 00:00:00 2001
From: Conor Horan-Kates <conor.code@gmail.com>
Date: Sun, 14 May 2017 19:57:45 -0700
Subject: [PATCH 4/4] embarassing.. and useful

---
 lib/rouster.rb              | 6 +++++-
 test/functional/test_new.rb | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/rouster.rb b/lib/rouster.rb
index a7e543d..ee86e94 100644
--- a/lib/rouster.rb
+++ b/lib/rouster.rb
@@ -69,7 +69,11 @@ def initialize(opts = nil)
     # set up logging
     Log4r::YamlConfigurator.load_yaml_file(File.expand_path(sprintf('%s/../log4r.yaml', File.dirname(__FILE__))))
     @logger = Log4r::Logger.get('rouster')
-    @logger.instance_variable_set(:@fullname, sprintf('rouster:%s', @name)) # want to use the same configs for everyone, but want to distinguish between VMs
+    begin
+      @logger.instance_variable_set(:@fullname, sprintf('rouster:%s', @name)) # want to use the same configs for everyone, but want to distinguish between VMs
+    rescue => e
+      @logger.error(sprintf('unadvised use of instance variables failed[%s], please file an issue', e.message))
+    end
 
     if opts.has_key?(:sudo)
       @sudo = opts[:sudo]
diff --git a/test/functional/test_new.rb b/test/functional/test_new.rb
index 364c91b..8a7609a 100644
--- a/test/functional/test_new.rb
+++ b/test/functional/test_new.rb
@@ -152,7 +152,7 @@ def test_good_remote_passthrough
           :user => ENV['USER'],
           :key  => @@user_sshkey,
           :paranoid => false,
-        },s
+        },
       )
     end