Remove redundancy in chef solo provisioner

lib/vagrant/provisioners/chef_solo.rb

@@ -19,7 +19,7 @@ module Vagrant
           env.config.vm.share_folder("vagrant-chef-solo-#{i}", cookbook_path(i), cookbook)
         end
       end
-      
+
       def share_role_folders
         host_role_paths.each_with_index do |role, i|
           env.config.vm.share_folder("vagrant-chef-solo-#{i}", role_path(i), role)
@@ -45,50 +45,48 @@ module Vagrant
         end
       end
 
-      def host_cookbook_paths
+      def host_folder_paths(paths)
-        cookbooks = env.config.chef.cookbooks_path
+        [paths].flatten.collect { |path| File.expand_path(path, env.root_path) }
-        cookbooks = [cookbooks] unless cookbooks.is_a?(Array)
-        cookbooks.collect! { |cookbook| File.expand_path(cookbook, env.root_path) }
-        return cookbooks
       end
-      
+
+      def folder_path(folder, i)
+        File.join(env.config.chef.provisioning_path, "#{folder}-#{i}")
+      end
+
+      def folders_path(folders, folder)
+        result = []
+        folders.each_with_index do |host_path, i|
+          result << folder_path(folder, i)
+        end
+
+        # We're lucky that ruby's string and array syntax for strings is the
+        # same as JSON, so we can just convert to JSON here and use that
+        result = result[0].to_s if result.length == 1
+        result.to_json
+      end
+
+      def host_cookbook_paths
+        host_folder_paths(env.config.chef.cookbooks_path)
+      end
+
       def host_role_paths
-        roles = env.config.chef.roles_path
+        host_folder_paths(env.config.chef.roles_path)
-        roles = [roles] unless roles.is_a?(Array)
-        roles.collect! { |role| File.expand_path(role, env.root_path) }
-        return roles
       end
 
       def cookbook_path(i)
-        File.join(env.config.chef.provisioning_path, "cookbooks-#{i}")
+        folder_path("cookbooks", i)
       end
-      
+
       def role_path(i)
-        File.join(env.config.chef.provisioning_path, "roles-#{i}")
+        folder_path("roles", i)
       end
 
       def cookbooks_path
-        result = []
+        folders_path(host_cookbook_paths, "cookbooks")
-        host_cookbook_paths.each_with_index do |host_path, i|
-          result << cookbook_path(i)
-        end
-
-        # We're lucky that ruby's string and array syntax for strings is the
-        # same as JSON, so we can just convert to JSON here and use that
-        result = result[0].to_s if result.length == 1
-        result.to_json
       end
-      
-      def roles_path
-        result = []
-        host_role_paths.each_with_index do |host_path, i|
-          result << role_path(i)
-        end
 
-        # We're lucky that ruby's string and array syntax for strings is the
+      def roles_path
-        # same as JSON, so we can just convert to JSON here and use that
+        folders_path(host_role_paths, "roles")
-        result = result[0].to_s if result.length == 1
-        result.to_json
       end
     end
   end

test/vagrant/provisioners/chef_solo_test.rb

@@ -11,7 +11,7 @@ class ChefSoloProvisionerTest < Test::Unit::TestCase
       @action.expects(:share_cookbook_folders).once
       @action.prepare
     end
-    
+
     should "share role folders" do
       @action.expects(:share_role_folders).once
       @action.prepare
@@ -44,7 +44,7 @@ class ChefSoloProvisionerTest < Test::Unit::TestCase
       @action.share_cookbook_folders
     end
   end
-  
+
   context "sharing role folders" do
     setup do
       @host_role_paths = ["foo", "bar"]
@@ -61,41 +61,60 @@ class ChefSoloProvisionerTest < Test::Unit::TestCase
     end
   end
 
-  context "host cookbooks paths" do
+  context "host folder paths" do
     should "return as an array if was originally a string" do
-      File.stubs(:expand_path).returns("foo")
+      folder = "foo"
-      @env.config.chef.cookbooks_path = "foo"
+      File.stubs(:expand_path).returns("bar")
-
+      assert_equal ["bar"], @action.host_folder_paths(folder)
-      assert_equal ["foo"], @action.host_cookbook_paths
     end
 
-    should "return the array of cookbooks if its an array" do
+    should "return the array of folders if its an array" do
-      cookbooks = ["foo", "bar"]
+      folders = ["foo", "bar"]
-      @env.config.chef.cookbooks_path = cookbooks
-
       expand_seq = sequence('expand_seq')
