Create utility method to clean up temporary directory that this created

Author: kpaulisse

lib/octocatalog-diff/util/util.rb

@@ -68,14 +68,27 @@ def self.temp_dir(prefix = 'ocd-', basedir = ENV['OCTOCATALOG_DIFF_TEMPDIR'])
         # If the base directory was not specified, then create a temporary directory, and
         # send the `at_exit` to clean it up at the conclusion.
         the_dir = Dir.mktmpdir(prefix)
-        at_exit do
+        at_exit { remove_temp_dir(the_dir) }
+        the_dir
+      end
+
+      # Utility method!
+      # Remove a directory recursively that has been used as a temporary directory. This
+      # should be called within an `at_exit` handler, and is only intended to be called via the
+      # `temp_dir` method above.
+      #
+      # dir - A String with the directory to remove.
+      def self.remove_temp_dir(dir)
+        retries = 0
+        while File.directory?(dir) && retries < 10
+          retries += 1
           begin
-            FileUtils.remove_entry_secure(the_dir) if File.directory?(the_dir)
-          rescue Errno::ENOENT # rubocop:disable Lint/HandleExceptions
-            # OK if the directory doesn't exist since we're trying to remove it anyway
+            FileUtils.remove_entry_secure(dir)
+          rescue Errno::ENOTEMPTY, Errno::ENOENT # rubocop:disable Lint/HandleExceptions
+            # Errno::ENOTEMPTY will trigger a retry because the directory exists
+            # Errno::ENOENT will break the loop because the directory won't exist next time it's checked
           end
         end
-        the_dir
       end
     end
   end

spec/octocatalog-diff/tests/util/util_spec.rb

@@ -109,4 +109,37 @@
       expect { described_class.temp_dir }.to raise_error(Errno::ENOENT, /temp_dir: Base dir/)
     end
   end
+
+  describe '#remove_temp_dir' do
+    let(:dir) { '/var/tmp/abcd/efgh/ijkl' }
+
+    it 'should do nothing if the directory does not exist' do
+      allow(File).to receive(:directory?).and_call_original
+      allow(File).to receive(:directory?).with(dir).and_return(false)
+      expect(FileUtils).not_to receive(:remove_entry_secure).with(dir)
+      described_class.remove_temp_dir(dir)
+    end
+
+    it 'should do nothing if Errno::ENOENT is raised due to race conditions' do
+      allow(File).to receive(:directory?).and_call_original
+      allow(File).to receive(:directory?).with(dir).and_return(true, false)
+      expect(FileUtils).to receive(:remove_entry_secure).with(dir).exactly(1).times.and_raise(Errno::ENOENT)
+      described_class.remove_temp_dir(dir)
+    end
+
+    it 'should retry if Errno::ENOTEMPTY is raised' do
+      allow(File).to receive(:directory?).and_call_original
+      allow(File).to receive(:directory?).with(dir).and_return(true, false)
+
+      call_count = 0
+      allow(FileUtils).to receive(:remove_entry_secure).with(dir) do
+        call_count += 1
+        raise Errno::ENOTEMPTY if call_count == 1
+        raise 'This should not occur' unless call_count == 2
+        true
+      end
+
+      described_class.remove_temp_dir(dir)
+    end
+  end
 end