From 776036e60c434ed2aad091874a60b5f5ce67a664 Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Wed, 30 Aug 2017 16:55:52 -0400 Subject: [PATCH 1/2] Create tempfile using the basename without extension: - 5f7845118f649b60862a9bc01f6b2a0f932e89d7 introduced a change to keep the filename extension when creating the tempfile - `Tempfile#path` is now returning the filename with the extension followed by random chars and the extension again ```ruby puts Rack::Test::UploadedFile.new('foo.txt').path # => '/var/xxx/foo.txt2017-30-08-dsad.txt' ``` - This PR modifies this behaviour to not include the file extension as part of the file basename when creating the Tempfile --- lib/rack/test/uploaded_file.rb | 3 ++- spec/rack/test/uploaded_file_spec.rb | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/rack/test/uploaded_file.rb b/lib/rack/test/uploaded_file.rb index 13a9245a..9d3b9757 100644 --- a/lib/rack/test/uploaded_file.rb +++ b/lib/rack/test/uploaded_file.rb @@ -62,8 +62,9 @@ def initialize_from_file_path(path) raise "#{path} file does not exist" unless ::File.exist?(path) @original_filename = ::File.basename(path) + extension = ::File.extname(@original_filename) - @tempfile = Tempfile.new([@original_filename, ::File.extname(path)]) + @tempfile = Tempfile.new([::File.basename(@original_filename, extension), extension]) @tempfile.set_encoding(Encoding::BINARY) if @tempfile.respond_to?(:set_encoding) ObjectSpace.define_finalizer(self, self.class.finalize(@tempfile)) diff --git a/spec/rack/test/uploaded_file_spec.rb b/spec/rack/test/uploaded_file_spec.rb index e6e138cb..7fbcd86a 100644 --- a/spec/rack/test/uploaded_file_spec.rb +++ b/spec/rack/test/uploaded_file_spec.rb @@ -25,6 +25,12 @@ def test_file_path expect(File.extname(uploaded_file.path)).to eq('.txt') end + it 'creates Tempfiles with a path that includes a single extension' do + uploaded_file = Rack::Test::UploadedFile.new(test_file_path) + + expect(uploaded_file.path).to_not match(/foo.txt/) + end + context 'it should call its destructor' do it 'calls the destructor' do expect(Rack::Test::UploadedFile).to receive(:actually_finalize).at_least(:once) From 5cf353d9a1f7e622973931de538a868507f0fa7e Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Thu, 30 Nov 2017 11:36:21 -0500 Subject: [PATCH 2/2] PR Review --- spec/rack/test/uploaded_file_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/rack/test/uploaded_file_spec.rb b/spec/rack/test/uploaded_file_spec.rb index 7fbcd86a..59f688b3 100644 --- a/spec/rack/test/uploaded_file_spec.rb +++ b/spec/rack/test/uploaded_file_spec.rb @@ -28,7 +28,8 @@ def test_file_path it 'creates Tempfiles with a path that includes a single extension' do uploaded_file = Rack::Test::UploadedFile.new(test_file_path) - expect(uploaded_file.path).to_not match(/foo.txt/) + regex = /foo#{Time.now.year}.*\.txt\Z/ + expect(uploaded_file.path).to match(regex) end context 'it should call its destructor' do