diff --git a/lib/instance_agent/plugins/codedeploy/install_instruction.rb b/lib/instance_agent/plugins/codedeploy/install_instruction.rb index b33cc81c..df044a6c 100644 --- a/lib/instance_agent/plugins/codedeploy/install_instruction.rb +++ b/lib/instance_agent/plugins/codedeploy/install_instruction.rb @@ -209,10 +209,10 @@ def execute FileUtils.rm(@file_path) elsif File.exist?(@file_path) if File.directory?(@file_path) - # TODO (AWSGLUE-713): handle the exception if the directory is non-empty; - # this might mean the customer has put files in this directory and we should - # probably ignore the error and move on - FileUtils.rmdir(@file_path) + begin + FileUtils.rmdir(@file_path) + rescue Errno::ENOTEMPTY + end else FileUtils.rm(@file_path) end diff --git a/test/instance_agent/plugins/codedeploy/install_instruction_test.rb b/test/instance_agent/plugins/codedeploy/install_instruction_test.rb index 9fd8821d..0a094e8e 100644 --- a/test/instance_agent/plugins/codedeploy/install_instruction_test.rb +++ b/test/instance_agent/plugins/codedeploy/install_instruction_test.rb @@ -231,40 +231,54 @@ class InstallInstructionTest < InstanceAgentTestCase context "Parsing a delete file" do context "an empty delete file" do setup do - @parse_string = <<-END - END - end + @parse_string = "" + end - should "return an empty command collection" do - commands = InstallInstruction.parse_remove_commands(@parse_string) - assert_equal 0, commands.length + should "return an empty command collection" do + commands = InstallInstruction.parse_remove_commands(@parse_string) + assert_equal 0, commands.length + end end - end - context "a single file to delete" do - setup do - @parse_string = <<-END - test_delete_path - END + context "a single file to delete" do + setup do + @parse_string = "test_delete_path\n" File.stubs(:exist?).with("test_delete_path").returns(true) end - should "produce a command that deletes test_delete_path" do + should "use rm for a regular file" do commands = InstallInstruction.parse_remove_commands(@parse_string) FileUtils.expects(:rm).with("test_delete_path") - assert_not_equal nil, commands commands.each do |command| command.execute end end + + should "use rmdir for a directory" do + File.stubs(:directory?).with("test_delete_path").returns(true) + commands = InstallInstruction.parse_remove_commands(@parse_string) + FileUtils.expects(:rmdir).with("test_delete_path") + commands.each do |command| + command.execute + end + end + + should "ignore a non-empty directory by rescuing Errno::ENOTEMPTY" do + File.stubs(:directory?).with("test_delete_path").returns(true) + commands = InstallInstruction.parse_remove_commands(@parse_string) + FileUtils.stubs(:rmdir).raises(Errno::ENOTEMPTY) + + assert_nothing_raised do + commands.each do |command| + command.execute + end + end + end end context "multiple files to delete" do setup do - @parse_string = <<-END - test_delete_path - another_delete_path - END + @parse_string = "test_delete_path\nanother_delete_path\n" File.stubs(:directory?).returns(false) File.stubs(:exist?).with("test_delete_path").returns(true) File.stubs(:exist?).with("another_delete_path").returns(true) @@ -295,11 +309,7 @@ class InstallInstructionTest < InstanceAgentTestCase context "removes mangled line at the end" do setup do - @parse_string = <<-END - test_delete_path - another_delete_path - END - @parse_string << "mangled" + @parse_string = "test_delete_path\nanother_delete_path\nmangled" File.stubs(:exist?).with("test_delete_path").returns(true) File.stubs(:exist?).with("another_delete_path").returns(true) end @@ -318,7 +328,7 @@ class InstallInstructionTest < InstanceAgentTestCase context "correctly determines method from file type" do setup do - @parse_string = 'foo' + @parse_string = "foo\n" @instruction_file = mock @instruction_file.stubs(:path).returns("test/123-cleanup") File.stubs(:open).with("test/123-cleanup", 'r').returns(@instruction_file)