convert chocolatey resource to use shell_out splat args

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

2 files changed, 46 insertions, 52 deletions

diff --git a/lib/chef/provider/package/chocolatey.rb b/lib/chef/provider/package/chocolatey.rb
index a09bbf55de..1bb379f471 100644
--- a/lib/chef/provider/package/chocolatey.rb
+++ b/lib/chef/provider/package/chocolatey.rb
@@ -1,5 +1,5 @@
 #
-# Copyright:: Copyright 2015-2016, Chef Software, Inc.
+# Copyright:: Copyright 2015-2019, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -84,13 +84,13 @@ class Chef
 
           # choco does not support installing multiple packages with version pins
           name_has_versions.each do |name, version|
-            choco_command("install -y --version", version, cmd_args, name)
+            choco_command("install", "-y", "--version", version, cmd_args, name)
           end
 
           # but we can do all the ones without version pins at once
           unless name_nil_versions.empty?
             cmd_names = name_nil_versions.keys
-            choco_command("install -y", cmd_args, *cmd_names)
+            choco_command("install", "-y", cmd_args, *cmd_names)
           end
         end
 
@@ -106,13 +106,13 @@ class Chef
 
           # choco does not support installing multiple packages with version pins
           name_has_versions.each do |name, version|
-            choco_command("upgrade -y --version", version, cmd_args, name)
+            choco_command("upgrade", "-y", "--version", version, cmd_args, name)
           end
 
           # but we can do all the ones without version pins at once
           unless name_nil_versions.empty?
             cmd_names = name_nil_versions.keys
-            choco_command("upgrade -y", cmd_args, *cmd_names)
+            choco_command("upgrade", "-y", cmd_args, *cmd_names)
           end
         end
 
@@ -121,7 +121,7 @@ class Chef
         # @param names [Array<String>] array of package names to install
         # @param versions [Array<String>] array of versions to install
         def remove_package(names, versions)
-          choco_command("uninstall -y", cmd_args(include_source: false), *names)
+          choco_command("uninstall", "-y", cmd_args(include_source: false), *names)
         end
 
         # Choco does not have dpkg's distinction between purge and remove
@@ -172,7 +172,7 @@ class Chef
         # @param args [String] variable number of string arguments
         # @return [Mixlib::ShellOut] object returned from shell_out!
         def choco_command(*args)
-          shell_out!(args_to_string(choco_exe, *args), returns: new_resource.returns)
+          shell_out!(choco_exe, *args, returns: new_resource.returns)
         end
 
         # Use the available_packages Hash helper to create an array suitable for
@@ -210,18 +210,8 @@ class Chef
         # @return [String] options from new_resource or empty string
         def cmd_args(include_source: true)
           cmd_args = [ new_resource.options ]
-          cmd_args.push( "-source #{new_resource.source}" ) if new_resource.source && include_source
-          args_to_string(*cmd_args)
-        end
-
-        # Helper to nicely convert variable string args into a single command line.  It
-        # will compact nulls or empty strings and join arguments with single spaces, without
-        # introducing any double-spaces for missing args.
-        #
-        # @param args [String] variable number of string arguments
-        # @return [String] nicely concatenated string or empty string
-        def args_to_string(*args)
-          args.reject { |i| i.nil? || i == "" }.join(" ")
+          cmd_args.push([ "-source", new_resource.source ]) if new_resource.source && include_source
+          cmd_args
         end
 
         # Available packages in chocolatey as a Hash of names mapped to versions
@@ -236,8 +226,8 @@ class Chef
           package_name_array.each do |pkg|
             available_versions =
               begin
-                cmd = [ "list -r #{pkg}" ]
-                cmd.push( "-source #{new_resource.source}" ) if new_resource.source
+                cmd = [ "list", "-r", pkg ]
+                cmd.push( [ "-source", new_resource.source ] ) if new_resource.source
                 cmd.push( new_resource.options ) if new_resource.options
 
                 raw = parse_list_output(*cmd)
@@ -255,7 +245,7 @@ class Chef
         #
         # @return [Hash] name-to-version mapping of installed packages
         def installed_packages
-          @installed_packages ||= Hash[*parse_list_output("list -l -r").flatten]
+          @installed_packages ||= Hash[*parse_list_output("list", "-l", "-r").flatten]
           @installed_packages
         end
 
diff --git a/spec/unit/provider/package/chocolatey_spec.rb b/spec/unit/provider/package/chocolatey_spec.rb
index 9b0cef4ef3..c263b3aa0a 100644
--- a/spec/unit/provider/package/chocolatey_spec.rb
+++ b/spec/unit/provider/package/chocolatey_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Adam Jacob (<adam@chef.io>)
-# Copyright:: Copyright 2008-2016, Chef Software, Inc.
+# Copyright:: Copyright 2008-2019, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -46,7 +46,7 @@ describe Chef::Provider::Package::Chocolatey do
     allow(provider).to receive(:choco_install_path).and_return(choco_install_path)
     allow(provider).to receive(:choco_exe).and_return(choco_exe)
     local_list_obj = double(stdout: local_list_stdout)
