preferred_manifest_record: fix pretty print.

The `preferred_manifest_record` function pretty prints the list of
existing files when the asked file can't be found. To do that, the root
path of the cookbook is removed from each existing file but the update
is done on the `existing_files` variable, not on a copy. When called
several times, this variable will eventually be equals to `[nil]`,
leading to a NoMethodError.

Without the fix, the new unit test fails with

> expected Chef::Exceptions::FileNotFound, got #<NoMethodError: undefined method `[]' for nil:NilClass> with backtrace:
>   # ./lib/chef/cookbook_version.rb:321:in `block in preferred_manifest_record'
>   # ./lib/chef/cookbook_version.rb:319:in `map!'
>   # ./lib/chef/cookbook_version.rb:319:in `preferred_manifest_record'
>   # ./spec/unit/cookbook_version_file_specificity_spec.rb:318:in `block (3 levels) in <top (required)>'
>   # ./spec/unit/cookbook_version_file_specificity_spec.rb:317:in `block (2 levels) in <top (required)>'

Fixes #2561.

2 files changed, 33 insertions, 4 deletions

diff --git a/lib/chef/cookbook_version.rb b/lib/chef/cookbook_version.rb
index 906b94baad..1e903608b5 100644
--- a/lib/chef/cookbook_version.rb
+++ b/lib/chef/cookbook_version.rb
@@ -316,11 +316,17 @@ class Chef
           error_message << error_locations.join("\n")
           existing_files = segment_filenames(segment)
           # Strip the root_dir prefix off all files for readability
-          existing_files.map! { |path| path[root_dir.length + 1..-1] } if root_dir
+          pretty_existing_files = existing_files.map { |path|
+            if root_dir
+              path[root_dir.length + 1..-1]
+            else
+              path
+            end
+          }
           # Show the files that the cookbook does have. If the user made a typo,
           # hopefully they'll see it here.
-          unless existing_files.empty?
-            error_message << "\n\nThis cookbook _does_ contain: ['#{existing_files.join("','")}']"
+          unless pretty_existing_files.empty?
+            error_message << "\n\nThis cookbook _does_ contain: ['#{pretty_existing_files.join("','")}']"
           end
           raise Chef::Exceptions::FileNotFound, error_message
         else
diff --git a/spec/unit/cookbook_version_file_specificity_spec.rb b/spec/unit/cookbook_version_file_specificity_spec.rb
index 2bc20ac64e..df9e6571d8 100644
--- a/spec/unit/cookbook_version_file_specificity_spec.rb
+++ b/spec/unit/cookbook_version_file_specificity_spec.rb
@@ -21,7 +21,7 @@ require "spec_helper"
 
 describe Chef::CookbookVersion, "file specificity" do
   before(:each) do
-    @cookbook = Chef::CookbookVersion.new "test-cookbook"
+    @cookbook = Chef::CookbookVersion.new("test-cookbook", "/cookbook-folder")
     @cookbook.manifest = {
       "files" =>
       [
@@ -301,6 +301,29 @@ describe Chef::CookbookVersion, "file specificity" do
     expect(manifest_record[:checksum]).to eq("csum4-platver-full")
   end
 
+  it "should raise a FileNotFound exception without match" do
+    node = Chef::Node.new
+
+    expect {
+      @cookbook.preferred_manifest_record(node, :files, "doesn't_exist.rb")
+    }.to raise_error(Chef::Exceptions::FileNotFound)
+  end
+  it "should raise a FileNotFound exception consistently without match" do
+    node = Chef::Node.new
+
+    expect {
+      @cookbook.preferred_manifest_record(node, :files, "doesn't_exist.rb")
+    }.to raise_error(Chef::Exceptions::FileNotFound)
+
+    expect {
+      @cookbook.preferred_manifest_record(node, :files, "doesn't_exist.rb")
+    }.to raise_error(Chef::Exceptions::FileNotFound)
+
+    expect {
+      @cookbook.preferred_manifest_record(node, :files, "doesn't_exist.rb")
+    }.to raise_error(Chef::Exceptions::FileNotFound)
+  end
+
   describe "when fetching the contents of a directory by file specificity" do
 
     it "should return a directory of manifest records based on priority preference: host" do