Modules now can find their own paths

Previously, when you created a module you had to specify
the path.  Now Module instances can use the module path
to look up their paths, and there are methods for determining
whether the module is present (if the path is present).

Also cleaned up the methods for figuring out what's in
the module (plugins, etc.).

Signed-off-by: Luke Kanies <luke@madstop.com>

4 files changed, 170 insertions, 82 deletions

diff --git a/spec/unit/file_serving/mount/plugins.rb b/spec/unit/file_serving/mount/plugins.rb
index c658b8cd6..ef842d12b 100755
--- a/spec/unit/file_serving/mount/plugins.rb
+++ b/spec/unit/file_serving/mount/plugins.rb
@@ -61,8 +61,8 @@ describe Puppet::FileServing::Mount::Plugins, "when searching for files" do
     end
 
     it "should return the plugin paths for each module that has plugins" do
-        one = stub 'module', :plugins? => true, :plugins => "/one"
-        two = stub 'module', :plugins? => true, :plugins => "/two"
+        one = stub 'module', :plugins? => true, :plugin_directory => "/one"
+        two = stub 'module', :plugins? => true, :plugin_directory => "/two"
 
         @environment.expects(:modules).returns [one, two]
         @mount.search("foo/bar").should == %w{/one /two}
diff --git a/spec/unit/module.rb b/spec/unit/module.rb
index 17c2065eb..4ff69695e 100755
--- a/spec/unit/module.rb
+++ b/spec/unit/module.rb
@@ -3,40 +3,137 @@
 require File.dirname(__FILE__) + '/../spec_helper'
 
 describe Puppet::Module do
+    it "should have a class method that returns a named module from a given environment" do
+        env = mock 'module'
+        env.expects(:module).with("mymod").returns "yep"
+        Puppet::Node::Environment.expects(:new).with("myenv").returns env
+
+        Puppet::Module.find("mymod", "myenv").should == "yep"
+    end
+
+    it "should return nil if asked for a named module that doesn't exist" do
+        env = mock 'module'
+        env.expects(:module).with("mymod").returns nil
+        Puppet::Node::Environment.expects(:new).with("myenv").returns env
+
+        Puppet::Module.find("mymod", "myenv").should be_nil
+    end
+
+    it "should return nil if asked for a module whose name is 'nil'" do
+        Puppet::Module.find(nil, "myenv").should be_nil
+    end
+
+    it "should require a name at initialization" do
+        lambda { Puppet::Module.new }.should raise_error(ArgumentError)
+    end
+
+    it "should convert an environment name into an Environment instance" do
+        Puppet::Module.new("foo", "prod").environment.should be_instance_of(Puppet::Node::Environment)
+    end
+
+    it "should accept an environment at initialization" do
+        Puppet::Module.new("foo", :prod).environment.name.should == :prod
+    end
+
+    it "should use the default environment if none is provided" do
+        env = Puppet::Node::Environment.new
+        Puppet::Module.new("foo").environment.should equal(env)
+    end
+
+    it "should use any provided Environment instance" do
+        env = Puppet::Node::Environment.new
+        Puppet::Module.new("foo", env).environment.should equal(env)
+    end
+
+    it "should return the path to the first found instance in its module paths as its path" do
+        mod = Puppet::Module.new("foo")
+        paths = %w{/a /b /c}
+        Puppet::Module.stubs(:modulepath).returns paths
+
+        FileTest.expects(:exist?).with("/a/foo").returns false
+        FileTest.expects(:exist?).with("/b/foo").returns true
+        FileTest.expects(:exist?).with("/c/foo").never
+
+        mod.path.should == "/b/foo"
+    end
+
+    it "should be considered existent if it exists in at least one module path" do
+        mod = Puppet::Module.new("foo")
+        mod.expects(:path).returns "/a/foo"
+        mod.should be_exist
+    end
+
+    it "should be considered nonexistent if it does not exist in any of the module paths" do
+        mod = Puppet::Module.new("foo")
+        mod.expects(:path).returns nil
+        mod.should_not be_exist
+    end
+
     [:plugins, :templates, :files, :manifests].each do |filetype|
-        it "should be able to indicate whether it has #{filetype}" do
-            Puppet::Module.new("foo", "/foo/bar").should respond_to(filetype.to_s + "?")
+        it "should be able to return individual #{filetype}" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            path = File.join("/a/foo", filetype.to_s, "my/file")
+            FileTest.expects(:exist?).with(path).returns true
+            mod.send(filetype.to_s.sub(/s$/, ''), "my/file").should == path
         end
 
-        it "should correctly detect when it has #{filetype}" do
-            FileTest.expects(:exist?).with("/foo/bar/#{filetype}").returns true
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s + "?").should be_true
+        it "should consider #{filetype} to be present if their base directory exists" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            path = File.join("/a/foo", filetype.to_s)
+            FileTest.expects(:exist?).with(path).returns true
+            mod.send(filetype.to_s + "?").should be_true
         end
 
