Please review pull request #516: (#12126) autoloader reloading opened by (pcarlisle)

Description:

This adds reloading capability to the autoloader. It tracks the mtime of autoloaded files and can reload all changed files on demand. It also converts pluginsync to using this capability.

  • Opened: Mon Feb 20 20:15:40 UTC 2012
  • Based on: puppetlabs:master (821a66975abdf11f5e89934dd99882a1b508dfc5)
  • Requested merge: pcarlisle:ticket/master/12126-autoloader-reload (20866d07a221402f338efd90b9de250912173f04)

Diff follows:

diff --git a/lib/puppet/configurer/plugin_handler.rb b/lib/puppet/configurer/plugin_handler.rb
index d2a12e2..57ab677 100644
--- a/lib/puppet/configurer/plugin_handler.rb
+++ b/lib/puppet/configurer/plugin_handler.rb
@@ -16,22 +16,7 @@ def download_plugins
       Puppet[:pluginsignore]
     )
 
-    plugin_downloader.evaluate.each { |file| load_plugin(file) }
-  end
-
-  def load_plugin(file)
-    return unless FileTest.exist?(file)
-    return if FileTest.directory?(file)
-
-    begin
-      if file =~ /.rb$/
-        Puppet.info "Loading downloaded plugin #{file}"
-        load file
-      else
-        Puppet.debug "Skipping downloaded plugin #{file}"
-      end
-    rescue Exception => detail
-      Puppet.err "Could not load downloaded file #{file}: #{detail}"
-    end
+    plugin_downloader.evaluate
+    Puppet::Util::Autoload.reload_changed
   end
 end
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 9a87429..306bf8d 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -107,7 +107,7 @@ def initialize(name, version, &block)
 
   # Try to find actions defined in other files.
   def load_actions
-    Puppet::Interface.autoloader.search_directories.each do |dir|
+    Puppet::Interface.autoloader.class.search_directories.each do |dir|
       Dir.glob(File.join(dir, "puppet/face/#{name}", "*.rb")).each do |file|
         action = "" '').sub(/^[\\\/]/, '').sub(/\.rb/, '')
         Puppet.debug "Loading action '#{action}' for '#{name}' from '#{dir}/#{action}.rb'"
diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb
index 4142658..f4218b9 100644
--- a/lib/puppet/util/autoload.rb
+++ b/lib/puppet/util/autoload.rb
@@ -1,49 +1,136 @@
+require 'pathname'
 require 'puppet/util/warnings'
 
 # Autoload paths, either based on names or all at once.
 class Puppet::Util::Autoload
-  require 'puppet/util/autoload/file_cache'
-
-  include Puppet::Util
-  include Puppet::Util::Warnings
-  include Puppet::Util::Autoload::FileCache
-
   @autoloaders = {}
-  @loaded = []
+  @loaded = {}
 
   class << self
     attr_reader :autoloaders
-    private :autoloaders
-  end
+    attr_accessor :loaded
+    private :autoloaders, :loaded
 
-  # Send [], []=, and :clear to the @autloaders hash
-  Puppet::Util.classproxy self, :autoloaders, "[]", "[]="
+    # List all loaded files.
+    def list_loaded
+      loaded.keys.sort { |a,b| a[0] <=> b[0] }.collect do |path, hash|
+        "#{path}: #{hash[:file]}"
+      end
+    end
 
-  # List all loaded files.
-  def self.list_loaded
-    @loaded.sort { |a,b| a[0] <=> b[0] }.collect do |path, hash|
-      "#{path}: #{hash[:file]}"
+    # Has a given path been loaded?  This is used for testing whether a
+    # changed file should be loaded or just ignored.  This is only
+    # used in network/client/master, when downloading plugins, to
+    # see if a given plugin is currently loaded and thus should be
+    # reloaded.
+    def loaded?(path)
+      path = path.to_s.sub(/\.rb$/, '')
+      loaded.include?(path)
     end
-  end
 
