Add latest changes from gitlab-org/gitlab@14-2-stable-eev14.2.0-rc42

2 files changed, 146 insertions, 132 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 4c96daea7b3..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
@@ -38,12 +44,7 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
     end)
   end
 
-  describe '#import', :clean_gitlab_redis_shared_state 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) }
-
+  describe '#import', :clean_gitlab_redis_cache do
     before do
       expect(worker)
         .to receive(:importer_class)
@@ -60,26 +61,23 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
       expect(importer_instance)
         .to receive(:execute)
 
-      expect_next_instance_of(Gitlab::Import::Logger) do |logger|
-        expect(logger)
-          .to receive(:info)
-          .with(
-            github_id: 1,
-            message: 'starting importer',
-            import_source: :github,
-            project_id: 1,
-            importer: 'klass_name'
-          )
-        expect(logger)
-          .to receive(:info)
-          .with(
-            github_id: 1,
-            message: 'importer finished',
-            import_source: :github,
-            project_id: 1,
-            importer: 'klass_name'
-          )
-      end
+      expect(Gitlab::GithubImport::Logger)
+        .to receive(:info)
+        .with(
+          github_id: 1,
+          message: 'starting importer',
+          project_id: project.id,
+          importer: 'klass_name'
+        )
+
+      expect(Gitlab::GithubImport::Logger)
+        .to receive(:info)
+        .with(
+          github_id: 1,
+          message: 'importer finished',
+          project_id: project.id,
+          importer: 'klass_name'
+        )
 
       worker.import(project, client, { 'number' => 10, 'github_id' => 1 })
 
@@ -100,74 +98,45 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter do
         .to receive(:execute)
         .and_raise(exception)
 
-      expect_next_instance_of(Gitlab::Import::Logger) do |logger|
-        expect(logger)
-          .to receive(:info)
-          .with(
-            github_id: 1,
-            message: 'starting importer',
-            import_source: :github,
-            project_id: project.id,
-            importer: 'klass_name'
-          )
-        expect(logger)
-          .to receive(:error)
-          .with(
-            github_id:  1,
-            message: 'importer failed',
-            import_source: :github,
-            project_id: project.id,
-            importer: 'klass_name',
-            'error.message': 'some error',
-            'github.data': {
-              'github_id' => 1,
-              'number' => 10
-            }
-          )
-      end
-
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_and_raise_exception)
+      expect(Gitlab::GithubImport::Logger)
+        .to receive(:info)
         .with(
-          exception,
-          import_source: :github,
           github_id: 1,
-          project_id: 1,
+          message: 'starting importer',
+          project_id: project.id,
           importer: 'klass_name'
-        ).and_call_original
+        )
+
+      expect(Gitlab::Import::ImportFailureService)
+        .to receive(:track)
+        .with(
+          project_id: project.id,
+          exception: exception,
+          error_source: 'klass_name'
+        )
+        .and_call_original
+
+      worker.import(project, client, { 'number' => 10, 'github_id' => 1 })
+
+      expect(project.import_state.reload.status).to eq('started')
 
-      expect { worker.import(project, client, { 'number' => 10, 'github_id' => 1 }) }
-        .to raise_error(exception)
+      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_next_instance_of(Gitlab::Import::Logger) do |logger|
-        expect(logger)
-          .to receive(:error)
-          .with(
-            github_id:  nil,
-            message: 'importer failed',
-            import_source: :github,
-            project_id: project.id,
-            importer: 'klass_name',
-            'error.message': 'key not found: :github_id',
-            'github.data': {
-              'number' => 10
-            }
-          )
-      end
-
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_and_raise_exception)
+      expect(Gitlab::Import::ImportFailureService)
+        .to receive(:track)
         .with(
-          an_instance_of(KeyError),
-          import_source: :github,
-          github_id: nil,
-          project_id: 1,
-          importer: 'klass_name'
-        ).and_call_original
+          project_id: project.id,
+          exception: a_kind_of(KeyError),
+          error_source: 'klass_name',
+          fail_import: true
+        )
+        .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 651ea77a71c..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)
 
@@ -36,71 +35,119 @@ RSpec.describe Gitlab::GithubImport::StageMethods do
           an_instance_of(Project)
         )
 
-      expect_next_instance_of(Gitlab::Import::Logger) do |logger|
-        expect(logger)
+      expect(Gitlab::GithubImport::Logger)
+        .to receive(:info)
+        .with(
+          message: 'starting stage',
+          project_id: project.id,
+          import_stage: 'DummyStage'
+        )
+
+      expect(Gitlab::GithubImport::Logger)
+        .to receive(:info)
+        .with(
+          message: 'stage finished',
+          project_id: project.id,
+          import_stage: 'DummyStage'
+        )
+
+      worker.perform(project.id)
+    end
+
+    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',
-            import_source: :github,
             project_id: project.id,
             import_stage: 'DummyStage'
           )
-        expect(logger)
-          .to receive(:info)
+
+        expect(Gitlab::Import::ImportFailureService)
+          .to receive(:track)
           .with(
-            message: 'stage finished',
-            import_source: :github,
             project_id: project.id,
-            import_stage: 'DummyStage'
-          )
-      end
+            exception: exception,
+            error_source: 'DummyStage',
+            fail_import: false
+          ).and_call_original
 
-      worker.perform(project.id)
+        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
 
-    it 'logs error when import fails' do
-      exception = StandardError.new('some error')
+    context 'when abort_on_failure is true' do
+      let(:worker) do
+        Class.new do
+          def self.name
+            'DummyStage'
+          end
 
-      allow(worker)
-        .to receive(:find_project)
-        .with(project.id)
-        .and_return(project)
+          def abort_on_failure
+            true
+          end
 
-      expect(worker)
-        .to receive(:try_import)
-        .and_raise(exception)
+          include(Gitlab::GithubImport::StageMethods)
+        end.new
+      end
+
+      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_next_instance_of(Gitlab::Import::Logger) do |logger|
-        expect(logger)
+        expect(worker)
+          .to receive(:try_import)
+          .and_raise(exception)
+
+        expect(Gitlab::GithubImport::Logger)
           .to receive(:info)
           .with(
             message: 'starting stage',
-            import_source: :github,
             project_id: project.id,
             import_stage: 'DummyStage'
           )
-        expect(logger)
-          .to receive(:error)
+
+        expect(Gitlab::Import::ImportFailureService)
+          .to receive(:track)
           .with(
-            message: 'stage failed',
-            import_source: :github,
             project_id: project.id,
-            import_stage: 'DummyStage',
-            'error.message': 'some error'
-          )
-      end
+            exception: exception,
+            error_source: 'DummyStage',
+            fail_import: true
+          ).and_call_original
 
-      expect(Gitlab::ErrorTracking)
-        .to receive(:track_and_raise_exception)
-        .with(
-          exception,
-          import_source: :github,
-          project_id: project.id,
-          import_stage: 'DummyStage'
-        )
-        .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 { worker.perform(project.id) }.to raise_error(exception)
+        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
 
@@ -132,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