Close ALL the files

Make sure that we close opened files after we're done with them.

Close ALL the files
Make sure that we close opened files after we're done with them.
4f6d482d · Prem Sichanugrist · 02eb7259 · 4f6d482d · 4f6d482d · 4f6d482d
Commit 4f6d482d authored Apr 17, 2012 by Prem Sichanugrist
10 changed files
--- a/test/attachment_adapter_test.rb
+++ b/test/attachment_adapter_test.rb
@@ -12,6 +12,10 @@ class AttachmentAdapterTest < Test::Unit::TestCase
    @subject = Paperclip.io_adapters.for(@attachment)
  end
+  def teardown
+    @file.close
+  end
  should "get the right filename" do
    assert_equal "5k.png", @subject.original_filename
  end

--- a/test/attachment_test.rb
+++ b/test/attachment_test.rb
@@ -1120,6 +1120,8 @@ class AttachmentTest < Test::Unit::TestCase
      @path = @attachment.path
    end
+    teardown { @file.close }
    should "not delete the files from storage when attachment is destroyed" do
      @attachment.destroy
      assert File.exists?(@path)
@@ -1142,6 +1144,8 @@ class AttachmentTest < Test::Unit::TestCase
      @path = @attachment.path
    end
+    teardown { @file.close }
    should "not be deleted when the model fails to destroy" do
      @dummy.stubs(:destroy).raises(Exception)

--- a/test/file_adapter_test.rb
+++ b/test/file_adapter_test.rb
@@ -2,12 +2,15 @@ require './test/helper'
 class FileAdapterTest < Test::Unit::TestCase
  context "a new instance" do
+    context "with normal file" do
      setup do
        @file = File.new(fixture_file("5k.png"))
        @file.binmode
        @subject = Paperclip.io_adapters.for(@file)
      end
+      teardown { @file.close }
      should "get the right filename" do
        assert_equal "5k.png", @subject.original_filename
      end
@@ -20,13 +23,6 @@ class FileAdapterTest < Test::Unit::TestCase
        assert_equal "image/png", @subject.content_type
      end
-    should "provide correct mime-type for empty file" do
-      @subject = Paperclip.io_adapters.for(Tempfile.new("file_adapter_test"))
-      # Content type contained '\n' at the end, for an empty file, on my Mac
-      assert_equal "application/x-empty", @subject.content_type
-    end
      should "get the file's size" do
        assert_equal 4456, @subject.size
      end
@@ -45,6 +41,19 @@ class FileAdapterTest < Test::Unit::TestCase
        assert expected.length > 0
        assert_equal expected, @subject.read
      end
+    end
+    context "empty file" do
+      setup do
+        @file = Tempfile.new("file_adapter_test")
+        @subject = Paperclip.io_adapters.for(@file)
+      end
+      teardown { @file.close }
+      should "provide correct mime-type" do
+        assert_equal "application/x-empty", @subject.content_type
+      end
+    end
  end
 end
--- a/test/integration_test.rb
+++ b/test/integration_test.rb
@@ -10,6 +10,8 @@ class IntegrationTest < Test::Unit::TestCase
      end
    end
+    teardown { @file.close }
    should "not exceed the open file limit" do
       assert_nothing_raised do
         dummies = Dummy.find(:all)
@@ -287,6 +289,7 @@ class IntegrationTest < Test::Unit::TestCase
      teardown do
        File.umask @umask
+        @file.close
      end
      should "respect the current umask" do
@@ -314,6 +317,8 @@ class IntegrationTest < Test::Unit::TestCase
      assert @dummy.save
    end