-  # Has a given path been loaded?  This is used for testing whether a
-  # changed file should be loaded or just ignored.  This is only
-  # used in network/client/master, when downloading plugins, to
-  # see if a given plugin is currently loaded and thus should be
-  # reloaded.
-  def self.loaded?(path)
-    path = path.to_s.sub(/\.rb$/, '')
-    @loaded.include?(path)
-  end
+    # Save the fact that a given path has been loaded.  This is so
+    # we can load downloaded plugins if they've already been loaded
+    # into memory.
+    def mark_loaded(name, file)
+      $" << name + ".rb" unless $".include?(name)
+      loaded[name] = [file, File.mtime(file)]
+    end
+
+    def changed?(name)
+      return true unless loaded.include?(name)
+      file, old_mtime = loaded[name]
+      return true unless file == get_file(name)
+      begin
+        old_mtime != File.mtime(file)
+      rescue Errno::ENOENT
+        true
+      end
+    end
+
+    # Load a single plugin by name.  We use 'load' here so we can reload a
+    # given plugin.
+    def load_file(name, env=nil)
+      file = get_file(name.to_s)
+      return false unless file
+      begin
+        Kernel.load file, @wrap
+        mark_loaded(name, file)
+        return true
+      rescue SystemExit,NoMemoryError
+        raise
+      rescue Exception => detail
+        message = "Could not autoload #{name}: #{detail}"
+        Puppet.log_exception(detail, message)
+        raise Puppet::Error, message
+      end
+    end
+
+    def loadall(path)
+      # Load every instance of everything we can find.
+      files_to_load(path).each do |file|
+        name = file.chomp(".rb")
+        load_file(name) unless loaded?(name)
+      end
+    end
+
+    def reload_changed
+      changed = loaded.keys.find_all { |f| changed?(f) }
+      changed.each do |name|
+        load_file(name)
+      end
+    end
+
+    # Get the correct file to load for a given path
+    # returns nil if no file is found
+    def get_file(name, env=nil)
+      name = name + '.rb' unless name.end_with?('.rb')
+      dirname, base = File.split(name)
+      path = search_directories(env).find { |dir| File.exist?(File.join(dir, name)) }
+      path and File.join(path, name)
+    end
+
+    def files_to_load(path)
+      search_directories.map {|dir| files_in_dir(dir, path) }.flatten.uniq
+    end
+
+    def files_in_dir(dir, path)
+      dir = Pathname.new(dir)
+      Dir.glob(File.join(dir, path, "*.rb")).collect do |file|
+        Pathname.new(file).relative_path_from(dir).to_s
+      end
+    end
+
+    def module_directories(env=nil)
+      # We have to require this late in the process because otherwise we might have
+      # load order issues.
+      require 'puppet/node/environment'
+
+      real_env = Puppet::Node::Environment.new(env)
+
+      # We're using a per-thread cache of said module directories, so that
+      # we don't scan the filesystem each time we try to load something with
+      # this autoload instance. But since we don't want to cache for the eternity
+      # this env_module_directories gets reset after the compilation on the master.
+      # This is also reset after an agent ran.
+      # One of the side effect of this change is that this module directories list will be
+      # shared among all autoload that we have running at a time. But that won't be an issue
+      # as by definition those directories are shared by all autoload.
+      Thread.current[:env_module_directories] ||= {}
+      Thread.current[:env_module_directories][real_env] ||= real_env.modulepath.collect do |dir|
+          Dir.entries(dir).reject { |f| f =~ /^\./ }.collect { |f| File.join(dir, f) }
+        end.flatten.collect { |d| [File.join(d, "plugins"), File.join(d, "lib")] }.flatten.find_all do |d|
+          FileTest.directory?(d)
+        end
+    end
 
-  # Save the fact that a given path has been loaded.  This is so
-  # we can load downloaded plugins if they've already been loaded
-  # into memory.
-  def self.loaded(file)
-    $" << file + ".rb" unless $".include?(file)
-    @loaded << file unless @loaded.include?(file)
+    def search_directories(env=nil)
+      [module_directories(env), Puppet[:libdir].split(File::PATH_SEPARATOR), $LOAD_PATH].flatten
+    end
   end
 
+  # Send [], []=, and :clear to the @autloaders hash
+  Puppet::Util.classproxy self, :autoloaders, "[]", "[]="
+
   attr_accessor :object, :path, :objwarn, :wrap
 
   def initialize(obj, path, options = {})
@@ -54,7 +141,6 @@ def initialize(obj, path, options = {})
     self.class[obj] = self
 
     options.each do |opt, value|
-      opt = opt.intern if opt.is_a? String
       begin
         self.send(opt.to_s + "=", value)
       rescue NoMethodError
@@ -65,94 +151,17 @@ def initialize(obj, path, options = {})
     @wrap = true unless defined?(@wrap)
   end
 
