Please review pull request #762: (#14387) user agent for module tool opened by (zaphod42)

Description:

This pull request changes the puppet module face to send a User-Agent header to the forge to identify itself. The User-Agent will contain the version of the module tool, the version of puppet, the operating system, and the version of ruby that is being used.

This pull request also ensures that Puppet::Forge does not depend on the Puppet::Face code. In order to achieve that the module face now is responsible for creating the correct Puppet::Forge object and handing
that to the various components of the face for execution.

The Puppet::Face now provides an interface for interacting with the repository in ways that various users were doing by asking for the repository directly.

  • Opened: Fri May 11 20:38:20 UTC 2012
  • Based on: puppetlabs:master (98481c8be4a0fea0141aeaa28eeb38a1fad302ca)
  • Requested merge: zaphod42:bug/master/14387-user-agent-for-module-tool (ebacce3583ae641f1385782602dfaf551cdea482)

Diff follows:

diff --git a/lib/puppet/face/module/install.rb b/lib/puppet/face/module/install.rb
index 2378c7a..13db76b 100644
--- a/lib/puppet/face/module/install.rb
+++ b/lib/puppet/face/module/install.rb
@@ -1,4 +1,5 @@
 # encoding: UTF-8
+require 'puppet/forge'
 
 Puppet::Face.define(:module, '1.0.0') do
   action(:install) do
@@ -119,7 +120,7 @@
     when_invoked do |name, options|
       Puppet::ModuleTool.set_option_defaults options
       Puppet.notice "Preparing to install into #{options[:target_dir]} ..."
-      Puppet::ModuleTool::Applications::Installer.run(name, options)
+      Puppet::ModuleTool::Applications::Installer.new(name, Puppet::Forge.new("PMT", self.version), options).run
     end
 
     when_rendering :console do |return_value, name, options|
diff --git a/lib/puppet/face/module/search.rb b/lib/puppet/face/module/search.rb
index a7e5821..36cca8f 100644
--- a/lib/puppet/face/module/search.rb
+++ b/lib/puppet/face/module/search.rb
@@ -1,4 +1,5 @@
 require 'puppet/util/terminal'
+require 'puppet/forge'
 
 Puppet::Face.define(:module, '1.0.0') do
   action(:search) do
@@ -22,7 +23,7 @@
 
     when_invoked do |term, options|
       Puppet::ModuleTool.set_option_defaults options
-      Puppet::ModuleTool::Applications::Searcher.run(term, options)
+      Puppet::ModuleTool::Applications::Searcher.new(term, Puppet::Forge.new("PMT", self.version), options).run
     end
 
     when_rendering :console do |results, term, options|
diff --git a/lib/puppet/face/module/upgrade.rb b/lib/puppet/face/module/upgrade.rb
index 5c4cd79..c16e3a3 100644
--- a/lib/puppet/face/module/upgrade.rb
+++ b/lib/puppet/face/module/upgrade.rb
@@ -57,7 +57,7 @@
       name = name.gsub('/', '-')
       Puppet.notice "Preparing to upgrade '#{name}' ..."
       Puppet::ModuleTool.set_option_defaults options
-      Puppet::ModuleTool::Applications::Upgrader.new(name, options).run
+      Puppet::ModuleTool::Applications::Upgrader.new(name, Puppet::Forge.new("PMT", self.version), options).run
     end
 
     when_rendering :console do |return_value|
diff --git a/lib/puppet/forge.rb b/lib/puppet/forge.rb
index 722f03b..eff05ed 100644
--- a/lib/puppet/forge.rb
+++ b/lib/puppet/forge.rb
@@ -5,7 +5,15 @@
 require 'puppet/forge/cache'
 require 'puppet/forge/repository'
 
-module Puppet::Forge
+class Puppet::Forge
+  # +consumer_name+ is a name to be used for identifying the consumer of the
+  # forge and +consumer_semver+ is a SemVer object to identify the version of
+  # the consumer
+  def initialize(consumer_name, consumer_semver)
+    @consumer_name = consumer_name
+    @consumer_semver = consumer_semver
+  end
+
   # Return a list of module metadata hashes that match the search query.
   # This return value is used by the module_tool face install search,
   # and displayed to on the console.
@@ -25,27 +33,24 @@ module Puppet::Forge
   #   }
   # ]
   #
