Please review pull request #333: Ticket/2.7.x/11802 puppet module list opened by (mmrobins)

Description:

This series of commits adds the ability to list the modules that are installed with options to let you look at other environments and module paths.

This involves some cleanup and enhancements to Puppet::Module and Puppet::Node::Environment.

  • Opened: Fri Jan 13 17:33:37 UTC 2012
  • Based on: puppetlabs:2.7.x (2ac94f94b9af6d8025ee0e771c4c7141ca9705c6)
  • Requested merge: mmrobins:ticket/2.7.x/11802-puppet_module_list (cd56926d4c36448668fd22ccefa400bc8c3d541a)

Diff follows:

diff --git a/lib/puppet/face/module/list.rb b/lib/puppet/face/module/list.rb
new file mode 100644
index 0000000..7729900
--- /dev/null
+++ b/lib/puppet/face/module/list.rb
@@ -0,0 +1,64 @@
+Puppet::Face.define(:module, '1.0.0') do
+  action(:list) do
+    summary "List installed modules"
+    description <<-HEREDOC
+      List puppet modules from a specific environment, specified modulepath or
+      default to listing modules in the default modulepath:
+      #{Puppet.settings[:modulepath]}
+    HEREDOC
+    returns "hash of paths to module objects"
+
+    option "--env ENVIRONMENT" do
+      summary "Which environments' modules to list"
+    end
+
+    option "--modulepath MODULEPATH" do
+      summary "Which directories to look for modules in"
+    end
+
+    examples <<-EOT
+      List installed modules:
+
+      $ puppet module list
+        /etc/puppet/modules
+          bacula (0.0.2)
+        /usr/share/puppet/modules
+          apache (0.0.3)
+          bacula (0.0.1)
+
+      List installed modules from a specified environment:
+
+      $ puppet module list --env 'test'
+        /tmp/puppet/modules
+          rrd (0.0.2)
+
+      List installed modules from a specified modulepath:
+
+      $ puppet module list --modulepath /tmp/facts1:/tmp/facts2
+        /tmp/facts1
+          stdlib
+        /tmp/facts2
+          nginx (1.0.0)
+    EOT
+
+    when_invoked do |options|
+      Puppet[:modulepath] = options[:modulepath] if options[:modulepath]
+      environment = Puppet::Node::Environment.new(options[:env])
+
+      environment.modules_by_path
+    end
+
+    when_rendering :console do |modules_by_path|
+      output = ''
+      modules_by_path.each do |path, modules|
+        output << "#{path}\n"
+        modules.each do |mod|
+          version_string = mod.version ? "(#{mod.version})" : ''
+          output << "  #{mod.name} #{version_string}\n"
+        end
+      end
+      output
+    end
+
+  end
+end
diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb
index 00468df..7f86df8 100644
--- a/lib/puppet/module.rb
+++ b/lib/puppet/module.rb
@@ -19,13 +19,6 @@ class InvalidName < Error; end
 
   FILETYPES = [MANIFESTS, FILES, TEMPLATES, PLUGINS]
 
-  # Return an array of paths by splitting the +modulepath+ config
-  # parameter. Only consider paths that are absolute and existing
-  # directories
-  def self.modulepath(environment = nil)
-    Puppet::Node::Environment.new(environment).modulepath
-  end
-
   # Find and return the +module+ that +path+ belongs to. If +path+ is
   # absolute, or if there is no module whose name is the first component
   # of +path+, return +nil+
@@ -48,15 +41,16 @@ def has_metadata?
     return metadata.is_a?(Hash) && !metadata.keys.empty?
   end
 
-  def initialize(name, environment = nil)
+  def initialize(name, options = {})
     @name = name
+    @path = options[:path]
 
     assert_validity
 
-    if environment.is_a?(Puppet::Node::Environment)
-      @environment = environment
+    if options[:environment].is_a?(Puppet::Node::Environment)
+      @environment = options[:environment]
     else
-      @environment = Puppet::Node::Environment.new(environment)
+      @environment = Puppet::Node::Environment.new(options[:environment])
     end
 
     load_metadata if has_metadata?
@@ -141,7 +135,7 @@ def metadata_file
 
   # Find this module in the modulepath.
   def path
