Bug #1550 - Rework to avoid regressing rspec tests, add new rspec tests for templatedir as a path

Signed-off-by: Paul Nasrat <pnasrat@googlemail.com>

4 files changed, 43 insertions, 13 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 61e46915a..74c8a93d7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
 0.24.x
+    Fixed #1554 - Added support for multiple template directories
+
     Fixed #1500 - puppetrun not working
 
     Fixed #1579 and #1580 - errors in the Puppet RPM spec file
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 87ccd62f6..a2900fd43 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -664,7 +664,8 @@ module Puppet
     setdefaults(:parser,
         :lexical => [false, "Whether to use lexical scoping (vs. dynamic)."],
         :templatedir => ["$vardir/templates",
-            "Where Puppet looks for template files."
+            "Where Puppet looks for template files.  Can be a list of colon-seperated
+             directories."
         ]
     )
 
diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb
index 4bdfab092..b34f2f8b0 100644
--- a/lib/puppet/module.rb
+++ b/lib/puppet/module.rb
@@ -72,28 +72,33 @@ class Puppet::Module
             return template
         end
 
+        template_paths = templatepath(environment)
+        default_template_path = File::join(template_paths.first, template)
+
         # If we can find the template in :templatedir, we return that.
-        td_file = templatepath(environment).collect { |path|
+        td_file = template_paths.collect { |path|
             File::join(path, template)
         }.find { |f| File.exists?(f) }
 
         return td_file unless td_file == nil
 
+        td_file = find_template_for_module(template, environment)
+        td_file ||= default_template_path
+    end
+
+    def self.find_template_for_module(template, environment = nil)
         path, file = split_path(template)
 
         # Because templates don't have an assumed template name, like manifests do,
         # we treat templates with no name as being templates in the main template
         # directory.
-        if file.nil?
-            mod = nil
-        else
+        if not file.nil?
             mod = find(path, environment)
+            if mod
+                return mod.template(file)
+            end
         end
-        if mod
-            return mod.template(file)
-        else
-            return td_file # Return this anyway, since we're going to fail.
-        end
+        nil
     end
 
     # Return a list of manifests (as absolute filenames) that match +pat+
@@ -156,4 +161,5 @@ class Puppet::Module
     end
 
     private :initialize
+    private_class_method :find_template_for_module
 end
diff --git a/spec/unit/module.rb b/spec/unit/module.rb
index e79001ae1..4d66550b5 100755
--- a/spec/unit/module.rb
+++ b/spec/unit/module.rb
@@ -90,23 +90,44 @@ describe Puppet::Module, " when searching for templates" do
     end
 
     it "should use the main templatedir if no module is found" do
-        Puppet.settings.expects(:value).with(:templatedir, nil).returns("/my/templates")
+        Puppet::Module.stubs(:templatepath).with(nil).returns(["/my/templates"])
         Puppet::Module.expects(:find).with("mymod", nil).returns(nil)
         Puppet::Module.find_template("mymod/mytemplate").should == "/my/templates/mymod/mytemplate"
     end
 
     it "should return unqualified templates directly in the template dir" do
-        Puppet.settings.expects(:value).with(:templatedir, nil).returns("/my/templates")
+        Puppet::Module.stubs(:templatepath).with(nil).returns(["/my/templates"])
         Puppet::Module.expects(:find).never
         Puppet::Module.find_template("mytemplate").should == "/my/templates/mytemplate"
     end
 
     it "should use the environment templatedir if no module is found and an environment is specified" do
-        Puppet.settings.expects(:value).with(:templatedir, "myenv").returns("/myenv/templates")
+        Puppet::Module.stubs(:templatepath).with("myenv").returns(["/myenv/templates"])
+        Puppet::Module.expects(:find).with("mymod", "myenv").returns(nil)
+        Puppet::Module.find_template("mymod/mytemplate", "myenv").should == "/myenv/templates/mymod/mytemplate"
+    end
+
+    it "should use first dir from environment templatedir if no module is found and an environment is specified" do
+        Puppet::Module.stubs(:templatepath).with("myenv").returns(["/myenv/templates", "/two/templates"])
         Puppet::Module.expects(:find).with("mymod", "myenv").returns(nil)
         Puppet::Module.find_template("mymod/mytemplate", "myenv").should == "/myenv/templates/mymod/mytemplate"
     end
 
+    it "should use a valid dir when templatedir is a path for unqualified templates and the first dir contains template" do
+        Puppet::Module.stubs(:templatepath).returns(["/one/templates", "/two/templates"])
+        File.expects(:exists?).with("/one/templates/mytemplate").returns(true)
+        Puppet::Module.expects(:find).never
+        Puppet::Module.find_template("mytemplate").should == "/one/templates/mytemplate"
+    end
+
+    it "should use a valid dir when templatedir is a path for unqualified templates and only second dir contains template" do
+        Puppet::Module.stubs(:templatepath).returns(["/one/templates", "/two/templates"])
+        File.expects(:exists?).with("/one/templates/mytemplate").returns(false)
+        File.expects(:exists?).with("/two/templates/mytemplate").returns(true)
+        Puppet::Module.expects(:find).never
+        Puppet::Module.find_template("mytemplate").should == "/two/templates/mytemplate"
+    end
+
     it "should use the node environment if specified" do
         Puppet.settings.stubs(:value).returns.returns("/my/directory")
         Puppet.settings.expects(:value).with(:modulepath, "myenv").returns("/my/modules")