-  def self.search(term)
+  def search(term)
     server = Puppet.settings[:module_repository].sub(/^(?!https?:\/\/)/, 'http://')
     Puppet.notice "Searching #{server} ..."
-    request = Net::HTTP::Get.new("/modules.json?q=#{URI.escape(term)}")
-    response = repository.make_http_request(request)
+    response = repository.make_http_request("/modules.json?q=#{URI.escape(term)}")
 
     case response.code
     when "200"
       matches = PSON.parse(response.body)
     else
       raise RuntimeError, "Could not execute search (HTTP #{response.code})"
-      matches = []
     end
 
     matches
   end
 
-  def self.remote_dependency_info(author, mod_name, version)
+  def remote_dependency_info(author, mod_name, version)
     version_string = version ? "&version=#{version}" : ''
-    request = Net::HTTP::Get.new("/api/v1/releases.json?module=#{author}/#{mod_name}" + version_string)
-    response = repository.make_http_request(request)
+    response = repository.make_http_request("/api/v1/releases.json?module=#{author}/#{mod_name}#{version_string}")
     json = PSON.parse(response.body) rescue {}
     case response.code
     when "200"
@@ -60,7 +65,7 @@ def self.remote_dependency_info(author, mod_name, version)
     end
   end
 
-  def self.get_release_packages_from_repository(install_list)
+  def get_release_packages_from_repository(install_list)
     install_list.map do |release|
       modname, version, file = release
       cache_path = nil
@@ -80,7 +85,7 @@ def self.get_release_packages_from_repository(install_list)
   # Locate a module release package on the local filesystem and move it
   # into the `Puppet.settings[:module_working_dir]`. Do not unpack it, just
   # return the location of the package on disk.
-  def self.get_release_package_from_filesystem(filename)
+  def get_release_package_from_filesystem(filename)
     if File.exist?(File.expand_path(filename))
       repository = Repository.new('file:///')
       uri = URI.parse("file://#{URI.escape(File.expand_path(filename))}")
@@ -92,7 +97,17 @@ def self.get_release_package_from_filesystem(filename)
     cache_path
   end
 
-  def self.repository
-    @repository ||= Puppet::Forge::Repository.new
+  def retrieve(release)
+    repository.retrieve(release)
+  end
+
+  def uri
+    repository.uri
+  end
+
+  def repository
+    version = "#{@consumer_name}/#{[@consumer_semver.major, @consumer_semver.minor, @consumer_semver.tiny].join('.')}#{@consumer_semver.special}"
+    @repository ||= Puppet::Forge::Repository.new(Puppet[:module_repository], version)
   end
+  private :repository
 end
diff --git a/lib/puppet/forge/cache.rb b/lib/puppet/forge/cache.rb
index 592bf1a..c9aeb55 100644
--- a/lib/puppet/forge/cache.rb
+++ b/lib/puppet/forge/cache.rb
@@ -1,6 +1,6 @@
 require 'uri'
 
-module Puppet::Forge
+class Puppet::Forge
   # = Cache
   #
   # Provides methods for reading files from local cache, filesystem or network.
diff --git a/lib/puppet/forge/repository.rb b/lib/puppet/forge/repository.rb
index cacfdc6..2528bd3 100644
--- a/lib/puppet/forge/repository.rb
+++ b/lib/puppet/forge/repository.rb
@@ -2,19 +2,20 @@
 require 'digest/sha1'
 require 'uri'
 
-module Puppet::Forge
+class Puppet::Forge
   # = Repository
   #
   # This class is a file for accessing remote repositories with modules.
   class Repository
-
     attr_reader :uri, :cache
 
