fix: fail with actionable errors on misconfigured verifier options

lib/kitchen/verifier/pester.rb

@@ -221,7 +221,13 @@ def resolve_downloads_paths!
         config[:downloads] = config[:downloads]
           .map do |source, destination|
             source = source.to_s
-            destination = destination.gsub("%{instance_name}", instance.name)
+            if destination.nil?
+              raise UserError, "The verifier's 'downloads' entry for '#{source}' has no local " \
+                               "destination. Every entry needs one, for example " \
+                               "'#{source}: ./testresults/'."
+            end
+
+            destination = destination.to_s.gsub("%{instance_name}", instance.name)
             info("  resolving remote source's absolute path.")
             unless source.match?(%r{^/|^[a-zA-Z]:[\\/]}) # is Absolute?
               info("  '#{source}' is a relative path, resolving to: #{File.join(config[:root_path], source)}")
@@ -373,12 +379,13 @@ def get_powershell_modules_from_nugetapi
         gallery_url_param = bootstrap[:repository_url] ? "-GalleryUrl '#{bootstrap[:repository_url]}'" : ""
 
         info("Bootstrapping environment without PowerShellGet Provider...")
-        Array(bootstrap[:modules]).map do |powershell_module|
+        config_list("bootstrap.modules", bootstrap[:modules]).map do |powershell_module|
           if powershell_module.is_a? Hash
+            module_name = module_name!("bootstrap.modules", powershell_module)
             <<-PS1