-    environment.modulepath.collect { |path| File.join(path, name) }.find { |d| FileTest.directory?(d) }
+    @path ||= environment.modulepath.collect { |path| File.join(path, name) }.find { |d| FileTest.directory?(d) }
   end
 
   # Find all plugin directories.  This is used by the Plugins fileserving mount.
@@ -149,11 +143,6 @@ def plugin_directory
     subpath("plugins")
   end
 
-  def requires(name, version = nil)
-    @requires ||= []
-    @requires << [name, version]
-  end
-
   def supports(name, version = nil)
     @supports ||= []
     @supports << [name, version]
@@ -204,4 +193,11 @@ def backward_compatible_plugins_dir
   def assert_validity
     raise InvalidName, "Invalid module name; module names must be alphanumeric (plus '-'), not '#{name}'" unless name =~ /^[-\w]+$/
   end
+
+  def ==(other)
+    self.name == other.name &&
+      self.version == other.version &&
+      self.path == other.path &&
+      self.environment == other.environment
+  end
 end
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 4fc314a..3268090 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -54,7 +54,6 @@ def self.root
     @root
   end
 
-  # This is only used for testing.
   def self.clear
     @seen.clear
   end
@@ -88,7 +87,7 @@ def known_resource_types
   end
 
   def module(name)
-    mod = Puppet::Module.new(name, self)
+    mod = Puppet::Module.new(name, :environment => self)
     return nil unless mod.exist?
     mod
   end
@@ -107,13 +106,27 @@ def module(name)
     module_names = modulepath.collect { |path| Dir.entries(path) }.flatten.uniq
     module_names.collect do |path|
       begin
-        Puppet::Module.new(path, self)
+        Puppet::Module.new(path, :environment => self)
       rescue Puppet::Module::Error => e
         nil
       end
     end.compact
   end
 
+  # Modules broken out by directory in the modulepath
+  def modules_by_path
+    modules_by_path = {}
+    modulepath.each do |path|
+      Dir.chdir(path) do
+        module_names = Dir.glob('*').select { |d| FileTest.directory? d }
+        modules_by_path[path] = module_names.map do |name|
+          Puppet::Module.new(name, :environment => self, :path => File.join(path, name))
+        end
+      end
+    end
+    modules_by_path
+  end
+
   def to_s
     name.to_s
   end
diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb
index 68def06..3cba895 100644
--- a/lib/puppet/parser/type_loader.rb
+++ b/lib/puppet/parser/type_loader.rb
@@ -112,7 +112,7 @@ def import_all
     # behavior) only load files from the first module of a given name.  E.g.,
     # given first/foo and second/foo, only files from first/foo will be loaded.
     module_names.each do |name|
-      mod = Puppet::Module.new(name, environment)
+      mod = Puppet::Module.new(name, :environment => environment)
       Find.find(File.join(mod.path, "manifests")) do |path|
         if path =~ /\.pp$/ or path =~ /\.rb$/
           import(path)
diff --git a/lib/puppet/util/rdoc/parser.rb b/lib/puppet/util/rdoc/parser.rb
index a8996ee..05e3aab 100644
--- a/lib/puppet/util/rdoc/parser.rb
+++ b/lib/puppet/util/rdoc/parser.rb
@@ -99,7 +99,7 @@ def split_module(path)
       modpath = $1
       name = $2
       Puppet.debug "rdoc: module #{name} into #{modpath} ?"
-      Puppet::Module.modulepath.each do |mp|
+      Puppet::Node::Environment.new.modulepath.each do |mp|
         if File.identical?(modpath,mp)
           Puppet.debug "rdoc: found module #{name}"
           return name
@@ -110,7 +110,7 @@ def split_module(path)
       # there can be paths we don't want to scan under modules
       # imagine a ruby or manifest that would be distributed as part as a module
       # but we don't want those to be hosted under <site>
-      Puppet::Module.modulepath.each do |mp|
+      Puppet::Node::Environment.new.modulepath.each do |mp|
         # check that fullpath is a descendant of mp
         dirname = fullpath
         previous = dirname