-  # Load a single plugin by name.  We use 'load' here so we can reload a
-  # given plugin.
-  def load(name,env=nil)
-    path = name.to_s + ".rb"
-
-    searchpath(env).each do |dir|
-      file = File.join(dir, path)
-      next unless file_exist?(file)
-      begin
-        Kernel.load file, @wrap
-        name = symbolize(name)
-        loaded name, file
-        return true
-      rescue SystemExit,NoMemoryError
-        raise
-      rescue Exception => detail
-        message = "Could not autoload #{name}: #{detail}"
-        Puppet.log_exception(detail, message)
-        raise Puppet::Error, message
-      end
-    end
-    false
-  end
-
-  # Mark the named object as loaded.  Note that this supports unqualified
-  # queries, while we store the result as a qualified query in the class.
-  def loaded(name, file)
-    self.class.loaded(File.join(@path, name.to_s))
-  end
-
-  # Indicate whether the specfied plugin has been loaded.
-  def loaded?(name)
-    self.class.loaded?(File.join(@path, name.to_s))
+  def load(name, env=nil)
+    self.class.load_file(File.join(@path, name.to_s), env)
   end
 
   # Load all instances that we can.  This uses require, rather than load,
   # so that already-loaded files don't get reloaded unnecessarily.
   def loadall
-    # Load every instance of everything we can find.
-    files_to_load.each do |file|
-      name = File.basename(file).chomp(".rb").intern
-      next if loaded?(name)
-      begin
-        Kernel.require file
-        loaded(name, file)
-      rescue SystemExit,NoMemoryError
-        raise
-      rescue Exception => detail
-        message = "Could not autoload #{file}: #{detail}"
-        Puppet.log_exception(detail, message)
-        raise Puppet::Error, message
-      end
-    end
+    self.class.loadall(@path)
   end
 
   def files_to_load
-    searchpath.map { |dir| Dir.glob("#{dir}/*.rb") }.flatten
-  end
-
-  # The list of directories to search through for loadable plugins.
-  def searchpath(env=nil)
-    search_directories(env).uniq.collect { |d| File.join(d, @path) }.find_all { |d| FileTest.directory?(d) }
-  end
-
-  def module_directories(env=nil)
-    # We have to require this late in the process because otherwise we might have
-    # load order issues.
-    require 'puppet/node/environment'
-
-    real_env = Puppet::Node::Environment.new(env)
-
-    # We're using a per-thread cache of said module directories, so that
-    # we don't scan the filesystem each time we try to load something with
-    # this autoload instance. But since we don't want to cache for the eternity
-    # this env_module_directories gets reset after the compilation on the master.
-    # This is also reset after an agent ran.
-    # One of the side effect of this change is that this module directories list will be
-    # shared among all autoload that we have running at a time. But that won't be an issue
-    # as by definition those directories are shared by all autoload.
-    Thread.current[:env_module_directories] ||= {}
-    Thread.current[:env_module_directories][real_env] ||= real_env.modulepath.collect do |dir|
-        Dir.entries(dir).reject { |f| f =~ /^\./ }.collect { |f| File.join(dir, f) }
-      end.flatten.collect { |d| [File.join(d, "plugins"), File.join(d, "lib")] }.flatten.find_all do |d|
-        FileTest.directory?(d)
-      end
-  end
-
-  def search_directories(env=nil)
-    [module_directories(env), Puppet[:libdir].split(File::PATH_SEPARATOR), $LOAD_PATH].flatten
+    self.class.files_to_load(@path)
   end
 end
