DRY'd up Chef command building

There's very little difference between the command building on Linux and Windows other than path formatting. All Chef provisioners support the --no-color argument now.

Added unit tests to verify changes.

plugins/provisioners/chef/command_builder.rb

@@ -1,15 +1,53 @@
 module VagrantPlugins
   module Chef
     class CommandBuilder
-      def initialize(machine, config, client_type)
+      def initialize(config, client_type, is_windows=false, is_ui_colored=false)
-        @machine     = machine
-        @config      = config
         @client_type   = client_type
+        @config        = config
+        @is_windows    = is_windows
+        @is_ui_colored = is_ui_colored
 
         if client_type != :solo && client_type != :client
           raise 'Invalid client_type, expected solo or client'
         end
       end
+
+      def build_command
+        "#{command_env}#{chef_binary_path} #{chef_arguments}"
+      end
+
+      protected
+
+      def command_env
+        @config.binary_env ? "#{@config.binary_env} " : ""
+      end
+
+      def chef_binary_path
+        binary_path = "chef-#{@client_type}"
+        if @config.binary_path
+          binary_path = guest_friendly_path(File.join(@config.binary_path, binary_path))
+        end
+        binary_path
+      end
+
+      def chef_arguments
+        chef_arguments = "-c #{provisioning_path("#{@client_type}.rb")}"
+        chef_arguments << " -j #{provisioning_path("dna.json")}"
+        chef_arguments << " #{@config.arguments}" if @config.arguments
+        chef_arguments << " --no-color" unless @is_ui_colored
+        chef_arguments.strip
+      end
+
+      def provisioning_path(file)
+        guest_friendly_path(File.join(@config.provisioning_path, file))
+      end
+
+      def guest_friendly_path(path)
+        return path unless @is_windows
+        path.gsub!("/", "\\")
+        path = "c:#{path}" if path.start_with?("\\")
+        path
+      end
     end
   end
 end

plugins/provisioners/chef/command_builder_linux.rb

@@ -1,47 +0,0 @@
-module VagrantPlugins
-  module Chef
-    class CommandBuilderLinux < CommandBuilder
-      def build_command
-        if @client_type == :solo
-          return build_command_solo
-        else
-          return build_command_client
-        end
-      end
-
-      protected
-
-      def build_command_client
-        command_env  = @config.binary_env ? "#{@config.binary_env} " : ""
-        command_args = @config.arguments ? " #{@config.arguments}" : ""
-
-        binary_path = "chef-client"
-        binary_path ||= File.join(@config.binary_path, binary_path)
-
-        return "#{command_env}#{binary_path} " +
-          "-c #{@config.provisioning_path}/client.rb " +
-          "-j #{@config.provisioning_path}/dna.json #{command_args}"
-      end
-
-      def build_command_solo
-        options = [
-          "-c #{@config.provisioning_path}/solo.rb",
-          "-j #{@config.provisioning_path}/dna.json"
-        ]
-
-        if !@machine.env.ui.is_a?(Vagrant::UI::Colored)
-          options << "--no-color"
-        end
-
-        command_env = @config.binary_env ? "#{@config.binary_env} " : ""
-        command_args = @config.arguments ? " #{@config.arguments}" : ""
-
-        binary_path = "chef-solo"
-        binary_path ||= File.join(@config.binary_path, binary_path)
-
-        return "#{command_env}#{binary_path} " +
-          "#{options.join(" ")} #{command_args}"
-      end
-    end
-  end
-end

plugins/provisioners/chef/command_builder_windows.rb