-      cookbooks.collect! { |cookbook| File.expand_path(cookbook, @env.root_path) }
+      folders.collect! { |folder| File.expand_path(folder, @env.root_path) }
 
-      assert_equal cookbooks, @action.host_cookbook_paths
+      assert_equal folders, @action.host_folder_paths(folders)
     end
   end
-  
-  context "host roles paths" do
-    should "return as an array if was originally a string" do
-      File.stubs(:expand_path).returns("foo")
-      @env.config.chef.roles_path = "foo"
 
-      assert_equal ["foo"], @action.host_role_paths
+  context "host cookbooks paths" do
+    should "get folders path for configured cookbooks path" do
+      result = mock("result")
+      @env.config.chef.stubs(:cookbooks_path).returns("foo")
+      @action.expects(:host_folder_paths).with(@env.config.chef.cookbooks_path).returns(result)
+      assert_equal result, @action.host_cookbook_paths
+    end
+  end
+
+  context "host roles paths" do
+    should "get folders path for configured roles path" do
+      result = mock("result")
+      @env.config.chef.stubs(:roles_path).returns("foo")
+      @action.expects(:host_folder_paths).with(@env.config.chef.roles_path).returns(result)
+      assert_equal result, @action.host_role_paths
+    end
+  end
+
+  context "folder path" do
+    should "return a proper path to a single folder" do
+      expected = File.join(@env.config.chef.provisioning_path, "cookbooks-5")
+      assert_equal expected, @action.folder_path("cookbooks", 5)
     end
 
-    should "return the array of roles if its an array" do
+    should "return array-representation of folder paths if multiple" do
-      roles = ["foo", "bar"]
+      @folders = (0..5).to_a
-      @env.config.chef.roles_path = roles
+      @cookbooks = @folders.inject([]) do |acc, i|
+        acc << @action.cookbook_path(i)
+      end
 
-      expand_seq = sequence('expand_seq')
+      assert_equal @cookbooks.to_json, @action.folders_path(@folders, "cookbooks")
-      roles.collect! { |role| File.expand_path(role, @env.root_path) }
+    end
 
-      assert_equal roles, @action.host_role_paths
+    should "return a single string representation if folder paths is single" do
+      @folder = "cookbooks"
+      @cookbooks = @action.folder_path(@folder, 0)
+
+      assert_equal @cookbooks.to_json, @action.folders_path([0], @folder)
     end
   end
 
@@ -105,43 +124,25 @@ class ChefSoloProvisionerTest < Test::Unit::TestCase
       assert_equal expected, @action.cookbook_path(5)
     end
 
-    should "return array-representation of cookbook paths if multiple" do
+    should "properly call folders path and return result" do
-      @cookbooks = (0..5).inject([]) do |acc, i|
+      result = mock("result")
-        acc << @action.cookbook_path(i)
+      @action.stubs(:host_cookbook_paths).returns([])
-      end
+      @action.expects(:folders_path).with(@action.host_cookbook_paths, "cookbooks").once.returns(result)
-
+      assert_equal result, @action.cookbooks_path
-      @env.config.chef.cookbooks_path = @cookbooks
-      assert_equal @cookbooks.to_json, @action.cookbooks_path
-    end
-
-    should "return a single string representation if cookbook paths is single" do
-      @cookbooks = @action.cookbook_path(0)
-
-      @env.config.chef.cookbooks_path = @cookbooks
-      assert_equal @cookbooks.to_json, @action.cookbooks_path
     end
   end
-  
+
   context "roles path" do
     should "return a proper path to a single role" do
       expected = File.join(@env.config.chef.provisioning_path, "roles-5")
       assert_equal expected, @action.role_path(5)
     end
 
-    should "return array-representation of role paths if multiple" do
+    should "properly call folders path and return result" do
-      @roles = (0..5).inject([]) do |acc, i|
+      result = mock("result")
-        acc << @action.role_path(i)
+      @action.stubs(:host_role_paths).returns([])
-      end
+      @action.expects(:folders_path).with(@action.host_role_paths, "roles").once.returns(result)
-
+      assert_equal result, @action.roles_path
-      @env.config.chef.roles_path = @roles
-      assert_equal @roles.to_json, @action.roles_path
-    end
-
-    should "return a single string representation if roles paths is single" do
-      @roles = @action.role_path(0)
-
-      @env.config.chef.roles_path = @roles
-      assert_equal @roles.to_json, @action.roles_path
     end
   end