Add latest changes from gitlab-org/gitlab@master

2 files changed, 119 insertions, 90 deletions

diff --git a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb
index ddcf922ad68..c1ac5ffebe8 100644
--- a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb
+++ b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb
@@ -21,6 +21,12 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
     end.new
   end
 
+  let_it_be(:project) { create(:project, :import_started) }
+
+  let(:importer_class) { double(:importer_class, name: 'klass_name') }
+  let(:importer_instance) { double(:importer_instance) }
+  let(:client) { double(:client) }
+
   before do
     stub_const('MockRepresantation', Class.new do
       include Gitlab::GithubImport::Representation::ToHash
@@ -39,11 +45,6 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
   end
 
   describe '#import', :clean_gitlab_redis_cache do
-    let(:importer_class) { double(:importer_class, name: 'klass_name') }
-    let(:importer_instance) { double(:importer_instance) }
-    let(:project) { double(:project, full_path: 'foo/bar', id: 1) }
-    let(:client) { double(:client) }
-
     before do
       expect(worker)
         .to receive(:importer_class)
@@ -65,7 +66,7 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
         .with(
           github_id: 1,
           message: 'starting importer',
-          project_id: 1,
+          project_id: project.id,
           importer: 'klass_name'
         )
 
@@ -74,7 +75,7 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
         .with(
           github_id: 1,
           message: 'importer finished',
-          project_id: 1,
+          project_id: project.id,
           importer: 'klass_name'
         )
 
@@ -106,59 +107,36 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
           importer: 'klass_name'
         )
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:error)
+      expect(Gitlab::Import::ImportFailureService)
+        .to receive(:track)
         .with(
-          github_id:  1,
-          message: 'importer failed',
           project_id: project.id,
-          importer: 'klass_name',
-          'error.message': 'some error',
-          'github.data': {
-            'github_id' => 1,
-            'number' => 10
-          }
+          exception: exception,
+          error_source: 'klass_name'
         )
+        .and_call_original
 
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_and_raise_exception)
-        .with(
-          exception,
-          import_source: :github,
-          github_id: 1,
-          project_id: 1,
-          importer: 'klass_name'
-        ).and_call_original
+      worker.import(project, client, { 'number' => 10, 'github_id' => 1 })
 
-      expect { worker.import(project, client, { 'number' => 10, 'github_id' => 1 }) }
-        .to raise_error(exception)
+      expect(project.import_state.reload.status).to eq('started')
+
+      expect(project.import_failures).not_to be_empty
+      expect(project.import_failures.last.exception_class).to eq('StandardError')
+      expect(project.import_failures.last.exception_message).to eq('some error')
     end
 
     it 'logs error when representation does not have a github_id' do
       expect(importer_class).not_to receive(:new)
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:error)
+      expect(Gitlab::Import::ImportFailureService)
+        .to receive(:track)
         .with(
-          github_id:  nil,
-          message: 'importer failed',
           project_id: project.id,
-          importer: 'klass_name',
-          'error.message': 'key not found: :github_id',
-          'github.data': {
-            'number' => 10
-          }
+          exception: a_kind_of(KeyError),
+          error_source: 'klass_name',
+          fail_import: true
         )
-
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_and_raise_exception)
-        .with(
-          an_instance_of(KeyError),
-          import_source: :github,
-          github_id: nil,
-          project_id: 1,
-          importer: 'klass_name'
-        ).and_call_original
+        .and_call_original
 
       expect { worker.import(project, client, { 'number' => 10 }) }
         .to raise_error(KeyError, 'key not found: :github_id')
diff --git a/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb b/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb
index c0e2df6f985..aeb86f5aa8c 100644
--- a/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb
+++ b/spec/workers/concerns/gitlab/github_import/stage_methods_spec.rb
@@ -3,7 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::GithubImport::StageMethods do
-  let(:project) { create(:project) }
+  let_it_be(:project) { create(:project, :import_started, import_url: 'https://t0ken@github.com/repo/repo.git') }
+
   let(:worker) do
     Class.new do
       def self.name
@@ -15,8 +16,6 @@ RSpec.describe Gitlab::GithubImport::StageMethods do
   end
 
   describe '#perform' do
-    let(:project) { create(:project, import_url: 'https://t0ken@github.com/repo/repo.git') }
-
     it 'returns if no project could be found' do
       expect(worker).not_to receive(:try_import)
 