diff --git a/lib/puppet/util/autoload/file_cache.rb b/lib/puppet/util/autoload/file_cache.rb
deleted file mode 100644
index b555473..0000000
--- a/lib/puppet/util/autoload/file_cache.rb
+++ /dev/null
@@ -1,92 +0,0 @@
-module Puppet::Util::Autoload::FileCache
-  @found_files = {}
-  @missing_files = {}
-  class << self
-    attr_reader :found_files, :missing_files
-  end
-
-  # Only used for testing.
-  def self.clear
-    @found_files.clear
-    @missing_files.clear
-  end
-
-  def found_files
-    Puppet::Util::Autoload::FileCache.found_files
-  end
-
-  def missing_files
-    Puppet::Util::Autoload::FileCache.missing_files
-  end
-
-  def directory_exist?(path)
-    cache = cached_data?(path, :directory?)
-    return cache unless cache.nil?
-
-    protect(path) do
-      stat = File.lstat(path)
-      if stat.directory?
-        found_file(path, stat)
-        return true
-      else
-        missing_file(path)
-        return false
-      end
-    end
-  end
-
-  def file_exist?(path)
-    cache = cached_data?(path)
-    return cache unless cache.nil?
-
-    protect(path) do
-      stat = File.lstat(path)
-      found_file(path, stat)
-      return true
-    end
-  end
-
-  def found_file?(path, type = nil)
-    if data = "" and ! data_expired?(data[:time])
-      return(type and ! data[:stat].send(type)) ? false : true
-    else
-      return false
-    end
-  end
-
-  def found_file(path, stat)
-    found_files[path] = {:stat => stat, :time => Time.now}
-  end
-
-  def missing_file?(path)
-    !!(time = missing_files[path] and ! data_expired?(time))
-  end
-
-  def missing_file(path)
-    missing_files[path] = Time.now
-  end
-
-  private
-
-  def cached_data?(path, type = nil)
-    if found_file?(path, type)
-      return true
-    elsif missing_file?(path)
-      return false
-    else
-      return nil
-    end
-  end
-
-  def data_expired?(time)
-    Time.now - time > 15
-  end
-
-  def protect(path)
-      yield
-  rescue => detail
-      raise unless detail.class.to_s.include?("Errno")
-      missing_file(path)
-      return false
-  end
-end
diff --git a/spec/integration/util/autoload_spec.rb b/spec/integration/util/autoload_spec.rb
index 92fc655..44f4e58 100755
--- a/spec/integration/util/autoload_spec.rb
+++ b/spec/integration/util/autoload_spec.rb
@@ -61,7 +61,7 @@ def with_loader(name, path)
     with_loader("foo", "bar") { |dir,loader|
       with_file(:mything, dir, "mything.rb") {
         loader.load(:mything).should be_true
-        loader.should be_loaded(:mything)
+        loader.class.should be_loaded("bar/mything")
         AutoloadIntegrator.should be_thing(:mything)
       }
     }
@@ -71,7 +71,7 @@ def with_loader(name, path)
     with_loader("foo", "bar") { |dir,loader|
       with_file(:noext, dir, "noext.rb") {
         loader.load(:noext)
-        loader.should be_loaded(:noext)
+        loader.class.should be_loaded("bar/noext")
       }
     }
   end
@@ -80,16 +80,7 @@ def with_loader(name, path)
     with_loader("foo", "bar") { |dir,loader|
       with_file(:noext, dir, "withext.rb") {
         loader.load(:withext)
-        loader.should be_loaded("withext.rb")
-      }
-    }
-  end
-
-  it "should register the fact that the instance is loaded with the Autoload base class" do
-    with_loader("foo", "bar") { |dir,loader|
-      with_file(:baseload, dir, "baseload.rb") {
-        loader.load(:baseload)
-        Puppet::Util::Autoload.should be_loaded("bar/withext.rb")
+        loader.class.should be_loaded("bar/withext.rb")
       }
     }
   end
@@ -106,7 +97,7 @@ def with_loader(name, path)
     with_loader("foo", "foo") do |dir, loader|
       with_file(:plugin, file.split("/")) do
         loader.load(:plugin)
-        loader.should be_loaded("plugin.rb")
+        loader.class.should be_loaded("foo/plugin.rb")
       end
     end
   end
diff --git a/spec/unit/configurer/plugin_handler_spec.rb b/spec/unit/configurer/plugin_handler_spec.rb
index 0fb7183..9ffb1ce 100755
--- a/spec/unit/configurer/plugin_handler_spec.rb
+++ b/spec/unit/configurer/plugin_handler_spec.rb
@@ -43,9 +43,9 @@ class PluginHandlerTester
   it "should use an Agent Downloader, with the name, source, destination, and ignore set correctly, to download plugins when downloading is enabled" do
     downloader = mock 'downloader'
 
-    Puppet.settings.expects(:value).with(:pluginsource).returns "psource"
-    Puppet.settings.expects(:value).with(:plugindest).returns "pdest"
-    Puppet.settings.expects(:value).with(:pluginsignore).returns "pignore"
+    Puppet[:pluginsource] = "psource"
+    Puppet[:plugindest] = "pdest"
+    Puppet[:pluginsignore] = "pignore"
 
     Puppet::Configurer::Downloader.expects(:new).with("plugin", "pdest", "psource", "pignore").returns downloader
 
@@ -54,69 +54,4 @@ class PluginHandlerTester
     @pluginhandler.expects(:download_plugins?).returns true
     @pluginhandler.download_plugins
   end