diff --git a/spec/unit/face/module/list_spec.rb b/spec/unit/face/module/list_spec.rb
new file mode 100644
index 0000000..7c85c68
--- /dev/null
+++ b/spec/unit/face/module/list_spec.rb
@@ -0,0 +1,100 @@
+require 'spec_helper'
+require 'puppet/face'
+require 'puppet/module_tool'
+
+describe "puppet module list" do
+  include PuppetSpec::Files
+
+  before do
+    dir = tmpdir("deep_path")
+
+    @modpath1 = File.join(dir, "modpath1")
+    @modpath2 = File.join(dir, "modpath2")
+
+    FileUtils.mkdir_p(@modpath1)
+    FileUtils.mkdir_p(@modpath2)
+  end
+
+  def mkmodule(name, path)
+    mod_path = File.join(path, name)
+    FileUtils.mkdir_p(mod_path)
+    mod_path
+  end
+
+  it "should return an empty list per dir in path if there are no modules" do
+    Puppet.settings[:modulepath] = "#{@modpath1}:#{@modpath2}"
+    Puppet::Face[:module, :current].list.should == {
+      @modpath1 => [],
+      @modpath2 => []
+    }
+  end
+
+  it "should include modules separated by the environment's modulepath" do
+    foomod1 = mkmodule('foo', @modpath1)
+    barmod1 = mkmodule('bar', @modpath1)
+    foomod2 = mkmodule('foo', @modpath2)
+
+    env = Puppet::Node::Environment.new
+    env.modulepath = [@modpath1, @modpath2]
+
+    Puppet::Face[:module, :current].list.should == {
+      @modpath1 => [
+        Puppet::Module.new('bar', :environment => env, :path => barmod1),
+        Puppet::Module.new('foo', :environment => env, :path => foomod1)
+      ],
+      @modpath2 => [Puppet::Module.new('foo', :environment => env, :path => foomod2)]
+    }
+  end
+
+  it "should use the specified environment" do
+    foomod1 = mkmodule('foo', @modpath1)
+    barmod1 = mkmodule('bar', @modpath1)
+
+    usedenv = Puppet::Node::Environment.new('useme')
+    usedenv.modulepath = [@modpath1, @modpath2]
+
+    Puppet::Face[:module, :current].list(:env => 'useme').should == {
+      @modpath1 => [
+        Puppet::Module.new('bar', :environment => usedenv),
+        Puppet::Module.new('foo', :environment => usedenv)
+      ],
+      @modpath2 => []
+    }
+  end
+
+  it "should use the specified modulepath" do
+    foomod1 = mkmodule('foo', @modpath1)
+    barmod2 = mkmodule('bar', @modpath2)
+
+    Puppet::Face[:module, :current].list(:modulepath => "#{@modpath1}:#{@modpath2}").should == {
+      @modpath1 => [ Puppet::Module.new('foo') ],
+      @modpath2 => [ Puppet::Module.new('bar') ]
+    }
+  end
+
+  it "should use the specified modulepath over the specified environment in place of the environment's default path" do
+    foomod1 = mkmodule('foo', @modpath1)
+    barmod2 = mkmodule('bar', @modpath2)
+    env = Puppet::Node::Environment.new('myenv')
+    env.modulepath = ['/tmp/notused']
+
+    list = Puppet::Face[:module, :current].list(:env => 'myenv', :modulepath => "#{@modpath1}:#{@modpath2}")
+
+    # Changing Puppet[:modulepath] causes Puppet::Node::Environment.new('myenv')
+    # to have a different object_id than the env above
+    env = Puppet::Node::Environment.new('myenv')
+    list.should == {
+      @modpath1 => [ Puppet::Module.new('foo', :environment => env, :path => foomod1) ],
+      @modpath2 => [ Puppet::Module.new('bar', :environment => env, :path => barmod2) ]
+    }
+  end
+
+  describe "inline documentation" do
+    subject { Puppet::Face[:module, :current].get_action :list }
+
+    its(:summary)     { should =~ /list.*module/im }
+    its(:description) { should =~ /list.*module/im }
+    its(:returns)     { should =~ /hash of paths to module objects/i }
+    its(:examples)    { should_not be_empty }
+  end
+end
diff --git a/spec/unit/module_spec.rb b/spec/unit/module_spec.rb
index a0f64c6..e117249 100755
--- a/spec/unit/module_spec.rb
+++ b/spec/unit/module_spec.rb
@@ -89,83 +89,6 @@
     lambda { mod.validate_puppet_version }.should raise_error(Puppet::Module::IncompatibleModule)
   end
 