@@ -55,46 +54,100 @@ RSpec.describe Gitlab::GithubImport::StageMethods do
       worker.perform(project.id)
     end
 
-    it 'logs error when import fails' do
-      exception = StandardError.new('some error')
-
-      allow(worker)
-        .to receive(:find_project)
-        .with(project.id)
-        .and_return(project)
-
-      expect(worker)
-        .to receive(:try_import)
-        .and_raise(exception)
+    context 'when abort_on_failure is false' do
+      it 'logs error when import fails' do
+        exception = StandardError.new('some error')
+
+        allow(worker)
+          .to receive(:find_project)
+          .with(project.id)
+          .and_return(project)
+
+        expect(worker)
+          .to receive(:try_import)
+          .and_raise(exception)
+
+        expect(Gitlab::GithubImport::Logger)
+          .to receive(:info)
+          .with(
+            message: 'starting stage',
+            project_id: project.id,
+            import_stage: 'DummyStage'
+          )
+
+        expect(Gitlab::Import::ImportFailureService)
+          .to receive(:track)
+          .with(
+            project_id: project.id,
+            exception: exception,
+            error_source: 'DummyStage',
+            fail_import: false
+          ).and_call_original
+
+        expect { worker.perform(project.id) }
+          .to raise_error(exception)
+
+        expect(project.import_state.reload.status).to eq('started')
+
+        expect(project.import_failures).not_to be_empty
+        expect(project.import_failures.last.exception_class).to eq('StandardError')
+        expect(project.import_failures.last.exception_message).to eq('some error')
+      end
+    end
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:info)
-        .with(
-          message: 'starting stage',
-          project_id: project.id,
-          import_stage: 'DummyStage'
-        )
+    context 'when abort_on_failure is true' do
+      let(:worker) do
+        Class.new do
+          def self.name
+            'DummyStage'
+          end
 
-      expect(Gitlab::GithubImport::Logger)
-        .to receive(:error)
-        .with(
-          message: 'stage failed',
-          project_id: project.id,
-          import_stage: 'DummyStage',
-          'error.message': 'some error'
-        )
+          def abort_on_failure
+            true
+          end
 
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_and_raise_exception)
-        .with(
-          exception,
-          import_source: :github,
-          project_id: project.id,
-          import_stage: 'DummyStage'
-        )
-        .and_call_original
+          include(Gitlab::GithubImport::StageMethods)
+        end.new
+      end
 
-      expect { worker.perform(project.id) }.to raise_error(exception)
+      it 'logs, captures and re-raises the exception and also marks the import as failed' do
+        exception = StandardError.new('some error')
+
+        allow(worker)
+          .to receive(:find_project)
+          .with(project.id)
+          .and_return(project)
+
+        expect(worker)
+          .to receive(:try_import)
+          .and_raise(exception)
+
+        expect(Gitlab::GithubImport::Logger)
+          .to receive(:info)
+          .with(
+            message: 'starting stage',
+            project_id: project.id,
+            import_stage: 'DummyStage'
+          )
+
+        expect(Gitlab::Import::ImportFailureService)
+          .to receive(:track)
+          .with(
+            project_id: project.id,
+            exception: exception,
+            error_source: 'DummyStage',
+            fail_import: true
+          ).and_call_original
+
+        expect { worker.perform(project.id) }.to raise_error(exception)
+
+        expect(project.import_state.reload.status).to eq('failed')
+        expect(project.import_state.last_error).to eq('some error')
+
+        expect(project.import_failures).not_to be_empty
+        expect(project.import_failures.last.exception_class).to eq('StandardError')
+        expect(project.import_failures.last.exception_message).to eq('some error')
+      end
     end
   end
 
@@ -126,16 +179,14 @@ RSpec.describe Gitlab::GithubImport::StageMethods do
   end
 
   describe '#find_project' do
-    let(:import_state) { create(:import_state, project: project) }
-
     it 'returns a Project for an existing ID' do
-      import_state.update_column(:status, 'started')
+      project.import_state.update_column(:status, 'started')
 
       expect(worker.find_project(project.id)).to eq(project)
     end
 
     it 'returns nil for a project that failed importing' do
-      import_state.update_column(:status, 'failed')
+      project.import_state.update_column(:status, 'failed')
 
       expect(worker.find_project(project.id)).to be_nil
     end