-        it "should correctly detect when it does not have #{filetype}" do
-            FileTest.expects(:exist?).with("/foo/bar/#{filetype}").returns false
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s + "?").should be_false
+        it "should consider #{filetype} to be absent if their base directory does not exist" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            path = File.join("/a/foo", filetype.to_s)
+            FileTest.expects(:exist?).with(path).returns false
+            mod.send(filetype.to_s + "?").should be_false
         end
 
-        it "should have a method for returning the full path to the #{filetype}" do
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s).should == File.join("/foo/bar", filetype.to_s)
+        it "should consider #{filetype} to be absent if the module base directory does not exist" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns nil
+            mod.send(filetype.to_s + "?").should be_false
         end
 
-        it "should be able to return individual #{filetype}" do
-            path = File.join("/foo/bar", filetype.to_s, "my/file")
-            FileTest.expects(:exist?).with(path).returns true
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s.sub(/s$/, ''), "my/file").should == path
+        it "should return nil if asked to return individual #{filetype} that don't exist" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            path = File.join("/a/foo", filetype.to_s, "my/file")
+            FileTest.expects(:exist?).with(path).returns false
+            mod.send(filetype.to_s.sub(/s$/, ''), "my/file").should be_nil
         end
 
-        it "should return nil if asked to return individual #{filetype} that don't exist" do
-            FileTest.expects(:exist?).with(File.join("/foo/bar", filetype.to_s, "my/file")).returns false
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s.sub(/s$/, ''), "my/file").should be_nil
+        it "should return nil when asked for individual #{filetype} if the module does not exist" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns nil
+            mod.send(filetype.to_s.sub(/s$/, ''), "my/file").should be_nil
         end
 
         it "should return the base directory if asked for a nil path" do
-            path = File.join("/foo/bar", filetype.to_s)
-            FileTest.expects(:exist?).with(path).returns true
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s.sub(/s$/, ''), nil).should == path
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            base = File.join("/a/foo", filetype.to_s)
+            FileTest.expects(:exist?).with(base).returns true
+            mod.send(filetype.to_s.sub(/s$/, ''), nil).should == base
+        end
+    end
+
+    %w{plugins files}.each do |type|
+        short = type.sub(/s$/, '')
+        it "should be able to return the #{short} directory" do
+            Puppet::Module.new("foo").should respond_to(short + "_directory")
+        end
+
+        it "should return the path to the #{short} directory" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+
+            mod.send(short + "_directory").should == "/a/foo/#{type}"
         end
     end
 end
@@ -99,8 +196,8 @@ describe Puppet::Module, "when yielding each module in a list of directories" do
         one = mock 'one'
         two = mock 'two'
 
-        Puppet::Module.expects(:new).with("f1", "/one/f1").returns one
-        Puppet::Module.expects(:new).with("f2", "/one/f2").returns two
+        Puppet::Module.expects(:new).with("f1").returns one
+        Puppet::Module.expects(:new).with("f2").returns two
 
         result = []
         Puppet::Module.each_module("/one") do |mod|
@@ -116,8 +213,8 @@ describe Puppet::Module, "when yielding each module in a list of directories" do
 
         one = mock 'one'
 
-        Puppet::Module.expects(:new).with("f1", "/one/f1").returns one
-        Puppet::Module.expects(:new).with("f1", "/two/f1").never
+        Puppet::Module.expects(:new).with("f1").returns one
+        Puppet::Module.expects(:new).with("f1").never
 
         result = []
         Puppet::Module.each_module("/one", "/two") do |mod|
@@ -146,27 +243,48 @@ describe Puppet::Module, " when building its search path" do
     end
 end
 
-describe Puppet::Module, " when searching for modules" do
-    it "should use the current environment to find the specified module if no environment is provided" do
-        env = mock 'env'
-        env.expects(:module).with("foo").returns "yay"
-        Puppet::Node::Environment.expects(:new).with(nil).returns env
+describe Puppet::Module, "when finding matching manifests" do
+    before do
+        @mod = Puppet::Module.new("mymod")
+        @mod.stubs(:path).returns "/a"
+    end
+
+    it "should return all manifests matching the glob pattern" do
+        Dir.expects(:glob).with("/a/manifests/yay/*.pp").returns(%w{foo bar})
 
-        Puppet::Module.find("foo").should == "yay"
+        @mod.match_manifests("yay/*.pp").should == %w{foo bar}
     end
 
-    it "should use the specified environment to find the specified module if an environment is provided" do
-        env = mock 'env'
-        env.expects(:module).with("foo").returns "yay"
-        Puppet::Node::Environment.expects(:new).with("myenv").returns env
+    it "should not return directories" do
+        Dir.expects(:glob).with("/a/manifests/yay/*.pp").returns(%w{foo bar})
 
-        Puppet::Module.find("foo", "myenv").should == "yay"
+        FileTest.expects(:directory?).with("foo").returns false
+        FileTest.expects(:directory?).with("bar").returns true
+        @mod.match_manifests("yay/*.pp").should == %w{foo}
     end