@@ -1,34 +0,0 @@
-module VagrantPlugins
-  module Chef
-    class CommandBuilderWindows < CommandBuilder
-      def build_command
-        "#{chef_binary_path} #{chef_arguments}"
-      end
-
-      protected
-
-      def chef_binary_path
-        binary_path = "chef-#{@client_type}"
-        binary_path = win_path(File.join(@config.binary_path, binary_path)) if @config.binary_path
-        binary_path
-      end
-
-      def chef_arguments
-        chef_arguments = "-c #{provisioning_path("#{@client_type}.rb")}"
-        chef_arguments << " -j #{provisioning_path("dna.json")}"
-        chef_arguments << " #{@config.arguments}" if @config.arguments
-        chef_arguments.strip
-      end
-
-      def provisioning_path(file)
-        win_path(File.join(@config.provisioning_path, file))
-      end
-
-      def win_path(path)
-        path.gsub!("/", "\\")
-        path = "c:#{path}" if path.start_with?("\\")
-        path
-      end
-    end
-  end
-end

plugins/provisioners/chef/plugin.rb

@@ -6,8 +6,6 @@ module VagrantPlugins
   module Chef
     root = Pathname.new(File.expand_path("../", __FILE__))
     autoload :CommandBuilder, root.join("command_builder")
-    autoload :CommandBuilderLinux, root.join("command_builder_linux")
-    autoload :CommandBuilderWindows, root.join("command_builder_windows")
 
     class Plugin < Vagrant.plugin("2")
       name "chef"

plugins/provisioners/chef/provisioner/base.rb

@@ -26,9 +26,7 @@ module VagrantPlugins
         # This returns the command to run Chef for the given client
         # type.
         def build_command(client)
-          builder_klass = CommandBuilderLinux
+          builder = CommandBuilder.new(@config, client, windows?, @machine.env.ui.is_a?(Vagrant::UI::Colored))
-          builder_klass = CommandBuilderWindows if windows?
-          builder = builder_klass.new(@machine, @config, client)
           return builder.build_command
         end
 

test/unit/plugins/provisioners/chef/command_build_windows_spec.rb

@@ -1,55 +0,0 @@
-require_relative "../../../base"
-
-require Vagrant.source_root.join("plugins/provisioners/chef/command_builder_windows")
-
-describe VagrantPlugins::Chef::CommandBuilderWindows do
-
-  let(:machine) { double("machine") }
-  let(:chef_config) { double("chef_config") }
-
-  subject do
-    VagrantPlugins::Chef::CommandBuilderWindows.new(machine, chef_config, :client)
-  end
-
-  before(:each) do
-    allow(chef_config).to receive(:provisioning_path).and_return('/tmp/vagrant-chef-1')
-    allow(chef_config).to receive(:arguments).and_return(nil)
-    allow(chef_config).to receive(:binary_env).and_return(nil)
-    allow(chef_config).to receive(:binary_path).and_return(nil)
-  end
-
-  describe '.initialize' do
-    it 'should raise when chef type is not client or solo' do
-      expect { VagrantPlugins::Chef::CommandBuilderWindows.new(machine, chef_config, :client_bad) }.
-        to raise_error
-    end
-  end
-
-  describe '.build_command' do
-    it "executes the chef-client in PATH by default" do
-      expect(subject.build_command()).to match(/^chef-client/)
-    end
-
-    it "executes the chef-client using full path if binary_path is specified" do
-      allow(chef_config).to receive(:binary_path).and_return(
-        "c:\\opscode\\chef\\bin\\chef-client")
-      expect(subject.build_command()).to match(/^c:\\opscode\\chef\\bin\\chef-client\\chef-client/)
-    end
-
-    it "builds a windows friendly client.rb path" do
-      expect(subject.build_command()).to include(
-        '-c c:\\tmp\\vagrant-chef-1\\client.rb')
-    end
-
-    it "builds a windows friendly solo.json path" do
-      expect(subject.build_command()).to include(
-        '-j c:\\tmp\\vagrant-chef-1\\dna.json')
-    end
-
-    it 'includes Chef arguments if specified' do
-      allow(chef_config).to receive(:arguments).and_return("-l DEBUG")
-      expect(subject.build_command()).to include(
-        '-l DEBUG')
-    end
-  end
-end