-
-  it "should be able to load plugins" do
-    @pluginhandler.should respond_to(:load_plugin)
-  end
-
-  it "should load each downloaded file" do
-    FileTest.stubs(:exist?).returns true
-    downloader = mock 'downloader'
-
-    Puppet::Configurer::Downloader.expects(:new).returns downloader
-
-    downloader.expects(:evaluate).returns %w{one two}
-
-    @pluginhandler.expects(:download_plugins?).returns true
-
-    @pluginhandler.expects(:load_plugin).with("one")
-    @pluginhandler.expects(:load_plugin).with("two")
-
-    @pluginhandler.download_plugins
-  end
-
-  it "should load ruby plugins when asked to do so" do
-    FileTest.stubs(:exist?).returns true
-    @pluginhandler.expects(:load).with("foo.rb")
-
-    @pluginhandler.load_plugin("foo.rb")
-  end
-
-  it "should skip non-ruby plugins when asked to do so" do
-    FileTest.stubs(:exist?).returns true
-    @pluginhandler.expects(:load).never
-
-    @pluginhandler.load_plugin("foo")
-  end
-
-  it "should not try to load files that don't exist" do
-    FileTest.expects(:exist?).with("foo.rb").returns false
-    @pluginhandler.expects(:load).never
-
-    @pluginhandler.load_plugin("foo.rb")
-  end
-
-  it "should not try to load directories" do
-    FileTest.stubs(:exist?).returns true
-    FileTest.expects(:directory?).with("foo").returns true
-    @pluginhandler.expects(:load).never
-
-    @pluginhandler.load_plugin("foo")
-  end
-
-  it "should warn but not fail if loading a file raises an exception" do
-    FileTest.stubs(:exist?).returns true
-    @pluginhandler.expects(:load).with("foo.rb").raises "eh"
-
-    Puppet.expects(:err)
-    @pluginhandler.load_plugin("foo.rb")
-  end
-
-  it "should warn but not fail if loading a file raises a LoadError" do
-    FileTest.stubs(:exist?).returns true
-    @pluginhandler.expects(:load).with("foo.rb").raises LoadError.new("eh")
-
-    Puppet.expects(:err)
-    @pluginhandler.load_plugin("foo.rb")
-  end
 end
