Merge pull request #7167 from chef/lcg/actuall-test-cheffs

actually test against cheffs

4 files changed, 56 insertions, 64 deletions

diff --git a/.travis.yml b/.travis.yml
index 8b6f61e54d..36fd343809 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -88,9 +88,10 @@ matrix:
     script: bundle exec tasks/bin/run_external_test $TEST_GEM master rake
     rvm: 2.5.1
   - env:
-      PEDANT_OPTS: --skip-oc_id
-      TEST_GEM: chef/chef-zero
-    script: bundle exec tasks/bin/run_external_test $TEST_GEM master rake spec cheffs
+      - PEDANT_OPTS=--skip-oc_id
+      - TEST_GEM=chef/chef-zero
+      - CHEF_FS=true
+    script: bundle exec tasks/bin/run_external_test $TEST_GEM master rake pedant
     rvm: 2.5.1
   - env:
       TEST_GEM: chef/cheffish
diff --git a/lib/chef/chef_fs/chef_fs_data_store.rb b/lib/chef/chef_fs/chef_fs_data_store.rb
index e44c2cb8b8..697c2db232 100644
--- a/lib/chef/chef_fs/chef_fs_data_store.rb
+++ b/lib/chef/chef_fs/chef_fs_data_store.rb
@@ -307,14 +307,25 @@ class Chef
 
         # GET /cookbooks/NAME/VERSION or /cookbook_artifacts/NAME/IDENTIFIER
         elsif %w{cookbooks cookbook_artifacts}.include?(path[0]) && path.length == 3
-          with_cookbook_manifest(path) do |manifest, entry|
+          with_entry(path) do |entry|
             cookbook_type = path[0]
+            result = nil
+            begin
+              result = Chef::CookbookManifest.new(entry.chef_object, policy_mode: cookbook_type == "cookbook_artifacts").to_hash
+            rescue Chef::ChefFS::FileSystem::NotFoundError => e
+              raise ChefZero::DataStore::DataNotFoundError.new(to_zero_path(e.entry), e)
+            end
 
-            manifest.each_pair do |key, value|
+            result.each_pair do |key, value|
               if value.is_a?(Array)
                 value.each do |file|
                   if file.is_a?(Hash) && file.has_key?("checksum")
-                    relative = ["file_store", "repo", cookbook_type, entry.name ]
+                    relative = ["file_store", "repo", cookbook_type]
+                    if chef_fs.versioned_cookbooks || cookbook_type == "cookbook_artifacts"
+                      relative << "#{path[1]}-#{path[2]}"
+                    else
+                      relative << path[1]
+                    end
                     relative += file[:path].split("/")
                     file["url"] = ChefZero::RestBase.build_uri(request.base_uri, relative)
                   end
@@ -323,8 +334,8 @@ class Chef
             end
 
             if cookbook_type == "cookbook_artifacts"
-              manifest["metadata"] = manifest["metadata"].to_hash
-              manifest["metadata"].delete_if do |key, value|
+              result["metadata"] = result["metadata"].to_hash
+              result["metadata"].delete_if do |key, value|
                 value == [] ||
                   (value == {} && !%w{dependencies attributes recipes}.include?(key)) ||
                   (value == "" && %w{source_url issues_url}.include?(key)) ||
@@ -332,8 +343,9 @@ class Chef
               end
             end
 
-            Chef::JSONCompat.to_json_pretty(manifest)
+            Chef::JSONCompat.to_json_pretty(result)
           end
+
         else
           with_entry(path) do |entry|
             begin
@@ -507,7 +519,14 @@ class Chef
         elsif %w{cookbooks cookbook_artifacts}.include?(path[0]) && path.length == 1
           with_entry(path) do |entry|
             begin
-              entry.children.map { |child| child.chef_object.name.to_s }.uniq
+              if path[0] == "cookbook_artifacts"
+                entry.children.map { |child| child.name.rpartition("-")[0] }.uniq
+              elsif chef_fs.versioned_cookbooks
+                # /cookbooks/name-version -> /cookbooks/name
+                entry.children.map { |child| split_name_version(child.name)[0] }.uniq
+              else
+                entry.children.map { |child| child.name }
+              end
             rescue Chef::ChefFS::FileSystem::NotFoundError
               # If the cookbooks dir doesn't exist, we have no cookbooks (not 404)
               []
