diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-12-17 14:59:07 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-12-17 14:59:07 +0300 |
commit | 8b573c94895dc0ac0e1d9d59cf3e8745e8b539ca (patch) | |
tree | 544930fb309b30317ae9797a9683768705d664c4 /spec/lib/gitlab/github_import | |
parent | 4b1de649d0168371549608993deac953eb692019 (diff) |
Add latest changes from gitlab-org/gitlab@13-7-stable-eev13.7.0-rc42
Diffstat (limited to 'spec/lib/gitlab/github_import')
11 files changed, 573 insertions, 12 deletions
diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb index bc734644d29..4000e0b2611 100644 --- a/spec/lib/gitlab/github_import/client_spec.rb +++ b/spec/lib/gitlab/github_import/client_spec.rb @@ -28,6 +28,17 @@ RSpec.describe Gitlab::GithubImport::Client do end end + describe '#pull_request_reviews' do + it 'returns the pull request reviews' do + client = described_class.new('foo') + + expect(client.octokit).to receive(:pull_request_reviews).with('foo/bar', 999) + expect(client).to receive(:with_rate_limit).and_yield + + client.pull_request_reviews('foo/bar', 999) + end + end + describe '#repository' do it 'returns the details of a repository' do client = described_class.new('foo') @@ -39,6 +50,17 @@ RSpec.describe Gitlab::GithubImport::Client do end end + describe '#pull_request' do + it 'returns the details of a pull_request' do + client = described_class.new('foo') + + expect(client.octokit).to receive(:pull_request).with('foo/bar', 999) + expect(client).to receive(:with_rate_limit).and_yield + + client.pull_request('foo/bar', 999) + end + end + describe '#labels' do it 'returns the labels' do client = described_class.new('foo') @@ -478,7 +500,7 @@ RSpec.describe Gitlab::GithubImport::Client do it 'searches for repositories based on name' do expected_search_query = 'test in:name is:public,private user:user repo:repo1 repo:repo2 org:org1 org:org2' - expect(client).to receive(:each_page).with(:search_repositories, expected_search_query) + expect(client.octokit).to receive(:search_repositories).with(expected_search_query, {}) client.search_repos_by_name('test') end 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 6188ba8ec3f..8ee534734f0 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 @@ -49,6 +49,57 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do importer.execute end end + + context 'when LFS list download fails' do + it 'rescues and logs the known exceptions' do + exception = StandardError.new('Invalid Project URL') + importer = described_class.new(project, client, parallel: false) + + expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service| + expect(service) + .to receive(:execute) + .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) + .with( + exception, + import_source: :github, + parallel: false, + project_id: project.id, + importer: 'Gitlab::GithubImport::Importer::LfsObjectImporter' + ).and_call_original + + importer.execute + end + + it 'raises and logs the unknown exceptions' do + exception = Exception.new('Really bad news') + importer = described_class.new(project, client, parallel: false) + + expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service| + expect(service) + .to receive(:execute) + .and_raise(exception) + end + + expect { importer.execute }.to raise_error(exception) + end + end end describe '#sequential_import' do @@ -56,18 +107,16 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do importer = described_class.new(project, client, parallel: false) lfs_object_importer = double(:lfs_object_importer) - allow(importer) - .to receive(:each_object_to_import) - .and_yield(lfs_download_object) + expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service| + expect(service).to receive(:execute).and_return([lfs_download_object]) + end expect(Gitlab::GithubImport::Importer::LfsObjectImporter) - .to receive(:new) - .with( + .to receive(:new).with( an_instance_of(Gitlab::GithubImport::Representation::LfsObject), project, client - ) - .and_return(lfs_object_importer) + ).and_return(lfs_object_importer) expect(lfs_object_importer).to receive(:execute) @@ -79,9 +128,9 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do it 'imports each lfs object in parallel' do importer = described_class.new(project, client) - allow(importer) - .to receive(:each_object_to_import) - .and_yield(lfs_download_object) + expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service| + expect(service).to receive(:execute).and_return([lfs_download_object]) + end expect(Gitlab::GithubImport::ImportLfsObjectWorker) .to receive(:perform_async) diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb index 46850618945..c7388314253 100644 --- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb @@ -43,7 +43,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitla describe '#execute' do it 'imports the pull request' do - mr = double(:merge_request, id: 10) + mr = double(:merge_request, id: 10, merged?: false) expect(importer) .to receive(:create_merge_request) diff --git a/spec/lib/gitlab/github_import/importer/pull_request_merged_by_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_merged_by_importer_spec.rb new file mode 100644 index 00000000000..2999dc5bb41 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/pull_request_merged_by_importer_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::PullRequestMergedByImporter, :clean_gitlab_redis_cache do + let_it_be(:merge_request) { create(:merged_merge_request) } + let(:project) { merge_request.project } + let(:created_at) { Time.new(2017, 1, 1, 12, 00).utc } + let(:client_double) { double(user: double(id: 999, login: 'merger', email: 'merger@email.com')) } + + let(:pull_request) do + instance_double( + Gitlab::GithubImport::Representation::PullRequest, + iid: merge_request.iid, + created_at: created_at, + merged_by: double(id: 999, login: 'merger') + ) + end + + subject { described_class.new(pull_request, project, client_double) } + + it 'assigns the merged by user when mapped' do + merge_user = create(:user, email: 'merger@email.com') + + subject.execute + + expect(merge_request.metrics.reload.merged_by).to eq(merge_user) + end + + it 'adds a note referencing the merger user when the user cannot be mapped' do + expect { subject.execute } + .to change(Note, :count).by(1) + .and not_change(merge_request, :updated_at) + + last_note = merge_request.notes.last + + expect(last_note.note).to eq("*Merged by: merger*") + expect(last_note.created_at).to eq(created_at) + expect(last_note.author).to eq(project.creator) + end +end diff --git a/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb new file mode 100644 index 00000000000..b2f993ac47c --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb @@ -0,0 +1,202 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::PullRequestReviewImporter, :clean_gitlab_redis_cache do + using RSpec::Parameterized::TableSyntax + + let_it_be(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let(:client_double) { double(user: double(id: 999, login: 'author', email: 'author@email.com')) } + let(:submitted_at) { Time.new(2017, 1, 1, 12, 00).utc } + + subject { described_class.new(review, project, client_double) } + + context 'when the review author can be mapped to a gitlab user' do + let_it_be(:author) { create(:user, email: 'author@email.com') } + + context 'when the review has no note text' do + context 'when the review is "APPROVED"' do + let(:review) { create_review(type: 'APPROVED', note: '') } + + it 'creates a note for the review' do + expect { subject.execute }.to change(Note, :count) + + last_note = merge_request.notes.last + expect(last_note.note).to eq('approved this merge request') + expect(last_note.author).to eq(author) + expect(last_note.created_at).to eq(submitted_at) + expect(last_note.system_note_metadata.action).to eq('approved') + + expect(merge_request.approved_by_users.reload).to include(author) + expect(merge_request.approvals.last.created_at).to eq(submitted_at) + end + end + + context 'when the review is "COMMENTED"' do + let(:review) { create_review(type: 'COMMENTED', note: '') } + + it 'creates a note for the review' do + expect { subject.execute }.not_to change(Note, :count) + end + end + + context 'when the review is "CHANGES_REQUESTED"' do + let(:review) { create_review(type: 'CHANGES_REQUESTED', note: '') } + + it 'creates a note for the review' do + expect { subject.execute }.not_to change(Note, :count) + end + end + end + + context 'when the review has a note text' do + context 'when the review is "APPROVED"' do + let(:review) { create_review(type: 'APPROVED') } + + it 'creates a note for the review' do + expect { subject.execute } + .to change(Note, :count).by(2) + .and change(Approval, :count).by(1) + + note = merge_request.notes.where(system: false).last + expect(note.note).to eq("**Review:** Approved\n\nnote") + expect(note.author).to eq(author) + expect(note.created_at).to eq(submitted_at) + + system_note = merge_request.notes.where(system: true).last + expect(system_note.note).to eq('approved this merge request') + expect(system_note.author).to eq(author) + expect(system_note.created_at).to eq(submitted_at) + expect(system_note.system_note_metadata.action).to eq('approved') + + expect(merge_request.approved_by_users.reload).to include(author) + expect(merge_request.approvals.last.created_at).to eq(submitted_at) + end + end + + context 'when the review is "COMMENTED"' do + let(:review) { create_review(type: 'COMMENTED') } + + it 'creates a note for the review' do + expect { subject.execute } + .to change(Note, :count).by(1) + .and not_change(Approval, :count) + + last_note = merge_request.notes.last + + expect(last_note.note).to eq("**Review:** Commented\n\nnote") + expect(last_note.author).to eq(author) + expect(last_note.created_at).to eq(submitted_at) + end + end + + context 'when the review is "CHANGES_REQUESTED"' do + let(:review) { create_review(type: 'CHANGES_REQUESTED') } + + it 'creates a note for the review' do + expect { subject.execute } + .to change(Note, :count).by(1) + .and not_change(Approval, :count) + + last_note = merge_request.notes.last + + expect(last_note.note).to eq("**Review:** Changes requested\n\nnote") + expect(last_note.author).to eq(author) + expect(last_note.created_at).to eq(submitted_at) + end + end + end + end + + context 'when the review author cannot be mapped to a gitlab user' do + context 'when the review has no note text' do + context 'when the review is "APPROVED"' do + let(:review) { create_review(type: 'APPROVED', note: '') } + + it 'creates a note for the review with *Approved by by<author>*' do + expect { subject.execute } + .to change(Note, :count).by(1) + + last_note = merge_request.notes.last + expect(last_note.note).to eq("*Created by author*\n\n**Review:** Approved") + expect(last_note.author).to eq(project.creator) + expect(last_note.created_at).to eq(submitted_at) + end + end + + context 'when the review is "COMMENTED"' do + let(:review) { create_review(type: 'COMMENTED', note: '') } + + it 'creates a note for the review with *Commented by<author>*' do + expect { subject.execute }.not_to change(Note, :count) + end + end + + context 'when the review is "CHANGES_REQUESTED"' do + let(:review) { create_review(type: 'CHANGES_REQUESTED', note: '') } + + it 'creates a note for the review with *Changes requested by <author>*' do + expect { subject.execute }.not_to change(Note, :count) + end + end + end + + context 'when the review has a note text' do + context 'when the review is "APPROVED"' do + let(:review) { create_review(type: 'APPROVED') } + + it 'creates a note for the review with *Approved by by<author>*' do + expect { subject.execute } + .to change(Note, :count).by(1) + + last_note = merge_request.notes.last + + expect(last_note.note).to eq("*Created by author*\n\n**Review:** Approved\n\nnote") + expect(last_note.author).to eq(project.creator) + expect(last_note.created_at).to eq(submitted_at) + end + end + + context 'when the review is "COMMENTED"' do + let(:review) { create_review(type: 'COMMENTED') } + + it 'creates a note for the review with *Commented by<author>*' do + expect { subject.execute } + .to change(Note, :count).by(1) + + last_note = merge_request.notes.last + + expect(last_note.note).to eq("*Created by author*\n\n**Review:** Commented\n\nnote") + expect(last_note.author).to eq(project.creator) + expect(last_note.created_at).to eq(submitted_at) + end + end + + context 'when the review is "CHANGES_REQUESTED"' do + let(:review) { create_review(type: 'CHANGES_REQUESTED') } + + it 'creates a note for the review with *Changes requested by <author>*' do + expect { subject.execute } + .to change(Note, :count).by(1) + + last_note = merge_request.notes.last + + expect(last_note.note).to eq("*Created by author*\n\n**Review:** Changes requested\n\nnote") + expect(last_note.author).to eq(project.creator) + expect(last_note.created_at).to eq(submitted_at) + end + end + end + end + + def create_review(type:, note: 'note') + Gitlab::GithubImport::Representation::PullRequestReview.from_json_hash( + merge_request_id: merge_request.id, + review_type: type, + note: note, + submitted_at: submitted_at.to_s, + author: { id: 999, login: 'author' } + ) + end +end 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 0835c6155b9..8a7867f3841 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 @@ -29,6 +29,7 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestsImporter do milestone: double(:milestone, number: 4), user: double(:user, id: 4, login: 'alice'), assignee: double(:user, id: 4, login: 'alice'), + merged_by: double(:user, id: 4, login: 'alice'), created_at: 1.second.ago, updated_at: 1.second.ago, merged_at: 1.second.ago diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb new file mode 100644 index 00000000000..b859cc727a6 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/pull_requests_merged_by_importer_spec.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::PullRequestsMergedByImporter do + let(:client) { double } + let(:project) { create(:project, import_source: 'http://somegithub.com') } + + subject { described_class.new(project, client) } + + it { is_expected.to include_module(Gitlab::GithubImport::ParallelScheduling) } + + describe '#representation_class' do + it { expect(subject.representation_class).to eq(Gitlab::GithubImport::Representation::PullRequest) } + end + + describe '#importer_class' do + it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequestMergedByImporter) } + end + + describe '#collection_method' do + it { expect(subject.collection_method).to eq(:pull_requests_merged_by) } + end + + describe '#id_for_already_imported_cache' do + it { expect(subject.id_for_already_imported_cache(double(number: 1))).to eq(1) } + end + + describe '#each_object_to_import' do + it 'fetchs the merged pull requests data' do + pull_request = double + create( + :merged_merge_request, + iid: 999, + source_project: project, + target_project: project + ) + + allow(client) + .to receive(:pull_request) + .with('http://somegithub.com', 999) + .and_return(pull_request) + + expect { |b| subject.each_object_to_import(&b) }.to yield_with_args(pull_request) + end + end +end 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 new file mode 100644 index 00000000000..5e2302f9662 --- /dev/null +++ b/spec/lib/gitlab/github_import/importer/pull_requests_reviews_importer_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Importer::PullRequestsReviewsImporter do + let(:client) { double } + let(:project) { create(:project, import_source: 'github/repo') } + + subject { described_class.new(project, client) } + + it { is_expected.to include_module(Gitlab::GithubImport::ParallelScheduling) } + + describe '#representation_class' do + it { expect(subject.representation_class).to eq(Gitlab::GithubImport::Representation::PullRequestReview) } + end + + describe '#importer_class' do + it { expect(subject.importer_class).to eq(Gitlab::GithubImport::Importer::PullRequestReviewImporter) } + end + + describe '#collection_method' do + it { expect(subject.collection_method).to eq(:pull_request_reviews) } + end + + describe '#id_for_already_imported_cache' do + it { expect(subject.id_for_already_imported_cache(double(github_id: 1))).to eq(1) } + end + + describe '#each_object_to_import' do + it 'fetchs the merged pull requests data' do + merge_request = create(:merge_request, source_project: project) + review = double + + expect(review) + .to receive(:merge_request_id=) + .with(merge_request.id) + + allow(client) + .to receive(:pull_request_reviews) + .with('github/repo', merge_request.iid) + .and_return([review]) + + expect { |b| subject.each_object_to_import(&b) }.to yield_with_args(review) + end + 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 578743be96b..1e31cd2f007 100644 --- a/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb +++ b/spec/lib/gitlab/github_import/parallel_scheduling_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do Class.new do include(Gitlab::GithubImport::ParallelScheduling) + def importer_class + Class + end + def collection_method :issues end @@ -63,6 +67,82 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do importer.execute end + + it 'logs the the process' do + importer = importer_class.new(project, client, parallel: false) + + expect(importer) + .to receive(:sequential_import) + .and_return([]) + + expect_next_instance_of(Gitlab::Import::Logger) do |logger| + expect(logger) + .to receive(:info) + .with( + message: 'starting importer', + import_source: :github, + parallel: false, + project_id: project.id, + importer: 'Class' + ) + expect(logger) + .to receive(:info) + .with( + message: 'importer finished', + import_source: :github, + parallel: false, + project_id: project.id, + importer: 'Class' + ) + end + + importer.execute + end + + 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_next_instance_of(Gitlab::Import::Logger) do |logger| + expect(logger) + .to receive(:info) + .with( + message: 'starting importer', + import_source: :github, + parallel: false, + project_id: project.id, + importer: 'Class' + ) + expect(logger) + .to receive(:error) + .with( + message: 'importer failed', + import_source: :github, + project_id: project.id, + parallel: false, + importer: 'Class', + 'error.message': 'some error' + ) + end + + 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) + end end describe '#sequential_import' do diff --git a/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb b/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb new file mode 100644 index 00000000000..f9763455468 --- /dev/null +++ b/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::Representation::PullRequestReview do + let(:submitted_at) { Time.new(2017, 1, 1, 12, 00).utc } + + shared_examples 'a PullRequest review' do + it 'returns an instance of PullRequest' do + expect(review).to be_an_instance_of(described_class) + expect(review.author).to be_an_instance_of(Gitlab::GithubImport::Representation::User) + expect(review.author.id).to eq(4) + expect(review.author.login).to eq('alice') + expect(review.note).to eq('note') + expect(review.review_type).to eq('APPROVED') + expect(review.submitted_at).to eq(submitted_at) + expect(review.github_id).to eq(999) + expect(review.merge_request_id).to eq(42) + end + end + + describe '.from_api_response' do + let(:response) do + double( + :response, + id: 999, + merge_request_id: 42, + body: 'note', + state: 'APPROVED', + user: double(:user, id: 4, login: 'alice'), + submitted_at: submitted_at + ) + end + + it_behaves_like 'a PullRequest review' do + let(:review) { described_class.from_api_response(response) } + end + + it 'does not set the user if the response did not include a user' do + allow(response) + .to receive(:user) + .and_return(nil) + + review = described_class.from_api_response(response) + + expect(review.author).to be_nil + end + end + + describe '.from_json_hash' do + let(:hash) do + { + 'github_id' => 999, + 'merge_request_id' => 42, + 'note' => 'note', + 'review_type' => 'APPROVED', + 'author' => { 'id' => 4, 'login' => 'alice' }, + 'submitted_at' => submitted_at.to_s + } + end + + it_behaves_like 'a PullRequest review' do + let(:review) { described_class.from_json_hash(hash) } + end + + it 'does not set the user if the response did not include a user' do + review = described_class.from_json_hash(hash.except('author')) + + expect(review.author).to be_nil + end + end +end diff --git a/spec/lib/gitlab/github_import/representation/pull_request_spec.rb b/spec/lib/gitlab/github_import/representation/pull_request_spec.rb index 370eac1d993..27a82951b01 100644 --- a/spec/lib/gitlab/github_import/representation/pull_request_spec.rb +++ b/spec/lib/gitlab/github_import/representation/pull_request_spec.rb @@ -115,6 +115,7 @@ RSpec.describe Gitlab::GithubImport::Representation::PullRequest do milestone: double(:milestone, number: 4), user: double(:user, id: 4, login: 'alice'), assignee: double(:user, id: 4, login: 'alice'), + merged_by: double(:user, id: 4, login: 'alice'), created_at: created_at, updated_at: updated_at, merged_at: merged_at |