diff --git a/spec/unit/util/autoload/file_cache_spec.rb b/spec/unit/util/autoload/file_cache_spec.rb
deleted file mode 100755
index cdde9bb..0000000
--- a/spec/unit/util/autoload/file_cache_spec.rb
+++ /dev/null
@@ -1,158 +0,0 @@
-#!/usr/bin/env rspec
-require 'spec_helper'
-require 'puppet/util/autoload/file_cache'
-
-class FileCacheTester
-  include Puppet::Util::Autoload::FileCache
-end
-
-describe Puppet::Util::Autoload::FileCache do
-  before do
-    @cacher = FileCacheTester.new
-  end
-
-  after do
-    Puppet::Util::Autoload::FileCache.clear
-  end
-
-  describe "when checking whether files exist" do
-    it "should have a method for testing whether a file exists" do
-      @cacher.should respond_to(:file_exist?)
-    end
-
-    it "should use lstat to determine whether a file exists" do
-      File.expects(:lstat).with("/my/file")
-      @cacher.file_exist?("/my/file")
-    end
-
-    it "should consider a file as absent if its lstat fails" do
-      File.expects(:lstat).with("/my/file").raises Errno::ENOENT
-      @cacher.should_not be_file_exist("/my/file")
-    end
-
-    it "should consider a file as absent if the directory is absent" do
-      File.expects(:lstat).with("/my/file").raises Errno::ENOTDIR
-      @cacher.should_not be_file_exist("/my/file")
-    end
-
-    it "should consider a file as absent permissions are missing" do
-      File.expects(:lstat).with("/my/file").raises Errno::EACCES
-      @cacher.should_not be_file_exist("/my/file")
-    end
-
-    it "should raise non-fs exceptions" do
-      File.expects(:lstat).with("/my/file").raises ArgumentError
-      lambda { @cacher.file_exist?("/my/file") }.should raise_error(ArgumentError)
-    end
-
-    it "should consider a file as present if its lstat succeeds" do
-      File.expects(:lstat).with("/my/file").returns mock("stat")
-      @cacher.should be_file_exist("/my/file")
-    end
-
-    it "should not stat a file twice in quick succession when the file is missing" do
-      File.expects(:lstat).with("/my/file").once.raises Errno::ENOENT
-      @cacher.should_not be_file_exist("/my/file")
-      @cacher.should_not be_file_exist("/my/file")
-    end
-
-    it "should not stat a file twice in quick succession when the file is present" do
-      File.expects(:lstat).with("/my/file").once.returns mock("stat")
-      @cacher.should be_file_exist("/my/file")
-      @cacher.should be_file_exist("/my/file")
-    end
-
-    it "should expire cached data after 15 seconds" do
-      now = Time.now
-
-      later = now + 16
-
-      Time.expects(:now).times(3).returns(now).then.returns(later).then.returns(later)
-      File.expects(:lstat).with("/my/file").times(2).returns(mock("stat")).then.raises Errno::ENOENT
-      @cacher.should be_file_exist("/my/file")
-      @cacher.should_not be_file_exist("/my/file")
-    end
-
-    it "should share cached data across autoload instances" do
-      File.expects(:lstat).with("/my/file").once.returns mock("stat")
-      other = Puppet::Util::Autoload.new("bar", "tmp")
-
-      @cacher.should be_file_exist("/my/file")
-      other.should be_file_exist("/my/file")
-    end
-  end
-
-  describe "when checking whether files exist" do
-    before do
-      @stat = stub 'stat', :directory? => true
-    end
-
-    it "should have a method for determining whether a directory exists" do
-      @cacher.should respond_to(:directory_exist?)
-    end
-
-    it "should use lstat to determine whether a directory exists" do
-      File.expects(:lstat).with("/my/file").returns @stat
-      @cacher.directory_exist?("/my/file")
-    end
-
-    it "should consider a directory as absent if its lstat fails" do
-      File.expects(:lstat).with("/my/file").raises Errno::ENOENT
-      @cacher.should_not be_directory_exist("/my/file")
-    end
-
-    it "should consider a file as absent if the directory is absent" do
-      File.expects(:lstat).with("/my/file").raises Errno::ENOTDIR
-      @cacher.should_not be_directory_exist("/my/file")
-    end
-
-    it "should consider a file as absent permissions are missing" do
-      File.expects(:lstat).with("/my/file").raises Errno::EACCES
-      @cacher.should_not be_directory_exist("/my/file")
-    end
-
-    it "should raise non-fs exceptions" do
-      File.expects(:lstat).with("/my/file").raises ArgumentError
-      lambda { @cacher.directory_exist?("/my/file") }.should raise_error(ArgumentError)
-    end
-
-    it "should consider a directory as present if its lstat succeeds and the stat is of a directory" do
-      @stat.expects(:directory?).returns true
-      File.expects(:lstat).with("/my/file").returns @stat
-      @cacher.should be_directory_exist("/my/file")
-    end
-
-    it "should consider a directory as absent if its lstat succeeds and the stat is not of a directory" do
-      @stat.expects(:directory?).returns false
-      File.expects(:lstat).with("/my/file").returns @stat
-      @cacher.should_not be_directory_exist("/my/file")
-    end
-
-    it "should not stat a directory twice in quick succession when the file is missing" do
-      File.expects(:lstat).with("/my/file").once.raises Errno::ENOENT
-      @cacher.should_not be_directory_exist("/my/file")
-      @cacher.should_not be_directory_exist("/my/file")
-    end
-
-    it "should not stat a directory twice in quick succession when the file is present" do
-      File.expects(:lstat).with("/my/file").once.returns @stat
-      @cacher.should be_directory_exist("/my/file")
-      @cacher.should be_directory_exist("/my/file")
-    end
-
-    it "should not consider a file to be a directory based on cached data" do
-      @stat.stubs(:directory?).returns false
-      File.stubs(:lstat).with("/my/file").returns @stat
-      @cacher.file_exist?("/my/file")
-      @cacher.should_not be_directory_exist("/my/file")
-    end
-
-    it "should share cached data across autoload instances" do
-      File.expects(:lstat).with("/my/file").once.returns @stat
-      other = Puppet::Util::Autoload.new("bar", "tmp")
-
-      @cacher.should be_directory_exist("/my/file")
-      other.should be_directory_exist("/my/file")
-    end
-  end
-end
diff --git a/spec/unit/util/autoload_spec.rb b/spec/unit/util/autoload_spec.rb
index 47ee54e..5f2267c 100755
--- a/spec/unit/util/autoload_spec.rb
+++ b/spec/unit/util/autoload_spec.rb
@@ -4,19 +4,19 @@
 require 'puppet/util/autoload'
 
 describe Puppet::Util::Autoload do