-    # Instantiate a new repository instance rooted at the optional string
-    # +url+, else an instance of the default Puppet modules repository.
-    def initialize(url=""
+    # Instantiate a new repository instance rooted at the +url+.
+    # The agent will report +consumer_version+ in the User-Agent to
+    # the repository.
+    def initialize(url, consumer_version)
       @uri = url.is_a?(::URI) ? url : ::URI.parse(url.sub(/^(?!https?:\/\/)/, 'http://'))
       @cache = Cache.new(self)
+      @consumer_version = consumer_version
     end
 
     # Read HTTP proxy configurationm from Puppet's config file, or the
@@ -53,8 +54,9 @@ def http_proxy_port
       return Puppet.settings[:http_proxy_port]
     end
 
-    # Return a Net::HTTPResponse read for this +request+.
-    def make_http_request(request, options = {})
+    # Return a Net::HTTPResponse read for this +request_path+.
+    def make_http_request(request_path)
+      request = Net::HTTP::Get.new(request_path, { "User-Agent" => user_agent })
       if ! @uri.user.nil? && ! @uri.password.nil?
         request.basic_auth(@uri.user, @uri.password)
       end
@@ -98,5 +100,10 @@ def cache_key
         Digest::SHA1.hexdigest(@uri.to_s)
       ].join('-')
     end
+
+    def user_agent
+      "#{@consumer_version} Puppet/#{Puppet.version} (#{Facter.value(:operatingsystem)} #{Facter.value(:operatingsystemrelease)}) Ruby/#{RUBY_VERSION}-p#{RUBY_PATCHLEVEL} (#{RUBY_RELEASE_DATE}; #{RUBY_PLATFORM})"
+    end
+    private :user_agent
   end
 end
diff --git a/lib/puppet/module_tool/applications/installer.rb b/lib/puppet/module_tool/applications/installer.rb
index 645a4ec..add1780 100644
--- a/lib/puppet/module_tool/applications/installer.rb
+++ b/lib/puppet/module_tool/applications/installer.rb
@@ -12,12 +12,13 @@ class Installer < Application
 
       include Puppet::ModuleTool::Errors
 
-      def initialize(name, options = {})
+      def initialize(name, forge, options = {})
         @action              = ""
         @environment         = Puppet::Node::Environment.new(Puppet.settings[:environment])
         @force               = options[:force]
         @ignore_dependencies = options[:force] || options[:ignore_dependencies]
         @name                = name
+        @forge               = forge
         super(options)
       end
 
@@ -102,7 +103,7 @@ def get_release_packages
             ]
           }
         else
-          get_remote_constraints
+          get_remote_constraints(@forge)
         end
 
         @graph = resolve_constraints({ @module_name => @version })
@@ -116,7 +117,7 @@ def get_release_packages
         # Long term we should just get rid of this caching behavior and cleanup downloaded modules after they install
         # but for now this is a quick fix to disable caching
         Puppet::Forge::Cache.clean
-        download_tarballs(@graph, @graph.last[:path])
+        download_tarballs(@graph, @graph.last[:path], @forge)
       end
 
       #
diff --git a/lib/puppet/module_tool/applications/searcher.rb b/lib/puppet/module_tool/applications/searcher.rb
index 28c7837..cc994ba 100644
--- a/lib/puppet/module_tool/applications/searcher.rb
+++ b/lib/puppet/module_tool/applications/searcher.rb
@@ -2,13 +2,14 @@ module Puppet::ModuleTool
   module Applications
     class Searcher < Application
 
-      def initialize(term, options = {})
+      def initialize(term, forge, options = {})
         @term = term
+        @forge = forge
         super(options)
       end
 
       def run
-        Puppet::Forge.search(@term)
+        @forge.search(@term)
       end
     end
   end
diff --git a/lib/puppet/module_tool/applications/upgrader.rb b/lib/puppet/module_tool/applications/upgrader.rb
index 94c1ddc..226f240 100644
--- a/lib/puppet/module_tool/applications/upgrader.rb
+++ b/lib/puppet/module_tool/applications/upgrader.rb
@@ -4,7 +4,7 @@ class Upgrader < Application
 
       include Puppet::ModuleTool::Errors
 
-      def initialize(name, options)
+      def initialize(name, forge, options)
         @action              = ""
         @environment         = Puppet::Node::Environment.new(Puppet.settings[:environment])
         @module_name         = name
@@ -12,6 +12,7 @@ def initialize(name, options)
         @force               = options[:force]
         @ignore_dependencies = options[:force] || options[:ignore_dependencies]
         @version             = options[:version]
+        @forge               = forge
       end
 
       def run
@@ -46,11 +47,11 @@ def run
           end
 
           begin
-            get_remote_constraints
+            get_remote_constraints(@forge)
           rescue => e
