Move exception handling to execute

3 files changed, 30 insertions, 44 deletions

diff --git a/spec/features/projects/show_project_spec.rb b/spec/features/projects/show_project_spec.rb
index 5aa0d8f0026..1bc6fae9e7f 100644
--- a/spec/features/projects/show_project_spec.rb
+++ b/spec/features/projects/show_project_spec.rb
@@ -3,28 +3,18 @@ require 'spec_helper'
 describe 'Project show page', feature: true do
   context 'when project pending delete' do
     let(:project) { create(:project, :empty_repo, pending_delete: true) }
-    let(:worker) { ProjectDestroyWorker.new }
 
     before do
       sign_in(project.owner)
     end
 
-    it 'shows flash error if deletion for project fails' do
-      error_message = "some error message"
-      project.update_attributes(delete_error: error_message, pending_delete: false)
+    it 'shows error message if deletion for project fails' do
+      project.update_attributes(delete_error: "Something went wrong", pending_delete: false)
 
-      visit namespace_project_path(project.namespace, project)
+      visit project_path(project)
 
       expect(page).to have_selector('.project-deletion-failed-message')
-      expect(page).to have_content("This project was scheduled for deletion, but failed with the following message: #{error_message}")
-    end
-
-    it 'renders 404 if project was successfully deleted' do
-      worker.perform(project.id, project.owner.id, {})
-
-      visit namespace_project_path(project.namespace, project)
-
-      expect(page).to have_http_status(404)
+      expect(page).to have_content("This project was scheduled for deletion, but failed with the following message: #{project.delete_error}")
     end
   end
 end
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index a629afe723d..357e09bee95 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -130,30 +130,29 @@ describe Projects::DestroyService, services: true do
         it_behaves_like 'handles errors thrown during async destroy', "Failed to remove project repository"
       end
 
-      context 'when `execute` raises any other error' do
+      context 'when `execute` raises expected error' do
         before do
-          expect_any_instance_of(Projects::DestroyService)
-            .to receive(:execute).and_raise(ArgumentError.new("Other error message"))
+          expect_any_instance_of(Project)
+            .to receive(:destroy!).and_raise(StandardError.new("Other error message"))
         end
 
         it_behaves_like 'handles errors thrown during async destroy', "Other error message"
       end
-    end
-  end
 
-  context 'with execute' do
-    it_behaves_like 'deleting the project with pipeline and build'
+      context 'when `execute` raises unexpected error' do
+        before do
+          expect_any_instance_of(Project)
+            .to receive(:destroy!).and_raise(Exception.new("Other error message"))
+        end
 
-    context 'when `execute` raises an error' do
-      before do
-        expect_any_instance_of(Projects::DestroyService)
-          .to receive(:execute).and_raise(ArgumentError)
-      end
+        it 'allows error to bubble up and rolls back project deletion' do
+          expect do
+            Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+          end.to raise_error
 
-      it 'allows the error to bubble up' do
-        expect do
-          Sidekiq::Testing.inline! { Projects::DestroyService.new(project, user, {}).execute }
-        end.to raise_error(ArgumentError)
+          expect(project.reload.pending_delete).to be(false)
+          expect(project.delete_error).to include("Other error message")
+        end
       end
     end
   end
@@ -182,8 +181,7 @@ describe Projects::DestroyService, services: true do
           expect_any_instance_of(ContainerRepository)
             .to receive(:delete_tags!).and_return(false)
 
-          expect{ destroy_project(project, user) }
-            .to raise_error(ActiveRecord::RecordNotDestroyed)
+          expect(destroy_project(project, user)).to be false
         end
       end
     end
@@ -208,8 +206,7 @@ describe Projects::DestroyService, services: true do
           expect_any_instance_of(ContainerRepository)
             .to receive(:delete_tags!).and_return(false)
 
-          expect { destroy_project(project, user) }
-            .to raise_error(Projects::DestroyService::DestroyError)
+          expect(destroy_project(project, user)).to be false
         end
       end
     end
diff --git a/spec/workers/project_destroy_worker_spec.rb b/spec/workers/project_destroy_worker_spec.rb
index 29f0295de42..f19c9dff941 100644
--- a/spec/workers/project_destroy_worker_spec.rb
+++ b/spec/workers/project_destroy_worker_spec.rb
@@ -21,17 +21,16 @@ describe ProjectDestroyWorker do
       expect(Dir.exist?(path)).to be_truthy
     end
 
-    describe 'when StandardError is raised' do
-      it 'reverts pending_delete attribute with a error message' do
-        allow_any_instance_of(::Projects::DestroyService).to receive(:execute).and_raise(StandardError, "some error message")
-
-        expect do
-          subject.perform(project.id, project.owner.id, {})
-        end.to change { project.reload.pending_delete }.from(true).to(false)
+    it 'does not raise error when project could not be found' do
+      expect do
+        subject.perform(-1, project.owner.id, {})
+      end.not_to raise_error
+    end
 
-        expect(Project.all).to include(project)
-        expect(project.delete_error).to eq("some error message")
-      end
+    it 'does not raise error when user could not be found' do
+      expect do
+        subject.perform(project.id, -1, {})
+      end.not_to raise_error
     end
   end
 end