-end
 
-describe Puppet::Module, " when returning files" do
-    it "should return the path to the module's 'files' directory" do
-        mod = Puppet::Module.send(:new, "mymod", "/my/mod")
-        mod.files.should == "/my/mod/files"
+    it "should default to the 'init.pp' file if no glob pattern is specified" do
+        FileTest.stubs(:exist?).returns true
+
+        @mod.match_manifests(nil).should == %w{/a/manifests/init.pp}
+    end
+
+    it "should return all manifests matching the glob pattern in all existing paths" do
+        Dir.expects(:glob).with("/a/manifests/yay/*.pp").returns(%w{a b})
+
+        @mod.match_manifests("yay/*.pp").should == %w{a b}
+    end
+
+    it "should match the glob pattern plus '.pp' if no results are found" do
+        Dir.expects(:glob).with("/a/manifests/yay/foo").returns([])
+        Dir.expects(:glob).with("/a/manifests/yay/foo.pp").returns(%w{yay})
+
+        @mod.match_manifests("yay/foo").should == %w{yay}
+    end
+
+    it "should return an empty array if no manifests matched" do
+        Dir.expects(:glob).with("/a/manifests/yay/*.pp").returns([])
+
+        @mod.match_manifests("yay/*.pp").should == []
     end
 end
diff --git a/spec/unit/node/environment.rb b/spec/unit/node/environment.rb
index dd6745f1e..5fac98b77 100755
--- a/spec/unit/node/environment.rb
+++ b/spec/unit/node/environment.rb
@@ -94,28 +94,24 @@ describe Puppet::Node::Environment do
             env.modules.should == %w{mod1 mod2}
         end
 
-        it "should be able to return an individual module by matching the module name" do
+        it "should be able to return an individual module that exists in its module path" do
             env = Puppet::Node::Environment.new("testing")
-            module_path = %w{/one /two}.join(File::PATH_SEPARATOR)
-            env.expects(:modulepath).returns module_path
 
-            one = stub 'one', :name => 'one'
-            two = stub 'two', :name => 'two'
-            Puppet::Module.expects(:each_module).with(module_path).multiple_yields(one, two)
+            mod = mock 'module'
+            Puppet::Module.expects(:new).with("one", env).returns mod
+            mod.expects(:exist?).returns true
 
-            env.module("two").should equal(two)
+            env.module("one").should equal(mod)
         end
 
-        it "should return nil if asked for a module that is not in its path" do
+        it "should return nil if asked for a module that does not exist in its path" do
             env = Puppet::Node::Environment.new("testing")
-            module_path = %w{/one /two}.join(File::PATH_SEPARATOR)
-            env.expects(:modulepath).returns module_path
 
-            one = stub 'one', :name => 'one'
-            two = stub 'two', :name => 'two'
-            Puppet::Module.expects(:each_module).with(module_path).multiple_yields(one, two)
+            mod = mock 'module'
+            Puppet::Module.expects(:new).with("one", env).returns mod
+            mod.expects(:exist?).returns false
 
-            env.module("three").should be_nil
+            env.module("one").should be_nil
         end
     end
 end
diff --git a/spec/unit/parser/files.rb b/spec/unit/parser/files.rb
index f401a902c..a7bd37b44 100644
--- a/spec/unit/parser/files.rb
+++ b/spec/unit/parser/files.rb
@@ -149,10 +149,6 @@ describe Puppet::Parser::Files do
     end
 
     describe "when searching for manifests in a found module" do
-        before do
-            @module = Puppet::Module.new("mymod", "/one")
-        end
-
         it "should return the manifests from the first found module" do
             mod = mock 'module'
             Puppet::Node::Environment.new.expects(:module).with("mymod").returns mod
@@ -167,28 +163,6 @@ describe Puppet::Parser::Files do
             Puppet::Parser::Files.find_manifests("mymod/init.pp", :environment => "myenv").should == ["/one/mymod/manifests/init.pp"]
         end
 
-        it "should return all manifests matching the glob pattern" do
-            File.stubs(:directory?).returns(true)
-            Dir.expects(:glob).with("/one/manifests/yay/*.pp").returns(%w{/one /two})
-
-            @module.match_manifests("yay/*.pp").should == %w{/one /two}
-        end
-
-        it "should not return directories" do
-            Dir.expects(:glob).with("/one/manifests/yay/*.pp").returns(%w{/one /two})
-
-            FileTest.expects(:directory?).with("/one").returns false
-            FileTest.expects(:directory?).with("/two").returns true
-
-            @module.match_manifests("yay/*.pp").should == %w{/one}
-        end
-
-        it "should default to the 'init.pp' file in the manifests directory" do
-            Dir.expects(:glob).with("/one/manifests/init.pp").returns(%w{/init.pp})
-
-            @module.match_manifests(nil).should == %w{/init.pp}
-        end
-
         after { Puppet.settings.clear }
     end
 end