diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-19 12:08:42 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-08-19 12:08:42 +0300 |
commit | b76ae638462ab0f673e5915986070518dd3f9ad3 (patch) | |
tree | bdab0533383b52873be0ec0eb4d3c66598ff8b91 /spec/lib/gitlab/github_import | |
parent | 434373eabe7b4be9593d18a585fb763f1e5f1a6f (diff) |
Add latest changes from gitlab-org/gitlab@14-2-stable-eev14.2.0-rc42
Diffstat (limited to 'spec/lib/gitlab/github_import')
13 files changed, 355 insertions, 210 deletions
diff --git a/spec/lib/gitlab/github_import/bulk_importing_spec.rb b/spec/lib/gitlab/github_import/bulk_importing_spec.rb index 63dce51c5da..6c94973b5a8 100644 --- a/spec/lib/gitlab/github_import/bulk_importing_spec.rb +++ b/spec/lib/gitlab/github_import/bulk_importing_spec.rb @@ -3,8 +3,20 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::BulkImporting do - let(:importer) do - Class.new { include(Gitlab::GithubImport::BulkImporting) }.new + let(:project) { instance_double(Project, id: 1) } + let(:importer) { MyImporter.new(project, double) } + let(:importer_class) do + Class.new do + include Gitlab::GithubImport::BulkImporting + + def object_type + :object_type + end + end + end + + before do + stub_const 'MyImporter', importer_class end describe '#build_database_rows' do @@ -21,6 +33,24 @@ RSpec.describe Gitlab::GithubImport::BulkImporting do .with(object) .and_return(false) + expect(Gitlab::Import::Logger) + .to receive(:info) + .with( + import_type: :github, + project_id: 1, + importer: 'MyImporter', + message: '1 object_types fetched' + ) + + expect(Gitlab::GithubImport::ObjectCounter) + .to receive(:increment) + .with( + project, + :object_type, + :fetched, + value: 1 + ) + enum = [[object, 1]].to_enum expect(importer.build_database_rows(enum)).to eq([{ title: 'Foo' }]) @@ -37,6 +67,24 @@ RSpec.describe Gitlab::GithubImport::BulkImporting do .with(object) .and_return(true) + expect(Gitlab::Import::Logger) + .to receive(:info) + .with( + import_type: :github, + project_id: 1, + importer: 'MyImporter', + message: '0 object_types fetched' + ) + + expect(Gitlab::GithubImport::ObjectCounter) + .to receive(:increment) + .with( + project, + :object_type, + :fetched, + value: 0 + ) + enum = [[object, 1]].to_enum expect(importer.build_database_rows(enum)).to be_empty @@ -48,12 +96,32 @@ RSpec.describe Gitlab::GithubImport::BulkImporting do rows = [{ title: 'Foo' }] * 10 model = double(:model, table_name: 'kittens') - expect(Gitlab::Database) + expect(Gitlab::Import::Logger) + .to receive(:info) + .twice + .with( + import_type: :github, + project_id: 1, + importer: 'MyImporter', + message: '5 object_types imported' + ) + + expect(Gitlab::GithubImport::ObjectCounter) + .to receive(:increment) + .twice + .with( + project, + :object_type, + :imported, + value: 5 + ) + + expect(Gitlab::Database.main) .to receive(:bulk_insert) .ordered .with('kittens', rows.first(5)) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .ordered .with('kittens', rows.last(5)) diff --git a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb index 9eea85526f5..0af840d2c10 100644 --- a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb @@ -36,7 +36,7 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter do describe '#execute' do context 'when the merge request no longer exists' do it 'does not import anything' do - expect(Gitlab::Database).not_to receive(:bulk_insert) + expect(Gitlab::Database.main).not_to receive(:bulk_insert) importer.execute end @@ -58,7 +58,7 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter do .to receive(:author_id_for) .and_return([user.id, true]) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .with( LegacyDiffNote.table_name, @@ -89,7 +89,7 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter do .to receive(:author_id_for) .and_return([project.creator_id, false]) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .with( LegacyDiffNote.table_name, @@ -133,7 +133,7 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter do .to receive(:author_id_for) .and_return([project.creator_id, false]) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key') diff --git a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb index fb826c987e1..0926000428c 100644 --- a/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/issue_importer_spec.rb @@ -190,7 +190,7 @@ RSpec.describe Gitlab::GithubImport::Importer::IssueImporter, :clean_gitlab_redi .with(issue.assignees[1]) .and_return(5) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .with( IssueAssignee.table_name, diff --git a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb index 6d143f78c66..241a0fef600 100644 --- a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb @@ -39,7 +39,7 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do .and_return(1) freeze_time do - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .with( LabelLink.table_name, @@ -64,7 +64,7 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do .with('bug') .and_return(nil) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .with(LabelLink.table_name, []) diff --git a/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb index 8ee534734f0..a2c7d51214a 100644 --- a/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/lfs_objects_importer_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do - let(:project) { double(:project, id: 4, import_source: 'foo/bar') } + let_it_be(:project) { create(:project, :import_started) } + let(:client) { double(:client) } let(:download_link) { "http://www.gitlab.com/lfs_objects/oid" } @@ -61,27 +62,12 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do .and_raise(exception) end - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) - .to receive(:error) - .with( - message: 'importer failed', - import_source: :github, - project_id: project.id, - parallel: false, - importer: 'Gitlab::GithubImport::Importer::LfsObjectImporter', - 'error.message': 'Invalid Project URL' - ) - end - - expect(Gitlab::ErrorTracking) - .to receive(:track_exception) + expect(Gitlab::Import::ImportFailureService) + .to receive(:track) .with( - exception, - import_source: :github, - parallel: false, project_id: project.id, - importer: 'Gitlab::GithubImport::Importer::LfsObjectImporter' + exception: exception, + error_source: 'Gitlab::GithubImport::Importer::LfsObjectImporter' ).and_call_original importer.execute diff --git a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb index ef0bb90db4a..820f46c7286 100644 --- a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb @@ -41,7 +41,7 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do .with(github_note) .and_return([user.id, true]) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .with( Note.table_name, @@ -71,7 +71,7 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do .with(github_note) .and_return([project.creator_id, false]) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .with( Note.table_name, @@ -115,7 +115,7 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do context 'when the noteable does not exist' do it 'does not import the note' do - expect(Gitlab::Database).not_to receive(:bulk_insert) + expect(Gitlab::Database.main).not_to receive(:bulk_insert) importer.execute end @@ -134,7 +134,7 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do .with(github_note) .and_return([user.id, true]) - expect(Gitlab::Database) + expect(Gitlab::Database.main) .to receive(:bulk_insert) .and_raise(ActiveRecord::InvalidForeignKey, 'invalid foreign key') diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb index 133d515246a..067b8b09516 100644 --- a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb @@ -148,7 +148,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do end end - shared_examples '#update_repository' do + describe '#update_repository' do it 'updates the repository' do importer = described_class.new(project, client) @@ -162,6 +162,10 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do .to receive(:increment) .and_call_original + expect(project.repository) + .to receive(:fetch_remote) + .with(url, forced: false, refmap: Gitlab::GithubImport.refmap) + freeze_time do importer.update_repository @@ -170,28 +174,6 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do end end - describe '#update_repository with :fetch_remote_params enabled' do - before do - stub_feature_flags(fetch_remote_params: true) - expect(project.repository) - .to receive(:fetch_remote) - .with('github', forced: false, url: url, refmap: Gitlab::GithubImport.refmap) - end - - it_behaves_like '#update_repository' - end - - describe '#update_repository with :fetch_remote_params disabled' do - before do - stub_feature_flags(fetch_remote_params: false) - expect(project.repository) - .to receive(:fetch_remote) - .with('github', forced: false) - end - - it_behaves_like '#update_repository' - end - describe '#update_repository?' do let(:importer) { described_class.new(project, client) } diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_reviews_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_reviews_importer_spec.rb index 08be350f0f9..c5fa67e50aa 100644 --- a/spec/lib/gitlab/github_import/importer/pull_requests_reviews_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_requests_reviews_importer_spec.rb @@ -27,100 +27,62 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsReviewsImporter do end describe '#each_object_to_import', :clean_gitlab_redis_cache do - context 'when github_review_importer_query_only_unimported_merge_requests is enabled' do - before do - stub_feature_flags(github_review_importer_query_only_unimported_merge_requests: true) - end - - let(:merge_request) do - create( - :merged_merge_request, - iid: 999, - source_project: project, - target_project: project - ) - end - - let(:review) { double(id: 1) } - - it 'fetches the pull requests reviews data' do - page = double(objects: [review], number: 1) - - expect(review) - .to receive(:merge_request_id=) - .with(merge_request.id) - - expect(client) - .to receive(:each_page) - .exactly(:once) # ensure to be cached on the second call - .with(:pull_request_reviews, 'github/repo', merge_request.iid, page: 1) - .and_yield(page) + let(:merge_request) do + create( + :merged_merge_request, + iid: 999, + source_project: project, + target_project: project + ) + end - expect { |b| subject.each_object_to_import(&b) } - .to yield_with_args(review) + let(:review) { double(id: 1) } - subject.each_object_to_import {} - end + it 'fetches the pull requests reviews data' do + page = double(objects: [review], number: 1) - it 'skips cached pages' do - Gitlab::GithubImport::PageCounter - .new(project, "merge_request/#{merge_request.id}/pull_request_reviews") - .set(2) + expect(review) + .to receive(:merge_request_id=) + .with(merge_request.id) - expect(review).not_to receive(:merge_request_id=) + expect(client) + .to receive(:each_page) + .exactly(:once) # ensure to be cached on the second call + .with(:pull_request_reviews, 'github/repo', merge_request.iid, page: 1) + .and_yield(page) - expect(client) - .to receive(:each_page) - .exactly(:once) # ensure to be cached on the second call - .with(:pull_request_reviews, 'github/repo', merge_request.iid, page: 2) + expect { |b| subject.each_object_to_import(&b) } + .to yield_with_args(review) - subject.each_object_to_import {} - end + subject.each_object_to_import {} + end - it 'skips cached merge requests' do - Gitlab::Cache::Import::Caching.set_add( - "github-importer/merge_request/already-imported/#{project.id}", - merge_request.id - ) + it 'skips cached pages' do + Gitlab::GithubImport::PageCounter + .new(project, "merge_request/#{merge_request.id}/pull_request_reviews") + .set(2) - expect(review).not_to receive(:merge_request_id=) + expect(review).not_to receive(:merge_request_id=) - expect(client).not_to receive(:each_page) + expect(client) + .to receive(:each_page) + .exactly(:once) # ensure to be cached on the second call + .with(:pull_request_reviews, 'github/repo', merge_request.iid, page: 2) - subject.each_object_to_import {} - end + subject.each_object_to_import {} end - context 'when github_review_importer_query_only_unimported_merge_requests is disabled' do - before do - stub_feature_flags(github_review_importer_query_only_unimported_merge_requests: false) - end - - it 'fetchs the merged pull requests data' do - merge_request = create( - :merged_merge_request, - iid: 999, - source_project: project, - target_project: project - ) - - review = double - - expect(review) - .to receive(:merge_request_id=) - .with(merge_request.id) + it 'skips cached merge requests' do + Gitlab::Cache::Import::Caching.set_add( + "github-importer/merge_request/already-imported/#{project.id}", + merge_request.id + ) - allow(client) - .to receive(:pull_request_reviews) - .exactly(:once) # ensure to be cached on the second call - .with('github/repo', merge_request.iid) - .and_return([review]) + expect(review).not_to receive(:merge_request_id=) - expect { |b| subject.each_object_to_import(&b) } - .to yield_with_args(review) + expect(client).not_to receive(:each_page) - subject.each_object_to_import {} - end + subject.each_object_to_import {} end end end diff --git a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb index 3839303b881..58a8fb1b7e4 100644 --- a/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/repository_importer_spec.rb @@ -202,7 +202,7 @@ RSpec.describe Gitlab::GithubImport::Importer::RepositoryImporter do expect(repository) .to receive(:fetch_as_mirror) - .with(project.import_url, refmap: Gitlab::GithubImport.refmap, forced: true, remote_name: 'github') + .with(project.import_url, refmap: Gitlab::GithubImport.refmap, forced: true) service = double expect(Repositories::HousekeepingService) @@ -211,17 +211,6 @@ RSpec.describe Gitlab::GithubImport::Importer::RepositoryImporter do expect(importer.import_repository).to eq(true) end - - it 'marks the import as failed when an error was raised' do - expect(project).to receive(:ensure_repository) - .and_raise(Gitlab::Git::Repository::NoRepository) - - expect(importer) - .to receive(:fail_import) - .and_return(false) - - expect(importer.import_repository).to eq(false) - end end describe '#import_wiki_repository' do @@ -234,28 +223,40 @@ RSpec.describe Gitlab::GithubImport::Importer::RepositoryImporter do expect(importer.import_wiki_repository).to eq(true) end - it 'marks the import as failed and creates an empty repo if an error was raised' do - expect(wiki_repository) - .to receive(:import_repository) - .with(importer.wiki_url) - .and_raise(Gitlab::Git::CommandError) + context 'when it raises a Gitlab::Git::CommandError' do + context 'when the error is not a "repository not exported"' do + it 'creates the wiki and re-raise the exception' do + exception = Gitlab::Git::CommandError.new - expect(importer) - .to receive(:fail_import) - .and_return(false) + expect(wiki_repository) + .to receive(:import_repository) + .with(importer.wiki_url) + .and_raise(exception) - expect(project) - .to receive(:create_wiki) + expect(project) + .to receive(:create_wiki) - expect(importer.import_wiki_repository).to eq(false) - end - end + expect { importer.import_wiki_repository } + .to raise_error(exception) + end + end + + context 'when the error is a "repository not exported"' do + it 'returns true' do + exception = Gitlab::Git::CommandError.new('repository not exported') - describe '#fail_import' do - it 'marks the import as failed' do - expect(project.import_state).to receive(:mark_as_failed).with('foo') + expect(wiki_repository) + .to receive(:import_repository) + .with(importer.wiki_url) + .and_raise(exception) - expect(importer.fail_import('foo')).to eq(false) + expect(project) + .not_to receive(:create_wiki) + + expect(importer.import_wiki_repository) + .to eq(true) + end + end end end diff --git a/spec/lib/gitlab/github_import/logger_spec.rb b/spec/lib/gitlab/github_import/logger_spec.rb new file mode 100644 index 00000000000..6fd0f5db93e --- /dev/null +++ b/spec/lib/gitlab/github_import/logger_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Logger do + subject(:logger) { described_class.new('/dev/null') } + + let(:now) { Time.zone.now } + + describe '#format_message' do + before do + allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('new-correlation-id') + end + + it 'formats strings' do + output = subject.format_message('INFO', now, 'test', 'Hello world') + + expect(Gitlab::Json.parse(output)).to eq({ + 'severity' => 'INFO', + 'time' => now.utc.iso8601(3), + 'message' => 'Hello world', + 'correlation_id' => 'new-correlation-id', + 'feature_category' => 'importers', + 'import_type' => 'github' + }) + end + + it 'formats hashes' do + output = subject.format_message('INFO', now, 'test', { hello: 1 }) + + expect(Gitlab::Json.parse(output)).to eq({ + 'severity' => 'INFO', + 'time' => now.utc.iso8601(3), + 'hello' => 1, + 'correlation_id' => 'new-correlation-id', + 'feature_category' => 'importers', + 'import_type' => 'github' + }) + end + end +end diff --git a/spec/lib/gitlab/github_import/object_counter_spec.rb b/spec/lib/gitlab/github_import/object_counter_spec.rb index 668c11667b5..c9e4ac67061 100644 --- a/spec/lib/gitlab/github_import/object_counter_spec.rb +++ b/spec/lib/gitlab/github_import/object_counter_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Gitlab::GithubImport::ObjectCounter, :clean_gitlab_redis_cache do it 'validates the operation being incremented' do expect { described_class.increment(project, :issue, :unknown) } - .to raise_error(ArgumentError, 'Operation must be fetched or imported') + .to raise_error(ArgumentError, 'operation must be fetched or imported') end it 'increments the counter and saves the key to be listed in the summary later' do @@ -33,4 +33,20 @@ RSpec.describe Gitlab::GithubImport::ObjectCounter, :clean_gitlab_redis_cache do 'imported' => { 'issue' => 2 } }) end + + it 'does not increment the counter if the given value is <= 0' do + expect(Gitlab::Metrics) + .not_to receive(:counter) + + expect(Gitlab::Metrics) + .not_to receive(:counter) + + described_class.increment(project, :issue, :fetched, value: 0) + described_class.increment(project, :issue, :imported, value: nil) + + expect(described_class.summary(project)).to eq({ + 'fetched' => {}, + 'imported' => {} + }) + end end diff --git a/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb b/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb index d56d4708385..1fc7d3c887f 100644 --- a/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb +++ b/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::ParallelScheduling do let(:importer_class) do Class.new do + def self.name + 'MyImporter' + end + include(Gitlab::GithubImport::ParallelScheduling) def importer_class @@ -21,7 +25,8 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do end end - let(:project) { double(:project, id: 4, import_source: 'foo/bar') } + let_it_be(:project) { create(:project, :import_started, import_source: 'foo/bar') } + let(:client) { double(:client) } describe '#parallel?' do @@ -79,73 +84,130 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do .to receive(:sequential_import) .and_return([]) - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) + expect(Gitlab::GithubImport::Logger) + .to receive(:info) + .with( + message: 'starting importer', + parallel: false, + project_id: project.id, + importer: 'Class' + ) + + expect(Gitlab::GithubImport::Logger) + .to receive(:info) + .with( + message: 'importer finished', + parallel: false, + project_id: project.id, + importer: 'Class' + ) + + importer.execute + end + + context 'when abort_on_failure is false' do + it 'logs the error when it fails' do + exception = StandardError.new('some error') + + importer = importer_class.new(project, client, parallel: false) + + expect(importer) + .to receive(:sequential_import) + .and_raise(exception) + + expect(Gitlab::GithubImport::Logger) .to receive(:info) .with( message: 'starting importer', - import_source: :github, parallel: false, project_id: project.id, importer: 'Class' ) - expect(logger) - .to receive(:info) + + expect(Gitlab::Import::ImportFailureService) + .to receive(:track) .with( - message: 'importer finished', - import_source: :github, - parallel: false, project_id: project.id, - importer: 'Class' - ) - end + exception: exception, + error_source: 'MyImporter', + fail_import: false + ).and_call_original - importer.execute + expect { importer.execute } + .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 the error when it fails' do - exception = StandardError.new('some error') + context 'when abort_on_failure is true' do + let(:importer_class) do + Class.new do + def self.name + 'MyImporter' + end - importer = importer_class.new(project, client, parallel: false) + include(Gitlab::GithubImport::ParallelScheduling) - expect(importer) - .to receive(:sequential_import) - .and_raise(exception) + def importer_class + Class + end + + def object_type + :dummy + end + + def collection_method + :issues + end + + def abort_on_failure + true + end + end + end + + it 'logs the error when it fails and marks import as failed' do + exception = StandardError.new('some error') + + importer = importer_class.new(project, client, parallel: false) - expect_next_instance_of(Gitlab::Import::Logger) do |logger| - expect(logger) + expect(importer) + .to receive(:sequential_import) + .and_raise(exception) + + expect(Gitlab::GithubImport::Logger) .to receive(:info) .with( message: 'starting importer', - import_source: :github, parallel: false, project_id: project.id, importer: 'Class' ) - expect(logger) - .to receive(:error) + + expect(Gitlab::Import::ImportFailureService) + .to receive(:track) .with( - message: 'importer failed', - import_source: :github, project_id: project.id, - parallel: false, - importer: 'Class', - 'error.message': 'some error' - ) - end + exception: exception, + error_source: 'MyImporter', + fail_import: true + ).and_call_original - expect(Gitlab::ErrorTracking) - .to receive(:track_exception) - .with( - exception, - import_source: :github, - parallel: false, - project_id: project.id, - importer: 'Class' - ) - .and_call_original + expect { importer.execute } + .to raise_error(exception) + + expect(project.import_state.reload.status).to eq('failed') + expect(project.import_state.last_error).to eq('some error') - expect { importer.execute }.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 diff --git a/spec/lib/gitlab/github_import/user_finder_spec.rb b/spec/lib/gitlab/github_import/user_finder_spec.rb index 20e67a784e1..f81fa3b1e2e 100644 --- a/spec/lib/gitlab/github_import/user_finder_spec.rb +++ b/spec/lib/gitlab/github_import/user_finder_spec.rb @@ -3,7 +3,14 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do - let(:project) { create(:project) } + let(:project) do + create( + :project, + import_type: 'github', + import_url: 'https://api.github.com/user/repo' + ) + end + let(:client) { double(:client) } let(:finder) { described_class.new(project, client) } @@ -263,6 +270,26 @@ RSpec.describe Gitlab::GithubImport::UserFinder, :clean_gitlab_redis_cache do finder.id_for_github_id(id) end + + context 'when importing from github enterprise' do + let(:project) do + create( + :project, + import_type: 'github', + import_url: 'https://othergithub.net/user/repo' + ) + end + + it 'does not look up the user by external id' do + expect(finder).not_to receive(:query_id_for_github_id) + + expect(Gitlab::Cache::Import::Caching) + .to receive(:write) + .with(described_class::ID_CACHE_KEY % id, nil) + + finder.id_for_github_id(id) + end + end end describe '#id_for_github_email' do |