This mostly focuses on adding all of the detailed tests for this new code, but it also cleans the code up just a little bit.
Signed-off-by: Luke Kanies <[email protected]> --- lib/puppet/util/backups.rb | 38 ++++++------ spec/unit/util/backups.rb | 148 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 139 insertions(+), 47 deletions(-) diff --git a/lib/puppet/util/backups.rb b/lib/puppet/util/backups.rb index b4bfbd2..4ab6777 100644 --- a/lib/puppet/util/backups.rb +++ b/lib/puppet/util/backups.rb @@ -3,13 +3,14 @@ module Puppet::Util::Backups # Deal with backups. def perform_backup(file = nil) - # let the path be specified - file ||= self[:path] - return true unless FileTest.exists?(file) # if they specifically don't want a backup, then just say # we're good return true unless self[:backup] + # let the path be specified + file ||= self[:path] + return true unless FileTest.exists?(file) + return perform_backup_with_bucket(file) if self.bucket return perform_backup_with_backuplocal(file, self[:backup]) end @@ -23,8 +24,7 @@ module Puppet::Util::Backups # we don't need to backup directories when recurse is on return true if self[:recurse] info "Recursively backing up to filebucket" - Find.find(self[:path]) { |f| backup_file_with_filebucket(f) if - File.file?(f) } + Find.find(self[:path]) { |f| backup_file_with_filebucket(f) if File.file?(f) } when "file"; backup_file_with_filebucket(file) when "link"; return true end @@ -33,9 +33,9 @@ module Puppet::Util::Backups def perform_backup_with_backuplocal(fileobj, backup) file = (fileobj.class == String) ? fileobj : fileobj.name newfile = file + backup - if FileTest.exists?(newfile) - remove_backup(newfile) - end + + remove_backup(newfile) + begin bfile = file + backup @@ -47,8 +47,7 @@ module Puppet::Util::Backups rescue => detail # since they said they want a backup, let's error out # if we couldn't make one - self.fail "Could not back %s up: %s" % - [file, detail.message] + self.fail "Could not back %s up: %s" % [file, detail.message] end end @@ -58,23 +57,24 @@ module Puppet::Util::Backups else method = :stat end - old = File.send(method, newfile).ftype - if old == "directory" - raise Puppet::Error, - "Will not remove directory backup %s; use a filebucket" % - newfile + begin + stat = File.send(method, newfile) + rescue Errno::ENOENT + return + end + + if stat.ftype == "directory" + raise Puppet::Error, "Will not remove directory backup %s; use a filebucket" % newfile end - info "Removing old backup of type %s" % - File.send(method, newfile).ftype + info "Removing old backup of type %s" % stat.ftype begin File.unlink(newfile) rescue => detail puts detail.backtrace if Puppet[:trace] - self.err "Could not remove old backup: %s" % detail - return false + self.fail "Could not remove old backup: %s" % detail end end diff --git a/spec/unit/util/backups.rb b/spec/unit/util/backups.rb index 2ddab5e..c4bf26a 100755 --- a/spec/unit/util/backups.rb +++ b/spec/unit/util/backups.rb @@ -7,62 +7,154 @@ require 'puppet/util/backups' include PuppetTest describe Puppet::Util::Backups do + before do + FileTest.stubs(:exists?).returns true + end + describe "when backing up a file" do - it "should succeed silently if the file does not exist" do - Puppet::Type::File.new(:name => '/no/such/file').perform_backup.should be_true + it "should noop if the file does not exist" do + FileTest.expects(:exists?).returns false + file = Puppet::Type::File.new(:name => '/no/such/file') + file.expects(:bucket).never + + file.perform_backup end + it "should succeed silently if self[:backup] is false" do - FileTest.stubs(:exists?).returns true - Puppet::Type::File.new(:name => '/some/file', :backup => false).perform_backup.should be_true + file = Puppet::Type::File.new(:name => '/no/such/file', :backup => false) + file.expects(:bucket).never + FileTest.expects(:exists?).never + file.perform_backup end - it "a bucket should work when provided" do + + it "a bucket should be used when provided" do path = '/my/file' - FileTest.stubs(:exists?).with(path).returns true File.stubs(:stat).with(path).returns(mock('stat', :ftype => 'file')) - bucket = mock('bucket', 'name' => 'foo') - bucket.expects(:backup).with(path) - file = Puppet::Type::File.new(:name => path, :backup => 'foo') + bucket = stub('bucket', 'name' => 'foo') file.stubs(:bucket).returns bucket - file.perform_backup.should be_nil + bucket.expects(:backup).with(path).returns("mysum") + + file.perform_backup end - it "a local backup should work" do + + it "should propagate any exceptions encountered when backing up to a filebucket" do path = '/my/file' - FileTest.stubs(:exists?).with(path).returns true - file = Puppet::Type::File.new(:name => path, :backup => '.foo') - file.stubs(:perform_backup_with_backuplocal).returns true - file.perform_backup.should be_true + File.stubs(:stat).with(path).returns(mock('stat', :ftype => 'file')) + + file = Puppet::Type::File.new(:name => path, :backup => 'foo') + bucket = stub('bucket', 'name' => 'foo') + file.stubs(:bucket).returns bucket + + bucket.expects(:backup).raises ArgumentError + + lambda { file.perform_backup }.should raise_error(ArgumentError) + end + + describe "and no filebucket is configured" do + it "should remove any local backup if one exists" do + path = '/my/file' + FileTest.stubs(:exists?).returns true + + backup = path + ".foo" + + File.expects(:lstat).with(backup).returns stub("stat", :ftype => "file") + File.expects(:unlink).with(backup) + + FileUtils.stubs(:cp_r) + + file = Puppet::Type::File.new(:name => path, :backup => '.foo') + file.perform_backup + end + + it "should fail when the old backup can't be removed" do + path = '/my/file' + FileTest.stubs(:exists?).returns true + + backup = path + ".foo" + + File.expects(:lstat).with(backup).returns stub("stat", :ftype => "file") + File.expects(:unlink).raises ArgumentError + + FileUtils.expects(:cp_r).never + + file = Puppet::Type::File.new(:name => path, :backup => '.foo') + lambda { file.perform_backup }.should raise_error(Puppet::Error) + end + + it "should not try to remove backups that don't exist" do + path = '/my/file' + FileTest.stubs(:exists?).returns true + + backup = path + ".foo" + + File.expects(:lstat).with(backup).raises(Errno::ENOENT) + File.expects(:unlink).never + + FileUtils.stubs(:cp_r) + + file = Puppet::Type::File.new(:name => path, :backup => '.foo') + file.perform_backup + end + + it "a copy should be created in the local directory" do + path = '/my/file' + FileTest.stubs(:exists?).with(path).returns true + + FileUtils.expects(:cp_r).with(path, path + ".foo", :preserve => true) + + file = Puppet::Type::File.new(:name => path, :backup => '.foo') + file.perform_backup.should be_true + end + + it "should propagate exceptions if no backup can be created" do + path = '/my/file' + FileTest.stubs(:exists?).with(path).returns true + + FileUtils.expects(:cp_r).raises ArgumentError + + file = Puppet::Type::File.new(:name => path, :backup => '.foo') + lambda { file.perform_backup }.should raise_error(Puppet::Error) + end end end + describe "when backing up a directory" do it "a bucket should work when provided" do path = '/my/dir' - FileTest.stubs(:exists?).with(path).returns true - File.stubs(:stat).with(path).returns(mock('stat', :ftype => 'directory')) - Find.stubs(:find).returns('') + File.stubs(:file?).returns true + Find.expects(:find).with(path).yields("/my/dir/file") - #bucket = mock('bucket', 'name' => 'foo') - bucket = mock('bucket') - bucket.stubs(:backup).with(path).returns true + bucket = stub('bucket', :name => "eh") + bucket.expects(:backup).with("/my/dir/file").returns true file = Puppet::Type::File.new(:name => path, :backup => 'foo') file.stubs(:bucket).returns bucket - file.perform_backup.should be_true + File.stubs(:stat).with(path).returns(stub('stat', :ftype => 'directory')) + + file.perform_backup end - it "a local backup should work" do + + it "should do nothing when recursing" do path = '/my/dir' - FileTest.stubs(:exists?).with(path).returns true - file = Puppet::Type::File.new(:name => path, :backup => '.foo') - file.stubs(:perform_backup_with_backuplocal).returns true - file.perform_backup.should be_true + bucket = stub('bucket', :name => "eh") + bucket.expects(:backup).never + + file = Puppet::Type::File.new(:name => path, :backup => 'foo', :recurse => true) + file.stubs(:bucket).returns bucket + + File.stubs(:stat).with(path).returns(stub('stat', :ftype => 'directory')) + + Find.expects(:find).never + + file.perform_backup end end end - -- 1.6.1 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
