Fix existing Uploader specs

Our current specs base are in many cases, not testing uploads correctly.

There are many variations on how uploads behave, depending on which
Uploader and which model it is related to, but because our test cases
have mocked important pieces, it was never testing them.

This fixes that by removing mocks and making the required adjustments.

app/uploaders/avatar_uploader.rb

@@ -19,7 +19,7 @@ class AvatarUploader < GitlabUploader
   end
 
   def absolute_path
-    self.class.absolute_path(model.avatar.upload)
+    self.class.absolute_path(upload)
   end
 
   private

ee/spec/lib/gitlab/geo/replication/file_retriever_spec.rb

@@ -9,10 +9,6 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do
     context 'when the upload exists' do
       let(:retriever) { described_class.new(upload.id, message) }
 
-      before do
-        expect(Upload).to receive(:find_by).with(id: upload.id).and_return(upload)
-      end
-
       context 'when the upload has a file' do
         before do
           FileUtils.mkdir_p(File.dirname(upload.absolute_path))
@@ -24,7 +20,7 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do
 
           it 'returns the file in a success hash' do
             expect(subject).to include(code: :ok, message: 'Success')
-            expect(subject[:file].file).to eq(upload.absolute_path)
+            expect(subject[:file].file.path).to eq(upload.absolute_path)
           end
         end
 
@@ -74,19 +70,19 @@ describe Gitlab::Geo::Replication::FileRetriever, :geo do
   describe '#execute' do
     context 'user avatar' do
       it_behaves_like "returns necessary params for sending a file from an API endpoint" do
-        let(:upload) { create(:upload, model: build(:user)) }
+        let(:upload) { create(:upload, model: create(:user)) }
       end
     end
 
     context 'group avatar' do
       it_behaves_like "returns necessary params for sending a file from an API endpoint" do
-        let(:upload) { create(:upload, model: build(:group)) }
+        let(:upload) { create(:upload, model: create(:group)) }
       end
     end
 
     context 'project avatar' do
       it_behaves_like "returns necessary params for sending a file from an API endpoint" do
-        let(:upload) { create(:upload, model: build(:project)) }
+        let(:upload) { create(:upload, model: create(:project)) }
       end
     end
 

ee/spec/models/geo/upload_registry_spec.rb

@@ -39,9 +39,10 @@ describe Geo::UploadRegistry, :geo, :geo_fdw do
 
   describe '#file' do
     it 'returns the path of the upload of a registry' do
-      registry = create(:geo_upload_registry, :file, :with_file)
+      upload = create(:upload, :with_file)
+      registry = create(:geo_upload_registry, :file, file_id: upload.id)
 
-      expect(registry.file).to eq('uploads/-/system/project/avatar/avatar.jpg')
+      expect(registry.file).to eq("uploads/-/system/project/avatar/#{upload.id}/avatar.jpg")
     end
 
     it 'return "removed" message when the upload no longer exists' do

spec/factories/uploads.rb

@@ -2,7 +2,7 @@
 
 FactoryBot.define do
   factory :upload do
-    model { build(:project) }
+    model { create(:project) }
     size { 100.kilobytes }
     uploader { "AvatarUploader" }
     mount_point { :avatar }
@@ -11,23 +11,27 @@ FactoryBot.define do
 
     # we should build a mount agnostic upload by default
     transient do
-      filename { 'myfile.jpg' }
+      filename { 'avatar.jpg' }
     end
 
-    # this needs to comply with RecordsUpload::Concern#upload_path
-    path { File.join("uploads/-/system", model.class.underscore, mount_point.to_s, 'avatar.jpg') }
+    path do
+      uploader_instance = Object.const_get(uploader.to_s).new(model, mount_point)
+      File.join(uploader_instance.store_dir, filename)
+    end
 
     trait :personal_snippet_upload do
-      uploader { "PersonalFileUploader" }
+      model { create(:personal_snippet) }
       path { File.join(secret, filename) }
-      model { build(:personal_snippet) }
+      uploader { "PersonalFileUploader" }
       secret { SecureRandom.hex }
+      mount_point { nil }
     end
 
     trait :issuable_upload do
       uploader { "FileUploader" }
       path { File.join(secret, filename) }
       secret { SecureRandom.hex }
+      mount_point { nil }
     end
 
     trait :with_file do
@@ -42,22 +46,23 @@ FactoryBot.define do
     end
 
     trait :namespace_upload do
-      model { build(:group) }
+      model { create(:group) }
       path { File.join(secret, filename) }
       uploader { "NamespaceFileUploader" }
       secret { SecureRandom.hex }
+      mount_point { nil }
     end
 
     trait :favicon_upload do
-      model { build(:appearance) }
-      path { File.join(secret, filename) }
+      model { create(:appearance) }
       uploader { "FaviconUploader" }
       secret { SecureRandom.hex }
+      mount_point { :favicon }
     end
 
     trait :attachment_upload do
       mount_point { :attachment }
-      model { build(:note) }
+      model { create(:note) }
       uploader { "AttachmentUploader" }
     end
   end

spec/support/shared_examples/models/with_uploads_shared_examples.rb

@@ -41,7 +41,8 @@ shared_examples_for 'model with uploads' do |supports_fileuploads|
         end
 
         it 'deletes remote files' do
-          expect_any_instance_of(Uploads::Fog).to receive(:delete_keys).with(uploads.map(&:path))
+          expected_array = array_including(*uploads.map(&:path))
+          expect_any_instance_of(Uploads::Fog).to receive(:delete_keys).with(expected_array)
 
           model_object.destroy
         end

spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb

@@ -42,33 +42,23 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
     end
 
     describe '.sanity_check!' do
-      shared_examples 'raises a SanityCheckError' do
+      shared_examples 'raises a SanityCheckError' do |expected_message|
         let(:mount_point) { nil }
 
         it do
           expect { described_class.sanity_check!(uploads, model_class, mount_point) }
-            .to raise_error(described_class::SanityCheckError)
+            .to raise_error(described_class::SanityCheckError).with_message(expected_message)
         end
       end
 
-      before do
-        stub_const("WrongModel", Class.new)
-      end
-
       context 'uploader types mismatch' do
         let!(:outlier) { create(:upload, uploader: 'GitlabUploader') }
 
-        include_examples 'raises a SanityCheckError'
-      end
-
-      context 'model types mismatch' do
-        let!(:outlier) { create(:upload, model_type: 'WrongModel') }
-
-        include_examples 'raises a SanityCheckError'
+        include_examples 'raises a SanityCheckError', /Multiple uploaders found/
       end
 
       context 'mount point not found' do
-        include_examples 'raises a SanityCheckError' do
+        include_examples 'raises a SanityCheckError', /Mount point [a-z:]+ not found in/ do
           let(:mount_point) { :potato }
         end
       end