Add tests, and make connector more testable

Signed-off-by: Marc A. Paradise <marc.paradise@gmail.com>

2 files changed, 153 insertions, 29 deletions

diff --git a/lib/chef/knife/bootstrap/train_connector.rb b/lib/chef/knife/bootstrap/train_connector.rb
index 40e0c565ee..5230d6638c 100644
--- a/lib/chef/knife/bootstrap/train_connector.rb
+++ b/lib/chef/knife/bootstrap/train_connector.rb
@@ -36,16 +36,12 @@ class Chef
         MKTEMP_NIX_COMMAND = "bash -c 'd=$(mktemp -d ${TMPDIR:-/tmp}/chef_XXXXXX); echo $d'".freeze
 
         def initialize(host_url, default_transport, opts)
-
           uri_opts = opts_from_uri(host_url)
           uri_opts[:backend] ||= @default_transport
           @transport_type = uri_opts[:backend]
 
           # opts in the URI will override user-provided options
           @config = transport_config(host_url, opts.merge(uri_opts))
-
-          Train.validate_backend(@config)
-          @train = Train.create(@transport_type, @config)
         end
 
         # Because creating a valid train connection for testing is a two-step process in which
@@ -59,25 +55,19 @@ class Chef
           # Specifying sudo: false ensures that attempted operations
           # don't fail because the mock platform doesn't support sudo
           tc = TrainConnector.new(url, protocol, { sudo: false }.merge(opts))
-
-          # Don't pull in the platform-specific mixins automatically during connect
-          # Otherwise, it will raise since it can't resolve the OS without the mock.
           tc.connect!