-            raise UnknownModuleError, results.merge(:repository => Puppet::Forge.repository.uri)
+            raise UnknownModuleError, results.merge(:repository => @forge.uri)
           else
-            raise UnknownVersionError, results.merge(:repository => Puppet::Forge.repository.uri) if @remote.empty?
+            raise UnknownVersionError, results.merge(:repository => @forge.uri) if @remote.empty?
           end
 
           if !@options[:force] && @versions["#{@module_name}"].last[:vstring].sub(/^(?=\d)/, 'v') == (@module.version || '0.0.0').sub(/^(?=\d)/, 'v')
@@ -70,7 +71,7 @@ def run
           # Long term we should just get rid of this caching behavior and cleanup downloaded modules after they install
           # but for now this is a quick fix to disable caching
           Puppet::Forge::Cache.clean
-          tarballs = download_tarballs(@graph, @graph.last[:path])
+          tarballs = download_tarballs(@graph, @graph.last[:path], @forge)
 
           unless @graph.empty?
             Puppet.notice 'Upgrading -- do not interrupt ...'
diff --git a/lib/puppet/module_tool/shared_behaviors.rb b/lib/puppet/module_tool/shared_behaviors.rb
index b7821fc..b43b166 100644
--- a/lib/puppet/module_tool/shared_behaviors.rb
+++ b/lib/puppet/module_tool/shared_behaviors.rb
@@ -24,14 +24,14 @@ def get_local_constraints
     end
   end
 
-  def get_remote_constraints
+  def get_remote_constraints(forge)
     @remote   = Hash.new { |h,k| h[k] = { } }
     @urls     = {}
     @versions = Hash.new { |h,k| h[k] = [] }
 
-    Puppet.notice "Downloading from #{Puppet::Forge.repository.uri} ..."
+    Puppet.notice "Downloading from #{forge.uri} ..."
     author, modname = Puppet::ModuleTool.username_and_modname_from(@module_name)
-    info = Puppet::Forge.remote_dependency_info(author, modname, @options[:version])
+    info = forge.remote_dependency_info(author, modname, @options[:version])
     info.each do |pair|
       mod_name, releases = pair
       mod_name = mod_name.gsub('/', '-')