-  describe "when specifying required modules" do
-    it "should support specifying a required module" do
-      mod = Puppet::Module.new("mymod")
-      mod.requires "foobar"
-    end
-
-    it "should support specifying multiple required modules" do
-      mod = Puppet::Module.new("mymod")
-      mod.requires "foobar"
-      mod.requires "baz"
-    end
-
-    it "should support specifying a required module and version" do
-      mod = Puppet::Module.new("mymod")
-      mod.requires "foobar", 1.0
-    end
-
-    it "should fail when required modules are missing" do
-      mod = Puppet::Module.new("mymod")
-      mod.requires "foobar"
-
-      mod.environment.expects(:module).with("foobar").returns nil
-
-      lambda { mod.validate_dependencies }.should raise_error(Puppet::Module::MissingModule)
-    end
-
-    it "should fail when required modules are present but of the wrong version" do
-      mod = Puppet::Module.new("mymod")
-      mod.requires "foobar", 1.0
-
-      foobar = Puppet::Module.new("foobar")
-      foobar.version = 2.0
-
-      mod.environment.expects(:module).with("foobar").returns foobar
-
-      lambda { mod.validate_dependencies }.should raise_error(Puppet::Module::IncompatibleModule)
-    end
-
-    it "should have valid dependencies when no dependencies have been specified" do
-      mod = Puppet::Module.new("mymod")
-
-      lambda { mod.validate_dependencies }.should_not raise_error
-    end
-
-    it "should fail when some dependencies are present but others aren't" do
-      mod = Puppet::Module.new("mymod")
-      mod.requires "foobar"
-      mod.requires "baz"
-
-      mod.environment.expects(:module).with("foobar").returns Puppet::Module.new("foobar")
-      mod.environment.expects(:module).with("baz").returns nil
-
-      lambda { mod.validate_dependencies }.should raise_error(Puppet::Module::MissingModule)
-    end
-
-    it "should have valid dependencies when all dependencies are met" do
-      mod = Puppet::Module.new("mymod")
-      mod.requires "foobar", 1.0
-      mod.requires "baz"
-
-      foobar = Puppet::Module.new("foobar")
-      foobar.version = 1.0
-
-      baz = Puppet::Module.new("baz")
-
-      mod.environment.expects(:module).with("foobar").returns foobar
-      mod.environment.expects(:module).with("baz").returns baz
-
-      lambda { mod.validate_dependencies }.should_not raise_error
-    end
-
-    it "should validate its dependendencies on initialization" do
-      Puppet::Module.any_instance.expects(:validate_dependencies)
-      Puppet::Module.new("mymod")
-    end
-  end
-
   describe "when managing supported platforms" do
     it "should support specifying a supported platform" do
       mod = Puppet::Module.new("mymod")
@@ -251,11 +174,11 @@
   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)
