Merge pull request #201 from what-is-quality/fix-same-directory-deployment

Ignore non-empty directories when cleanup

Author: Helen1987

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

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)