-  include PuppetSpec::Files
-
   before do
     @autoload = Puppet::Util::Autoload.new("foo", "tmp")
 
     @autoload.stubs(:eachdir).yields "/my/dir"
+    @loaded = {}
+    @autoload.class.stubs(:loaded).returns(@loaded)
   end
 
   describe "when building the search path" do
     before :each do
-      @dira = make_absolute('/a')
-      @dirb = make_absolute('/b')
-      @dirc = make_absolute('/c')
+      @dira = File.expand_path('/a')
+      @dirb = File.expand_path('/b')
+      @dirc = File.expand_path('/c')
     end
 
     it "should collect all of the plugins and lib directories that exist in the current environment's module path" do
@@ -32,7 +32,7 @@
         FileTest.expects(:directory?).with(d).returns true
       end
 
-      @autoload.module_directories.should == ["#{@dira}/one/plugins", "#{@dira}/two/lib", "#{@dirb}/one/plugins", "#{@dirb}/two/lib"]
+      @autoload.class.module_directories.should == ["#{@dira}/one/plugins", "#{@dira}/two/lib", "#{@dirb}/one/plugins", "#{@dirb}/two/lib"]
     end
 
     it "should not look for lib directories in directories starting with '.'" do
@@ -46,37 +46,27 @@
       FileTest.expects(:directory?).with("#{@dira}/../lib").never
       FileTest.expects(:directory?).with("#{@dira}/../plugins").never
 
-      @autoload.module_directories
+      @autoload.class.module_directories
     end
 
     it "should include the module directories, the Puppet libdir, and all of the Ruby load directories" do
-      Puppet.stubs(:[]).with(:libdir).returns(%w{/libdir1 /lib/dir/two /third/lib/dir}.join(File::PATH_SEPARATOR))
-      @autoload.expects(:module_directories).returns %w{/one /two}
-      @autoload.search_directories.should == %w{/one /two /libdir1 /lib/dir/two /third/lib/dir} + $LOAD_PATH
-    end
-
-    it "should include in its search path all of the unique search directories that have a subdirectory matching the autoload path" do
-      @autoload = Puppet::Util::Autoload.new("foo", "loaddir")
-      @autoload.expects(:search_directories).returns %w{/one /two /three /three}
-      FileTest.expects(:directory?).with("/one/loaddir").returns true
-      FileTest.expects(:directory?).with("/two/loaddir").returns false
-      FileTest.expects(:directory?).with("/three/loaddir").returns true
-      @autoload.searchpath.should == ["/one/loaddir", "/three/loaddir"]
+      Puppet[:libdir] = %w{/libdir1 /lib/dir/two /third/lib/dir}.join(File::PATH_SEPARATOR)
+      @autoload.class.expects(:module_directories).returns %w{/one /two}
+      @autoload.class.search_directories.should == %w{/one /two /libdir1 /lib/dir/two /third/lib/dir} + $LOAD_PATH
     end
   end
 
-  it "should include its FileCache module" do
-    Puppet::Util::Autoload.ancestors.should be_include(Puppet::Util::Autoload::FileCache)
-  end
-
   describe "when loading a file" do
     before do
-      @autoload.stubs(:searchpath).returns %w{/a}
+      @autoload.class.stubs(:search_directories).returns %w{/a}
+      FileTest.stubs(:directory?).returns true
+      @time_a = Time.utc(2010, 'jan', 1, 6, 30)
+      File.stubs(:mtime).returns @time_a
     end
 
     [RuntimeError, LoadError, SyntaxError].each do |error|
       it "should die with Puppet::Error if a #{error.to_s} exception is thrown" do
-        @autoload.stubs(:file_exist?).returns true
+        File.stubs(:exist?).returns true
 
         Kernel.expects(:load).raises error
 
@@ -88,36 +78,129 @@
       @autoload.load("foo").should == false
     end
 
+    it "should register loaded files with the autoloader" do
+      File.stubs(:exist?).returns true
+      Kernel.stubs(:load)
+      @autoload.load("myfile")
+
+      @autoload.class.loaded?("tmp/myfile.rb").should be
+
+      $LOADED_FEATURES.delete("tmp/myfile.rb")
+    end
+
     it "should register loaded files with the main loaded file list so they are not reloaded by ruby" do
-      @autoload.stubs(:file_exist?).returns true
+      File.stubs(:exist?).returns true
       Kernel.stubs(:load)
 
       @autoload.load("myfile")
 
       $LOADED_FEATURES.should be_include("tmp/myfile.rb")