+    Puppet::Module.new("foo", :environment => "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
+    Puppet::Module.new("foo", :environment => :prod).environment.name.should == :prod
   end
 
   it "should use the default environment if none is provided" do
@@ -265,43 +188,53 @@
 
   it "should use any provided Environment instance" do
     env = Puppet::Node::Environment.new
-    Puppet::Module.new("foo", env).environment.should equal(env)
+    Puppet::Module.new("foo", :environment => env).environment.should equal(env)
   end
 
-  it "should return the path to the first found instance in its environment's module paths as its path" do
-    dir = tmpdir("deep_path")
-    first = File.join(dir, "first")
-    second = File.join(dir, "second")
+  describe ".path" do
+    before do
+      dir = tmpdir("deep_path")
 
-    FileUtils.mkdir_p(first)
-    FileUtils.mkdir_p(second)
-    Puppet[:modulepath] = "#{first}#{File::PATH_SEPARATOR}#{second}"
+      @first = File.join(dir, "first")
+      @second = File.join(dir, "second")
+      Puppet[:modulepath] = "#{@first}#{File::PATH_SEPARATOR}#{@second}"
 
-    modpath = File.join(first, "foo")
-    FileUtils.mkdir_p(modpath)
+      FileUtils.mkdir_p(@first)
+      FileUtils.mkdir_p(@second)
+    end
 
-    # Make a second one, which we shouldn't find
-    FileUtils.mkdir_p(File.join(second, "foo"))
+    it "should return the path to the first found instance in its environment's module paths as its path" do
+      modpath = File.join(@first, "foo")
+      FileUtils.mkdir_p(modpath)
 
-    mod = Puppet::Module.new("foo")
-    mod.path.should == modpath
-  end
+      # Make a second one, which we shouldn't find
+      FileUtils.mkdir_p(File.join(@second, "foo"))
+
+      mod = Puppet::Module.new("foo")
+      mod.path.should == modpath
+    end
 
-  it "should be able to find itself in a directory other than the first directory in the module path" do
-    dir = tmpdir("deep_path")
-    first = File.join(dir, "first")
-    second = File.join(dir, "second")
+    it "should be able to find itself in a directory other than the first directory in the module path" do
+      modpath = File.join(@second, "foo")
+      FileUtils.mkdir_p(modpath)
 
-    FileUtils.mkdir_p(first)
-    FileUtils.mkdir_p(second)
-    Puppet[:modulepath] = "#{first}#{File::PATH_SEPARATOR}#{second}"
+      mod = Puppet::Module.new("foo")
+      mod.should be_exist
+      mod.path.should == modpath
+    end
 
-    modpath = File.join(second, "foo")
-    FileUtils.mkdir_p(modpath)
+    it "should be able to find itself in a directory other than the first directory in the module path even when it exists in the first" do
+      environment = Puppet::Node::Environment.new
 
-    mod = Puppet::Module.new("foo")
-    mod.should be_exist
-    mod.path.should == modpath
+      first_modpath = File.join(@first, "foo")
+      FileUtils.mkdir_p(first_modpath)
+      second_modpath = File.join(@second, "foo")
+      FileUtils.mkdir_p(second_modpath)
+
+      mod = Puppet::Module.new("foo", :environment => environment, :path => second_modpath)
+      mod.path.should == File.join(@second, "foo")
+      mod.environment.should == environment
+    end
   end
 
   it "should be considered existent if it exists in at least one module path" do
@@ -403,24 +336,6 @@
   end
 end
 
-describe Puppet::Module, " when building its search path" do
-  it "should use the current environment's search path if no environment is specified" do
-    env = mock 'env'
-    env.expects(:modulepath).returns "eh"
-    Puppet::Node::Environment.expects(:new).with(nil).returns env
-
-    Puppet::Module.modulepath.should == "eh"
-  end
-
-  it "should use the specified environment's search path if an environment is specified" do
-    env = mock 'env'
-    env.expects(:modulepath).returns "eh"
-    Puppet::Node::Environment.expects(:new).with("foo").returns env
-
-    Puppet::Module.modulepath("foo").should == "eh"
-  end
-end
-
 describe Puppet::Module, "when finding matching manifests" do
   before do
     @mod = Puppet::Module.new("mymod")
@@ -592,7 +507,6 @@
       @module.puppetversion.should == @data[:puppetversion]
     end
 
-
     it "should fail if the discovered name is different than the metadata name"
   end
 end
diff --git a/spec/unit/node/environment_spec.rb b/spec/unit/node/environment_spec.rb
index 78d3834..d5d3068 100755
--- a/spec/unit/node/environment_spec.rb
+++ b/spec/unit/node/environment_spec.rb
@@ -7,6 +7,8 @@
 require 'puppet/util/execution'
 
 describe Puppet::Node::Environment do
+  let(:env) { Puppet::Node::Environment.new("testing") }
+
   include PuppetSpec::Files
   after do
     Puppet::Node::Environment.clear
@@ -44,59 +46,57 @@
 
   describe "when managing known resource types" do
     before do
-      @env = Puppet::Node::Environment.new("dev")
-      @collection = Puppet::Resource::TypeCollection.new(@env)
-      @env.stubs(:perform_initial_import).returns(Puppet::Parser::AST::Hostclass.new(''))
+      @collection = Puppet::Resource::TypeCollection.new(env)
+      env.stubs(:perform_initial_import).returns(Puppet::Parser::AST::Hostclass.new(''))
       Thread.current[:known_resource_types] = nil
     end
 
     it "should create a resource type collection if none exists" do
-      Puppet::Resource::TypeCollection.expects(:new).with(@env).returns @collection
-      @env.known_resource_types.should equal(@collection)
+      Puppet::Resource::TypeCollection.expects(:new).with(env).returns @collection
+      env.known_resource_types.should equal(@collection)
     end
 
     it "should reuse any existing resource type collection" do
-      @env.known_resource_types.should equal(@env.known_resource_types)
+      env.known_resource_types.should equal(env.known_resource_types)
     end
 
     it "should perform the initial import when creating a new collection" do
-      @env = Puppet::Node::Environment.new("dev")
-      @env.expects(:perform_initial_import).returns(Puppet::Parser::AST::Hostclass.new(''))
-      @env.known_resource_types
+      env.expects(:perform_initial_import).returns(Puppet::Parser::AST::Hostclass.new(''))
+      env.known_resource_types
     end
 
     it "should return the same collection even if stale if it's the same thread" do
       Puppet::Resource::TypeCollection.stubs(:new).returns @collection
-      @env.known_resource_types.stubs(:stale?).returns true
+      env.known_resource_types.stubs(:stale?).returns true
 
-      @env.known_resource_types.should equal(@collection)
+      env.known_resource_types.should equal(@collection)
     end
 
     it "should return the current thread associated collection if there is one" do
       Thread.current[:known_resource_types] = @collection
 
-      @env.known_resource_types.should equal(@collection)
+      env.known_resource_types.should equal(@collection)
     end
 
     it "should give to all threads using the same environment the same collection if the collection isn't stale" do
-      original_thread_type_collection = Puppet::Resource::TypeCollection.new(@env)
-      Puppet::Resource::TypeCollection.expects(:new).with(@env).returns original_thread_type_collection
-      @env.known_resource_types.should equal(original_thread_type_collection)
+      original_thread_type_collection = Puppet::Resource::TypeCollection.new(env)
+      Puppet::Resource::TypeCollection.expects(:new).with(env).returns original_thread_type_collection
+      env.known_resource_types.should equal(original_thread_type_collection)
 
       original_thread_type_collection.expects(:require_reparse?).returns(false)
-      Puppet::Resource::TypeCollection.stubs(:new).with(@env).returns @collection
+      Puppet::Resource::TypeCollection.stubs(:new).with(env).returns @collection
 
       t = Thread.new {
-        @env.known_resource_types.should equal(original_thread_type_collection)
+        env.known_resource_types.should equal(original_thread_type_collection)
       }
       t.join
     end
 
     it "should generate a new TypeCollection if the current one requires reparsing" do
-      old_type_collection = @env.known_resource_types
+      old_type_collection = env.known_resource_types
       old_type_collection.stubs(:require_reparse?).returns true
       Thread.current[:known_resource_types] = nil
-      new_type_collection = @env.known_resource_types
+      new_type_collection = env.known_resource_types
 
       new_type_collection.should be_a Puppet::Resource::TypeCollection
       new_type_collection.should_not equal(old_type_collection)
@@ -109,14 +109,11 @@
 
     Puppet[:modulepath] = path
 
-    env = Puppet::Node::Environment.new("testing")
-
     env.modulepath.should == [real_file]
   end
 
   it "should prefix the value of the 'PUPPETLIB' environment variable to the module path if present" do
     Puppet::Util::Execution.withenv("PUPPETLIB" => %w{/l1 /l2}.join(File::PATH_SEPARATOR)) do
-      env = Puppet::Node::Environment.new("testing")
       module_path = %w{/one /two}.join(File::PATH_SEPARATOR)
       env.expects(:validate_dirs).with(%w{/l1 /l2 /one /two}).returns %w{/l1 /l2 /one /two}
       env.expects(:[]).with(:modulepath).returns module_path
@@ -132,8 +129,6 @@
     end
 
     it "should not return non-directories" do
-      env = Puppet::Node::Environment.new("testing")
-
       FileTest.expects(:directory?).with(@path_one).returns true
       FileTest.expects(:directory?).with(@path_two).returns false
 
@@ -142,7 +137,6 @@
 
     it "should use the current working directory to fully-qualify unqualified paths" do
       FileTest.stubs(:directory?).returns true
-      env = Puppet::Node::Environment.new("testing")
 
       two = File.expand_path(File.join(Dir.getwd, "two"))
       env.validate_dirs([@path_one, 'two']).should == [@path_one, two]
@@ -155,44 +149,75 @@
     end
 
     it "should provide an array-like accessor method for returning any environment-specific setting" do
-      env = Puppet::Node::Environment.new("testing")
       env.should respond_to(:[])
     end
 
     it "should ask the Puppet settings instance for the setting qualified with the environment name" do
       Puppet.settings.expects(:value).with("myvar", :testing).returns("myval")
-      env = Puppet::Node::Environment.new("testing")
       env["myvar"].should == "myval"
     end
 
     it "should be able to return an individual module that exists in its module path" do
-      env = Puppet::Node::Environment.new("testing")
 
       mod = mock 'module'
-      Puppet::Module.expects(:new).with("one", env).returns mod
+      Puppet::Module.expects(:new).with("one", :environment => env).returns mod
       mod.expects(:exist?).returns true
 
       env.module("one").should equal(mod)
     end
 
     it "should return nil if asked for a module that does not exist in its path" do
-      env = Puppet::Node::Environment.new("testing")
 
       mod = mock 'module'
-      Puppet::Module.expects(:new).with("one", env).returns mod
+      Puppet::Module.expects(:new).with("one", :environment => env).returns mod
       mod.expects(:exist?).returns false
 
       env.module("one").should be_nil
     end
 
-    it "should be able to return its modules" do
-      Puppet::Node::Environment.new("testing").should respond_to(:modules)
+    describe ".modules_by_path" do
+      before do
+        dir = tmpdir("deep_path")
+
+        @first = File.join(dir, "first")
+        @second = File.join(dir, "second")
+        Puppet[:modulepath] = "#{@first}#{File::PATH_SEPARATOR}#{@second}"
+
+        FileUtils.mkdir_p(@first)
+        FileUtils.mkdir_p(@second)
+      end
+
+      it "should return an empty list if there are no modules" do
+        env.modules_by_path.should == {
+          @first  => [],
+          @second => []
+        }
+      end
+
+      it "should include modules even if they exist in multiple dirs in the modulepath" do
+        modpath1 = File.join(@first, "foo")
+        FileUtils.mkdir_p(modpath1)
+        modpath2 = File.join(@second, "foo")
+        FileUtils.mkdir_p(modpath2)
+
+        env.modules_by_path.should == {
+          @first  => [Puppet::Module.new('foo', :environment => env, :path => modpath1)],
+          @second => [Puppet::Module.new('foo', :environment => env, :path => modpath2)]
+        }
+      end
     end
 
     describe ".modules" do
+      it "should return an empty list if there are no modules" do
+        env.modulepath = %w{/a /b}
+        Dir.expects(:entries).with("/a").returns []
+        Dir.expects(:entries).with("/b").returns []
+
+        env.modules.should == []
+      end
+
       it "should return a module named for every directory in each module path" do
-        env = Puppet::Node::Environment.new("testing")
-        env.expects(:modulepath).at_least_once.returns %w{/a /b}
+        env.modulepath = %w{/a /b}
         Dir.expects(:entries).with("/a").returns %w{foo bar}
         Dir.expects(:entries).with("/b").returns %w{bee baz}
 
@@ -200,8 +225,7 @@
       end
 
       it "should remove duplicates" do
-        env = Puppet::Node::Environment.new("testing")
-        env.expects(:modulepath).returns( %w{/a /b} ).at_least_once
+        env.modulepath = %w{/a /b}
         Dir.expects(:entries).with("/a").returns %w{foo}
         Dir.expects(:entries).with("/b").returns %w{foo}
 
@@ -209,8 +233,7 @@
       end
 
       it "should ignore invalid modules" do
-        env = Puppet::Node::Environment.new("testing")
-        env.stubs(:modulepath).returns %w{/a}
+        env.modulepath = %w{/a}
         Dir.expects(:entries).with("/a").returns %w{foo bar}
 
         Puppet::Module.expects(:new).with { |name, env| name == "foo" }.returns mock("foomod", :name => "foo")
@@ -220,16 +243,14 @@
       end
 
       it "should create modules with the correct environment" do
-        env = Puppet::Node::Environment.new("testing")
-        env.expects(:modulepath).at_least_once.returns %w{/a}
+        env.modulepath = %w{/a}
         Dir.expects(:entries).with("/a").returns %w{foo}
 
         env.modules.each {|mod| mod.environment.should == env }
       end
 
       it "should cache the module list" do
-        env = Puppet::Node::Environment.new("testing")
-        env.expects(:modulepath).at_least_once.returns %w{/a}
+        env.modulepath = %w{/a}
         Dir.expects(:entries).once.with("/a").returns %w{foo}
 
         env.modules
@@ -244,20 +265,18 @@
       @helper.extend(Puppet::Node::Environment::Helper)
     end
 
-    it "should be able to set and retrieve the environment" do
+    it "should be able to set and retrieve the environment as a symbol" do
       @helper.environment = :foo
       @helper.environment.name.should == :foo
     end
 
     it "should accept an environment directly" do
-      env = Puppet::Node::Environment.new :foo
-      @helper.environment = env
+      @helper.environment = Puppet::Node::Environment.new(:foo)
       @helper.environment.name.should == :foo
     end
 
     it "should accept an environment as a string" do
-      env = Puppet::Node::Environment.new "foo"
-      @helper.environment = env
+      @helper.environment = 'foo'
       @helper.environment.name.should == :foo
     end
   end
@@ -266,14 +285,13 @@
     before do
       @parser = Puppet::Parser::Parser.new("test")
       Puppet::Parser::Parser.stubs(:new).returns @parser
-      @env = Puppet::Node::Environment.new("env")
     end
 
     it "should set the parser's string to the 'code' setting and parse if code is available" do
       Puppet.settings[:code] = "my code"
       @parser.expects(:string=).with "my code"
       @parser.expects(:parse)
-      @env.instance_eval { perform_initial_import }
+      env.instance_eval { perform_initial_import }
     end
 
     it "should set the parser's file to the 'manifest' setting and parse if no code is available and the manifest is available" do
@@ -282,7 +300,7 @@
       Puppet.settings[:manifest] = filename
       @parser.expects(:file=).with filename
       @parser.expects(:parse)
-      @env.instance_eval { perform_initial_import }
+      env.instance_eval { perform_initial_import }
     end
 
     it "should pass the manifest file to the parser even if it does not exist on disk" do
@@ -291,15 +309,15 @@
       Puppet.settings[:manifest] = filename
       @parser.expects(:file=).with(filename).once
       @parser.expects(:parse).once
-      @env.instance_eval { perform_initial_import }
+      env.instance_eval { perform_initial_import }
     end
 
     it "should fail helpfully if there is an error importing" do
       File.stubs(:exist?).returns true
-      @env.stubs(:known_resource_types).returns Puppet::Resource::TypeCollection.new(@env)
+      env.stubs(:known_resource_types).returns Puppet::Resource::TypeCollection.new(env)
       @parser.expects(:file=).once
       @parser.expects(:parse).raises ArgumentError
-      lambda { @env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error)
+      lambda { env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error)
     end
 
     it "should not do anything if the ignore_import settings is set" do
@@ -307,15 +325,15 @@
       @parser.expects(:string=).never
       @parser.expects(:file=).never
       @parser.expects(:parse).never
-      @env.instance_eval { perform_initial_import }
+      env.instance_eval { perform_initial_import }
     end
 
     it "should mark the type collection as needing a reparse when there is an error parsing" do
       @parser.expects(:parse).raises Puppet::ParseError.new("Syntax error at ...")
-      @env.stubs(:known_resource_types).returns Puppet::Resource::TypeCollection.new(@env)
+      env.stubs(:known_resource_types).returns Puppet::Resource::TypeCollection.new(env)
 
-      lambda { @env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error, /Syntax error at .../)
-      @env.known_resource_types.require_reparse?.should be_true
+      lambda { env.instance_eval { perform_initial_import } }.should raise_error(Puppet::Error, /Syntax error at .../)
+      env.known_resource_types.require_reparse?.should be_true
     end
   end
 end
diff --git a/spec/unit/util/rdoc/parser_spec.rb b/spec/unit/util/rdoc/parser_spec.rb
index 9c8cc75..892b932 100755
--- a/spec/unit/util/rdoc/parser_spec.rb
+++ b/spec/unit/util/rdoc/parser_spec.rb
@@ -135,7 +135,7 @@
 
   describe "when finding modules from filepath" do
     before :each do
-      Puppet::Module.stubs(:modulepath).returns("/path/to/modules")
+      Puppet::Node::Environment.any_instance.stubs(:modulepath).returns("/path/to/modules")
     end
 
     it "should return the module name for modulized puppet manifests" do

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to