-              ${#{powershell_module[:Name]}} = #{ps_hash(powershell_module)}
+              ${#{module_name}} = #{ps_hash(powershell_module)}
 
-              Install-ModuleFromNuget -Module ${#{powershell_module[:Name]}} #{gallery_url_param}
+              Install-ModuleFromNuget -Module ${#{module_name}} #{gallery_url_param}
             PS1
           else
             <<-PS1
@@ -397,13 +404,14 @@ def register_psrepository_scriptblock
         return if config[:register_repository].nil?
 
         info("Registering a new PowerShellGet Repository")
-        Array(config[:register_repository]).map do |psrepo|
+        config_list("register_repository", config[:register_repository]).map do |psrepo|
+          repo_name = module_name!("register_repository", psrepo)
           # Using Set-PSRepo from ../../*/*/*/PesterUtil.psm1
-          debug("Command to set PSRepo #{psrepo[:Name]}.")
+          debug("Command to set PSRepo #{repo_name}.")
           <<-PS1
-            Write-Host 'Registering psrepo #{psrepo[:Name]}...'
-            ${#{psrepo[:Name]}} = #{ps_hash(psrepo)}
-            Set-PSRepo -Repository ${#{psrepo[:Name]}}
+            Write-Host 'Registering psrepo #{repo_name}...'
+            ${#{repo_name}} = #{ps_hash(psrepo)}
+            Set-PSRepo -Repository ${#{repo_name}}
           PS1
         end
       end
@@ -439,10 +447,10 @@ def install_pester
       def install_modules_from_gallery
         return if config[:install_modules].nil?
 
-        Array(config[:install_modules]).map do |powershell_module|
+        config_list("install_modules", config[:install_modules]).map do |powershell_module|
           if powershell_module.is_a? Hash
             # Sanitize variable name so that $powershell-yaml becomes $powershell_yaml
-            module_name = powershell_module[:Name].gsub(/[\W]/, "_")
+            module_name = module_name!("install_modules", powershell_module).gsub(/[\W]/, "_")
             # so we can splat that variable to install module
             <<-PS1
               $#{module_name} = #{ps_hash(powershell_module)}
@@ -785,7 +793,7 @@ def prepare_copy_folders
 
         info("Preparing to copy specified folders to #{sandbox_module_path}.")
         kitchen_root_path = config[:kitchen_root]
-        config[:copy_folders].each do |folder|
+        config_list("copy_folders", config[:copy_folders]).each do |folder|
           debug("copying #{folder}")
           folder_to_copy = File.join(kitchen_root_path, folder)
           copy_if_src_exists(folder_to_copy, sandbox_module_path)
@@ -867,6 +875,61 @@ def absolute_test_folder
         path = (Pathname.new config[:test_folder]).realpath
         integration_path = File.join(path, "integration")
         Dir.exist?(integration_path) ? integration_path : path.to_s
+      rescue Errno::ENOENT
+        raise UserError, "The verifier's 'test_folder' is set to " \
+                         "'#{config[:test_folder]}', which does not exist. It is resolved " \
+                         "relative to the directory kitchen runs in, so give it a path that " \
+                         "exists there or an absolute one."
+      end
+
+      # Returns the entries of a config option that is documented as a list.
+      #
+      # YAML makes it easy to write a single mapping where a list of mappings
+      # was meant -- leaving off the leading `- ` is enough. `Array()` turns
+      # such a mapping into a list of `[key, value]` pairs, which then renders
+      # as nonsense PowerShell instead of failing, so reject it here where we
+      # can still say which option is at fault.
+      #
+      # @param key [String] the option's name, for the error message
+      # @param value [Object] the configured value
+      # @return [Array] the entries to iterate over
+      # @raise [Kitchen::UserError] when a single mapping was given
+      # @api private
+      def config_list(key, value)
+        if value.is_a?(Hash)
+          raise UserError, "The verifier's '#{key}' must be a list, but a single mapping " \
+                           "was given. Put a '- ' in front of each entry in kitchen.yml."
+        end
+
+        Array(value)
+      end
+
+      # Returns the Name of a mapping-shaped entry in one of the module or
+      # repository lists.
+      #
+      # The name becomes a PowerShell variable that the generated script splats,
+      # so a missing one either blows up here or emits an empty `${}` that fails
+      # on the instance a long way from its cause.
+      #
+      # @param key [String] the option's name, for the error message
+      # @param entry [Hash] the entry to read the name from
+      # @return [String] the entry's Name
+      # @raise [Kitchen::UserError] when the entry is not a mapping, or has no
+      #   usable Name
+      # @api private
+      def module_name!(key, entry)
+        unless entry.is_a?(Hash)
+          raise UserError, "Every entry under the verifier's '#{key}' must be a mapping with " \
+                           "at least a 'Name'; #{entry.inspect} is a #{entry.class}."
+        end
+
+        name = entry[:Name] || entry["Name"]
+        if name.to_s.empty?
+          raise UserError, "Every mapping under the verifier's '#{key}' needs a 'Name'; " \
+                           "#{entry.inspect} has none."
+        end
+
+        name.to_s
       end
 
       # Returns the final segment of a path that lives on the SUT.

spec/kitchen/verifier/pester_config_errors_spec.rb

@@ -0,0 +1,147 @@
+# frozen_string_literal: true
+
+# Copyright (c) 2015 Steven Murawski
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+# THE SOFTWARE.
+
+require_relative "../../spec_helper"
+
+# Covers what happens when kitchen.yml is wrong rather than when it is right.
+#
+# Every option below is documented as a list of mappings or as a mapping with a
+# Name, and YAML makes all of them easy to get subtly wrong -- a missing `- `,
+# a forgotten key, a value left blank. The verifier interpolates these straight
+# into PowerShell, so an unchecked mistake either raises somewhere unrelated in
+# Ruby or ships a broken script to the instance and fails there. These specs
+# pin the messages that say which option is at fault.
+describe Kitchen::Verifier::Pester do
+  describe "a list option given as a single mapping" do
+    it "rejects install_modules" do
+      verifier = build_verifier({ install_modules: { Name: "PSScriptAnalyzer" } })
+
+      error = _ { verifier.install_modules_from_gallery }.must_raise Kitchen::UserError
+      _(error.message).must_include "'install_modules' must be a list"
+      _(error.message).must_include "Put a '- ' in front of each entry"
+    end
+
+    it "rejects register_repository" do
+      verifier = build_verifier({ register_repository: { Name: "MyRepo", SourceLocation: "https://example.test/api/v2" } })
+
+      error = _ { verifier.register_psrepository_scriptblock }.must_raise Kitchen::UserError
+      _(error.message).must_include "'register_repository' must be a list"
+    end
+
+    it "rejects bootstrap.modules" do
+      verifier = build_verifier({ bootstrap: { repository_url: "https://example.test/api/v2", modules: { Name: "PowerShellGet" } } })
+
+      error = _ { verifier.get_powershell_modules_from_nugetapi }.must_raise Kitchen::UserError
+      _(error.message).must_include "'bootstrap.modules' must be a list"
+    end
+
+    it "rejects copy_folders" do
+      verifier = build_verifier({ copy_folders: { output: "MyModule" }, kitchen_root: "/kitchen" })
+      verifier.stubs(:sandbox_path).returns("/kitchen/sandbox")
+
+      error = _ { verifier.send(:prepare_copy_folders) }.must_raise Kitchen::UserError
+      _(error.message).must_include "'copy_folders' must be a list"
+    end
+
+    it "still accepts a list, which is the documented shape" do
+      verifier = build_verifier({ install_modules: [{ Name: "PSScriptAnalyzer" }] })
+
+      _(verifier.install_modules_from_gallery.first).must_include "Install-Module @PSScriptAnalyzer"
+    end
+
+    it "still accepts a bare string entry" do
+      verifier = build_verifier({ install_modules: %w{PSScriptAnalyzer} })
+
+      _(verifier.install_modules_from_gallery.first).must_include "Install-Module -Name 'PSScriptAnalyzer'"
+    end
+  end
+
+  describe "a mapping entry with no Name" do
+    it "rejects an install_modules entry" do
+      verifier = build_verifier({ install_modules: [{ Repository: "MyRepo" }] })
+
+      error = _ { verifier.install_modules_from_gallery }.must_raise Kitchen::UserError
+      _(error.message).must_include "'install_modules' needs a 'Name'"
+      _(error.message).must_include "Repository"
+    end
+
+    it "rejects a register_repository entry" do
+      verifier = build_verifier({ register_repository: [{ SourceLocation: "https://example.test/api/v2" }] })
+
+      error = _ { verifier.register_psrepository_scriptblock }.must_raise Kitchen::UserError
+      _(error.message).must_include "'register_repository' needs a 'Name'"
+    end
+
+    it "rejects a bootstrap.modules entry" do
+      verifier = build_verifier({ bootstrap: { repository_url: "https://example.test/api/v2", modules: [{ Version: "2.2.5" }] } })
+
+      error = _ { verifier.get_powershell_modules_from_nugetapi }.must_raise Kitchen::UserError
+      _(error.message).must_include "'bootstrap.modules' needs a 'Name'"
+    end
+
+    it "rejects a Name that is present but empty" do
+      verifier = build_verifier({ install_modules: [{ Name: "" }] })
+
+      _ { verifier.install_modules_from_gallery }.must_raise Kitchen::UserError
+    end
+
+    it "rejects a register_repository entry that is a bare string, since it is splatted" do
+      verifier = build_verifier({ register_repository: %w{MyRepo} })
+
+      error = _ { verifier.register_psrepository_scriptblock }.must_raise Kitchen::UserError
+      _(error.message).must_include "must be a mapping"
+    end
+
+    it "accepts a string key, since not every config path symbolizes keys" do
+      verifier = build_verifier({ install_modules: [{ "Name" => "PSScriptAnalyzer" }] })
+
+      _(verifier.install_modules_from_gallery.first).must_include "Install-Module @PSScriptAnalyzer"
+    end
+  end
+
+  describe "a test_folder that does not exist" do
+    it "names the option and the path instead of raising Errno::ENOENT" do
+      verifier = build_verifier({ test_folder: "definitely/not/here" })
+
+      error = _ { verifier.send(:test_folder) }.must_raise Kitchen::UserError
+      _(error.message).must_include "'test_folder'"
+      _(error.message).must_include "definitely/not/here"
+      _(error.message).must_include "does not exist"
+    end
+
+    it "surfaces through create_sandbox, where the folder is first read" do
+      verifier = build_verifier({ test_folder: "definitely/not/here" })
+
+      _ { verifier.create_sandbox }.must_raise Kitchen::UserError
+    end
+  end
+
+  describe "a downloads entry with no destination" do
+    it "names the source it belongs to" do
+      verifier = build_verifier({ downloads: { "PesterTestResults.xml" => nil }, root_path: "/tmp/verifier" })
+
+      error = _ { verifier.resolve_downloads_paths! }.must_raise Kitchen::UserError
+      _(error.message).must_include "has no local destination"
+      _(error.message).must_include "PesterTestResults.xml"
+    end
+  end
+end