+
+      $LOADED_FEATURES.delete("tmp/myfile.rb")
+    end
+
+    it "should load the first file in the searchpath" do
+      @autoload.stubs(:search_directories).returns %w{/a /b}
+      FileTest.stubs(:directory?).returns true
+      File.stubs(:exist?).returns true
+      Kernel.expects(:load).with("/a/tmp/myfile.rb", optionally(anything))
+
+      @autoload.load("myfile")
+
+      $LOADED_FEATURES.delete("tmp/myfile.rb")
     end
   end
 
   describe "when loading all files" do
     before do
-      @autoload.stubs(:searchpath).returns %w{/a}
-      Dir.stubs(:glob).returns "/path/to/file.rb"
+      @autoload.class.stubs(:search_directories).returns %w{/a}
+      FileTest.stubs(:directory?).returns true
+      Dir.stubs(:glob).returns "/a/foo/file.rb"
+      File.stubs(:exist?).returns true
+      @time_a = Time.utc(2010, 'jan', 1, 6, 30)
+      File.stubs(:mtime).returns @time_a
 
       @autoload.class.stubs(:loaded?).returns(false)
     end
 
     [RuntimeError, LoadError, SyntaxError].each do |error|
       it "should die an if a #{error.to_s} exception is thrown", :'fails_on_ruby_1.9.2' => true do
-        Kernel.expects(:require).raises error
+        Kernel.expects(:load).raises error
 
         lambda { @autoload.loadall }.should raise_error(Puppet::Error)
       end
     end
 
     it "should require the full path to the file", :'fails_on_ruby_1.9.2' => true do
-      Kernel.expects(:require).with("/path/to/file.rb")
+      Kernel.expects(:load).with("/a/foo/file.rb", optionally(anything))
 
       @autoload.loadall
     end
   end
+
+  describe "when reloading files" do
+    before :each do
+      @file_a = "/a/file.rb"
+      @file_b = "/b/file.rb"
+      @first_time = Time.utc(2010, 'jan', 1, 6, 30)
+      @second_time = @first_time + 60
+    end
+
+    after :each do
+      $LOADED_FEATURES.delete("a/file.rb")
+      $LOADED_FEATURES.delete("b/file.rb")
+    end
+
+    describe "in one directory" do
+      before :each do
+        @autoload.class.stubs(:search_directories).returns %w{/a}
+        File.expects(:mtime).with(@file_a).returns(@first_time)
+        @autoload.class.mark_loaded("file", @file_a)
+      end
+
+      it "should reload if mtime changes" do
+        File.stubs(:mtime).with(@file_a).returns(@first_time + 60)
+        File.stubs(:exist?).with(@file_a).returns true
+        Kernel.expects(:load).with(@file_a, optionally(anything))
+        @autoload.class.reload_changed
+      end
+
+      it "should do nothing if the file is deleted" do
+        File.stubs(:mtime).with(@file_a).raises(Errno::ENOENT)
+        File.stubs(:exist?).with(@file_a).returns false
+        Kernel.expects(:load).never
+        @autoload.class.reload_changed
+      end
+    end
+
+    describe "in two directories" do
+      before :each do
+        @autoload.class.stubs(:search_directories).returns %w{/a /b}
+      end
+
+      it "should load b/file when a/file is deleted" do
+        File.expects(:mtime).with(@file_a).returns(@first_time)
+        @autoload.class.mark_loaded("file", @file_a)
+        File.stubs(:mtime).with(@file_a).raises(Errno::ENOENT)
+        File.stubs(:exist?).with(@file_a).returns false
+        File.stubs(:exist?).with(@file_b).returns true
+        File.stubs(:mtime).with(@file_b).returns @first_time
+        Kernel.expects(:load).with(@file_b, optionally(anything))
+        @autoload.class.reload_changed
+        @autoload.class.send(:loaded)["file"].should == [@file_b, @first_time]
+      end
+
+      it "should load a/file when b/file is loaded and a/file is created" do
+        File.stubs(:mtime).with(@file_b).returns @first_time
+        File.stubs(:exist?).with(@file_b).returns true
+        @autoload.class.mark_loaded("file", @file_b)
+
+        File.stubs(:mtime).with(@file_a).returns @first_time
+        File.stubs(:exist?).with(@file_a).returns true
+        Kernel.expects(:load).with(@file_a, optionally(anything))
+        @autoload.class.reload_changed
+        @autoload.class.send(:loaded)["file"].should == [@file_a, @first_time]
+      end
+    end
+  end
 end

    

--
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