-    allow(provider).to receive(:shell_out_compacted!).with("#{choco_exe} list -l -r", { returns: [0], timeout: timeout }).and_return(local_list_obj)
+    allow(provider).to receive(:shell_out_compacted!).with(choco_exe, "list", "-l", "-r", { returns: [0], timeout: timeout }).and_return(local_list_obj)
   end
 
   def allow_remote_list(package_names, args = nil)
@@ -60,7 +60,11 @@ describe Chef::Provider::Package::Chocolatey do
     EOF
     remote_list_obj = double(stdout: remote_list_stdout)
     package_names.each do |pkg|
-      allow(provider).to receive(:shell_out_compacted!).with("#{choco_exe} list -r #{pkg}#{args}", { returns: [0], timeout: timeout }).and_return(remote_list_obj)
+      if args
+        allow(provider).to receive(:shell_out_compacted!).with(choco_exe, "list", "-r", pkg, *args, { returns: [0], timeout: timeout }).and_return(remote_list_obj)
+      else
+        allow(provider).to receive(:shell_out_compacted!).with(choco_exe, "list", "-r", pkg, { returns: [0], timeout: timeout }).and_return(remote_list_obj)
+      end
     end
   end
 
@@ -182,7 +186,7 @@ describe Chef::Provider::Package::Chocolatey do
     it "should install a single package" do
       allow_remote_list(["git"])
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y git", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "git", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:install)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -193,7 +197,7 @@ describe Chef::Provider::Package::Chocolatey do
         allow_remote_list(["git"])
         new_resource.timeout(timeout)
         provider.load_current_resource
-        expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y git", { returns: [0], timeout: timeout }).and_return(double)
+        expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "git", { returns: [0], timeout: timeout }).and_return(double)
         provider.run_action(:install)
         expect(new_resource).to be_updated_by_last_action
       end
@@ -222,7 +226,7 @@ describe Chef::Provider::Package::Chocolatey do
       new_resource.package_name("ConEmu")
       new_resource.version("15.10.25.1")
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y --version 15.10.25.1 conemu", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "--version", "15.10.25.1", "conemu", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:install)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -235,7 +239,7 @@ describe Chef::Provider::Package::Chocolatey do
       new_resource.package_name(%w{chocolatey ConEmu})
       new_resource.version([nil, "15.10.25.1"])
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y --version 15.10.25.1 conemu", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "--version", "15.10.25.1", "conemu", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:install)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -245,7 +249,7 @@ describe Chef::Provider::Package::Chocolatey do
       new_resource.package_name("conemu")
       new_resource.version("15.10.25.1")
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y --version 15.10.25.1 conemu", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "--version", "15.10.25.1", "conemu", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:install)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -255,8 +259,8 @@ describe Chef::Provider::Package::Chocolatey do
       new_resource.package_name(%w{ConEmu git})
       new_resource.version(["15.10.25.1", nil])
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y --version 15.10.25.1 conemu", { returns: [0], timeout: timeout }).and_return(double)
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y git", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "--version", "15.10.25.1", "conemu", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "git", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:install)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -265,27 +269,27 @@ describe Chef::Provider::Package::Chocolatey do
       allow_remote_list(%w{git munin-node})
       new_resource.package_name(%w{git munin-node})
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y git munin-node", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "git", "munin-node", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:install)
       expect(new_resource).to be_updated_by_last_action
     end
 
     context "when passing a source argument" do
       it "should pass options into the install command" do
-        allow_remote_list(["git"], " -source localpackages")
+        allow_remote_list(["git"], ["-source", "localpackages"])
         new_resource.source("localpackages")
         provider.load_current_resource
-        expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y -source localpackages git", { returns: [0], timeout: timeout }).and_return(double)
+        expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "-source", "localpackages", "git", { returns: [0], timeout: timeout }).and_return(double)
         provider.run_action(:install)
         expect(new_resource).to be_updated_by_last_action
       end
     end
 
     it "should pass options into the install command" do
-      allow_remote_list(["git"], " -force")
+      allow_remote_list(["git"], "-force")
       new_resource.options("-force")
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y -force git", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "-force", "git", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:install)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -306,7 +310,7 @@ describe Chef::Provider::Package::Chocolatey do
 
     context "alternate source" do
       it "installing a package that does not exist throws an error" do
-        allow_remote_list(["package-does-not-exist"], " -source alternate_source")
+        allow_remote_list(["package-does-not-exist"], ["-source", "alternate_source"])
         new_resource.package_name("package-does-not-exist")
         new_resource.source("alternate_source")
         provider.load_current_resource
@@ -316,7 +320,7 @@ describe Chef::Provider::Package::Chocolatey do
 
     context "private source" do
       it "installing a package without auth options throws an error" do
-        allow_remote_list(["package-without-auth"], " -source auth_source")
+        allow_remote_list(["package-without-auth"], ["-source", "auth_source"])
         new_resource.package_name("package-without-auth")
         new_resource.source("auth_source")
         provider.load_current_resource