-          # We need to provide this mock before invoking mix_in_target_platform,
-          # otherwise it will fail with an unknown OS (since we don't have a real connection).
-          tc.backend.mock_os(
+          tc.connection.mock_os(
             family: family,
             name: name,
             release: release,
             arch: arch
           )
           tc
-
         end
 
         def connect!
-          # Force connection to establish:
-          backend.wait_until_ready
+          # Force connection to establish
+          connection.wait_until_ready
           true
         end
 
@@ -91,7 +81,7 @@ class Chef
 
         # True if we're connected to a linux host
         def linux?
-          backend.platform.linux?
+          connection.platform.linux?
         end
 
         # True if we're connected to a unix host.
@@ -99,12 +89,12 @@ class Chef
         # for a linux host because train classifies
         # linux as a unix
         def unix?
-          backend.platform.unix?
+          connection.platform.unix?
         end
 
         # True if we're connected to a windows host
         def windows?
-          backend.platform.windows?
+          connection.platform.windows?
         end
 
         def winrm?
@@ -118,13 +108,17 @@ class Chef
         # Creates a temporary directory on the remote host if it
         # hasn't already. Caches directory location.
         #
-        # Returns the path.
+        # Returns the path on the remote host.
         def temp_dir
           cmd = windows? ? MKTEMP_WIN_COMMAND : MKTEMP_NIX_COMMAND
           @tmpdir ||= begin
                         res = run_command!(cmd)
                         dir = res.stdout.chomp.strip
                         unless windows?
+                          # Ensure that dir has the correct owner.  We are possibly
+                          # running with sudo right now - so this directory would be owned by root.
+                          # File upload is performed over SCP as the current logged-in user,
+                          # so we'll set ownership to ensure that works.
                           run_command!("chown #{@config[:user]} '#{dir}'")
                         end
                         dir
@@ -132,7 +126,7 @@ class Chef
         end
 
         def upload_file!(local_path, remote_path)
-          backend.upload(local_path, remote_path)
+          connection.upload(local_path, remote_path)
         end
 
         def upload_file_content!(content, remote_path)
@@ -159,7 +153,7 @@ class Chef
         end
 
         def run_command(command, &data_handler)
-          backend.run_command(command, &data_handler)
+          connection.run_command(command, &data_handler)
         end
 
         def run_command!(command, &data_handler)
@@ -170,12 +164,12 @@ class Chef
           result
         end
 
-        # Should be private, but we use them for test validation/mocking
-        def train
-          @train
-        end
-        def backend
-          @train.connection
+        def connection
+          @connection ||= begin
+                       Train.validate_backend(@config)
+                       train = Train.create(@transport_type, @config)
+                       train.connection
+                     end
         end
 
         private
@@ -185,7 +179,6 @@ class Chef
         def transport_config(host_url, opts_in)
           opts = { target: host_url,
                    sudo: opts_in[:sudo] === false ? false : true,
-                   # TODO - we should be always encoding password
                    www_form_encoded_password: true,
                    key_files: opts_in[:key_files],
                    non_interactive: true, # Prevent password prompts
@@ -234,8 +227,8 @@ class Chef
           # to the transport type we're using.
           valid_opts = Train.options(config[:backend])
           opts_in.select do |key, _v|
-            valid_opts.key?(key) &&
-              !config.key?(key) end
+            valid_opts.key?(key) && !config.key?(key)
+          end
         end
 
         # Extract any of username/password/host/port/transport
diff --git a/spec/unit/knife/bootstrap/train_connector_spec.rb b/spec/unit/knife/bootstrap/train_connector_spec.rb
index 872bf5481d..08bf21dd42 100644
--- a/spec/unit/knife/bootstrap/train_connector_spec.rb
+++ b/spec/unit/knife/bootstrap/train_connector_spec.rb
@@ -20,5 +20,136 @@ require "ostruct"
 require "chef/knife/bootstrap/train_connector"
 
 describe Chef::Knife::Bootstrap::TrainConnector do
-  # Tests in flight, will be pushed up
+  let(:protocol) { "mock" }
+  let(:family) { "unknown" }
+  let(:release) { "unknown" } # version
+  let(:name) { "unknown" }
+  let(:arch) { "x86_64" }
+  let(:host_url) { "mock://user1@example.com" }
+  let(:opts) { {} }
+  subject do
+    # Create a valid TargetHost with the backend stubbed out.
+    Chef::Knife::Bootstrap::TrainConnector.test_instance(host_url,
+                                                         protocol: protocol,
+                                                         family: family,
+                                                         name: name,
+                                                         release: release,
+                                                         arch: arch,
+                                                         opts: opts)
+  end
+
+  context "connect!" do
+  end
+
+  describe "platform helpers" do
+    context "on linux" do
+      let(:family) { "debian" }
+      let(:name) { "ubuntu" }
+      it "reports that it is linux and unix, because that is how train classifies it" do
+        expect(subject.unix?).to eq true
+        expect(subject.linux?).to eq true
+        expect(subject.windows?).to eq false
+      end
+    end
+    context "on unix" do
+      let(:family) { "os" }
+      let(:name) { "mac_os_x" }
+      it "reports only a unix OS" do
+        expect(subject.unix?).to eq true
+        expect(subject.linux?).to eq false
+        expect(subject.windows?).to eq false
+      end
+    end
+    context "on windows" do
+      let(:family) { "windows" }
+      let(:name) { "windows" }
+      it "reports only a windows OS" do
+        expect(subject.unix?).to eq false
+        expect(subject.linux?).to eq false
+        expect(subject.windows?).to eq true
+      end
+    end
+  end
+
+  describe "#connect!" do
+    it "establishes the connection to the remote host by waiting for it" do
+      expect(subject.connection).to receive(:wait_until_ready)
+      subject.connect!
+    end
+  end
+
+  describe "#temp_dir" do
+    context "under windows" do
+      let(:family) { "windows" }
+      let(:name) { "windows" }
+
+      it "uses the windows command to create the temp dir" do
+        expected_command = Chef::Knife::Bootstrap::TrainConnector::MKTEMP_WIN_COMMAND
+        expect(subject).to receive(:run_command!).with(expected_command)
+          .and_return double("result", stdout: "C:/a/path")
+        expect(subject.temp_dir).to eq "C:/a/path"
+      end
+
+    end
+    context "under linux and unix-like" do
+      let(:family) { "debian" }
+      let(:name) { "ubuntu" }
+      it "uses the *nix command to create the temp dir and sets ownership to logged-in user" do
+        expected_command = Chef::Knife::Bootstrap::TrainConnector::MKTEMP_NIX_COMMAND
+        expect(subject).to receive(:run_command!).with(expected_command)
+          .and_return double("result", stdout: "/a/path")
+        expect(subject).to receive(:run_command!).with("chown user1 '/a/path'")
+        expect(subject.temp_dir).to eq "/a/path"
+      end
+
+    end
+  end
+  context "#upload_file_content!" do
+    it "creates a local file with expected content and uploads it" do
+      expect(subject).to receive(:upload_file!) do |local_path, remote_path|
+        expect(File.read(local_path)).to eq "test data"
+        expect(remote_path).to eq "/target/path"
+      end
+      subject.upload_file_content!("test data", "/target/path")
+    end
+  end
+
+  context "del_file" do
+    context "on windows" do
+      let(:family) { "windows" }
+      let(:name) { "windows" }
+      it "deletes the file with a windows command" do
+        expect(subject).to receive(:run_command!) do |cmd, &_handler|
+          expect(cmd).to match(/Test-Path "deleteme\.txt".*/)
+        end
+        subject.del_file!("deleteme.txt")
+      end
+    end
+    context "on unix-like" do
+      let(:family) { "debian" }
+      let(:name) { "ubuntu" }
+      it "deletes the file with a windows command" do
+        expect(subject).to receive(:run_command!) do |cmd, &_handler|
+          expect(cmd).to match(/rm -f "deleteme\.txt".*/)
+        end
+        subject.del_file!("deleteme.txt")
+      end
+    end
+  end
+
+  context "#run_command!" do
+    it "raises a RemoteExecutionFailed when the remote execution failed" do
+      command_result = double("results", stdout: "", stderr: "failed", exit_status: 1)
+      expect(subject).to receive(:run_command).and_return command_result
+
+      expect { subject.run_command!("test") }.to raise_error do |e|
+        expect(e.hostname).to eq subject.hostname
+        expect(e.class).to eq Chef::Knife::Bootstrap::RemoteExecutionFailed
+        expect(e.stderr).to eq "failed"
+        expect(e.stdout).to eq ""
+        expect(e.exit_status).to eq 1
+      end
+    end
+  end
+
 end