FIX: randomize file name when created from fixtures (#9731)

* FIX: randomize file name when created from fixtures

When a temporary file is created from fixtures it should have a unique name.
It is to prevent a collision in parallel specs evaluation

* FIX: use /tmp/pid folder to keep fixture files

spec/rails_helper.rb

@@ -237,6 +237,12 @@ RSpec.configure do |config|
     end
   end
 
+  config.after(:suite) do
+    if SpecSecureRandom.value
+      FileUtils.remove_dir(file_from_fixtures_tmp_folder, true)
+    end
+  end
+
   config.before :each, &TestSetup.method(:test_setup)
 
   config.before(:each, type: :multisite) do
@@ -368,9 +374,15 @@ def unfreeze_time
 end
 
 def file_from_fixtures(filename, directory = "images")
-  FileUtils.mkdir_p("#{Rails.root}/tmp/spec") unless Dir.exists?("#{Rails.root}/tmp/spec")
-  FileUtils.cp("#{Rails.root}/spec/fixtures/#{directory}/#{filename}", "#{Rails.root}/tmp/spec/#{filename}")
-  File.new("#{Rails.root}/tmp/spec/#{filename}")
+  SpecSecureRandom.value ||= SecureRandom.hex
+  FileUtils.mkdir_p(file_from_fixtures_tmp_folder) unless Dir.exists?(file_from_fixtures_tmp_folder)
+  tmp_file_path = File.join(file_from_fixtures_tmp_folder, SecureRandom.hex << filename)
+  FileUtils.cp("#{Rails.root}/spec/fixtures/#{directory}/#{filename}", tmp_file_path)
+  File.new(tmp_file_path)
+end
+
+def file_from_fixtures_tmp_folder
+  File.join(Dir.tmpdir, "rspec_#{Process.pid}_#{SpecSecureRandom.value}")
 end
 
 def has_trigger?(trigger_name)
@@ -410,3 +422,9 @@ def track_log_messages(level: nil)
 ensure
   Rails.logger = old_logger
 end
+
+class SpecSecureRandom
+  class << self
+    attr_accessor :value
+  end
+end

spec/requests/admin/themes_controller_spec.rb

@@ -24,22 +24,24 @@ describe Admin::ThemesController do
   end
 
   describe '#upload_asset' do
+    let(:file) { file_from_fixtures("fake.woff2", "woff2") }
+    let(:filename) { File.basename(file) }
     let(:upload) do
-      Rack::Test::UploadedFile.new(file_from_fixtures("fake.woff2", "woff2"))
+      Rack::Test::UploadedFile.new(file)
     end
 
     it 'can create a theme upload' do
       post "/admin/themes/upload_asset.json", params: { file: upload }
       expect(response.status).to eq(201)
 
-      upload = Upload.find_by(original_filename: "fake.woff2")
+      upload = Upload.find_by(original_filename: filename)
 
       expect(upload.id).not_to be_nil
       expect(response.parsed_body["upload_id"]).to eq(upload.id)
     end
 
     context "when trying to upload an existing file" do
-      let(:uploaded_file) { Upload.find_by(original_filename: "fake.woff2") }
+      let(:uploaded_file) { Upload.find_by(original_filename: filename) }
       let(:response_json) { response.parsed_body }
 
       before do

spec/requests/uploads_controller_spec.rb

@@ -16,9 +16,11 @@ describe UploadsController do
         sign_in(user)
       end
 
+      let(:logo_file) { file_from_fixtures("logo.png") }
       let(:logo) do
-        Rack::Test::UploadedFile.new(file_from_fixtures("logo.png"))
+        Rack::Test::UploadedFile.new(logo_file)
       end
+      let(:logo_filename) { File.basename(logo_file) }
 
       let(:fake_jpg) do
         Rack::Test::UploadedFile.new(file_from_fixtures("fake.jpg"))
@@ -160,7 +162,7 @@ describe UploadsController do
         upload = Upload.last
 
         expect(upload.id).to eq(id)
-        expect(upload.original_filename).to eq('logo.png')
+        expect(upload.original_filename).to eq(logo_filename)
       end
 
       it 'respects `authorized_extensions_for_staff` setting when staff upload file' do
@@ -265,7 +267,7 @@ describe UploadsController do
       expect(response.status).to eq(200)
 
       expect(response.headers["Content-Disposition"])
-        .to eq(%Q|attachment; filename="logo.png"; filename*=UTF-8''logo.png|)
+        .to eq(%Q|attachment; filename="#{upload.original_filename}"; filename*=UTF-8''#{upload.original_filename}|)
     end
 
     it 'returns 200 when js file' do
@@ -282,7 +284,7 @@ describe UploadsController do
       get "/uploads/#{site}/#{upload.sha1}.json"
       expect(response.status).to eq(200)
       expect(response.headers["Content-Disposition"])
-        .to eq(%Q|attachment; filename="image_no_extension.png"; filename*=UTF-8''image_no_extension.png|)
+        .to eq(%Q|attachment; filename="#{upload.original_filename}"; filename*=UTF-8''#{upload.original_filename}|)
     end
 
     it "handles file without extension" do
@@ -292,7 +294,7 @@ describe UploadsController do
       get "/uploads/#{site}/#{upload.sha1}.json"
       expect(response.status).to eq(200)
       expect(response.headers["Content-Disposition"])
-        .to eq(%Q|attachment; filename="not_an_image"; filename*=UTF-8''not_an_image|)
+        .to eq(%Q|attachment; filename="#{upload.original_filename}"; filename*=UTF-8''#{upload.original_filename}|)
     end
 
     context "prevent anons from downloading files" do