From 78cb48cea0b14eabdfd728c2f9fc183f587faded Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Fri, 8 Apr 2011 13:39:37 -0700 Subject: maint: disable garbage collector during individual test cases. This reduced overhead of garbage collection from 50 percent to 20 percent in my test runs, and reduced wall-time to match. This seems to be a reasonable win: we allow the GC to run on demand, but only outside the testing. Memory use went from ~ 300MB to ~550MB between runs, which suggests that we are generating a *lot* of garbage at times, but that we also benefit from cleaning it automatically on a regular basis. Reviewed-By: Matt Robinson --- spec/spec_helper.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index fc63c6d19..e8ab66611 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -38,6 +38,8 @@ RSpec.configure do |config| config.mock_with :mocha config.before :each do + GC.disable + # these globals are set by Application $puppet_application_mode = nil $puppet_application_name = nil @@ -65,6 +67,8 @@ RSpec.configure do |config| @logs.clear Puppet::Util::Log.close_all + + GC.enable end end -- cgit From f9a2ffd53054b67924790b0386cf74435497c3f1 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Fri, 8 Apr 2011 13:57:31 -0700 Subject: maint: use FileUtil to remove files, not exec We used to shell out to chmod and rm to clean up temporary files; this lead to the cleanup method here being one of the largest consumers of walltime. Replacing that with FileUtil calls is as, or more, secure, and performs sufficiently well that we can just delegate. Reviewed-By: Matt Robinson --- spec/lib/puppet_spec/files.rb | 56 +++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/spec/lib/puppet_spec/files.rb b/spec/lib/puppet_spec/files.rb index 38c51a572..30fb4fc42 100644 --- a/spec/lib/puppet_spec/files.rb +++ b/spec/lib/puppet_spec/files.rb @@ -3,41 +3,51 @@ require 'tempfile' # A support module for testing files. module PuppetSpec::Files + # This code exists only to support tests that run as root, pretty much. + # Once they have finally been eliminated this can all go... --daniel 2011-04-08 + if Puppet.features.posix? then + def self.in_tmp(path) + path =~ /^\/tmp/ or path =~ /^\/var\/folders/ + end + elsif Puppet.features.microsoft_windows? + def self.in_tmp(path) + tempdir = File.expand_path(File.join(Dir::LOCAL_APPDATA, "Temp")) + path =~ /^#{tempdir}/ + end + else + fail "Help! Can't find in_tmp for this platform" + end + def self.cleanup - if defined?($tmpfiles) - $tmpfiles.each do |file| - file = File.expand_path(file) - if Puppet.features.posix? and file !~ /^\/tmp/ and file !~ /^\/var\/folders/ - puts "Not deleting tmpfile #{file} outside of /tmp or /var/folders" - next - elsif Puppet.features.microsoft_windows? - tempdir = File.expand_path(File.join(Dir::LOCAL_APPDATA, "Temp")) - if file !~ /^#{tempdir}/ - puts "Not deleting tmpfile #{file} outside of #{tempdir}" - next - end - end - if FileTest.exist?(file) - system("chmod -R 755 '#{file}'") - system("rm -rf '#{file}'") - end + $global_tempfiles ||= [] + while path = $global_tempfiles.pop do + fail "Not deleting tmpfile #{path} outside regular tmpdir" unless in_tmp(path) + + begin + FileUtils.rm_r path, :secure => true + rescue Errno::ENOENT + # nothing to do end - $tmpfiles.clear end end def tmpfile(name) + # Generate a temporary file, just for the name... source = Tempfile.new(name) path = source.path source.close! - $tmpfiles ||= [] - $tmpfiles << path + + # ...record it for cleanup, + $global_tempfiles ||= [] + $global_tempfiles << File.expand_path(path) + + # ...and bam. path end def tmpdir(name) - file = tmpfile(name) - FileUtils.mkdir_p(file) - file + path = tmpfile(name) + FileUtils.mkdir_p(path) + path end end -- cgit From a19fbb417b54f0456b02bea295efd310761f6b86 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Fri, 8 Apr 2011 15:06:37 -0700 Subject: maint: don't take over signal handling in tests... We had a problem where we installed a signal handler during a :before block, which wasn't stubbed, so ended up leaving that in place forever. Which bites. We stub it out locally, which is ugly but functional. Paired-With: Matt Robinson --- spec/unit/application/faces_base_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/unit/application/faces_base_spec.rb b/spec/unit/application/faces_base_spec.rb index 6d8815f44..6d8456612 100755 --- a/spec/unit/application/faces_base_spec.rb +++ b/spec/unit/application/faces_base_spec.rb @@ -59,6 +59,13 @@ describe Puppet::Application::FacesBase do describe "parsing the command line" do context "with just an action" do before :all do + # We have to stub Signal.trap to avoid a crazy mess where we take + # over signal handling and make it impossible to cancel the test + # suite run. + # + # It would be nice to fix this elsewhere, but it is actually hard to + # capture this in rspec 2.5 and all. :( --daniel 2011-04-08 + Signal.stubs(:trap) app.command_line.stubs(:args).returns %w{foo} app.preinit end -- cgit From f9271b918833b250da304efa8efdd0fdb64e89ca Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Fri, 8 Apr 2011 15:24:26 -0700 Subject: maint: delete dead darwinport package provider The DarwinPorts package provider was actually entirely broken; we are not shipping or supporting it with the 2.7 release. Plans exist to introduce a newer, functional MacPorts provider, but this dead code can be removed early. Paired-With: Nigel Kersten --- lib/puppet/provider/package/darwinport.rb | 86 ------------------------------- 1 file changed, 86 deletions(-) delete mode 100755 lib/puppet/provider/package/darwinport.rb diff --git a/lib/puppet/provider/package/darwinport.rb b/lib/puppet/provider/package/darwinport.rb deleted file mode 100755 index c5f9ba28f..000000000 --- a/lib/puppet/provider/package/darwinport.rb +++ /dev/null @@ -1,86 +0,0 @@ -require 'puppet/provider/package' - -Puppet::Type.type(:package).provide :darwinport, :parent => Puppet::Provider::Package do - desc "Package management using DarwinPorts on OS X." - - confine :operatingsystem => :darwin - commands :port => "/opt/local/bin/port" - - def self.eachpkgashash - # list out all of the packages - open("| #{command(:port)} list installed") { |process| - regex = %r{(\S+)\s+@(\S+)\s+(\S+)} - fields = [:name, :ensure, :location] - hash = {} - - # now turn each returned line into a package object - process.each { |line| - hash.clear - - if match = regex.match(line) - fields.zip(match.captures) { |field,value| - hash[field] = value - } - - hash.delete :location - hash[:provider] = self.name - yield hash.dup - else - raise Puppet::DevError, - "Failed to match dpkg line #{line}" - end - } - } - end - - def self.instances - packages = [] - - eachpkgashash do |hash| - packages << new(hash) - end - - packages - end - - def install - should = @resource.should(:ensure) - - # Seems like you can always say 'upgrade' - output = port "upgrade", @resource[:name] - if output =~ /^Error: No port/ - raise Puppet::ExecutionFailure, "Could not find package #{@resource[:name]}" - end - end - - def query - version = nil - self.class.eachpkgashash do |hash| - return hash if hash[:name] == @resource[:name] - end - - nil - end - - def latest - info = port :search, "^#{@resource[:name]}$" - - if $CHILD_STATUS != 0 or info =~ /^Error/ - return nil - end - - ary = info.split(/\s+/) - version = ary[2].sub(/^@/, '') - - version - end - - def uninstall - port :uninstall, @resource[:name] - end - - def update - install - end -end - -- cgit