From 983a0bba5d2a042c4a3bbb22432ec192c7501d82 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 20 Apr 2020 18:38:24 +0000 Subject: Add latest changes from gitlab-org/gitlab@12-10-stable-ee --- .../merge_when_pipeline_succeeds_service_spec.rb | 22 ++- spec/services/auto_merge_service_spec.rb | 69 ++++++++- .../create_cross_project_pipeline_service_spec.rb | 40 +++++ spec/services/ci/update_runner_service_spec.rb | 13 ++ spec/services/emails/destroy_service_spec.rb | 5 +- .../git/process_ref_changes_service_spec.rb | 43 ++++++ spec/services/issues/export_csv_service_spec.rb | 170 +++++++++++++++++++++ .../jira_import/start_import_service_spec.rb | 35 +++-- .../merge_orchestration_service_spec.rb | 116 ++++++++++++++ .../merge_requests/pushed_branches_service_spec.rb | 42 +++++ .../services/merge_requests/update_service_spec.rb | 14 +- .../dashboard/transient_embed_service_spec.rb | 50 ++++-- .../personal_access_tokens/create_service_spec.rb | 24 +++ spec/services/pod_logs/base_service_spec.rb | 27 ++-- .../pod_logs/elasticsearch_service_spec.rb | 63 +++++++- spec/services/pod_logs/kubernetes_service_spec.rb | 32 +++- .../quick_actions/interpret_service_spec.rb | 29 +++- .../resources/create_access_token_service_spec.rb | 163 ++++++++++++++++++++ spec/services/snippets/create_service_spec.rb | 37 +++++ .../terraform/remote_state_handler_spec.rb | 143 +++++++++++++++++ spec/services/users/build_service_spec.rb | 20 +++ .../x509_certificate_revoke_service_spec.rb | 2 - 22 files changed, 1088 insertions(+), 71 deletions(-) create mode 100644 spec/services/issues/export_csv_service_spec.rb create mode 100644 spec/services/merge_requests/merge_orchestration_service_spec.rb create mode 100644 spec/services/merge_requests/pushed_branches_service_spec.rb create mode 100644 spec/services/personal_access_tokens/create_service_spec.rb create mode 100644 spec/services/resources/create_access_token_service_spec.rb create mode 100644 spec/services/terraform/remote_state_handler_spec.rb (limited to 'spec/services') diff --git a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb index e03d87e9d49..b6e8d3c636a 100644 --- a/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb +++ b/spec/services/auto_merge/merge_when_pipeline_succeeds_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe AutoMerge::MergeWhenPipelineSucceedsService do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } let(:mr_merge_if_green_enabled) do create(:merge_request, merge_when_pipeline_succeeds: true, merge_user: user, @@ -20,6 +20,10 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do described_class.new(project, user, commit_message: 'Awesome message') end + before_all do + project.add_maintainer(user) + end + describe "#available_for?" do subject { service.available_for?(mr_merge_if_green_enabled) } @@ -34,11 +38,25 @@ describe AutoMerge::MergeWhenPipelineSucceedsService do it { is_expected.to be_truthy } + it 'memoizes the result' do + expect(mr_merge_if_green_enabled).to receive(:can_be_merged_by?).once.and_call_original + + 2.times { is_expected.to be_truthy } + end + context 'when the head pipeline succeeded' do let(:pipeline_status) { :success } it { is_expected.to be_falsy } end + + context 'when the user does not have permission to merge' do + before do + allow(mr_merge_if_green_enabled).to receive(:can_be_merged_by?) { false } + end + + it { is_expected.to be_falsy } + end end describe "#execute" do diff --git a/spec/services/auto_merge_service_spec.rb b/spec/services/auto_merge_service_spec.rb index 221cf695331..bab69fb4aa3 100644 --- a/spec/services/auto_merge_service_spec.rb +++ b/spec/services/auto_merge_service_spec.rb @@ -3,22 +3,36 @@ require 'spec_helper' describe AutoMergeService do - let_it_be(:project) { create(:project) } + let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } let(:service) { described_class.new(project, user) } - describe '.all_strategies' do - subject { described_class.all_strategies } + before_all do + project.add_maintainer(user) + end - it 'includes merge when pipeline succeeds' do - is_expected.to include(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) + describe '.all_strategies_ordered_by_preference' do + subject { described_class.all_strategies_ordered_by_preference } + + it 'returns all strategies in preference order' do + if Gitlab.ee? + is_expected.to eq( + [AutoMergeService::STRATEGY_MERGE_TRAIN, + AutoMergeService::STRATEGY_ADD_TO_MERGE_TRAIN_WHEN_PIPELINE_SUCCEEDS, + AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS]) + else + is_expected.to eq([AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS]) + end end end describe '#available_strategies' do subject { service.available_strategies(merge_request) } - let(:merge_request) { create(:merge_request) } + let(:merge_request) do + create(:merge_request, source_project: project) + end + let(:pipeline_status) { :running } before do @@ -42,6 +56,36 @@ describe AutoMergeService do end end + describe '#preferred_strategy' do + subject { service.preferred_strategy(merge_request) } + + let(:merge_request) do + create(:merge_request, source_project: project) + end + + let(:pipeline_status) { :running } + + before do + create(:ci_pipeline, pipeline_status, ref: merge_request.source_branch, + sha: merge_request.diff_head_sha, + project: merge_request.source_project) + + merge_request.update_head_pipeline + end + + it 'returns preferred strategy' do + is_expected.to eq('merge_when_pipeline_succeeds') + end + + context 'when the head piipeline succeeded' do + let(:pipeline_status) { :success } + + it 'returns available strategies' do + is_expected.to be_nil + end + end + end + describe '.get_service_class' do subject { described_class.get_service_class(strategy) } @@ -63,7 +107,10 @@ describe AutoMergeService do describe '#execute' do subject { service.execute(merge_request, strategy) } - let(:merge_request) { create(:merge_request) } + let(:merge_request) do + create(:merge_request, source_project: project) + end + let(:pipeline_status) { :running } let(:strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS } @@ -90,6 +137,14 @@ describe AutoMergeService do is_expected.to eq(:failed) end end + + context 'when strategy is not specified' do + let(:strategy) { } + + it 'chooses the most preferred strategy' do + is_expected.to eq(:merge_when_pipeline_succeeds) + end + end end describe '#update' do diff --git a/spec/services/ci/create_cross_project_pipeline_service_spec.rb b/spec/services/ci/create_cross_project_pipeline_service_spec.rb index a411244e57f..5c59aaa4ce9 100644 --- a/spec/services/ci/create_cross_project_pipeline_service_spec.rb +++ b/spec/services/ci/create_cross_project_pipeline_service_spec.rb @@ -475,5 +475,45 @@ describe Ci::CreateCrossProjectPipelineService, '#execute' do expect(bridge.failure_reason).to eq 'insufficient_bridge_permissions' end end + + context 'when there is no such branch in downstream project' do + let(:trigger) do + { + trigger: { + project: downstream_project.full_path, + branch: 'invalid_branch' + } + } + end + + it 'does not create a pipeline and drops the bridge' do + service.execute(bridge) + + expect(bridge.reload).to be_failed + expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed') + end + end + + context 'when downstream pipeline has a branch rule and does not satisfy' do + before do + stub_ci_pipeline_yaml_file(config) + end + + let(:config) do + <<-EOY + hello: + script: echo world + only: + - invalid_branch + EOY + end + + it 'does not create a pipeline and drops the bridge' do + service.execute(bridge) + + expect(bridge.reload).to be_failed + expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed') + end + end end end diff --git a/spec/services/ci/update_runner_service_spec.rb b/spec/services/ci/update_runner_service_spec.rb index 2b07dad7248..abe575eebc8 100644 --- a/spec/services/ci/update_runner_service_spec.rb +++ b/spec/services/ci/update_runner_service_spec.rb @@ -23,6 +23,19 @@ describe Ci::UpdateRunnerService do end end + context 'with cost factor params' do + let(:params) { { public_projects_minutes_cost_factor: 1.1, private_projects_minutes_cost_factor: 2.2 }} + + it 'updates the runner cost factors' do + expect(update).to be_truthy + + runner.reload + + expect(runner.public_projects_minutes_cost_factor).to eq(1.1) + expect(runner.private_projects_minutes_cost_factor).to eq(2.2) + end + end + context 'when params are not valid' do let(:params) { { run_untagged: false } } diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 5abe8da2529..9e14a13aa4f 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -10,7 +10,10 @@ describe Emails::DestroyService do describe '#execute' do it 'removes an email' do - expect { service.execute(email) }.to change { user.emails.count }.by(-1) + response = service.execute(email) + + expect(user.emails).not_to include(email) + expect(response).to be true end end end diff --git a/spec/services/git/process_ref_changes_service_spec.rb b/spec/services/git/process_ref_changes_service_spec.rb index fc5e379f51d..924e913a9ec 100644 --- a/spec/services/git/process_ref_changes_service_spec.rb +++ b/spec/services/git/process_ref_changes_service_spec.rb @@ -160,6 +160,49 @@ describe Git::ProcessRefChangesService do let(:ref_prefix) { 'refs/heads' } it_behaves_like 'service for processing ref changes', Git::BranchPushService + + context 'when there are merge requests associated with branches' do + let(:tag_changes) do + [ + { index: 0, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789012', ref: "refs/tags/v10.0.0" } + ] + end + let(:branch_changes) do + [ + { index: 0, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789012', ref: "#{ref_prefix}/create1" }, + { index: 1, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789013', ref: "#{ref_prefix}/create2" }, + { index: 2, oldrev: Gitlab::Git::BLANK_SHA, newrev: '789014', ref: "#{ref_prefix}/create3" } + ] + end + let(:git_changes) { double(branch_changes: branch_changes, tag_changes: tag_changes) } + + it 'schedules job for existing merge requests' do + expect_next_instance_of(MergeRequests::PushedBranchesService) do |service| + expect(service).to receive(:execute).and_return(%w(create1 create2)) + end + + expect(UpdateMergeRequestsWorker).to receive(:perform_async) + .with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789012', "#{ref_prefix}/create1").ordered + expect(UpdateMergeRequestsWorker).to receive(:perform_async) + .with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789013', "#{ref_prefix}/create2").ordered + expect(UpdateMergeRequestsWorker).not_to receive(:perform_async) + .with(project.id, user.id, Gitlab::Git::BLANK_SHA, '789014', "#{ref_prefix}/create3").ordered + + subject.execute + end + + context 'refresh_only_existing_merge_requests_on_push disabled' do + before do + stub_feature_flags(refresh_only_existing_merge_requests_on_push: false) + end + + it 'refreshes all merge requests' do + expect(UpdateMergeRequestsWorker).to receive(:perform_async).exactly(3).times + + subject.execute + end + end + end end context 'tag changes' do diff --git a/spec/services/issues/export_csv_service_spec.rb b/spec/services/issues/export_csv_service_spec.rb new file mode 100644 index 00000000000..419e29d92a8 --- /dev/null +++ b/spec/services/issues/export_csv_service_spec.rb @@ -0,0 +1,170 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Issues::ExportCsvService do + let_it_be(:user) { create(:user) } + let(:group) { create(:group) } + let(:project) { create(:project, :public, group: group) } + let!(:issue) { create(:issue, project: project, author: user) } + let!(:bad_issue) { create(:issue, project: project, author: user) } + let(:subject) { described_class.new(Issue.all, project) } + + it 'renders csv to string' do + expect(subject.csv_data).to be_a String + end + + describe '#email' do + it 'emails csv' do + expect { subject.email(user) }.to change(ActionMailer::Base.deliveries, :count) + end + + it 'renders with a target filesize' do + expect(subject.csv_builder).to receive(:render).with(described_class::TARGET_FILESIZE) + + subject.email(user) + end + end + + def csv + CSV.parse(subject.csv_data, headers: true) + end + + context 'includes' do + let(:milestone) { create(:milestone, title: 'v1.0', project: project) } + let(:idea_label) { create(:label, project: project, title: 'Idea') } + let(:feature_label) { create(:label, project: project, title: 'Feature') } + + before do + # Creating a timelog touches the updated_at timestamp of issue, + # so create these first. + issue.timelogs.create(time_spent: 360, user: user) + issue.timelogs.create(time_spent: 200, user: user) + issue.update!(milestone: milestone, + assignees: [user], + description: 'Issue with details', + state: :opened, + due_date: DateTime.new(2014, 3, 2), + created_at: DateTime.new(2015, 4, 3, 2, 1, 0), + updated_at: DateTime.new(2016, 5, 4, 3, 2, 1), + closed_at: DateTime.new(2017, 6, 5, 4, 3, 2), + weight: 4, + discussion_locked: true, + labels: [feature_label, idea_label], + time_estimate: 72000) + end + + it 'includes the columns required for import' do + expect(csv.headers).to include('Title', 'Description') + end + + specify 'iid' do + expect(csv[0]['Issue ID']).to eq issue.iid.to_s + end + + specify 'url' do + expect(csv[0]['URL']).to match(/http.*#{project.full_path}.*#{issue.iid}/) + end + + specify 'title' do + expect(csv[0]['Title']).to eq issue.title + end + + specify 'state' do + expect(csv[0]['State']).to eq 'Open' + end + + specify 'description' do + expect(csv[0]['Description']).to eq issue.description + expect(csv[1]['Description']).to eq nil + end + + specify 'author name' do + expect(csv[0]['Author']).to eq issue.author_name + end + + specify 'author username' do + expect(csv[0]['Author Username']).to eq issue.author.username + end + + specify 'assignee name' do + expect(csv[0]['Assignee']).to eq user.name + expect(csv[1]['Assignee']).to eq '' + end + + specify 'assignee username' do + expect(csv[0]['Assignee Username']).to eq user.username + expect(csv[1]['Assignee Username']).to eq '' + end + + specify 'confidential' do + expect(csv[0]['Confidential']).to eq 'No' + end + + specify 'milestone' do + expect(csv[0]['Milestone']).to eq issue.milestone.title + expect(csv[1]['Milestone']).to eq nil + end + + specify 'labels' do + expect(csv[0]['Labels']).to eq 'Feature,Idea' + expect(csv[1]['Labels']).to eq nil + end + + specify 'due_date' do + expect(csv[0]['Due Date']).to eq '2014-03-02' + expect(csv[1]['Due Date']).to eq nil + end + + specify 'created_at' do + expect(csv[0]['Created At (UTC)']).to eq '2015-04-03 02:01:00' + end + + specify 'updated_at' do + expect(csv[0]['Updated At (UTC)']).to eq '2016-05-04 03:02:01' + end + + specify 'closed_at' do + expect(csv[0]['Closed At (UTC)']).to eq '2017-06-05 04:03:02' + expect(csv[1]['Closed At (UTC)']).to eq nil + end + + specify 'discussion_locked' do + expect(csv[0]['Locked']).to eq 'Yes' + end + + specify 'weight' do + expect(csv[0]['Weight']).to eq '4' + end + + specify 'time estimate' do + expect(csv[0]['Time Estimate']).to eq '72000' + expect(csv[1]['Time Estimate']).to eq '0' + end + + specify 'time spent' do + expect(csv[0]['Time Spent']).to eq '560' + expect(csv[1]['Time Spent']).to eq '0' + end + + context 'with issues filtered by labels and project' do + let(:subject) do + described_class.new( + IssuesFinder.new(user, + project_id: project.id, + label_name: %w(Idea Feature)).execute, project) + end + + it 'returns only filtered objects' do + expect(csv.count).to eq(1) + expect(csv[0]['Issue ID']).to eq issue.iid.to_s + end + end + end + + context 'with minimal details' do + it 'renders labels as nil' do + expect(csv[0]['Labels']).to eq nil + end + end +end diff --git a/spec/services/jira_import/start_import_service_spec.rb b/spec/services/jira_import/start_import_service_spec.rb index ae0c4f63fee..90f38945a9f 100644 --- a/spec/services/jira_import/start_import_service_spec.rb +++ b/spec/services/jira_import/start_import_service_spec.rb @@ -5,8 +5,9 @@ require 'spec_helper' describe JiraImport::StartImportService do let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project) } + let(:key) { 'KEY' } - subject { described_class.new(user, project, '').execute } + subject { described_class.new(user, project, key).execute } context 'when feature flag disabled' do before do @@ -23,6 +24,8 @@ describe JiraImport::StartImportService do context 'when user does not have permissions to run the import' do before do + create(:jira_service, project: project, active: true) + project.add_developer(user) end @@ -38,19 +41,21 @@ describe JiraImport::StartImportService do it_behaves_like 'responds with error', 'Jira integration not configured.' end - context 'when issues feature are disabled' do - let_it_be(:project, reload: true) { create(:project, :issues_disabled) } - - it_behaves_like 'responds with error', 'Cannot import because issues are not available in this project.' - end - context 'when Jira service exists' do let!(:jira_service) { create(:jira_service, project: project, active: true) } context 'when Jira project key is not provided' do + let(:key) { '' } + it_behaves_like 'responds with error', 'Unable to find Jira project to import data from.' end + context 'when issues feature are disabled' do + let_it_be(:project, reload: true) { create(:project, :issues_disabled) } + + it_behaves_like 'responds with error', 'Cannot import because issues are not available in this project.' + end + context 'when correct data provided' do let(:fake_key) { 'some-key' } @@ -62,15 +67,17 @@ describe JiraImport::StartImportService do it_behaves_like 'responds with error', 'Jira import is already running.' end - it 'returns success response' do - expect(subject).to be_a(ServiceResponse) - expect(subject).to be_success - end + context 'when everything is ok' do + it 'returns success response' do + expect(subject).to be_a(ServiceResponse) + expect(subject).to be_success + end - it 'schedules jira import' do - subject + it 'schedules jira import' do + subject - expect(project.latest_jira_import).to be_scheduled + expect(project.latest_jira_import).to be_scheduled + end end it 'creates jira import data' do diff --git a/spec/services/merge_requests/merge_orchestration_service_spec.rb b/spec/services/merge_requests/merge_orchestration_service_spec.rb new file mode 100644 index 00000000000..c50f20d7703 --- /dev/null +++ b/spec/services/merge_requests/merge_orchestration_service_spec.rb @@ -0,0 +1,116 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MergeRequests::MergeOrchestrationService do + let_it_be(:maintainer) { create(:user) } + let(:merge_params) { { sha: merge_request.diff_head_sha } } + let(:user) { maintainer } + let(:service) { described_class.new(project, user, merge_params) } + + let!(:merge_request) do + create(:merge_request, source_project: project, source_branch: 'feature', + target_project: project, target_branch: 'master') + end + + shared_context 'fresh repository' do + let_it_be(:project) { create(:project, :repository) } + + before_all do + project.add_maintainer(maintainer) + end + end + + describe '#execute' do + subject { service.execute(merge_request) } + + include_context 'fresh repository' + + context 'when merge request is mergeable' do + context 'when merge request can be merged automatically' do + before do + create(:ci_pipeline, :detached_merge_request_pipeline, project: project, merge_request: merge_request) + merge_request.update_head_pipeline + end + + it 'schedules auto merge' do + expect_next_instance_of(AutoMergeService, project, user, merge_params) do |service| + expect(service).to receive(:execute).with(merge_request).and_call_original + end + + subject + + expect(merge_request).to be_auto_merge_enabled + expect(merge_request.auto_merge_strategy).to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) + expect(merge_request).not_to be_merged + end + end + + context 'when merge request cannot be merged automatically' do + it 'merges immediately', :sidekiq_inline do + expect(merge_request) + .to receive(:merge_async).with(user.id, merge_params) + .and_call_original + + subject + + merge_request.reset + expect(merge_request).to be_merged + expect(merge_request).not_to be_auto_merge_enabled + end + end + end + + context 'when merge request is not mergeable' do + before do + allow(merge_request).to receive(:mergeable_state?) { false } + end + + it 'does nothing' do + subject + + expect(merge_request).not_to be_auto_merge_enabled + expect(merge_request).not_to be_merged + end + end + end + + describe '#can_merge?' do + subject { service.can_merge?(merge_request) } + + include_context 'fresh repository' + + context 'when merge request is mergeable' do + it { is_expected.to eq(true) } + end + + context 'when merge request is not mergeable' do + before do + allow(merge_request).to receive(:mergeable_state?) { false } + end + + it { is_expected.to eq(false) } + end + end + + describe '#preferred_auto_merge_strategy' do + subject { service.preferred_auto_merge_strategy(merge_request) } + + include_context 'fresh repository' + + context 'when merge request can be merged automatically' do + before do + create(:ci_pipeline, :detached_merge_request_pipeline, project: project, merge_request: merge_request) + merge_request.update_head_pipeline + end + + it 'fetches perferred auto merge strategy' do + is_expected.to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS) + end + end + + context 'when merge request cannot be merged automatically' do + it { is_expected.to be_nil } + end + end +end diff --git a/spec/services/merge_requests/pushed_branches_service_spec.rb b/spec/services/merge_requests/pushed_branches_service_spec.rb new file mode 100644 index 00000000000..7b5d505f4d9 --- /dev/null +++ b/spec/services/merge_requests/pushed_branches_service_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MergeRequests::PushedBranchesService do + let(:project) { create(:project) } + let!(:service) { described_class.new(project, nil, changes: pushed_branches) } + + context 'when branches pushed' do + let(:pushed_branches) do + %w(branch1 branch2 extra1 extra2 extra3).map do |branch| + { ref: "refs/heads/#{branch}" } + end + end + + it 'returns only branches which have a merge request' do + create(:merge_request, source_branch: 'branch1', source_project: project) + create(:merge_request, source_branch: 'branch2', source_project: project) + create(:merge_request, target_branch: 'branch2', source_project: project) + create(:merge_request, :closed, target_branch: 'extra1', source_project: project) + create(:merge_request, source_branch: 'extra2') + + expect(service.execute).to contain_exactly('branch1', 'branch2') + end + end + + context 'when tags pushed' do + let(:pushed_branches) do + %w(v10.0.0 v11.0.2 v12.1.0).map do |branch| + { ref: "refs/tags/#{branch}" } + end + end + + it 'returns empty result without any SQL query performed' do + control_count = ActiveRecord::QueryRecorder.new do + expect(service.execute).to be_empty + end.count + + expect(control_count).to be_zero + end + end +end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index dd5d90b2d07..8c1800c495f 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -208,7 +208,7 @@ describe MergeRequests::UpdateService, :mailer do end end - context 'merge' do + shared_examples_for 'correct merge behavior' do let(:opts) do { merge: merge_request.diff_head_sha @@ -311,6 +311,18 @@ describe MergeRequests::UpdateService, :mailer do end end + describe 'merge' do + it_behaves_like 'correct merge behavior' + + context 'when merge_orchestration_service feature flag is disabled' do + before do + stub_feature_flags(merge_orchestration_service: false) + end + + it_behaves_like 'correct merge behavior' + end + end + context 'todos' do let!(:pending_todo) { create(:todo, :assigned, user: user, project: project, target: merge_request, author: user2) } diff --git a/spec/services/metrics/dashboard/transient_embed_service_spec.rb b/spec/services/metrics/dashboard/transient_embed_service_spec.rb index fddfbe15281..4982f56cddc 100644 --- a/spec/services/metrics/dashboard/transient_embed_service_spec.rb +++ b/spec/services/metrics/dashboard/transient_embed_service_spec.rb @@ -38,21 +38,7 @@ describe Metrics::Dashboard::TransientEmbedService, :use_clean_rails_memory_stor end describe '#get_dashboard' do - let(:embed_json) do - { - panel_groups: [{ - panels: [{ - type: 'line-graph', - title: 'title', - y_label: 'y_label', - metrics: [{ - query_range: 'up', - label: 'y_label' - }] - }] - }] - }.to_json - end + let(:embed_json) { get_embed_json } let(:service_params) { [project, user, { environment: environment, embedded: 'true', embed_json: embed_json }] } let(:service_call) { described_class.new(*service_params).get_dashboard } @@ -68,5 +54,39 @@ describe Metrics::Dashboard::TransientEmbedService, :use_clean_rails_memory_stor described_class.new(*service_params).get_dashboard described_class.new(*service_params).get_dashboard end + + it 'caches unique requests separately' do + alt_embed_json = get_embed_json('area-chart') + alt_service_params = [project, user, { environment: environment, embedded: 'true', embed_json: alt_embed_json }] + + embed = described_class.new(*service_params).get_dashboard + alt_embed = described_class.new(*alt_service_params).get_dashboard + + expect(embed).not_to eq(alt_embed) + expect(get_type_for_embed(embed)).to eq('line-graph') + expect(get_type_for_embed(alt_embed)).to eq('area-chart') + end + + private + + def get_embed_json(type = 'line-graph') + { + panel_groups: [{ + panels: [{ + type: type, + title: 'title', + y_label: 'y_label', + metrics: [{ + query_range: 'up', + label: 'y_label' + }] + }] + }] + }.to_json + end + + def get_type_for_embed(embed) + embed[:dashboard][:panel_groups][0][:panels][0][:type] + end end end diff --git a/spec/services/personal_access_tokens/create_service_spec.rb b/spec/services/personal_access_tokens/create_service_spec.rb new file mode 100644 index 00000000000..9190434b96a --- /dev/null +++ b/spec/services/personal_access_tokens/create_service_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PersonalAccessTokens::CreateService do + describe '#execute' do + context 'with valid params' do + it 'creates personal access token record' do + user = create(:user) + params = { name: 'Test token', impersonation: true, scopes: [:api], expires_at: Date.today + 1.month } + + response = described_class.new(user, params).execute + personal_access_token = response.payload[:personal_access_token] + + expect(response.success?).to be true + expect(personal_access_token.name).to eq(params[:name]) + expect(personal_access_token.impersonation).to eq(params[:impersonation]) + expect(personal_access_token.scopes).to eq(params[:scopes]) + expect(personal_access_token.expires_at).to eq(params[:expires_at]) + expect(personal_access_token.user).to eq(user) + end + end + end +end diff --git a/spec/services/pod_logs/base_service_spec.rb b/spec/services/pod_logs/base_service_spec.rb index fb53321352b..3ec5dc68c60 100644 --- a/spec/services/pod_logs/base_service_spec.rb +++ b/spec/services/pod_logs/base_service_spec.rb @@ -13,10 +13,16 @@ describe ::PodLogs::BaseService do let(:container_name) { 'container-0' } let(:params) { {} } let(:raw_pods) do - JSON.parse([ - kube_pod(name: pod_name), - kube_pod(name: pod_name_2) - ].to_json, object_class: OpenStruct) + [ + { + name: pod_name, + container_names: %w(container-0-0 container-0-1) + }, + { + name: pod_name_2, + container_names: %w(container-1-0 container-1-1) + } + ] end subject { described_class.new(cluster, namespace, params: params) } @@ -99,19 +105,6 @@ describe ::PodLogs::BaseService do end end - describe '#get_raw_pods' do - let(:service) { create(:cluster_platform_kubernetes, :configured) } - - it 'returns success with passthrough k8s response' do - stub_kubeclient_pods(namespace) - - result = subject.send(:get_raw_pods, {}) - - expect(result[:status]).to eq(:success) - expect(result[:raw_pods].first).to be_a(Kubeclient::Resource) - end - end - describe '#get_pod_names' do it 'returns success with a list of pods' do result = subject.send(:get_pod_names, raw_pods: raw_pods) diff --git a/spec/services/pod_logs/elasticsearch_service_spec.rb b/spec/services/pod_logs/elasticsearch_service_spec.rb index 39aa910d878..e3efce1134b 100644 --- a/spec/services/pod_logs/elasticsearch_service_spec.rb +++ b/spec/services/pod_logs/elasticsearch_service_spec.rb @@ -21,8 +21,63 @@ describe ::PodLogs::ElasticsearchService do ] end + let(:raw_pods) do + [ + { + name: pod_name, + container_names: [container_name, "#{container_name}-1"] + } + ] + end + subject { described_class.new(cluster, namespace, params: params) } + describe '#get_raw_pods' do + before do + create(:clusters_applications_elastic_stack, :installed, cluster: cluster) + end + + it 'returns success with elasticsearch response' do + allow_any_instance_of(::Clusters::Applications::ElasticStack) + .to receive(:elasticsearch_client) + .and_return(Elasticsearch::Transport::Client.new) + allow_any_instance_of(::Gitlab::Elasticsearch::Logs::Pods) + .to receive(:pods) + .with(namespace) + .and_return(raw_pods) + + result = subject.send(:get_raw_pods, {}) + + expect(result[:status]).to eq(:success) + expect(result[:raw_pods]).to eq(raw_pods) + end + + it 'returns an error when ES is unreachable' do + allow_any_instance_of(::Clusters::Applications::ElasticStack) + .to receive(:elasticsearch_client) + .and_return(nil) + + result = subject.send(:get_raw_pods, {}) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq('Unable to connect to Elasticsearch') + end + + it 'handles server errors from elasticsearch' do + allow_any_instance_of(::Clusters::Applications::ElasticStack) + .to receive(:elasticsearch_client) + .and_return(Elasticsearch::Transport::Client.new) + allow_any_instance_of(::Gitlab::Elasticsearch::Logs::Pods) + .to receive(:pods) + .and_raise(Elasticsearch::Transport::Transport::Errors::ServiceUnavailable.new) + + result = subject.send(:get_raw_pods, {}) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq('Elasticsearch returned status code: ServiceUnavailable') + end + end + describe '#check_times' do context 'with start and end provided and valid' do let(:params) do @@ -168,7 +223,7 @@ describe ::PodLogs::ElasticsearchService do allow_any_instance_of(::Clusters::Applications::ElasticStack) .to receive(:elasticsearch_client) .and_return(Elasticsearch::Transport::Client.new) - allow_any_instance_of(::Gitlab::Elasticsearch::Logs) + allow_any_instance_of(::Gitlab::Elasticsearch::Logs::Lines) .to receive(:pod_logs) .with(namespace, pod_name: pod_name, container_name: container_name, search: search, start_time: start_time, end_time: end_time, cursor: cursor) .and_return({ logs: expected_logs, cursor: expected_cursor }) @@ -195,7 +250,7 @@ describe ::PodLogs::ElasticsearchService do allow_any_instance_of(::Clusters::Applications::ElasticStack) .to receive(:elasticsearch_client) .and_return(Elasticsearch::Transport::Client.new) - allow_any_instance_of(::Gitlab::Elasticsearch::Logs) + allow_any_instance_of(::Gitlab::Elasticsearch::Logs::Lines) .to receive(:pod_logs) .and_raise(Elasticsearch::Transport::Transport::Errors::ServiceUnavailable.new) @@ -209,9 +264,9 @@ describe ::PodLogs::ElasticsearchService do allow_any_instance_of(::Clusters::Applications::ElasticStack) .to receive(:elasticsearch_client) .and_return(Elasticsearch::Transport::Client.new) - allow_any_instance_of(::Gitlab::Elasticsearch::Logs) + allow_any_instance_of(::Gitlab::Elasticsearch::Logs::Lines) .to receive(:pod_logs) - .and_raise(::Gitlab::Elasticsearch::Logs::InvalidCursor.new) + .and_raise(::Gitlab::Elasticsearch::Logs::Lines::InvalidCursor.new) result = subject.send(:pod_logs, result_arg) diff --git a/spec/services/pod_logs/kubernetes_service_spec.rb b/spec/services/pod_logs/kubernetes_service_spec.rb index ff0554bbe5c..da89c7ee117 100644 --- a/spec/services/pod_logs/kubernetes_service_spec.rb +++ b/spec/services/pod_logs/kubernetes_service_spec.rb @@ -20,14 +20,36 @@ describe ::PodLogs::KubernetesService do end let(:raw_pods) do - JSON.parse([ - kube_pod(name: pod_name), - kube_pod(name: pod_name_2, container_name: container_name_2) - ].to_json, object_class: OpenStruct) + [ + { + name: pod_name, + container_names: [container_name, "#{container_name}-1"] + }, + { + name: pod_name_2, + container_names: [container_name_2, "#{container_name_2}-1"] + } + ] end subject { described_class.new(cluster, namespace, params: params) } + describe '#get_raw_pods' do + let(:service) { create(:cluster_platform_kubernetes, :configured) } + + it 'returns success with passthrough k8s response' do + stub_kubeclient_pods(namespace) + + result = subject.send(:get_raw_pods, {}) + + expect(result[:status]).to eq(:success) + expect(result[:raw_pods]).to eq([{ + name: 'kube-pod', + container_names: %w(container-0 container-0-1) + }]) + end + end + describe '#pod_logs' do let(:result_arg) do { @@ -233,7 +255,7 @@ describe ::PodLogs::KubernetesService do end it 'returns error if container_name was not specified and there are no containers on the pod' do - raw_pods.first.spec.containers = [] + raw_pods.first[:container_names] = [] result = subject.send(:check_container_name, pod_name: pod_name, diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index 6cc2e2b6abe..36f9966c0ef 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -492,7 +492,7 @@ describe QuickActions::InterpretService do end end - shared_examples 'merge command' do + shared_examples 'merge immediately command' do let(:project) { create(:project, :repository) } it 'runs merge command if content contains /merge' do @@ -504,7 +504,18 @@ describe QuickActions::InterpretService do it 'returns them merge message' do _, _, message = service.execute(content, issuable) - expect(message).to eq('Scheduled to merge this merge request when the pipeline succeeds.') + expect(message).to eq('Merged this merge request.') + end + end + + shared_examples 'merge automatically command' do + let(:project) { create(:project, :repository) } + + it 'runs merge command if content contains /merge and returns merge message' do + _, updates, message = service.execute(content, issuable) + + expect(updates).to eq(merge: merge_request.diff_head_sha) + expect(message).to eq('Scheduled to merge this merge request (Merge when pipeline succeeds).') end end @@ -675,11 +686,23 @@ describe QuickActions::InterpretService do context 'merge command' do let(:service) { described_class.new(project, developer, { merge_request_diff_head_sha: merge_request.diff_head_sha }) } - it_behaves_like 'merge command' do + it_behaves_like 'merge immediately command' do let(:content) { '/merge' } let(:issuable) { merge_request } end + context 'when the head pipeline of merge request is running' do + before do + create(:ci_pipeline, :detached_merge_request_pipeline, merge_request: merge_request) + merge_request.update_head_pipeline + end + + it_behaves_like 'merge automatically command' do + let(:content) { '/merge' } + let(:issuable) { merge_request } + end + end + context 'can not be merged when logged user does not have permissions' do let(:service) { described_class.new(project, create(:user)) } diff --git a/spec/services/resources/create_access_token_service_spec.rb b/spec/services/resources/create_access_token_service_spec.rb new file mode 100644 index 00000000000..8c108d9937a --- /dev/null +++ b/spec/services/resources/create_access_token_service_spec.rb @@ -0,0 +1,163 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Resources::CreateAccessTokenService do + subject { described_class.new(resource_type, resource, user, params).execute } + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:params) { {} } + + describe '#execute' do + # Created shared_examples as it will easy to include specs for group bots in https://gitlab.com/gitlab-org/gitlab/-/issues/214046 + shared_examples 'fails when user does not have the permission to create a Resource Bot' do + before do + resource.add_developer(user) + end + + it 'returns error' do + response = subject + + expect(response.error?).to be true + expect(response.message).to eq("User does not have permission to create #{resource_type} Access Token") + end + end + + shared_examples 'fails when flag is disabled' do + before do + stub_feature_flags(resource_access_token: false) + end + + it 'returns nil' do + expect(subject).to be nil + end + end + + shared_examples 'allows creation of bot with valid params' do + it { expect { subject }.to change { User.count }.by(1) } + + it 'creates resource bot user' do + response = subject + + access_token = response.payload[:access_token] + + expect(access_token.user.reload.user_type).to eq("#{resource_type}_bot") + end + + context 'bot name' do + context 'when no value is passed' do + it 'uses default value' do + response = subject + access_token = response.payload[:access_token] + + expect(access_token.user.name).to eq("#{resource.name.to_s.humanize} bot") + end + end + + context 'when user provides value' do + let(:params) { { name: 'Random bot' } } + + it 'overrides the default value' do + response = subject + access_token = response.payload[:access_token] + + expect(access_token.user.name).to eq(params[:name]) + end + end + end + + it 'adds the bot user as a maintainer in the resource' do + response = subject + access_token = response.payload[:access_token] + bot_user = access_token.user + + expect(resource.members.maintainers.map(&:user_id)).to include(bot_user.id) + end + + context 'personal access token' do + it { expect { subject }.to change { PersonalAccessToken.count }.by(1) } + + context 'when user does not provide scope' do + it 'has default scopes' do + response = subject + access_token = response.payload[:access_token] + + expect(access_token.scopes).to eq(Gitlab::Auth::API_SCOPES + Gitlab::Auth::REPOSITORY_SCOPES + Gitlab::Auth.registry_scopes - [:read_user]) + end + end + + context 'when user provides scope explicitly' do + let(:params) { { scopes: Gitlab::Auth::REPOSITORY_SCOPES } } + + it 'overrides the default value' do + response = subject + access_token = response.payload[:access_token] + + expect(access_token.scopes).to eq(Gitlab::Auth::REPOSITORY_SCOPES) + end + end + + context 'expires_at' do + context 'when no value is passed' do + it 'uses default value' do + response = subject + access_token = response.payload[:access_token] + + expect(access_token.expires_at).to eq(nil) + end + end + + context 'when user provides value' do + let(:params) { { expires_at: Date.today + 1.month } } + + it 'overrides the default value' do + response = subject + access_token = response.payload[:access_token] + + expect(access_token.expires_at).to eq(params[:expires_at]) + end + end + + context 'when invalid scope is passed' do + let(:params) { { scopes: [:invalid_scope] } } + + it 'returns error' do + response = subject + + expect(response.error?).to be true + end + end + end + end + + context 'when access provisioning fails' do + before do + allow(resource).to receive(:add_maintainer).and_return(nil) + end + + it 'returns error' do + response = subject + + expect(response.error?).to be true + end + end + end + + context 'when resource is a project' do + let(:resource_type) { 'project' } + let(:resource) { project } + + it_behaves_like 'fails when user does not have the permission to create a Resource Bot' + it_behaves_like 'fails when flag is disabled' + + context 'user with valid permission' do + before do + resource.add_maintainer(user) + end + + it_behaves_like 'allows creation of bot with valid params' + end + end + end +end diff --git a/spec/services/snippets/create_service_spec.rb b/spec/services/snippets/create_service_spec.rb index 690aa2c066e..c1a8a026b90 100644 --- a/spec/services/snippets/create_service_spec.rb +++ b/spec/services/snippets/create_service_spec.rb @@ -252,6 +252,39 @@ describe Snippets::CreateService do end end + shared_examples 'after_save callback to store_mentions' do + context 'when mentionable attributes change' do + let(:extra_opts) { { description: "Description with #{user.to_reference}" } } + + it 'saves mentions' do + expect_next_instance_of(Snippet) do |instance| + expect(instance).to receive(:store_mentions!).and_call_original + end + expect(snippet.user_mentions.count).to eq 1 + end + end + + context 'when mentionable attributes do not change' do + it 'does not call store_mentions' do + expect_next_instance_of(Snippet) do |instance| + expect(instance).not_to receive(:store_mentions!) + end + expect(snippet.user_mentions.count).to eq 0 + end + end + + context 'when save fails' do + it 'does not call store_mentions' do + base_opts.delete(:title) + + expect_next_instance_of(Snippet) do |instance| + expect(instance).not_to receive(:store_mentions!) + end + expect(snippet.valid?).to be false + end + end + end + context 'when ProjectSnippet' do let_it_be(:project) { create(:project) } @@ -265,6 +298,7 @@ describe Snippets::CreateService do it_behaves_like 'snippet create data is tracked' it_behaves_like 'an error service response when save fails' it_behaves_like 'creates repository and files' + it_behaves_like 'after_save callback to store_mentions' end context 'when PersonalSnippet' do @@ -276,6 +310,9 @@ describe Snippets::CreateService do it_behaves_like 'snippet create data is tracked' it_behaves_like 'an error service response when save fails' it_behaves_like 'creates repository and files' + pending('See https://gitlab.com/gitlab-org/gitlab/issues/30742') do + it_behaves_like 'after_save callback to store_mentions' + end end end end diff --git a/spec/services/terraform/remote_state_handler_spec.rb b/spec/services/terraform/remote_state_handler_spec.rb new file mode 100644 index 00000000000..f4e1831b2e8 --- /dev/null +++ b/spec/services/terraform/remote_state_handler_spec.rb @@ -0,0 +1,143 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Terraform::RemoteStateHandler do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + + describe '#find_with_lock' do + context 'without a state name' do + subject { described_class.new(project, user) } + + it 'raises an exception' do + expect { subject.find_with_lock }.to raise_error(ArgumentError) + end + end + + context 'with a state name' do + subject { described_class.new(project, user, name: 'state') } + + context 'with no matching state' do + it 'raises an exception' do + expect { subject.find_with_lock }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'with a matching state' do + let!(:state) { create(:terraform_state, project: project, name: 'state') } + + it 'returns the state' do + expect(subject.find_with_lock).to eq(state) + end + end + end + end + + describe '#create_or_find!' do + it 'requires passing a state name' do + handler = described_class.new(project, user) + + expect { handler.create_or_find! }.to raise_error(ArgumentError) + end + + it 'allows to create states with same name in different projects' do + project_b = create(:project) + + state_a = described_class.new(project, user, name: 'my-state').create_or_find! + state_b = described_class.new(project_b, user, name: 'my-state').create_or_find! + + expect(state_a).to be_persisted + expect(state_b).to be_persisted + expect(state_a.id).not_to eq state_b.id + end + + it 'loads the same state upon subsequent call in the project scope' do + state_a = described_class.new(project, user, name: 'my-state').create_or_find! + state_b = described_class.new(project, user, name: 'my-state').create_or_find! + + expect(state_a).to be_persisted + expect(state_a.id).to eq state_b.id + end + end + + context 'when state locking is not being used' do + subject { described_class.new(project, user, name: 'my-state') } + + describe '#handle_with_lock' do + it 'allows to modify a state using database locking' do + state = subject.handle_with_lock do |state| + state.name = 'updated-name' + end + + expect(state.name).to eq 'updated-name' + end + + it 'returns the state object itself' do + state = subject.create_or_find! + + expect(state.name).to eq 'my-state' + end + end + + describe '#lock!' do + it 'raises an error' do + expect { subject.lock! }.to raise_error(ArgumentError) + end + end + end + + context 'when using locking' do + describe '#handle_with_lock' do + it 'handles a locked state using exclusive read lock' do + handler = described_class + .new(project, user, name: 'new-state', lock_id: 'abc-abc') + + handler.lock! + + state = handler.handle_with_lock do |state| + state.name = 'new-name' + end + + expect(state.name).to eq 'new-name' + end + end + + it 'raises exception if lock has not been acquired before' do + handler = described_class + .new(project, user, name: 'new-state', lock_id: 'abc-abc') + + expect { handler.handle_with_lock } + .to raise_error(described_class::StateLockedError) + end + + describe '#lock!' do + it 'allows to lock state if it does not exist yet' do + handler = described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc') + + state = handler.lock! + + expect(state).to be_persisted + expect(state.name).to eq 'new-state' + end + + it 'allows to lock state if it exists and is not locked' do + state = described_class.new(project, user, name: 'new-state').create_or_find! + handler = described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc') + + handler.lock! + + expect(state.reload.lock_xid).to eq 'abc-abc' + expect(state).to be_locked + end + + it 'raises an exception when trying to unlocked state locked by someone else' do + described_class.new(project, user, name: 'new-state', lock_id: 'abc-abc').lock! + + handler = described_class.new(project, user, name: 'new-state', lock_id: '12a-23f') + + expect { handler.lock! }.to raise_error(described_class::StateLockedError) + end + end + end +end diff --git a/spec/services/users/build_service_spec.rb b/spec/services/users/build_service_spec.rb index 146819c7f44..7588be833ae 100644 --- a/spec/services/users/build_service_spec.rb +++ b/spec/services/users/build_service_spec.rb @@ -157,6 +157,26 @@ describe Users::BuildService do end end + context 'when user_type is provided' do + subject(:user) { service.execute } + + context 'when project_bot' do + before do + params.merge!({ user_type: :project_bot }) + end + + it { expect(user.project_bot?).to be true} + end + + context 'when not a project_bot' do + before do + params.merge!({ user_type: :alert_bot }) + end + + it { expect(user.user_type).to be nil } + end + end + context 'with "user_default_external" application setting' do using RSpec::Parameterized::TableSyntax diff --git a/spec/services/x509_certificate_revoke_service_spec.rb b/spec/services/x509_certificate_revoke_service_spec.rb index ef76f616c93..c2b2576904c 100644 --- a/spec/services/x509_certificate_revoke_service_spec.rb +++ b/spec/services/x509_certificate_revoke_service_spec.rb @@ -24,8 +24,6 @@ describe X509CertificateRevokeService do end context 'for good certificates' do - RSpec::Matchers.define_negated_matcher :not_change, :change - let(:x509_certificate) { create(:x509_certificate) } it 'do not update any commit signature' do -- cgit v1.2.3