@@ -515,15 +534,22 @@ class Chef
           end
 
         elsif %w{cookbooks cookbook_artifacts}.include?(path[0]) && path.length == 2
-          result = with_entry([ path[0] ]) do |entry|
-            cookbooks = entry.children.map { |child| child.chef_object }
-            cookbooks.select { |cookbook| cookbook.name.to_s == path[1] }.
-            map { |cookbook| cookbook.version }
-          end
-          if result.empty?
-            raise ChefZero::DataStore::DataNotFoundError.new(path)
+          if chef_fs.versioned_cookbooks || path[0] == "cookbook_artifacts"
+            result = with_entry([ path[0] ]) do |entry|
+              # list /cookbooks/name = filter /cookbooks/name-version down to name
+              entry.children.map { |child| split_name_version(child.name) }.
+              select { |name, version| name == path[1] }.
+              map { |name, version| version }
+            end
+            if result.empty?
+              raise ChefZero::DataStore::DataNotFoundError.new(path)
+            end
+            result
+          else
+            # list /cookbooks/name = <single version>
+            version = get_single_cookbook_version(path)
+            [version]
           end
-          result
 
         else
           result = with_entry(path) do |entry|
@@ -756,26 +782,6 @@ class Chef
         path.length == 1 && BASE_DIRNAMES.include?(path[0])
       end
 
-      def with_cookbook_manifest(path)
-        cookbook_type = path[0]
-        begin
-          # this is fast and equivalent to with_entry() that also returns the cb manifest
-          entry = Chef::ChefFS::FileSystem.resolve_path(chef_fs, to_chef_fs_path(path))
-          yield Chef::CookbookManifest.new(entry.chef_object, policy_mode: cookbook_type == "cookbook_artifacts").to_hash, entry
-        rescue Chef::ChefFS::FileSystem::NotFoundError, ChefZero::DataStore::DataNotFoundError => e
-          # this is very slow and we walk through all the cookbook versions to find ones that have the correct name in the metadata
-          dir = Chef::ChefFS::FileSystem.resolve_path(chef_fs, to_chef_fs_path([path[0]]))
-          entry = dir.children.select do |child|
-            child.chef_object.full_name == "#{path[1]}-#{path[2]}" ||
-              (child.chef_object.name.to_s == path[1] && child.chef_object.identifier == path[2])
-          end[0]
-          raise ChefZero::DataStore::DataNotFoundError.new(path) if entry.nil?
-          yield Chef::CookbookManifest.new(entry.chef_object, policy_mode: cookbook_type == "cookbook_artifacts").to_hash, entry
-        end
-      rescue Chef::ChefFS::FileSystem::NotFoundError => e
-        raise ChefZero::DataStore::DataNotFoundError.new(to_zero_path(e.entry), e)
-      end
-
       def with_entry(path)
         yield Chef::ChefFS::FileSystem.resolve_path(chef_fs, to_chef_fs_path(path))
       rescue Chef::ChefFS::FileSystem::NotFoundError => e
diff --git a/spec/integration/knife/chef_fs_data_store_spec.rb b/spec/integration/knife/chef_fs_data_store_spec.rb
index d50968f327..02508b799d 100644
--- a/spec/integration/knife/chef_fs_data_store_spec.rb
+++ b/spec/integration/knife/chef_fs_data_store_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: John Keiser (<jkeiser@chef.io>)
-# Copyright:: Copyright 2013-2018, Chef Software Inc.
+# Copyright:: Copyright 2013-2016, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -28,7 +28,6 @@ describe "ChefFSDataStore tests", :workstation do
 
   let(:cookbook_x_100_metadata_rb) { cb_metadata("x", "1.0.0") }
   let(:cookbook_z_100_metadata_rb) { cb_metadata("z", "1.0.0") }
-  let(:cookbook_y_102_metadata_rb) { cb_metadata("z", "1.0.2") }
 
   describe "with repo mode 'hosted_everything' (default)" do
     before do
@@ -40,8 +39,6 @@ describe "ChefFSDataStore tests", :workstation do
         file "clients/x.json", {}
         file "cookbook_artifacts/x-111/metadata.rb", cookbook_x_100_metadata_rb
         file "cookbooks/x/metadata.rb", cookbook_x_100_metadata_rb