+    teardown { [@file, @bad_file].each(&:close) }
    should "write and delete its files" do
      [["434x66", :original],
       ["300x46", :large],
@@ -407,6 +412,8 @@ class IntegrationTest < Test::Unit::TestCase
        @dummy2.save
      end
+      teardown { @file2.close }
      should "work when assigned a file" do
        assert_not_equal `identify -format "%wx%h" "#{@dummy.avatar.path(:original)}"`,
                         `identify -format "%wx%h" "#{@dummy2.avatar.path(:original)}"`
@@ -427,18 +434,15 @@ class IntegrationTest < Test::Unit::TestCase
        has_many :attachments, :class_name => 'Dummy'
      end
+      @file = File.new(File.join(File.dirname(__FILE__), "fixtures", "5k.png"), 'rb')
      @dummy = Dummy.new
-      @dummy.avatar = File.new(File.join(File.dirname(__FILE__),
+      @dummy.avatar = @file
-                               "fixtures",
-                               "5k.png"), 'rb')
    end
    should "should not error when saving" do
-      assert_nothing_raised do
      @dummy.save!
    end
  end
-  end
  if ENV['S3_TEST_BUCKET']
    def s3_files_for attachment
@@ -482,6 +486,12 @@ class IntegrationTest < Test::Unit::TestCase
        @files_on_s3 = s3_files_for @dummy.avatar
      end
+      teardown do
+        @file.close
+        @bad_file.close
+        @files_on_s3.values.each(&:close)
+      end
      context 'assigning itself to a new model' do
        setup do
          @d2 = Dummy.new

--- a/test/paperclip_test.rb
+++ b/test/paperclip_test.rb
@@ -67,6 +67,8 @@ class PaperclipTest < Test::Unit::TestCase
      @expected = [d1, d3]
    end
+    teardown { @file.close }
    should "yield every instance of a model that has an attachment" do
      actual = []
      Paperclip.each_instance_with_attachment("Dummy", "avatar") do |instance|

--- a/test/storage/filesystem_test.rb
+++ b/test/storage/filesystem_test.rb
@@ -2,13 +2,17 @@ require './test/helper'
 class FileSystemTest < Test::Unit::TestCase
  context "Filesystem" do
+    context "normal file" do
      setup do
        rebuild_model :styles => { :thumbnail => "25x25#" }
        @dummy = Dummy.create!
-      @dummy.avatar = File.open(fixture_file('5k.png'))
+        @file = File.open(fixture_file('5k.png'))
+        @dummy.avatar = @file
      end
+      teardown { @file.close }
      should "allow file assignment" do
        assert @dummy.save
      end
@@ -22,16 +26,20 @@ class FileSystemTest < Test::Unit::TestCase
        @dummy.save
        assert File.exists?(@dummy.avatar.path(:thumbnail))
      end
+    end
    context "with file that has space in file name" do
      setup do
        rebuild_model :styles => { :thumbnail => "25x25#" }
        @dummy = Dummy.create!
-        @dummy.avatar = File.open(fixture_file('spaced file.png'))
+        @file = File.open(fixture_file('spaced file.png'))
+        @dummy.avatar = @file
        @dummy.save
      end
+      teardown { @file.close }
      should "store the file" do
        assert File.exists?(@dummy.avatar.path)
      end

--- a/test/storage/fog_test.rb
+++ b/test/storage/fog_test.rb
@@ -12,10 +12,13 @@ class FogTest < Test::Unit::TestCase
                      :url => '/:attachment/:filename',
                      :fog_directory => "paperclip",
                      :fog_credentials => fixture_file('fog.yml')
+        @file = File.new(fixture_file('5k.png'), 'rb')
        @dummy = Dummy.new
-        @dummy.avatar = File.new(fixture_file('5k.png'), 'rb')
+        @dummy.avatar = @file
      end
+      teardown { @file.close }
      should "have the proper information loading credentials from a file" do
        assert_equal @dummy.avatar.fog_credentials[:provider], 'AWS'
      end
@@ -28,10 +31,13 @@ class FogTest < Test::Unit::TestCase
                      :url => '/:attachment/:filename',
                      :fog_directory => "paperclip",
                      :fog_credentials => File.open(fixture_file('fog.yml'))
+        @file = File.new(fixture_file('5k.png'), 'rb')
        @dummy = Dummy.new
-        @dummy.avatar = File.new(fixture_file('5k.png'), 'rb')
+        @dummy.avatar = @file
      end
+      teardown { @file.close }
      should "have the proper information loading credentials from a file" do
        assert_equal @dummy.avatar.fog_credentials[:provider], 'AWS'
      end
@@ -48,9 +54,13 @@ class FogTest < Test::Unit::TestCase
                        :aws_access_key_id => 'AWS_ID',
                        :aws_secret_access_key => 'AWS_SECRET'
                      }
+        @file = File.new(fixture_file('5k.png'), 'rb')
        @dummy = Dummy.new
-        @dummy.avatar = File.new(fixture_file('5k.png'), 'rb')
+        @dummy.avatar = @file
      end
+      teardown { @file.close }
      should "be able to interpolate the path without blowing up" do
        assert_equal File.expand_path(File.join(File.dirname(__FILE__), "../../public/avatars/5k.png")),
                     @dummy.avatar.path

--- a/test/storage/s3_live_test.rb
+++ b/test/storage/s3_live_test.rb
@@ -5,11 +5,12 @@ unless ENV["S3_BUCKET"].blank?
  class S3LiveTest < Test::Unit::TestCase
    context "when assigning an S3 attachment directly to another model" do
      setup do
+        @s3_credentials = File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml"))
        rebuild_model :styles => { :thumb => "100x100", :square => "32x32#" },
                      :storage => :s3,
                      :bucket => ENV["S3_BUCKET"],
                      :path => ":class/:attachment/:id/:style.:extension",
-                      :s3_credentials => File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml"))
+                      :s3_credentials => @s3_credentials
        @dummy = Dummy.new
        @attachment = Dummy.new.avatar
@@ -21,16 +22,18 @@ unless ENV["S3_BUCKET"].blank?
        @attachment2.assign(@file)
        @attachment2.save
      end