@@ -140,13 +140,13 @@ def resolve_constraints(dependencies, source = [{:name => :you}], seen = {}, act
     return dependencies
   end
 
-  def download_tarballs(graph, default_path)
+  def download_tarballs(graph, default_path, forge)
     graph.map do |release|
       begin
         if release[:tarball]
           cache_path = Pathname(release[:tarball])
         else
-          cache_path = Puppet::Forge.repository.retrieve(release[:file])
+          cache_path = forge.retrieve(release[:file])
         end
       rescue OpenURI::HTTPError => e
         raise RuntimeError, "Could not download module: #{e.message}"
@@ -154,7 +154,7 @@ def download_tarballs(graph, default_path)
 
       [
         { (release[:path] ||= default_path) => cache_path},
-        *download_tarballs(release[:dependencies], default_path)
+        *download_tarballs(release[:dependencies], default_path, forge)
       ]
     end.flatten
   end
diff --git a/spec/unit/face/module/search_spec.rb b/spec/unit/face/module/search_spec.rb
index 999b24c..699dc11 100644
--- a/spec/unit/face/module/search_spec.rb
+++ b/spec/unit/face/module/search_spec.rb
@@ -140,8 +140,14 @@
     end
 
     it "should accept the --module-repository option" do
+      forge = mock("Puppet::Forge")
+      searcher = mock("Searcher")
       options[:module_repository] = "http://forge.example.com"
-      Puppet::ModuleTool::Applications::Searcher.expects(:run).with("puppetlabs-apache", has_entries(options)).once
+
+      Puppet::Forge.expects(:new).with("PMT", subject.version).returns(forge)
+      Puppet::ModuleTool::Applications::Searcher.expects(:new).with("puppetlabs-apache", forge, has_entries(options)).returns(searcher)
+      searcher.expects(:run)
+
       subject.search("puppetlabs-apache", options)
     end
   end
diff --git a/spec/unit/forge/repository_spec.rb b/spec/unit/forge/repository_spec.rb
index bbfc0d1..7841688 100644
--- a/spec/unit/forge/repository_spec.rb
+++ b/spec/unit/forge/repository_spec.rb
@@ -4,53 +4,85 @@
 require 'puppet/forge/cache'
 
 describe Puppet::Forge::Repository do
-  describe 'instances' do
+  let(:consumer_version) { "Test/1.0" }
+  let(:repository) { Puppet::Forge::Repository.new('http://fake.com', consumer_version) }
 
-    let(:repository) { Puppet::Forge::Repository.new('http://fake.com') }
+  it "retrieve accesses the cache" do
+    uri = URI.parse('http://some.url.com')
+    repository.cache.expects(:retrieve).with(uri)
 
-    describe '#retrieve' do
-      before do
-        @uri = URI.parse('http://some.url.com')
-      end
+    repository.retrieve(uri)
+  end
 
-      it "should access the cache" do
-        repository.cache.expects(:retrieve).with(@uri)
-        repository.retrieve(@uri)
-      end
+  describe 'http_proxy support' do
+    after :each do
+      ENV["http_proxy"] = nil
     end
 
-    describe 'http_proxy support' do
-      before :each do
-        ENV["http_proxy"] = nil
-      end
+    it "supports environment variable for port and host" do
+      ENV["http_proxy"] = "http://test.com:8011"
 
-      after :each do
-        ENV["http_proxy"] = nil
-      end
+      repository.http_proxy_host.should == "test.com"
+      repository.http_proxy_port.should == 8011
+    end
 
-      it "should support environment variable for port and host" do
-        ENV["http_proxy"] = "http://test.com:8011"
-        repository.http_proxy_host.should == "test.com"
-        repository.http_proxy_port.should == 8011
-      end
+    it "supports puppet configuration for port and host" do
+      ENV["http_proxy"] = nil
+      proxy_settings_of('test.com', 7456)
+
+      repository.http_proxy_port.should == 7456
+      repository.http_proxy_host.should == "test.com"
+    end
+
+    it "uses environment variable before puppet settings" do
+      ENV["http_proxy"] = "http://test1.com:8011"
+      proxy_settings_of('test2.com', 7456)
+
+      repository.http_proxy_host.should == "test1.com"
+      repository.http_proxy_port.should == 8011
+    end
+  end
 
-      it "should support puppet configuration for port and host" do
-        ENV["http_proxy"] = nil
-        Puppet.settings.stubs(:[]).with(:http_proxy_host).returns('test.com')
-        Puppet.settings.stubs(:[]).with(:http_proxy_port).returns(7456)
+  describe "making a request" do
+    before :each do
+      proxy_settings_of("proxy", 1234)
+    end
 
-        repository.http_proxy_port.should == 7456
-        repository.http_proxy_host.should == "test.com"
+    it "returns the result object from the request" do
+      result = "the http response" 
+      performs_an_http_request result do |http|
+        http.expects(:request).with(responds_with(:path, "the_path"))
       end
 
-      it "should use environment variable before puppet settings" do
-        ENV["http_proxy"] = "http://test1.com:8011"
-        Puppet.settings.stubs(:[]).with(:http_proxy_host).returns('test2.com')
-        Puppet.settings.stubs(:[]).with(:http_proxy_port).returns(7456)
+      repository.make_http_request("the_path").should == result
+    end
+
+    it "sets the user agent for the request" do
+      performs_an_http_request do |http|
+        http.expects(:request).with() do |request|
+          puppet_version = /Puppet\/\d+\..*/
+          os_info = /\(.*\)/
+          ruby_version = /Ruby\/\d+\.\d+\.\d+-p\d+ \(\d{4}-\d{2}-\d{2}; .*\)/
 
-        repository.http_proxy_host.should == "test1.com"
-        repository.http_proxy_port.should == 8011
+          request["User-Agent"] =~ /^#{consumer_version} #{puppet_version} #{os_info} #{ruby_version}/
+        end
       end
+
+      repository.make_http_request("the_path")
     end
+
+    def performs_an_http_request(result = nil, &block)
+      http = mock("http client")
+      yield http
+
+      proxy = mock("http proxy")
+      proxy.expects(:start).with("fake.com", 80).yields(http).returns(result)
+      Net::HTTP.expects(:Proxy).with("proxy", 1234).returns(proxy)
+    end
+  end
+
+  def proxy_settings_of(host, port)
+    Puppet.settings.stubs(:[]).with(:http_proxy_host).returns(host)
+    Puppet.settings.stubs(:[]).with(:http_proxy_port).returns(port)
   end
 end
diff --git a/spec/unit/forge_spec.rb b/spec/unit/forge_spec.rb
index 95e47a0..6e11d0d 100644
--- a/spec/unit/forge_spec.rb
+++ b/spec/unit/forge_spec.rb
@@ -4,8 +4,6 @@
 require 'puppet/module_tool'
 
 describe Puppet::Forge do
-  include PuppetSpec::Files
-
   let(:response_body) do
   <<-EOF
     [
@@ -22,35 +20,31 @@
     ]
   EOF
   end
-  let(:response) { stub(:body => response_body, :code => '200') }
 
-  before do
+  let(:forge) { Puppet::Forge.new("test_agent", SemVer.new("v1.0.0")) }
+
+  def repository_responds_with(response)
     Puppet::Forge::Repository.any_instance.stubs(:make_http_request).returns(response)
-    Puppet::Forge::Repository.any_instance.stubs(:retrieve).returns("/tmp/foo")
   end
 
-  describe "the behavior of the search method" do
-    context "when there are matches for the search term" do
-      before do
-        Puppet::Forge::Repository.any_instance.stubs(:make_http_request).returns(response)
-      end
+  it "returns a list of matches from the forge when there are matches for the search term" do
+    response = stub(:body => response_body, :code => '200')
+    repository_responds_with(response)
 
-      it "should return a list of matches from the forge" do
-        Puppet::Forge.search('bacula').should == PSON.load(response_body)
-      end
-    end
+    forge.search('bacula').should == PSON.load(response_body)
+  end
 
-    context "when the connection to the forge fails" do
-      let(:response)  { stub(:body => '{}', :code => '404') }
+  context "when the connection to the forge fails" do
+    before :each do
+      repository_responds_with(stub(:body => '{}', :code => '404'))
+    end
 
-      it "should raise an error for search" do
-        lambda { Puppet::Forge.search('bacula') }.should raise_error RuntimeError
-      end
+    it "raises an error for search" do
+      expect { forge.search('bacula') }.should raise_error RuntimeError
+    end
 
-      it "should raise an error for remote_dependency_info" do
-        lambda { Puppet::Forge.remote_dependency_info('puppetlabs', 'bacula', '0.0.1') }.should raise_error RuntimeError
-      end
+    it "raises an error for remote_dependency_info" do
+      expect { forge.remote_dependency_info('puppetlabs', 'bacula', '0.0.1') }.should raise_error RuntimeError
     end
   end
-
 end
diff --git a/spec/unit/module_tool/applications/installer_spec.rb b/spec/unit/module_tool/applications/installer_spec.rb
index 6b3c8fd..cddcdaf 100644
--- a/spec/unit/module_tool/applications/installer_spec.rb
+++ b/spec/unit/module_tool/applications/installer_spec.rb
@@ -11,8 +11,6 @@
     fake_env.modulepath = [modpath1]
     FileUtils.touch(stdlib_pkg)
     Puppet.settings[:modulepath] = modpath1
-    Puppet::Forge.stubs(:remote_dependency_info).returns(remote_dependency_info)
-    Puppet::Forge.stubs(:repository).returns(repository)
   end
 
   let(:unpacker)        { stub(:run) }
@@ -22,18 +20,18 @@
   let(:fake_env)        { Puppet::Node::Environment.new('fake_env') }
   let(:options)         { Hash[:target_dir => modpath1] }
 
-  let(:repository) do
-    repository = mock()
-    repository.stubs(:uri => 'forge-dev.puppetlabs.com')
+  let(:forge)           do 
+    forge = mock("Puppet::Forge")
 
-    releases = remote_dependency_info.each_key do |mod|
+    forge.stubs(:remote_dependency_info).returns(remote_dependency_info)
+    forge.stubs(:uri).returns('forge-dev.puppetlabs.com')
+    remote_dependency_info.each_key do |mod|
       remote_dependency_info[mod].each do |release|
-        repository.stubs(:retrieve).with(release['file'])\
-                  .returns("/fake_cache#{release['file']}")
+        forge.stubs(:retrieve).with(release['file']).returns("/fake_cache#{release['file']}")
       end
     end
 
-    repository
+    forge
   end
 
   let(:remote_dependency_info) do
@@ -77,14 +75,14 @@
   describe "the behavior of .is_module_package?" do
     it "should return true when file is a module package" do
       pending("porting to Windows", :if => Puppet.features.microsoft_windows?) do
-        installer = installer_class.new("foo", options)
+        installer = installer_class.new("foo", forge, options)
         installer.send(:is_module_package?, stdlib_pkg).should be_true
       end
     end
 
     it "should return false when file is not a module package" do
       pending("porting to Windows", :if => Puppet.features.microsoft_windows?) do
-        installer = installer_class.new("foo", options)
+        installer = installer_class.new("foo", forge, options)
         installer.send(:is_module_package?, "pmtacceptance-apollo-0.0.2.tar").
           should be_false
       end
@@ -102,7 +100,7 @@
         Puppet::ModuleTool::Applications::Unpacker.expects(:new).
           with('/fake_cache/pmtacceptance-stdlib-1.0.0.tar.gz', options).
           returns(unpacker)
-        results = installer_class.run('pmtacceptance-stdlib', options)
+        results = installer_class.run('pmtacceptance-stdlib', forge, options)
         results[:installed_modules].length == 1
         results[:installed_modules][0][:module].should == "pmtacceptance-stdlib"
         results[:installed_modules][0][:version][:vstring].should == "1.0.0"
@@ -122,7 +120,7 @@
             with('/fake_cache/pmtacceptance-java-1.7.1.tar.gz', options).
             returns(unpacker)
 
-          results = installer_class.run('pmtacceptance-apollo', options)
+          results = installer_class.run('pmtacceptance-apollo', forge, options)
           installed_dependencies = results[:installed_modules][0][:dependencies]
 
           dependencies = installed_dependencies.inject({}) do |result, dep|
@@ -142,7 +140,7 @@
           Puppet::ModuleTool::Applications::Unpacker.expects(:new).
             with('/fake_cache/pmtacceptance-apollo-0.0.2.tar.gz', options).
             returns(unpacker)
-          results = installer_class.run('pmtacceptance-apollo', options)
+          results = installer_class.run('pmtacceptance-apollo', forge, options)
           results[:installed_modules][0][:module].should == "pmtacceptance-apollo"
         end
       end
@@ -153,7 +151,7 @@
           Puppet::ModuleTool::Applications::Unpacker.expects(:new).
             with('/fake_cache/pmtacceptance-apollo-0.0.2.tar.gz', options).
             returns(unpacker)
-          results = installer_class.run('pmtacceptance-apollo', options)
+          results = installer_class.run('pmtacceptance-apollo', forge, options)
           dependencies = results[:installed_modules][0][:dependencies]
           dependencies.should == []
         end
@@ -165,7 +163,7 @@
           Puppet::ModuleTool::Applications::Unpacker.expects(:new).
             with('/fake_cache/pmtacceptance-apollo-0.0.2.tar.gz', options).
             returns(unpacker)
-          results = installer_class.run('pmtacceptance-apollo', options)
+          results = installer_class.run('pmtacceptance-apollo', forge, options)
           dependencies = results[:installed_modules][0][:dependencies]
           dependencies.should == []
         end
@@ -184,7 +182,7 @@
     Use `puppet module install --force` to install this module anyway
 MSG
 
-          results = installer_class.run('pmtacceptance-apollo', options)
+          results = installer_class.run('pmtacceptance-apollo', forge, options)
           results[:result].should == :failure
           results[:error][:oneline].should == oneline
           results[:error][:multiline].should == multiline
diff --git a/spec/unit/module_tool/applications/upgrader_spec.rb b/spec/unit/module_tool/applications/upgrader_spec.rb
index 3707f8b..63a7a21 100644
--- a/spec/unit/module_tool/applications/upgrader_spec.rb
+++ b/spec/unit/module_tool/applications/upgrader_spec.rb
@@ -6,9 +6,6 @@
 describe Puppet::ModuleTool::Applications::Upgrader do
   include PuppetSpec::Files
 
-  before do
-  end
-
   it "should update the requested module"
   it "should not update dependencies"
   it "should fail when updating a dependency to an unsupported version"

    

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