DRY the Shell: Don't duplicate the Config, especially since there were differing default values

plugins/communicators/winrm/communicator.rb

@@ -106,12 +106,8 @@ module VagrantPlugins
 
         WinRMShell.new(
           winrm_info[:host],
-          @machine.config.winrm.username,
+          winrm_info[:port],
-          @machine.config.winrm.password,
+          @machine.config.winrm
-          port: winrm_info[:port],
-          timeout_in_seconds: @machine.config.winrm.timeout,
-          max_tries: @machine.config.winrm.max_tries,
-          ssl: @machine.config.winrm.ssl,
         )
       end
 

plugins/communicators/winrm/config.rb

@@ -29,7 +29,7 @@ module VagrantPlugins
         @port = (@ssl ? 5986 : 5985)       if @port == UNSET_VALUE
         @guest_port = (@ssl ? 5986 : 5985) if @guest_port == UNSET_VALUE
         @max_tries = 20       if @max_tries == UNSET_VALUE
-        @timeout = 1800       if @timeout == UNSET_VALUE
+        @timeout = 60         if @timeout == UNSET_VALUE
         @ssl = false          if @ssl == UNSET_VALUE
         @ssl_peer_verification = true if @ssl_peer_verification == UNSET_VALUE
       end

plugins/communicators/winrm/shell.rb

@@ -32,27 +32,17 @@ module VagrantPlugins
       ]
 
       attr_reader :logger
-      attr_reader :username
-      attr_reader :password
       attr_reader :host
       attr_reader :port
-      attr_reader :timeout_in_seconds
+      attr_reader :config
-      attr_reader :max_tries
-      attr_reader :ssl
-      attr_reader :ssl_peer_verification
 
-      def initialize(host, username, password, options = {})
+      def initialize(host, port, config)
         @logger = Log4r::Logger.new("vagrant::communication::winrmshell")
         @logger.debug("initializing WinRMShell")
 
         @host                  = host
-        @port                  = options[:port] || (options[:ssl] ? 5986 : 5985)
+        @port                  = port
-        @username              = username
+        @config                = config
-        @password              = password
-        @timeout_in_seconds    = options[:timeout_in_seconds] || 60
-        @max_tries             = options[:max_tries] || 20
-        @ssl                   = options[:ssl] || false
-        @ssl_peer_verification = options[:ssl_peer_verification] || true
       end
 
       def powershell(command, &block)
@@ -122,11 +112,11 @@ module VagrantPlugins
         @logger.info("Attempting to connect to WinRM...")
         @logger.info("  - Host: #{@host}")
         @logger.info("  - Port: #{@port}")
-        @logger.info("  - Username: #{@username}")
+        @logger.info("  - Username: #{@config.username}")
-        @logger.info("  - SSL: #{@ssl}")
+        @logger.info("  - Transport: #{@config.transport}")
 
-        client = ::WinRM::WinRMWebService.new(endpoint, :plaintext, endpoint_options)
+        client = ::WinRM::WinRMWebService.new(endpoint, transport.to_sym, endpoint_options)
-        client.set_timeout(@timeout_in_seconds)
+        client.set_timeout(@config.timeout)
         client.toggle_nori_type_casting(:off) #we don't want coersion of types
         client
       end
@@ -140,13 +130,12 @@ module VagrantPlugins
       end
 
       def endpoint_options
-        { user: @username,
+        { user: @config.username,
-          pass: @password,
+          pass: @config.password,
           host: @host,
           port: @port,
-          operation_timeout: @timeout_in_seconds,
           basic_auth_only: true,
-          no_ssl_peer_verification: !@ssl_peer_verification }
+          no_ssl_peer_verification: !@config.ssl_peer_verification }
       end
     end #WinShell class
   end

test/unit/plugins/communicators/winrm/communicator_test.rb

@@ -101,6 +101,6 @@ describe VagrantPlugins::CommunicatorWinRM::Communicator do
       expect(shell).to receive(:download).with("from", "to")
       subject.download("from", "to")
     end
-  end  
+  end
 
 end

test/unit/plugins/communicators/winrm/shell_test.rb

@@ -1,14 +1,22 @@
 require File.expand_path("../../../../base", __FILE__)
 
 require Vagrant.source_root.join("plugins/communicators/winrm/shell")
+require Vagrant.source_root.join("plugins/communicators/winrm/config")
 
 describe VagrantPlugins::CommunicatorWinRM::WinRMShell do
   include_context "unit"
 
   let(:session) { double("winrm_session") }
+  let(:config)  {
+    VagrantPlugins::CommunicatorWinRM::Config.new.tap do |c|
+      c.username = 'username'
+      c.password = 'password'
+      c.finalize!
+    end
+  }
 
   subject do
-    described_class.new('localhost', 'username', 'password').tap do |comm|
+    described_class.new('localhost', 5985, config).tap do |comm|
       allow(comm).to receive(:new_session).and_return(session)
     end
   end
@@ -51,7 +59,7 @@ describe VagrantPlugins::CommunicatorWinRM::WinRMShell do
     it "should create endpoint options" do
       expect(subject.send(:endpoint_options)).to eq(
         { user: "username", pass: "password", host: "localhost", port: 5985,
-          operation_timeout: 60, basic_auth_only: true, no_ssl_peer_verification: false })
+          basic_auth_only: true, no_ssl_peer_verification: false })
     end
   end