test/unit/plugins/provisioners/chef/command_builder_spec.rb

@@ -0,0 +1,105 @@
+require_relative "../../../base"
+
+require Vagrant.source_root.join("plugins/provisioners/chef/command_builder")
+
+describe VagrantPlugins::Chef::CommandBuilder do
+
+  let(:machine) { double("machine") }
+  let(:chef_config) { double("chef_config") }
+
+  before(:each) do
+    allow(chef_config).to receive(:provisioning_path).and_return('/tmp/vagrant-chef-1')
+    allow(chef_config).to receive(:arguments).and_return(nil)
+    allow(chef_config).to receive(:binary_env).and_return(nil)
+    allow(chef_config).to receive(:binary_path).and_return(nil)
+    allow(chef_config).to receive(:binary_env).and_return(nil)
+  end
+
+  describe '.initialize' do
+    it 'should raise when chef type is not client or solo' do
+      expect { VagrantPlugins::Chef::CommandBuilder.new(chef_config, :client_bad) }.
+        to raise_error
+    end
+  end
+
+  describe 'build_command' do
+    describe 'windows' do
+      subject do
+        VagrantPlugins::Chef::CommandBuilder.new(chef_config, :client, true)
+      end
+
+      it "executes the chef-client in PATH by default" do
+        expect(subject.build_command()).to match(/^chef-client/)
+      end
+
+      it "executes the chef-client using full path if binary_path is specified" do
+        allow(chef_config).to receive(:binary_path).and_return(
+          "c:\\opscode\\chef\\bin\\chef-client")
+        expect(subject.build_command()).to match(/^c:\\opscode\\chef\\bin\\chef-client\\chef-client/)
+      end
+
+      it "builds a guest friendly client.rb path" do
+        expect(subject.build_command()).to include(
+          '-c c:\\tmp\\vagrant-chef-1\\client.rb')
+      end
+
+      it "builds a guest friendly solo.json path" do
+        expect(subject.build_command()).to include(
+          '-j c:\\tmp\\vagrant-chef-1\\dna.json')
+      end
+
+      it 'includes Chef arguments if specified' do
+        allow(chef_config).to receive(:arguments).and_return("-l DEBUG")
+        expect(subject.build_command()).to include(
+          '-l DEBUG')
+      end
+
+      it 'includes --no-color if UI is not colored' do
+        expect(subject.build_command()).to include(
+          ' --no-color')
+      end
+    end
+
+    describe 'linux' do
+      subject do
+        VagrantPlugins::Chef::CommandBuilder.new(chef_config, :client, false)
+      end
+
+      it "executes the chef-client in PATH by default" do
+        expect(subject.build_command()).to match(/^chef-client/)
+      end
+
+      it "executes the chef-client using full path if binary_path is specified" do
+        allow(chef_config).to receive(:binary_path).and_return(
+          "/opt/chef/chef-client")
+        expect(subject.build_command()).to match(/^\/opt\/chef\/chef-client/)
+      end
+
+      it "builds a guest friendly client.rb path" do
+        expect(subject.build_command()).to include(
+          '-c /tmp/vagrant-chef-1/client.rb')
+      end
+
+      it "builds a guest friendly solo.json path" do
+        expect(subject.build_command()).to include(
+          '-j /tmp/vagrant-chef-1/dna.json')
+      end
+
+      it 'includes Chef arguments if specified' do
+        allow(chef_config).to receive(:arguments).and_return("-l DEBUG")
+        expect(subject.build_command()).to include(
+          '-l DEBUG')
+      end
+
+      it 'includes --no-color if UI is not colored' do
+        expect(subject.build_command()).to include(
+          ' --no-color')
+      end
+
+      it 'includes environment variables if specified' do
+        allow(chef_config).to receive(:binary_env).and_return("ENVVAR=VAL")
+        expect(subject.build_command()).to match(/^ENVVAR=VAL /)
+      end
+    end
+  end
+end