-        file "cookbooks/y/metadata.rb", cookbook_y_102_metadata_rb
-        file "cookbooks/z/metadata.rb", cookbook_z_100_metadata_rb
         file "data_bags/x/y.json", {}
         file "environments/x.json", {}
         file "nodes/x.json", {}
@@ -67,7 +64,6 @@ describe "ChefFSDataStore tests", :workstation do
 /acls/cookbook_artifacts/x.json
 /acls/cookbooks/
 /acls/cookbooks/x.json
-/acls/cookbooks/z.json
 /acls/data_bags/
 /acls/data_bags/x.json
 /acls/environments/
@@ -88,13 +84,11 @@ describe "ChefFSDataStore tests", :workstation do
 /containers/
 /containers/x.json
 /cookbook_artifacts/
-/cookbook_artifacts/x-1.0.0/
-/cookbook_artifacts/x-1.0.0/metadata.rb
+/cookbook_artifacts/x-111/
+/cookbook_artifacts/x-111/metadata.rb
 /cookbooks/
 /cookbooks/x/
 /cookbooks/x/metadata.rb
-/cookbooks/z/
-/cookbooks/z/metadata.rb
 /data_bags/
 /data_bags/x/
 /data_bags/x/y.json
@@ -117,12 +111,6 @@ EOM
         end
       end
 
-      context "LIST /TYPE/NAME" do
-        it "knife cookbook show -z z" do
-          knife("cookbook show -z z").should_succeed "z   1.0.2  1.0.0\n"
-        end
-      end
-
       context "DELETE /TYPE/NAME" do
         it "knife delete -z /clients/x.json works" do
           knife("delete -z /clients/x.json").should_succeed "Deleted /clients/x.json\n"
@@ -131,7 +119,7 @@ EOM
 
         it "knife delete -z -r /cookbooks/x works" do
           knife("delete -z -r /cookbooks/x").should_succeed "Deleted /cookbooks/x\n"
-          knife("list -z -Rfp /cookbooks").should_succeed "/cookbooks/z/\n/cookbooks/z/metadata.rb\n"
+          knife("list -z -Rfp /cookbooks").should_succeed ""
         end
 
         it "knife delete -z -r /data_bags/x works" do
@@ -206,14 +194,7 @@ EOM
 Uploading x              [1.0.0]
 Uploaded 1 cookbook.
 EOM
-          knife("list --local -Rfp /cookbooks").should_succeed <<EOM
-/cookbooks/x/
-/cookbooks/x/metadata.rb
-/cookbooks/y/
-/cookbooks/y/metadata.rb
-/cookbooks/z/
-/cookbooks/z/metadata.rb
-EOM
+          knife("list --local -Rfp /cookbooks").should_succeed "/cookbooks/x/\n/cookbooks/x/metadata.rb\n"
         end
 
         it "knife raw -z -i empty.json -m PUT /data/x/y" do
diff --git a/tasks/bin/run_external_test b/tasks/bin/run_external_test
index 1925c3e125..01eba18943 100755
--- a/tasks/bin/run_external_test
+++ b/tasks/bin/run_external_test
@@ -11,6 +11,10 @@ include Chef::Mixin::ShellOut
 github_repo = ARGV.shift
 git_thing = ARGV.shift
 
+build_dir = File.expand_path(ENV["TRAVIS_BUILD_DIR"] || Dir.pwd)
+
+env = { "GEMFILE_MOD" => "gem 'chef', path: '#{build_dir}'" }
+
 Dir.mktmpdir("chef-external-test") do |dir|
   git_url = "https://github.com/#{github_repo}"
   Dir.rmdir dir
@@ -18,8 +22,8 @@ Dir.mktmpdir("chef-external-test") do |dir|
   Dir.chdir(dir) do
     shell_out!("git checkout #{git_thing}", live_stream: STDOUT)
     Bundler.with_clean_env do
-      shell_out!("bundle install", live_stream: STDOUT)
-      shell_out!("bundle exec #{ARGV.join(" ")}", live_stream: STDOUT)
+      shell_out!("bundle install", live_stream: STDOUT, env: env)
+      shell_out!("bundle exec #{ARGV.join(" ")}", live_stream: STDOUT, env: env)
     end
   end
 end