@@ -324,7 +328,7 @@ describe Chef::Provider::Package::Chocolatey do
       end
 
       it "installing a package with invalid credentials throws an error" do
-        allow_remote_list(["package-invalid-auth"], " -source auth_source -u user -p password")
+        allow_remote_list(["package-invalid-auth"], [ "-source", "auth_source", "-u user -p password"])
         new_resource.package_name("package-invalid-auth")
         new_resource.source("auth_source")
         new_resource.options("-u user -p password")
@@ -333,11 +337,11 @@ describe Chef::Provider::Package::Chocolatey do
       end
 
       it "installing a package with valid credentials" do
-        allow_remote_list(["git"], " -source auth_source -u user -p password")
+        allow_remote_list(["git"], [ "-source", "auth_source", "-u user -p password" ])
         new_resource.source("auth_source")
         new_resource.options("-u user -p password")
         provider.load_current_resource
-        expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} install -y -u user -p password -source auth_source git", { returns: [0], timeout: timeout }).and_return(double)
+        expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "install", "-y", "-u user -p password", "-source", "auth_source", "git", { returns: [0], timeout: timeout }).and_return(double)
         provider.run_action(:install)
         expect(new_resource).to be_updated_by_last_action
       end
@@ -348,7 +352,7 @@ describe Chef::Provider::Package::Chocolatey do
     it "should install a package that is not installed" do
       allow_remote_list(["git"])
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} upgrade -y git", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "upgrade", "-y", "git", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:upgrade)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -357,7 +361,7 @@ describe Chef::Provider::Package::Chocolatey do
       allow_remote_list(["ConEmu"])
       new_resource.package_name("ConEmu")
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} upgrade -y conemu", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "upgrade", "-y", "conemu", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:upgrade)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -366,7 +370,7 @@ describe Chef::Provider::Package::Chocolatey do
       allow_remote_list(["conemu"])
       new_resource.package_name("conemu")
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} upgrade -y conemu", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "upgrade", "-y", "conemu", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:upgrade)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -375,7 +379,7 @@ describe Chef::Provider::Package::Chocolatey do
       allow_remote_list(["chocolatey"])
       new_resource.package_name("chocolatey")
       provider.load_current_resource
-      expect(provider).not_to receive(:shell_out_compacted!).with("#{choco_exe} upgrade -y chocolatey", { returns: [0], timeout: timeout })
+      expect(provider).not_to receive(:shell_out_compacted!).with(choco_exe, "upgrade", "-y", "chocolatey", { returns: [0], timeout: timeout })
       provider.run_action(:upgrade)
       expect(new_resource).not_to be_updated_by_last_action
     end
@@ -384,7 +388,7 @@ describe Chef::Provider::Package::Chocolatey do
       allow_remote_list(["git"])
       new_resource.version("2.6.2")
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} upgrade -y --version 2.6.2 git", { returns: [0], timeout: timeout })
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "upgrade", "-y", "--version", "2.6.2", "git", { returns: [0], timeout: timeout })
       provider.run_action(:upgrade)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -392,7 +396,7 @@ describe Chef::Provider::Package::Chocolatey do
     it "upgrading multiple packages uses a single command" do
       allow_remote_list(%w{conemu git})
       new_resource.package_name(%w{conemu git})
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} upgrade -y conemu git", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "upgrade", "-y", "conemu", "git", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:upgrade)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -413,7 +417,7 @@ describe Chef::Provider::Package::Chocolatey do
 
     context "alternate source" do
       it "installing a package that does not exist throws an error" do
-        allow_remote_list(["package-does-not-exist"], " -source alternate_source")
+        allow_remote_list(["package-does-not-exist"], ["-source", "alternate_source"])
         new_resource.package_name("package-does-not-exist")
         new_resource.source("alternate_source")
         provider.load_current_resource
@@ -444,7 +448,7 @@ describe Chef::Provider::Package::Chocolatey do
       allow_remote_list(["ConEmu"])
       new_resource.package_name("ConEmu")
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} uninstall -y ConEmu", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "uninstall", "-y", "ConEmu", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:remove)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -453,7 +457,7 @@ describe Chef::Provider::Package::Chocolatey do
       allow_remote_list(["conemu"])
       new_resource.package_name("conemu")
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} uninstall -y conemu", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "uninstall", "-y", "conemu", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:remove)
       expect(new_resource).to be_updated_by_last_action
     end
@@ -463,7 +467,7 @@ describe Chef::Provider::Package::Chocolatey do
       allow_remote_list(%w{git conemu})
       new_resource.package_name(%w{git conemu})
       provider.load_current_resource
-      expect(provider).to receive(:shell_out_compacted!).with("#{choco_exe} uninstall -y conemu", { returns: [0], timeout: timeout }).and_return(double)
+      expect(provider).to receive(:shell_out_compacted!).with(choco_exe, "uninstall", "-y", "conemu", { returns: [0], timeout: timeout }).and_return(double)
       provider.run_action(:remove)
       expect(new_resource).to be_updated_by_last_action
     end