-    end
+      teardown { [@s3_credentials, @file].each(&:close) }
+    end
    context "Generating an expiring url on a nonexistant attachment" do
      setup do
+        @s3_credentials = File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml"))
        rebuild_model :styles => { :thumb => "100x100", :square => "32x32#" },
                      :storage => :s3,
                      :bucket => ENV["S3_BUCKET"],
                      :path => ":class/:attachment/:id/:style.:extension",
-                      :s3_credentials => File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml"))
+                      :s3_credentials => @s3_credentials
        @dummy = Dummy.new
      end
@@ -41,16 +44,19 @@ unless ENV["S3_BUCKET"].blank?
    context "Using S3 for real, an attachment with S3 storage" do
      setup do
+        @s3_credentials = File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml"))
        rebuild_model :styles => { :thumb => "100x100", :square => "32x32#" },
                      :storage => :s3,
                      :bucket => ENV["S3_BUCKET"],
                      :path => ":class/:attachment/:id/:style.:extension",
-                      :s3_credentials => File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml"))
+                      :s3_credentials => @s3_credentials
        Dummy.delete_all
        @dummy = Dummy.new
      end
+      teardown { @s3_credentials.close }
      should "be extended by the S3 module" do
        assert Dummy.new.avatar.is_a?(Paperclip::Storage::S3)
      end
@@ -80,17 +86,21 @@ unless ENV["S3_BUCKET"].blank?
    context "An attachment that uses S3 for storage and has spaces in file name" do
      setup do
+        @s3_credentials = File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml"))
        rebuild_model :styles => { :thumb => "100x100", :square => "32x32#" },
          :storage => :s3,
          :bucket => ENV["S3_BUCKET"],
-          :s3_credentials => File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml"))
+          :s3_credentials => @s3_credentials
        Dummy.delete_all
+        @file = File.new(fixture_file('spaced file.png'), 'rb')
        @dummy = Dummy.new
-        @dummy.avatar = File.new(fixture_file('spaced file.png'), 'rb')
+        @dummy.avatar = @file
        @dummy.save
      end
+      teardown { @s3_credentials.close }
      should "return a replaced version for path" do
        assert_match /.+\/spaced_file\.png/, @dummy.avatar.path
      end
@@ -116,17 +126,20 @@ unless ENV["S3_BUCKET"].blank?
    context "An attachment that uses S3 for storage and uses AES256 encryption" do
      setup do
+        @s3_credentials = File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml"))
        rebuild_model :styles => { :thumb => "100x100", :square => "32x32#" },
                      :storage => :s3,
                      :bucket => ENV["S3_BUCKET"],
                      :path => ":class/:attachment/:id/:style.:extension",
-                      :s3_credentials => File.new(File.join(File.dirname(__FILE__), "..", "fixtures", "s3.yml")),
+                      :s3_credentials => @s3_credentials,
                      :s3_server_side_encryption => :aes256
        Dummy.delete_all
        @dummy = Dummy.new
      end
+      teardown { @s3_credentials.close }
      context "when assigned" do
        setup do
          @file = File.new(fixture_file('5k.png'), 'rb')

--- a/test/storage/s3_test.rb
+++ b/test/storage/s3_test.rb
@@ -186,8 +186,10 @@ class S3Test < Test::Unit::TestCase
                      'secret_access_key' => "54321"
                    }
+      File.open(fixture_file('5k.png'), 'rb') do |file|
        @dummy = Dummy.new
-      @dummy.avatar = File.new(fixture_file('5k.png'), 'rb')
+        @dummy.avatar = file
+      end
    end
    should "return a url containing the correct original file mime type" do
@@ -217,8 +219,10 @@ class S3Test < Test::Unit::TestCase
                      'secret_access_key' => "54321"
                    }
+      File.open(fixture_file('spaced file.png'), 'rb') do |file|
        @dummy = Dummy.new
-      @dummy.avatar = File.new(fixture_file('spaced file.png'), 'rb')
+        @dummy.avatar = file
+      end
    end
    should "return a replaced version for path" do

--- a/test/thumbnail_test.rb
+++ b/test/thumbnail_test.rb
@@ -7,6 +7,8 @@ class ThumbnailTest < Test::Unit::TestCase
      @tempfile = Paperclip::Tempfile.new(["file", ".jpg"])
    end
+    teardown { @tempfile.close }
    should "have its path contain a real extension" do
      assert_equal ".jpg", File.extname(@tempfile.path)
    end
@@ -21,6 +23,8 @@ class ThumbnailTest < Test::Unit::TestCase
      @tempfile = Paperclip::Tempfile.new("file")
    end
+    teardown { @tempfile.close }
    should "not have an extension if not given one" do
      assert_equal "", File.extname(@tempfile.path)
    end