summaryrefslogtreecommitdiff
path: root/spec/services
diff options
context:
space:
mode:
authorRobert Speicher <rspeicher@gmail.com>2021-01-20 13:34:23 -0600
committerRobert Speicher <rspeicher@gmail.com>2021-01-20 13:34:23 -0600
commit6438df3a1e0fb944485cebf07976160184697d72 (patch)
tree00b09bfd170e77ae9391b1a2f5a93ef6839f2597 /spec/services
parent42bcd54d971da7ef2854b896a7b34f4ef8601067 (diff)
downloadgitlab-ce-6438df3a1e0fb944485cebf07976160184697d72.tar.gz
Add latest changes from gitlab-org/gitlab@13-8-stable-eev13.8.0-rc42
Diffstat (limited to 'spec/services')
-rw-r--r--spec/services/alert_management/alerts/todo/create_service_spec.rb4
-rw-r--r--spec/services/alert_management/sync_alert_service_data_service_spec.rb55
-rw-r--r--spec/services/ci/build_report_result_service_spec.rb6
-rw-r--r--spec/services/ci/create_downstream_pipeline_service_spec.rb47
-rw-r--r--spec/services/ci/create_pipeline_service/dry_run_spec.rb2
-rw-r--r--spec/services/ci/create_pipeline_service_spec.rb73
-rw-r--r--spec/services/ci/destroy_expired_job_artifacts_service_spec.rb76
-rw-r--r--spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb (renamed from spec/services/ci/pipelines/create_artifact_service_spec.rb)2
-rw-r--r--spec/services/ci/pipeline_artifacts/destroy_expired_artifacts_service_spec.rb81
-rw-r--r--spec/services/ci/pipeline_processing/test_cases/dag_build_fails_deploy_is_delayed_and_needs_test.yml41
-rw-r--r--spec/services/ci/pipeline_processing/test_cases/dag_build_fails_deploy_is_manual_and_needs_test.yml40
-rw-r--r--spec/services/ci/pipeline_trigger_service_spec.rb14
-rw-r--r--spec/services/ci/play_build_service_spec.rb25
-rw-r--r--spec/services/ci/process_build_service_spec.rb40
-rw-r--r--spec/services/ci/retry_build_service_spec.rb16
-rw-r--r--spec/services/ci/retry_pipeline_service_spec.rb22
-rw-r--r--spec/services/ci/test_failure_history_service_spec.rb21
-rw-r--r--spec/services/ci/update_build_state_service_spec.rb21
-rw-r--r--spec/services/container_expiration_policies/cleanup_service_spec.rb43
-rw-r--r--spec/services/draft_notes/create_service_spec.rb17
-rw-r--r--spec/services/draft_notes/publish_service_spec.rb22
-rw-r--r--spec/services/feature_flags/create_service_spec.rb24
-rw-r--r--spec/services/feature_flags/update_service_spec.rb24
-rw-r--r--spec/services/git/branch_push_service_spec.rb4
-rw-r--r--spec/services/groups/create_service_spec.rb8
-rw-r--r--spec/services/groups/destroy_service_spec.rb111
-rw-r--r--spec/services/incident_management/pager_duty/process_webhook_service_spec.rb2
-rw-r--r--spec/services/issuable/bulk_update_service_spec.rb51
-rw-r--r--spec/services/issues/close_service_spec.rb27
-rw-r--r--spec/services/issues/export_csv_service_spec.rb4
-rw-r--r--spec/services/jira_connect/sync_service_spec.rb4
-rw-r--r--spec/services/jira_connect_subscriptions/create_service_spec.rb12
-rw-r--r--spec/services/members/create_service_spec.rb51
-rw-r--r--spec/services/merge_requests/after_create_service_spec.rb22
-rw-r--r--spec/services/merge_requests/cleanup_refs_service_spec.rb20
-rw-r--r--spec/services/merge_requests/close_service_spec.rb9
-rw-r--r--spec/services/merge_requests/create_from_issue_service_spec.rb3
-rw-r--r--spec/services/merge_requests/create_service_spec.rb6
-rw-r--r--spec/services/merge_requests/export_csv_service_spec.rb4
-rw-r--r--spec/services/merge_requests/merge_service_spec.rb30
-rw-r--r--spec/services/merge_requests/mergeability_check_service_spec.rb49
-rw-r--r--spec/services/merge_requests/post_merge_service_spec.rb9
-rw-r--r--spec/services/merge_requests/reopen_service_spec.rb9
-rw-r--r--spec/services/merge_requests/update_service_spec.rb29
-rw-r--r--spec/services/namespaces/package_settings/update_service_spec.rb102
-rw-r--r--spec/services/notes/create_service_spec.rb30
-rw-r--r--spec/services/notes/destroy_service_spec.rb8
-rw-r--r--spec/services/notes/quick_actions_service_spec.rb8
-rw-r--r--spec/services/notes/update_service_spec.rb14
-rw-r--r--spec/services/notification_service_spec.rb12
-rw-r--r--spec/services/onboarding_progress_service_spec.rb36
-rw-r--r--spec/services/packages/create_event_service_spec.rb25
-rw-r--r--spec/services/packages/debian/create_package_file_service_spec.rb106
-rw-r--r--spec/services/packages/debian/extract_metadata_service_spec.rb59
-rw-r--r--spec/services/packages/debian/get_or_create_incoming_service_spec.rb36
-rw-r--r--spec/services/packages/maven/find_or_create_package_service_spec.rb54
-rw-r--r--spec/services/packages/nuget/search_service_spec.rb150
-rw-r--r--spec/services/pages/migrate_legacy_storage_to_deployment_service_spec.rb118
-rw-r--r--spec/services/pages/zip_directory_service_spec.rb28
-rw-r--r--spec/services/post_receive_service_spec.rb11
-rw-r--r--spec/services/projects/after_import_service_spec.rb6
-rw-r--r--spec/services/projects/container_repository/cleanup_tags_service_spec.rb116
-rw-r--r--spec/services/projects/container_repository/delete_tags_service_spec.rb4
-rw-r--r--spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb17
-rw-r--r--spec/services/projects/fork_service_spec.rb18
-rw-r--r--spec/services/projects/housekeeping_service_spec.rb134
-rw-r--r--spec/services/projects/schedule_bulk_repository_shard_moves_service_spec.rb43
-rw-r--r--spec/services/projects/update_pages_service_spec.rb13
-rw-r--r--spec/services/projects/update_service_spec.rb43
-rw-r--r--spec/services/quick_actions/interpret_service_spec.rb212
-rw-r--r--spec/services/repositories/housekeeping_service_spec.rb14
-rw-r--r--spec/services/resource_events/change_state_service_spec.rb9
-rw-r--r--spec/services/service_desk_settings/update_service_spec.rb13
-rw-r--r--spec/services/snippets/schedule_bulk_repository_shard_moves_service_spec.rb12
-rw-r--r--spec/services/snippets/update_repository_storage_service_spec.rb137
-rw-r--r--spec/services/todo_service_spec.rb258
-rw-r--r--spec/services/users/update_service_spec.rb2
-rw-r--r--spec/services/web_hook_service_spec.rb9
78 files changed, 2232 insertions, 805 deletions
diff --git a/spec/services/alert_management/alerts/todo/create_service_spec.rb b/spec/services/alert_management/alerts/todo/create_service_spec.rb
index e3d9de8b4df..fa4fd8ed0b2 100644
--- a/spec/services/alert_management/alerts/todo/create_service_spec.rb
+++ b/spec/services/alert_management/alerts/todo/create_service_spec.rb
@@ -58,6 +58,10 @@ RSpec.describe AlertManagement::Alerts::Todo::CreateService do
create(:todo, :pending, **todo_params)
end
+ before do
+ stub_feature_flags(multiple_todos: false)
+ end
+
it 'does not create a todo' do
expect { result }.not_to change { Todo.count }
end
diff --git a/spec/services/alert_management/sync_alert_service_data_service_spec.rb b/spec/services/alert_management/sync_alert_service_data_service_spec.rb
deleted file mode 100644
index ecec60011db..00000000000
--- a/spec/services/alert_management/sync_alert_service_data_service_spec.rb
+++ /dev/null
@@ -1,55 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe AlertManagement::SyncAlertServiceDataService do
- let_it_be(:alerts_service) do
- AlertsService.skip_callback(:save, :after, :update_http_integration)
- service = create(:alerts_service, :active)
- AlertsService.set_callback(:save, :after, :update_http_integration)
-
- service
- end
-
- describe '#execute' do
- subject(:execute) { described_class.new(alerts_service).execute }
-
- context 'without http integration' do
- it 'creates the integration' do
- expect { execute }
- .to change { AlertManagement::HttpIntegration.count }.by(1)
- end
-
- it 'returns a success' do
- expect(subject.success?).to eq(true)
- end
- end
-
- context 'existing legacy http integration' do
- let_it_be(:integration) { create(:alert_management_http_integration, :legacy, project: alerts_service.project) }
-
- it 'updates the integration' do
- expect { execute }
- .to change { integration.reload.encrypted_token }.to(alerts_service.data.encrypted_token)
- .and change { integration.encrypted_token_iv }.to(alerts_service.data.encrypted_token_iv)
- end
-
- it 'returns a success' do
- expect(subject.success?).to eq(true)
- end
- end
-
- context 'existing other http integration' do
- let_it_be(:integration) { create(:alert_management_http_integration, project: alerts_service.project) }
-
- it 'creates the integration' do
- expect { execute }
- .to change { AlertManagement::HttpIntegration.count }.by(1)
- end
-
- it 'returns a success' do
- expect(subject.success?).to eq(true)
- end
- end
- end
-end
diff --git a/spec/services/ci/build_report_result_service_spec.rb b/spec/services/ci/build_report_result_service_spec.rb
index 244ffbf4bbd..7c2702af086 100644
--- a/spec/services/ci/build_report_result_service_spec.rb
+++ b/spec/services/ci/build_report_result_service_spec.rb
@@ -6,12 +6,6 @@ RSpec.describe Ci::BuildReportResultService do
describe '#execute', :clean_gitlab_redis_shared_state do
subject(:build_report_result) { described_class.new.execute(build) }
- around do |example|
- travel_to(DateTime.parse('2020-07-01')) do
- example.run
- end
- end
-
context 'when build is finished' do
let(:build) { create(:ci_build, :success, :test_reports) }
diff --git a/spec/services/ci/create_downstream_pipeline_service_spec.rb b/spec/services/ci/create_downstream_pipeline_service_spec.rb
index 03cea4074bf..860932d4fde 100644
--- a/spec/services/ci/create_downstream_pipeline_service_spec.rb
+++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb
@@ -371,6 +371,26 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
expect { service.execute(bridge) }.to change { Ci::Pipeline.count }.by(1)
end
end
+
+ context 'when downstream project does not allow user-defined variables for child pipelines' do
+ before do
+ bridge.yaml_variables = [{ key: 'BRIDGE', value: '$PIPELINE_VARIABLE-var', public: true }]
+
+ upstream_pipeline.project.update!(restrict_user_defined_variables: true)
+ end
+
+ it 'creates a new pipeline allowing variables to be passed downstream' do
+ expect { service.execute(bridge) }.to change { Ci::Pipeline.count }.by(1)
+ end
+
+ it 'passes variables downstream from the bridge' do
+ pipeline = service.execute(bridge)
+
+ pipeline.variables.map(&:key).tap do |variables|
+ expect(variables).to include 'BRIDGE'
+ end
+ end
+ end
end
end
@@ -460,6 +480,33 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do
expect(variable.value).to eq 'my-value-var'
end
end
+
+ context 'when downstream project does not allow user-defined variables for multi-project pipelines' do
+ before do
+ downstream_project.update!(restrict_user_defined_variables: true)
+ end
+
+ it 'does not create a new pipeline' do
+ expect { service.execute(bridge) }
+ .not_to change { Ci::Pipeline.count }
+ end
+
+ it 'ignores variables passed downstream from the bridge' do
+ pipeline = service.execute(bridge)
+
+ pipeline.variables.map(&:key).tap do |variables|
+ expect(variables).not_to include 'BRIDGE'
+ end
+ end
+
+ it 'sets errors', :aggregate_failures do
+ service.execute(bridge)
+
+ expect(bridge.reload).to be_failed
+ expect(bridge.failure_reason).to eq('downstream_pipeline_creation_failed')
+ expect(bridge.options[:downstream_errors]).to eq(['Insufficient permissions to set pipeline variables'])
+ end
+ end
end
end
diff --git a/spec/services/ci/create_pipeline_service/dry_run_spec.rb b/spec/services/ci/create_pipeline_service/dry_run_spec.rb
index 60c56ed0f67..c21a4ef0917 100644
--- a/spec/services/ci/create_pipeline_service/dry_run_spec.rb
+++ b/spec/services/ci/create_pipeline_service/dry_run_spec.rb
@@ -108,7 +108,7 @@ RSpec.describe Ci::CreatePipelineService do
it_behaves_like 'returns a non persisted pipeline'
it 'returns a pipeline with errors', :aggregate_failures do
- error_message = "test: needs 'build'"
+ error_message = "'test' job needs 'build' job, but it was not added to the pipeline"
expect(subject.error_messages.map(&:content)).to eq([error_message])
expect(subject.errors).not_to be_empty
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index f9015752644..e1f1bdc41a1 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -91,6 +91,23 @@ RSpec.describe Ci::CreatePipelineService do
.with({ source: 'push' }, 5)
end
+ it 'tracks included template usage' do
+ expect_next_instance_of(Gitlab::Ci::Pipeline::Chain::TemplateUsage) do |instance|
+ expect(instance).to receive(:perform!)
+ end
+
+ execute_service
+ end
+
+ describe 'recording a conversion event' do
+ it 'schedules a record conversion event worker' do
+ expect(Experiments::RecordConversionEventWorker).to receive(:perform_async).with(:ci_syntax_templates, user.id)
+ expect(Experiments::RecordConversionEventWorker).to receive(:perform_async).with(:pipelines_empty_state, user.id)
+
+ pipeline
+ end
+ end
+
context 'when merge requests already exist for this source branch' do
let(:merge_request_1) do
create(:merge_request, source_branch: 'feature', target_branch: "master", source_project: project)
@@ -481,6 +498,7 @@ RSpec.describe Ci::CreatePipelineService do
expect(execute_service).not_to be_persisted
expect(Ci::Pipeline.count).to eq(0)
+ expect(Namespaces::OnboardingPipelineCreatedWorker).not_to receive(:perform_async)
end
shared_examples 'a failed pipeline' do
@@ -1418,6 +1436,13 @@ RSpec.describe Ci::CreatePipelineService do
pipeline
end
+ it 'schedules a namespace onboarding create action worker' do
+ expect(Namespaces::OnboardingPipelineCreatedWorker)
+ .to receive(:perform_async).with(project.namespace_id)
+
+ pipeline
+ end
+
context 'when target sha is specified' do
let(:target_sha) { merge_request.target_branch_sha }
@@ -1688,9 +1713,11 @@ RSpec.describe Ci::CreatePipelineService do
shared_examples 'has errors' do
it 'contains the expected errors' do
expect(pipeline.builds).to be_empty
- expect(pipeline.yaml_errors).to eq("test_a: needs 'build_a'")
- expect(pipeline.error_messages.map(&:content)).to contain_exactly("test_a: needs 'build_a'")
- expect(pipeline.errors[:base]).to contain_exactly("test_a: needs 'build_a'")
+
+ error_message = "'test_a' job needs 'build_a' job, but it was not added to the pipeline"
+ expect(pipeline.yaml_errors).to eq(error_message)
+ expect(pipeline.error_messages.map(&:content)).to contain_exactly(error_message)
+ expect(pipeline.errors[:base]).to contain_exactly(error_message)
end
end
@@ -2385,16 +2412,6 @@ RSpec.describe Ci::CreatePipelineService do
expect(build_names).to contain_exactly('regular-job')
end
- context 'when FF ci_seed_block_run_before_workflow_rules is disabled' do
- before do
- stub_feature_flags(ci_seed_block_run_before_workflow_rules: false)
- end
-
- it 'does not a pipeline' do
- expect(pipeline).not_to be_persisted
- end
- end
-
context 'when a job requires the same variable' do
let(:config) do
<<-EOY
@@ -2423,16 +2440,6 @@ RSpec.describe Ci::CreatePipelineService do
expect(pipeline).to be_persisted
expect(build_names).to contain_exactly('build', 'test1', 'test2')
end
-
- context 'when FF ci_seed_block_run_before_workflow_rules is disabled' do
- before do
- stub_feature_flags(ci_seed_block_run_before_workflow_rules: false)
- end
-
- it 'does not a pipeline' do
- expect(pipeline).not_to be_persisted
- end
- end
end
end
@@ -2443,16 +2450,6 @@ RSpec.describe Ci::CreatePipelineService do
expect(pipeline).not_to be_persisted
end
- context 'when FF ci_seed_block_run_before_workflow_rules is disabled' do
- before do
- stub_feature_flags(ci_seed_block_run_before_workflow_rules: false)
- end
-
- it 'does not create a pipeline' do
- expect(pipeline).not_to be_persisted
- end
- end
-
context 'when a job requires the same variable' do
let(:config) do
<<-EOY
@@ -2480,16 +2477,6 @@ RSpec.describe Ci::CreatePipelineService do
it 'does not create a pipeline' do
expect(pipeline).not_to be_persisted
end
-
- context 'when FF ci_seed_block_run_before_workflow_rules is disabled' do
- before do
- stub_feature_flags(ci_seed_block_run_before_workflow_rules: false)
- end
-
- it 'does not create a pipeline' do
- expect(pipeline).not_to be_persisted
- end
- end
end
end
end
diff --git a/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb b/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb
index c8d426ee657..1edcef2977b 100644
--- a/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb
+++ b/spec/services/ci/destroy_expired_job_artifacts_service_spec.rb
@@ -10,7 +10,7 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
describe '.execute' do
subject { service.execute }
- let_it_be(:artifact, reload: true) do
+ let_it_be(:artifact, refind: true) do
create(:ci_job_artifact, expire_at: 1.day.ago)
end
@@ -30,14 +30,16 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
it 'performs the smallest number of queries for job_artifacts' do
log = ActiveRecord::QueryRecorder.new { subject }
- # SELECT expired ci_job_artifacts
+ # SELECT expired ci_job_artifacts - 3 queries from each_batch
# PRELOAD projects, routes, project_statistics
# BEGIN
# INSERT into ci_deleted_objects
# DELETE loaded ci_job_artifacts
# DELETE security_findings -- for EE
# COMMIT
- expect(log.count).to be_within(1).of(8)
+ # SELECT next expired ci_job_artifacts
+
+ expect(log.count).to be_within(1).of(11)
end
end
@@ -162,13 +164,21 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
end
context 'when timeout happens' do
+ let!(:second_artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) }
+
before do
- stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_TIMEOUT', 1.second)
- allow_any_instance_of(described_class).to receive(:destroy_artifacts_batch) { true }
+ stub_const('Ci::DestroyExpiredJobArtifactsService::LOOP_TIMEOUT', 0.seconds)
+ stub_const('Ci::DestroyExpiredJobArtifactsService::BATCH_SIZE', 1)
+
+ second_artifact.job.pipeline.unlocked!
+ end
+
+ it 'destroys one artifact' do
+ expect { subject }.to change { Ci::JobArtifact.count }.by(-1)
end
- it 'returns false and does not continue destroying' do
- is_expected.to be_falsy
+ it 'reports the number of destroyed artifacts' do
+ is_expected.to eq(1)
end
end
@@ -182,13 +192,13 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
let!(:second_artifact) { create(:ci_job_artifact, expire_at: 1.day.ago) }
- it 'raises an error and does not continue destroying' do
- is_expected.to be_falsy
- end
-
it 'destroys one artifact' do
expect { subject }.to change { Ci::JobArtifact.count }.by(-1)
end
+
+ it 'reports the number of destroyed artifacts' do
+ is_expected.to eq(1)
+ end
end
context 'when there are no artifacts' do
@@ -199,6 +209,10 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
it 'does not raise error' do
expect { subject }.not_to raise_error
end
+
+ it 'reports the number of destroyed artifacts' do
+ is_expected.to eq(0)
+ end
end
context 'when there are artifacts more than batch sizes' do
@@ -213,33 +227,9 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
it 'destroys all expired artifacts' do
expect { subject }.to change { Ci::JobArtifact.count }.by(-2)
end
- end
-
- context 'when artifact is a pipeline artifact' do
- context 'when artifacts are expired' do
- let!(:pipeline_artifact_1) { create(:ci_pipeline_artifact, expire_at: 1.week.ago) }
- let!(:pipeline_artifact_2) { create(:ci_pipeline_artifact, expire_at: 1.week.ago) }
- before do
- [pipeline_artifact_1, pipeline_artifact_2].each { |pipeline_artifact| pipeline_artifact.pipeline.unlocked! }
- end
-
- it 'destroys pipeline artifacts' do
- expect { subject }.to change { Ci::PipelineArtifact.count }.by(-2)
- end
- end
-
- context 'when artifacts are not expired' do
- let!(:pipeline_artifact_1) { create(:ci_pipeline_artifact, expire_at: 2.days.from_now) }
- let!(:pipeline_artifact_2) { create(:ci_pipeline_artifact, expire_at: 2.days.from_now) }
-
- before do
- [pipeline_artifact_1, pipeline_artifact_2].each { |pipeline_artifact| pipeline_artifact.pipeline.unlocked! }
- end
-
- it 'does not destroy pipeline artifacts' do
- expect { subject }.not_to change { Ci::PipelineArtifact.count }
- end
+ it 'reports the number of destroyed artifacts' do
+ is_expected.to eq(2)
end
end
@@ -255,16 +245,4 @@ RSpec.describe Ci::DestroyExpiredJobArtifactsService, :clean_gitlab_redis_shared
end
end
end
-
- describe '.destroy_job_artifacts_batch' do
- it 'returns a falsy value without artifacts' do
- expect(service.send(:destroy_job_artifacts_batch)).to be_falsy
- end
- end
-
- describe '.destroy_pipeline_artifacts_batch' do
- it 'returns a falsy value without artifacts' do
- expect(service.send(:destroy_pipeline_artifacts_batch)).to be_falsy
- end
- end
end
diff --git a/spec/services/ci/pipelines/create_artifact_service_spec.rb b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb
index 4e9248d9d1a..b48ea70aa4c 100644
--- a/spec/services/ci/pipelines/create_artifact_service_spec.rb
+++ b/spec/services/ci/pipeline_artifacts/coverage_report_service_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe ::Ci::Pipelines::CreateArtifactService do
+RSpec.describe ::Ci::PipelineArtifacts::CoverageReportService do
describe '#execute' do
subject { described_class.new.execute(pipeline) }
diff --git a/spec/services/ci/pipeline_artifacts/destroy_expired_artifacts_service_spec.rb b/spec/services/ci/pipeline_artifacts/destroy_expired_artifacts_service_spec.rb
new file mode 100644
index 00000000000..ac1a590face
--- /dev/null
+++ b/spec/services/ci/pipeline_artifacts/destroy_expired_artifacts_service_spec.rb
@@ -0,0 +1,81 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::PipelineArtifacts::DestroyExpiredArtifactsService do
+ let(:service) { described_class.new }
+
+ describe '.execute' do
+ subject { service.execute }
+
+ context 'when timeout happens' do
+ before do
+ stub_const('Ci::PipelineArtifacts::DestroyExpiredArtifactsService::LOOP_TIMEOUT', 0.1.seconds)
+ allow(service).to receive(:destroy_artifacts_batch) { true }
+ end
+
+ it 'returns 0 and does not continue destroying' do
+ is_expected.to eq(0)
+ end
+ end
+
+ context 'when there are no artifacts' do
+ it 'does not raise error' do
+ expect { subject }.not_to raise_error
+ end
+ end
+
+ context 'when the loop limit is reached' do
+ before do
+ stub_const('::Ci::PipelineArtifacts::DestroyExpiredArtifactsService::LOOP_LIMIT', 1)
+ stub_const('::Ci::PipelineArtifacts::DestroyExpiredArtifactsService::BATCH_SIZE', 1)
+
+ create_list(:ci_pipeline_artifact, 2, expire_at: 1.week.ago)
+ end
+
+ it 'destroys one artifact' do
+ expect { subject }.to change { Ci::PipelineArtifact.count }.by(-1)
+ end
+
+ it 'reports the number of destroyed artifacts' do
+ is_expected.to eq(1)
+ end
+ end
+
+ context 'when there are artifacts more than batch sizes' do
+ before do
+ stub_const('Ci::PipelineArtifacts::DestroyExpiredArtifactsService::BATCH_SIZE', 1)
+
+ create_list(:ci_pipeline_artifact, 2, expire_at: 1.week.ago)
+ end
+
+ it 'destroys all expired artifacts' do
+ expect { subject }.to change { Ci::PipelineArtifact.count }.by(-2)
+ end
+
+ it 'reports the number of destroyed artifacts' do
+ is_expected.to eq(2)
+ end
+ end
+
+ context 'when artifacts are not expired' do
+ before do
+ create(:ci_pipeline_artifact, expire_at: 2.days.from_now)
+ end
+
+ it 'does not destroy pipeline artifacts' do
+ expect { subject }.not_to change { Ci::PipelineArtifact.count }
+ end
+
+ it 'reports the number of destroyed artifacts' do
+ is_expected.to eq(0)
+ end
+ end
+ end
+
+ describe '.destroy_artifacts_batch' do
+ it 'returns a falsy value without artifacts' do
+ expect(service.send(:destroy_artifacts_batch)).to be_falsy
+ end
+ end
+end
diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_build_fails_deploy_is_delayed_and_needs_test.yml b/spec/services/ci/pipeline_processing/test_cases/dag_build_fails_deploy_is_delayed_and_needs_test.yml
new file mode 100644
index 00000000000..b729efaeab2
--- /dev/null
+++ b/spec/services/ci/pipeline_processing/test_cases/dag_build_fails_deploy_is_delayed_and_needs_test.yml
@@ -0,0 +1,41 @@
+config:
+ build:
+ stage: build
+ script: exit 1
+
+ test:
+ stage: test
+ script: exit 0
+
+ deploy:
+ stage: deploy
+ script: exit 0
+ when: delayed
+ start_in: 5 seconds
+ needs: [test]
+
+init:
+ expect:
+ pipeline: pending
+ stages:
+ build: pending
+ test: created
+ deploy: created
+ jobs:
+ build: pending
+ test: created
+ deploy: created
+
+transitions:
+ - event: drop
+ jobs: [build]
+ expect:
+ pipeline: failed
+ stages:
+ build: failed
+ test: skipped
+ deploy: skipped
+ jobs:
+ build: failed
+ test: skipped
+ deploy: skipped
diff --git a/spec/services/ci/pipeline_processing/test_cases/dag_build_fails_deploy_is_manual_and_needs_test.yml b/spec/services/ci/pipeline_processing/test_cases/dag_build_fails_deploy_is_manual_and_needs_test.yml
new file mode 100644
index 00000000000..479fc8fd72d
--- /dev/null
+++ b/spec/services/ci/pipeline_processing/test_cases/dag_build_fails_deploy_is_manual_and_needs_test.yml
@@ -0,0 +1,40 @@
+config:
+ build:
+ stage: build
+ script: exit 1
+
+ test:
+ stage: test
+ script: exit 0
+
+ deploy:
+ stage: deploy
+ script: exit 0
+ when: manual
+ needs: [test]
+
+init:
+ expect:
+ pipeline: pending
+ stages:
+ build: pending
+ test: created
+ deploy: created
+ jobs:
+ build: pending
+ test: created
+ deploy: created
+
+transitions:
+ - event: drop
+ jobs: [build]
+ expect:
+ pipeline: failed
+ stages:
+ build: failed
+ test: skipped
+ deploy: skipped
+ jobs:
+ build: failed
+ test: skipped
+ deploy: skipped
diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb
index ac077e3c30e..0cc66e67b91 100644
--- a/spec/services/ci/pipeline_trigger_service_spec.rb
+++ b/spec/services/ci/pipeline_trigger_service_spec.rb
@@ -3,14 +3,16 @@
require 'spec_helper'
RSpec.describe Ci::PipelineTriggerService do
- let(:project) { create(:project, :repository) }
+ include AfterNextHelpers
+
+ let_it_be(:project) { create(:project, :repository) }
before do
stub_ci_pipeline_to_return_yaml_file
end
describe '#execute' do
- let(:user) { create(:user) }
+ let_it_be(:user) { create(:user) }
let(:result) { described_class.new(project, user, params).execute }
before do
@@ -29,8 +31,8 @@ RSpec.describe Ci::PipelineTriggerService do
end
end
- context 'when params have an existsed trigger token' do
- context 'when params have an existsed ref' do
+ context 'when params have an existing trigger token' do
+ context 'when params have an existing ref' do
let(:params) { { token: trigger.token, ref: 'master', variables: nil } }
it 'triggers a pipeline' do
@@ -45,9 +47,7 @@ RSpec.describe Ci::PipelineTriggerService do
context 'when commit message has [ci skip]' do
before do
- allow_next_instance_of(Ci::Pipeline) do |instance|
- allow(instance).to receive(:git_commit_message) { '[ci skip]' }
- end
+ allow_next(Ci::Pipeline).to receive(:git_commit_message) { '[ci skip]' }
end
it 'ignores [ci skip] and create as general' do
diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb
index c9ecbad3167..00c6de7681d 100644
--- a/spec/services/ci/play_build_service_spec.rb
+++ b/spec/services/ci/play_build_service_spec.rb
@@ -72,6 +72,31 @@ RSpec.describe Ci::PlayBuildService, '#execute' do
expect(build.reload.job_variables.map(&:key)).to contain_exactly('first', 'second')
end
+
+ context 'when user defined variables are restricted' do
+ before do
+ project.update!(restrict_user_defined_variables: true)
+ end
+
+ context 'when user is maintainer' do
+ before do
+ project.add_maintainer(user)
+ end
+
+ it 'assigns the variables to the build' do
+ service.execute(build, job_variables)
+
+ expect(build.reload.job_variables.map(&:key)).to contain_exactly('first', 'second')
+ end
+ end
+
+ context 'when user is developer' do
+ it 'raises an error' do
+ expect { service.execute(build, job_variables) }
+ .to raise_error Gitlab::Access::AccessDeniedError
+ end
+ end
+ end
end
end
diff --git a/spec/services/ci/process_build_service_spec.rb b/spec/services/ci/process_build_service_spec.rb
index a6e8732f5ff..6d2af81a6e8 100644
--- a/spec/services/ci/process_build_service_spec.rb
+++ b/spec/services/ci/process_build_service_spec.rb
@@ -124,24 +124,46 @@ RSpec.describe Ci::ProcessBuildService, '#execute' do
end
context 'when build is scheduled with DAG' do
+ using RSpec::Parameterized::TableSyntax
+
let(:pipeline) { create(:ci_pipeline, ref: 'master', project: project) }
- let!(:build) { create(:ci_build, :created, when: :on_success, pipeline: pipeline, scheduling_type: :dag) }
+ let!(:build) { create(:ci_build, :created, when: build_when, pipeline: pipeline, scheduling_type: :dag) }
let!(:other_build) { create(:ci_build, :created, when: :on_success, pipeline: pipeline) }
let!(:build_on_other_build) { create(:ci_build_need, build: build, name: other_build.name) }
- context 'when current status is success' do
- let(:current_status) { 'success' }
+ where(:build_when, :current_status, :after_status) do
+ :on_success | 'success' | 'pending'
+ :on_success | 'skipped' | 'skipped'
+ :manual | 'success' | 'manual'
+ :manual | 'skipped' | 'skipped'
+ :delayed | 'success' | 'manual'
+ :delayed | 'skipped' | 'skipped'
+ end
- it 'enqueues the build' do
- expect { subject }.to change { build.status }.to('pending')
+ with_them do
+ it 'proceeds the build' do
+ expect { subject }.to change { build.status }.to(after_status)
end
end
- context 'when current status is skipped' do
- let(:current_status) { 'skipped' }
+ context 'when FF skip_dag_manual_and_delayed_jobs is disabled' do
+ before do
+ stub_feature_flags(skip_dag_manual_and_delayed_jobs: false)
+ end
- it 'skips the build' do
- expect { subject }.to change { build.status }.to('skipped')
+ where(:build_when, :current_status, :after_status) do
+ :on_success | 'success' | 'pending'
+ :on_success | 'skipped' | 'skipped'
+ :manual | 'success' | 'manual'
+ :manual | 'skipped' | 'manual'
+ :delayed | 'success' | 'manual'
+ :delayed | 'skipped' | 'manual'
+ end
+
+ with_them do
+ it 'proceeds the build' do
+ expect { subject }.to change { build.status }.to(after_status)
+ end
end
end
end
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index 81d56a0e42a..bdf60bb3fdc 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -206,6 +206,22 @@ RSpec.describe Ci::RetryBuildService do
expect(subsequent_build.reload).to be_created
expect(subsequent_bridge.reload).to be_created
end
+
+ it 'updates ownership for subsequent builds' do
+ expect { service.execute(build) }.to change { subsequent_build.reload.user }.to(user)
+ end
+
+ it 'updates ownership for subsequent bridges' do
+ expect { service.execute(build) }.to change { subsequent_bridge.reload.user }.to(user)
+ end
+
+ it 'does not cause n+1 when updaing build ownership' do
+ control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { service.execute(build) }.count
+
+ create_list(:ci_build, 2, :skipped, stage_idx: build.stage_idx + 1, pipeline: pipeline, stage: 'deploy')
+
+ expect { service.execute(build) }.not_to exceed_all_query_limit(control_count)
+ end
end
context 'when pipeline has other builds' do
diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb
index 526c2f39b46..3c6a99efbf8 100644
--- a/spec/services/ci/retry_pipeline_service_spec.rb
+++ b/spec/services/ci/retry_pipeline_service_spec.rb
@@ -64,6 +64,18 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do
expect(build('spinach 1')).to be_created
expect(pipeline.reload).to be_running
end
+
+ it 'changes ownership of subsequent builds' do
+ expect(build('rspec 2').user).not_to eq(user)
+ expect(build('rspec 3').user).not_to eq(user)
+ expect(build('spinach 1').user).not_to eq(user)
+
+ service.execute(pipeline)
+
+ expect(build('rspec 2').user).to eq(user)
+ expect(build('rspec 3').user).to eq(user)
+ expect(build('spinach 1').user).to eq(user)
+ end
end
context 'when there is failed build present which was run on failure' do
@@ -161,6 +173,16 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do
expect(build('rspec 2')).to be_created
expect(pipeline.reload).to be_running
end
+
+ it 'changes ownership of subsequent builds' do
+ expect(build('staging').user).not_to eq(user)
+ expect(build('rspec 2').user).not_to eq(user)
+
+ service.execute(pipeline)
+
+ expect(build('staging').user).to eq(user)
+ expect(build('rspec 2').user).to eq(user)
+ end
end
end
diff --git a/spec/services/ci/test_failure_history_service_spec.rb b/spec/services/ci/test_failure_history_service_spec.rb
index e858c85490d..d9c1c8dc3fa 100644
--- a/spec/services/ci/test_failure_history_service_spec.rb
+++ b/spec/services/ci/test_failure_history_service_spec.rb
@@ -22,19 +22,6 @@ RSpec.describe Ci::TestFailureHistoryService, :aggregate_failures do
expect(Ci::TestCaseFailure.count).to eq(2)
end
- context 'when feature flag for test failure history is disabled' do
- before do
- stub_feature_flags(test_failure_history: false)
- end
-
- it 'does not persist data' do
- execute_service
-
- expect(Ci::TestCase.count).to eq(0)
- expect(Ci::TestCaseFailure.count).to eq(0)
- end
- end
-
context 'when pipeline is not for the default branch' do
before do
pipeline.update_column(:ref, 'new-feature')
@@ -136,14 +123,6 @@ RSpec.describe Ci::TestFailureHistoryService, :aggregate_failures do
it { is_expected.to eq(true) }
end
- context 'when feature flag is disabled' do
- before do
- stub_feature_flags(test_failure_history: false)
- end
-
- it { is_expected.to eq(false) }
- end
-
context 'when pipeline is not equal to the project default branch' do
before do
pipeline.update_column(:ref, 'some-other-branch')
diff --git a/spec/services/ci/update_build_state_service_spec.rb b/spec/services/ci/update_build_state_service_spec.rb
index 3112e5dda1b..63190cc5d49 100644
--- a/spec/services/ci/update_build_state_service_spec.rb
+++ b/spec/services/ci/update_build_state_service_spec.rb
@@ -82,8 +82,9 @@ RSpec.describe Ci::UpdateBuildStateService do
let(:params) do
{
output: { checksum: 'crc32:12345678', bytesize: 123 },
+ state: 'failed',
failure_reason: 'script_failure',
- state: 'failed'
+ exit_code: 42
}
end
@@ -95,6 +96,15 @@ RSpec.describe Ci::UpdateBuildStateService do
expect(result.status).to eq 200
end
+ it 'updates the allow_failure flag' do
+ expect(build)
+ .to receive(:drop_with_exit_code!)
+ .with('script_failure', 42)
+ .and_call_original
+
+ subject.execute
+ end
+
it 'does not increment invalid trace metric' do
execute_with_stubbed_metrics!
@@ -115,6 +125,15 @@ RSpec.describe Ci::UpdateBuildStateService do
expect(build).to be_failed
end
+ it 'updates the allow_failure flag' do
+ expect(build)
+ .to receive(:drop_with_exit_code!)
+ .with('script_failure', 42)
+ .and_call_original
+
+ subject.execute
+ end
+
it 'responds with 200 OK status' do
result = subject.execute
diff --git a/spec/services/container_expiration_policies/cleanup_service_spec.rb b/spec/services/container_expiration_policies/cleanup_service_spec.rb
index 8438073ceb0..34f69d24141 100644
--- a/spec/services/container_expiration_policies/cleanup_service_spec.rb
+++ b/spec/services/container_expiration_policies/cleanup_service_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
it 'completely clean up the repository' do
expect(Projects::ContainerRepository::CleanupTagsService)
- .to receive(:new).with(project, nil, cleanup_tags_service_params).and_return(cleanup_tags_service)
+ .to receive(:new).with(project, nil, cleanup_tags_service_params).and_return(cleanup_tags_service)
expect(cleanup_tags_service).to receive(:execute).with(repository).and_return(status: :success)
response = subject
@@ -34,10 +34,14 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
end
context 'without a successful cleanup tags service execution' do
- it 'partially clean up the repository' do
+ let(:cleanup_tags_service_response) { { status: :error, message: 'timeout' } }
+
+ before do
expect(Projects::ContainerRepository::CleanupTagsService)
- .to receive(:new).and_return(double(execute: { status: :error, message: 'timeout' }))
+ .to receive(:new).and_return(double(execute: cleanup_tags_service_response))
+ end
+ it 'partially clean up the repository' do
response = subject
aggregate_failures "checking the response and container repositories" do
@@ -49,6 +53,39 @@ RSpec.describe ContainerExpirationPolicies::CleanupService do
expect(repository.expiration_policy_completed_at).to eq(nil)
end
end
+
+ context 'with a truncated cleanup tags service response' do
+ let(:cleanup_tags_service_response) do
+ {
+ status: :error,
+ original_size: 1000,
+ before_truncate_size: 800,
+ after_truncate_size: 200,
+ before_delete_size: 100
+ }
+ end
+
+ it 'partially clean up the repository' do
+ response = subject
+
+ aggregate_failures "checking the response and container repositories" do
+ expect(response.success?).to eq(true)
+ expect(response.payload)
+ .to include(
+ cleanup_status: :unfinished,
+ container_repository_id: repository.id,
+ cleanup_tags_service_original_size: 1000,
+ cleanup_tags_service_before_truncate_size: 800,
+ cleanup_tags_service_after_truncate_size: 200,
+ cleanup_tags_service_before_delete_size: 100
+ )
+ expect(ContainerRepository.waiting_for_cleanup.count).to eq(1)
+ expect(repository.reload.cleanup_unfinished?).to be_truthy
+ expect(repository.expiration_policy_started_at).not_to eq(nil)
+ expect(repository.expiration_policy_completed_at).to eq(nil)
+ end
+ end
+ end
end
context 'with no repository' do
diff --git a/spec/services/draft_notes/create_service_spec.rb b/spec/services/draft_notes/create_service_spec.rb
index f0291067777..9e084dbed1c 100644
--- a/spec/services/draft_notes/create_service_spec.rb
+++ b/spec/services/draft_notes/create_service_spec.rb
@@ -20,6 +20,23 @@ RSpec.describe DraftNotes::CreateService do
expect(draft.discussion_id).to be_nil
end
+ it 'tracks the start event when the draft is persisted' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .to receive(:track_create_review_note_action)
+ .with(user: user)
+
+ draft = create_draft(note: 'This is a test')
+ expect(draft).to be_persisted
+ end
+
+ it 'does not track the start event when the draft is not persisted' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .not_to receive(:track_create_review_note_action)
+
+ draft = create_draft(note: 'Not a reply!', resolve_discussion: true)
+ expect(draft).not_to be_persisted
+ end
+
it 'cannot resolve when there is nothing to resolve' do
draft = create_draft(note: 'Not a reply!', resolve_discussion: true)
diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb
index ae0c8113904..f83e91b683f 100644
--- a/spec/services/draft_notes/publish_service_spec.rb
+++ b/spec/services/draft_notes/publish_service_spec.rb
@@ -43,6 +43,13 @@ RSpec.describe DraftNotes::PublishService do
expect(result[:status]).to eq(:success)
end
+ it 'does not track the publish event' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .not_to receive(:track_publish_review_action)
+
+ publish(draft: drafts.first)
+ end
+
context 'commit_id is set' do
let(:commit_id) { commit.id }
@@ -74,6 +81,13 @@ RSpec.describe DraftNotes::PublishService do
expect { publish }.not_to change { DraftNote.count }
end
+ it 'does not track the publish event' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .not_to receive(:track_publish_review_action)
+
+ publish
+ end
+
it 'returns an error' do
result = publish
@@ -105,6 +119,14 @@ RSpec.describe DraftNotes::PublishService do
publish
end
+ it 'tracks the publish event' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .to receive(:track_publish_review_action)
+ .with(user: user)
+
+ publish
+ end
+
context 'commit_id is set' do
let(:commit_id) { commit.id }
diff --git a/spec/services/feature_flags/create_service_spec.rb b/spec/services/feature_flags/create_service_spec.rb
index 2cd19000f99..e115d8098c9 100644
--- a/spec/services/feature_flags/create_service_spec.rb
+++ b/spec/services/feature_flags/create_service_spec.rb
@@ -34,6 +34,12 @@ RSpec.describe FeatureFlags::CreateService do
it 'does not create audit log' do
expect { subject }.not_to change { AuditEvent.count }
end
+
+ it 'does not sync the feature flag to Jira' do
+ expect(::JiraConnect::SyncFeatureFlagsWorker).not_to receive(:perform_async)
+
+ subject
+ end
end
context 'when feature flag is saved correctly' do
@@ -54,6 +60,24 @@ RSpec.describe FeatureFlags::CreateService do
expect { subject }.to change { Operations::FeatureFlag.count }.by(1)
end
+ it 'syncs the feature flag to Jira' do
+ expect(::JiraConnect::SyncFeatureFlagsWorker).to receive(:perform_async).with(Integer, Integer)
+
+ subject
+ end
+
+ context 'the feature flag is disabled' do
+ before do
+ stub_feature_flags(jira_sync_feature_flags: false)
+ end
+
+ it 'does not sync the feature flag to Jira' do
+ expect(::JiraConnect::SyncFeatureFlagsWorker).not_to receive(:perform_async)
+
+ subject
+ end
+ end
+
it 'creates audit event' do
expected_message = 'Created feature flag <strong>feature_flag</strong> '\
'with description <strong>"description"</strong>. '\
diff --git a/spec/services/feature_flags/update_service_spec.rb b/spec/services/feature_flags/update_service_spec.rb
index 66a75a2c24e..8c4055ddd9e 100644
--- a/spec/services/feature_flags/update_service_spec.rb
+++ b/spec/services/feature_flags/update_service_spec.rb
@@ -26,6 +26,24 @@ RSpec.describe FeatureFlags::UpdateService do
expect(subject[:status]).to eq(:success)
end
+ context 'the feature flag is disabled' do
+ before do
+ stub_feature_flags(jira_sync_feature_flags: false)
+ end
+
+ it 'does not sync the feature flag to Jira' do
+ expect(::JiraConnect::SyncFeatureFlagsWorker).not_to receive(:perform_async)
+
+ subject
+ end
+ end
+
+ it 'syncs the feature flag to Jira' do
+ expect(::JiraConnect::SyncFeatureFlagsWorker).to receive(:perform_async).with(Integer, Integer)
+
+ subject
+ end
+
it 'creates audit event with correct message' do
name_was = feature_flag.name
@@ -52,6 +70,12 @@ RSpec.describe FeatureFlags::UpdateService do
it 'does not create audit event' do
expect { subject }.not_to change { AuditEvent.count }
end
+
+ it 'does not sync the feature flag to Jira' do
+ expect(::JiraConnect::SyncFeatureFlagsWorker).not_to receive(:perform_async)
+
+ subject
+ end
end
context 'when user is reporter' do
diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb
index c7bf006dab0..cc3ba21f002 100644
--- a/spec/services/git/branch_push_service_spec.rb
+++ b/spec/services/git/branch_push_service_spec.rb
@@ -554,7 +554,7 @@ RSpec.describe Git::BranchPushService, services: true do
end
describe "housekeeping" do
- let(:housekeeping) { Projects::HousekeepingService.new(project) }
+ let(:housekeeping) { Repositories::HousekeepingService.new(project) }
before do
# Flush any raw key-value data stored by the housekeeping code.
@@ -562,7 +562,7 @@ RSpec.describe Git::BranchPushService, services: true do
Gitlab::Redis::Queues.with { |conn| conn.flushall }
Gitlab::Redis::SharedState.with { |conn| conn.flushall }
- allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping)
+ allow(Repositories::HousekeepingService).to receive(:new).and_return(housekeeping)
end
after do
diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb
index 4f5bc3a3d5a..f0cd42c1948 100644
--- a/spec/services/groups/create_service_spec.rb
+++ b/spec/services/groups/create_service_spec.rb
@@ -63,6 +63,10 @@ RSpec.describe Groups::CreateService, '#execute' do
end
it { is_expected.to be_persisted }
+
+ it 'adds an onboarding progress record' do
+ expect { subject }.to change(OnboardingProgress, :count).from(0).to(1)
+ end
end
context 'when user can not create a group' do
@@ -84,6 +88,10 @@ RSpec.describe Groups::CreateService, '#execute' do
end
it { is_expected.to be_persisted }
+
+ it 'does not add an onboarding progress record' do
+ expect { subject }.not_to change(OnboardingProgress, :count).from(0)
+ end
end
context 'as guest' do
diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb
index e06f09d0463..2f9bb72939a 100644
--- a/spec/services/groups/destroy_service_spec.rb
+++ b/spec/services/groups/destroy_service_spec.rb
@@ -135,51 +135,120 @@ RSpec.describe Groups::DestroyService do
end
describe 'authorization updates', :sidekiq_inline do
- context 'shared groups' do
+ context 'for solo groups' do
+ context 'group is deleted' do
+ it 'updates project authorization' do
+ expect { destroy_group(group, user, false) }.to(
+ change { user.can?(:read_project, project) }.from(true).to(false))
+ end
+
+ it 'does not make use of a specific service to update project_authorizations records' do
+ expect(UserProjectAccessChangedService)
+ .not_to receive(:new).with(group.user_ids_for_project_authorizations)
+
+ destroy_group(group, user, false)
+ end
+ end
+ end
+
+ context 'for shared groups within different hierarchies' do
+ let(:shared_with_group) { group }
let!(:shared_group) { create(:group, :private) }
let!(:shared_group_child) { create(:group, :private, parent: shared_group) }
+ let!(:shared_group_user) { create(:user) }
let!(:project) { create(:project, group: shared_group) }
let!(:project_child) { create(:project, group: shared_group_child) }
before do
- create(:group_group_link, shared_group: shared_group, shared_with_group: group)
- group.refresh_members_authorized_projects
+ shared_group.add_user(shared_group_user, Gitlab::Access::OWNER)
+
+ create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group)
+ shared_with_group.refresh_members_authorized_projects
+ end
+
+ context 'the shared group is deleted' do
+ it 'updates project authorization' do
+ expect(shared_group_user.can?(:read_project, project)).to eq(true)
+ expect(shared_group_user.can?(:read_project, project_child)).to eq(true)
+
+ destroy_group(shared_group, shared_group_user, false)
+
+ expect(shared_group_user.can?(:read_project, project)).to eq(false)
+ expect(shared_group_user.can?(:read_project, project_child)).to eq(false)
+ end
+
+ it 'does not make use of specific service to update project_authorizations records' do
+ expect(UserProjectAccessChangedService)
+ .not_to receive(:new).with(shared_group.user_ids_for_project_authorizations).and_call_original
+
+ destroy_group(shared_group, shared_group_user, false)
+ end
end
- it 'updates project authorization' do
- expect(user.can?(:read_project, project)).to eq(true)
- expect(user.can?(:read_project, project_child)).to eq(true)
+ context 'the shared_with group is deleted' do
+ it 'updates project authorization' do
+ expect(user.can?(:read_project, project)).to eq(true)
+ expect(user.can?(:read_project, project_child)).to eq(true)
- destroy_group(group, user, false)
+ destroy_group(shared_with_group, user, false)
- expect(user.can?(:read_project, project)).to eq(false)
- expect(user.can?(:read_project, project_child)).to eq(false)
+ expect(user.can?(:read_project, project)).to eq(false)
+ expect(user.can?(:read_project, project_child)).to eq(false)
+ end
+
+ it 'makes use of a specific service to update project_authorizations records' do
+ expect(UserProjectAccessChangedService)
+ .to receive(:new).with(shared_with_group.user_ids_for_project_authorizations).and_call_original
+
+ destroy_group(shared_with_group, user, false)
+ end
end
end
- context 'shared groups in the same group hierarchy' do
- let!(:subgroup) { create(:group, :private, parent: group) }
- let!(:subgroup_user) { create(:user) }
+ context 'for shared groups in the same group hierarchy' do
+ let(:shared_group) { group }
+ let(:shared_with_group) { nested_group }
+ let!(:shared_with_group_user) { create(:user) }
before do
- subgroup.add_user(subgroup_user, Gitlab::Access::MAINTAINER)
+ shared_with_group.add_user(shared_with_group_user, Gitlab::Access::MAINTAINER)
- create(:group_group_link, shared_group: group, shared_with_group: subgroup)
- subgroup.refresh_members_authorized_projects
+ create(:group_group_link, shared_group: shared_group, shared_with_group: shared_with_group)
+ shared_with_group.refresh_members_authorized_projects
end
- context 'group is deleted' do
+ context 'the shared group is deleted' do
it 'updates project authorization' do
- expect { destroy_group(group, user, false) }.to(
- change { subgroup_user.can?(:read_project, project) }.from(true).to(false))
+ expect { destroy_group(shared_group, user, false) }.to(
+ change { shared_with_group_user.can?(:read_project, project) }.from(true).to(false))
+ end
+
+ it 'does not make use of a specific service to update project authorizations' do
+ # Due to the recursive nature of `Groups::DestroyService`, `UserProjectAccessChangedService`
+ # will still be executed for the nested group as they fall under the same hierarchy
+ # and hence we need to account for this scenario.
+ expect(UserProjectAccessChangedService)
+ .to receive(:new).with(shared_with_group.user_ids_for_project_authorizations).and_call_original
+
+ expect(UserProjectAccessChangedService)
+ .not_to receive(:new).with(shared_group.user_ids_for_project_authorizations)
+
+ destroy_group(shared_group, user, false)
end
end
- context 'subgroup is deleted' do
+ context 'the shared_with group is deleted' do
it 'updates project authorization' do
- expect { destroy_group(subgroup, user, false) }.to(
- change { subgroup_user.can?(:read_project, project) }.from(true).to(false))
+ expect { destroy_group(shared_with_group, user, false) }.to(
+ change { shared_with_group_user.can?(:read_project, project) }.from(true).to(false))
+ end
+
+ it 'makes use of a specific service to update project authorizations' do
+ expect(UserProjectAccessChangedService)
+ .to receive(:new).with(shared_with_group.user_ids_for_project_authorizations).and_call_original
+
+ destroy_group(shared_with_group, user, false)
end
end
end
diff --git a/spec/services/incident_management/pager_duty/process_webhook_service_spec.rb b/spec/services/incident_management/pager_duty/process_webhook_service_spec.rb
index 4c8aebe5fe2..0caffb16f42 100644
--- a/spec/services/incident_management/pager_duty/process_webhook_service_spec.rb
+++ b/spec/services/incident_management/pager_duty/process_webhook_service_spec.rb
@@ -17,7 +17,7 @@ RSpec.describe IncidentManagement::PagerDuty::ProcessWebhookService do
let(:webhook_payload) { Gitlab::Json.parse(fixture_file('pager_duty/webhook_incident_trigger.json')) }
let(:token) { nil }
- subject(:execute) { described_class.new(project, nil, webhook_payload).execute(token) }
+ subject(:execute) { described_class.new(project, webhook_payload).execute(token) }
context 'when PagerDuty webhook setting is active' do
let_it_be(:incident_management_setting) { create(:project_incident_management_setting, project: project, pagerduty_active: true) }
diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb
index f2bc4f717af..79543fe9f5d 100644
--- a/spec/services/issuable/bulk_update_service_spec.rb
+++ b/spec/services/issuable/bulk_update_service_spec.rb
@@ -3,8 +3,8 @@
require 'spec_helper'
RSpec.describe Issuable::BulkUpdateService do
- let(:user) { create(:user) }
- let(:project) { create(:project, :repository, namespace: user.namespace) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:project) { create(:project, :repository, namespace: user.namespace) }
def bulk_update(issuables, extra_params = {})
bulk_update_params = extra_params
@@ -31,6 +31,23 @@ RSpec.describe Issuable::BulkUpdateService do
end
end
+ shared_examples 'updates iterations' do
+ it 'succeeds' do
+ result = bulk_update(issuables, sprint_id: iteration.id)
+
+ expect(result.success?).to be_truthy
+ expect(result.payload[:count]).to eq(issuables.count)
+ end
+
+ it 'updates the issuables iteration' do
+ bulk_update(issuables, sprint_id: iteration.id)
+
+ issuables.each do |issuable|
+ expect(issuable.reload.iteration).to eq(iteration)
+ end
+ end
+ end
+
shared_examples 'updating labels' do
def create_issue_with_labels(labels)
create(:labeled_issue, project: project, labels: labels)
@@ -233,6 +250,21 @@ RSpec.describe Issuable::BulkUpdateService do
it_behaves_like 'updates milestones'
end
+ describe 'updating iterations' do
+ let_it_be(:group) { create(:group) }
+ let_it_be(:project) { create(:project, group: group) }
+ let_it_be(:issuables) { [create(:issue, project: project)] }
+ let_it_be(:iteration) { create(:iteration, group: group) }
+
+ let(:parent) { project }
+
+ before do
+ group.add_reporter(user)
+ end
+
+ it_behaves_like 'updates iterations'
+ end
+
describe 'updating labels' do
let(:bug) { create(:label, project: project) }
let(:regression) { create(:label, project: project) }
@@ -283,7 +315,7 @@ RSpec.describe Issuable::BulkUpdateService do
end
context 'with issuables at a group level' do
- let(:group) { create(:group) }
+ let_it_be(:group) { create(:group) }
let(:parent) { group }
before do
@@ -315,6 +347,19 @@ RSpec.describe Issuable::BulkUpdateService do
end
end
+ describe 'updating iterations' do
+ let_it_be(:iteration) { create(:iteration, group: group) }
+ let_it_be(:project) { create(:project, :repository, group: group) }
+
+ context 'when issues' do
+ let_it_be(:issue1) { create(:issue, project: project) }
+ let_it_be(:issue2) { create(:issue, project: project) }
+ let_it_be(:issuables) { [issue1, issue2] }
+
+ it_behaves_like 'updates iterations'
+ end
+ end
+
describe 'updating labels' do
let(:project) { create(:project, :repository, group: group) }
let(:bug) { create(:group_label, group: group) }
diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb
index 9076fb11c9b..dc545f57d23 100644
--- a/spec/services/issues/close_service_spec.rb
+++ b/spec/services/issues/close_service_spec.rb
@@ -112,10 +112,14 @@ RSpec.describe Issues::CloseService do
end
context "closed by a merge request", :sidekiq_might_not_need_inline do
- it 'mentions closure via a merge request' do
+ subject(:close_issue) do
perform_enqueued_jobs do
described_class.new(project, user).close_issue(issue, closed_via: closing_merge_request)
end
+ end
+
+ it 'mentions closure via a merge request' do
+ close_issue
email = ActionMailer::Base.deliveries.last
@@ -124,12 +128,15 @@ RSpec.describe Issues::CloseService do
expect(email.body.parts.map(&:body)).to all(include(closing_merge_request.to_reference))
end
+ it_behaves_like 'records an onboarding progress action', :issue_auto_closed do
+ let(:namespace) { project.namespace }
+ end
+
context 'when user cannot read merge request' do
it 'does not mention merge request' do
project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
- perform_enqueued_jobs do
- described_class.new(project, user).close_issue(issue, closed_via: closing_merge_request)
- end
+
+ close_issue
email = ActionMailer::Base.deliveries.last
body_text = email.body.parts.map(&:body).join(" ")
@@ -141,13 +148,11 @@ RSpec.describe Issues::CloseService do
end
context 'updating `metrics.first_mentioned_in_commit_at`' do
- subject { described_class.new(project, user).close_issue(issue, closed_via: closing_merge_request) }
-
context 'when `metrics.first_mentioned_in_commit_at` is not set' do
it 'uses the first commit authored timestamp' do
expected = closing_merge_request.commits.first.authored_date
- subject
+ close_issue
expect(issue.metrics.first_mentioned_in_commit_at).to eq(expected)
end
@@ -159,7 +164,7 @@ RSpec.describe Issues::CloseService do
end
it 'does not update the metrics' do
- expect { subject }.not_to change { issue.metrics.first_mentioned_in_commit_at }
+ expect { close_issue }.not_to change { issue.metrics.first_mentioned_in_commit_at }
end
end
@@ -167,7 +172,7 @@ RSpec.describe Issues::CloseService do
let(:closing_merge_request) { create(:merge_request, :without_diffs, source_project: project) }
it 'does not update the metrics' do
- subject
+ close_issue
expect(issue.metrics.first_mentioned_in_commit_at).to be_nil
end
@@ -206,7 +211,7 @@ RSpec.describe Issues::CloseService do
end
context "valid params" do
- def close_issue
+ subject(:close_issue) do
perform_enqueued_jobs do
described_class.new(project, user).close_issue(issue)
end
@@ -290,6 +295,8 @@ RSpec.describe Issues::CloseService do
close_issue
end
+
+ it_behaves_like 'does not record an onboarding progress action'
end
context 'when issue is not confidential' do
diff --git a/spec/services/issues/export_csv_service_spec.rb b/spec/services/issues/export_csv_service_spec.rb
index 8072b7a478e..fd1bcf82ccd 100644
--- a/spec/services/issues/export_csv_service_spec.rb
+++ b/spec/services/issues/export_csv_service_spec.rb
@@ -20,7 +20,9 @@ RSpec.describe Issues::ExportCsvService do
end
it 'renders with a target filesize' do
- expect(subject.csv_builder).to receive(:render).with(described_class::TARGET_FILESIZE)
+ expect_next_instance_of(CsvBuilder) do |csv_builder|
+ expect(csv_builder).to receive(:render).with(described_class::TARGET_FILESIZE).once
+ end
subject.email(user)
end
diff --git a/spec/services/jira_connect/sync_service_spec.rb b/spec/services/jira_connect/sync_service_spec.rb
index 4b434348146..edd0bad70f5 100644
--- a/spec/services/jira_connect/sync_service_spec.rb
+++ b/spec/services/jira_connect/sync_service_spec.rb
@@ -45,11 +45,11 @@ RSpec.describe JiraConnect::SyncService do
it 'logs the response as an error' do
expect_next(client).to store_info([
{ 'errorMessages' => ['some error message'] },
- { 'rejectedBuilds' => ['x'] }
+ { 'errorMessages' => ['x'] }
])
expect_log(:error, { 'errorMessages' => ['some error message'] })
- expect_log(:error, { 'rejectedBuilds' => ['x'] })
+ expect_log(:error, { 'errorMessages' => ['x'] })
subject
end
diff --git a/spec/services/jira_connect_subscriptions/create_service_spec.rb b/spec/services/jira_connect_subscriptions/create_service_spec.rb
index 9750c671fa2..5f467a07a78 100644
--- a/spec/services/jira_connect_subscriptions/create_service_spec.rb
+++ b/spec/services/jira_connect_subscriptions/create_service_spec.rb
@@ -49,18 +49,6 @@ RSpec.describe JiraConnectSubscriptions::CreateService do
subject
end
-
- context 'when the jira_connect_full_namespace_sync feature flag is disabled' do
- before do
- stub_feature_flags(jira_connect_full_namespace_sync: false)
- end
-
- specify do
- expect(JiraConnect::SyncProjectWorker).not_to receive(:bulk_perform_in_with_contexts)
-
- subject
- end
- end
end
end
diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb
index e8a4a798b20..50efee9f43c 100644
--- a/spec/services/members/create_service_spec.rb
+++ b/spec/services/members/create_service_spec.rb
@@ -2,26 +2,41 @@
require 'spec_helper'
-RSpec.describe Members::CreateService do
- let_it_be(:project) { create(:project) }
+RSpec.describe Members::CreateService, :clean_gitlab_redis_shared_state, :sidekiq_inline do
+ let_it_be(:source) { create(:project) }
let_it_be(:user) { create(:user) }
- let_it_be(:project_user) { create(:user) }
- let_it_be(:user_ids) { project_user.id.to_s }
+ let_it_be(:member) { create(:user) }
+ let_it_be(:user_ids) { member.id.to_s }
let_it_be(:access_level) { Gitlab::Access::GUEST }
let(:params) { { user_ids: user_ids, access_level: access_level } }
- subject(:execute_service) { described_class.new(user, params).execute(project) }
+ subject(:execute_service) { described_class.new(user, params).execute(source) }
before do
- project.add_maintainer(user)
- allow(Namespaces::OnboardingUserAddedWorker).to receive(:idempotent?).and_return(false)
+ if source.is_a?(Project)
+ source.add_maintainer(user)
+ OnboardingProgress.onboard(source.namespace)
+ else
+ source.add_owner(user)
+ OnboardingProgress.onboard(source)
+ end
end
context 'when passing valid parameters' do
it 'adds a user to members' do
expect(execute_service[:status]).to eq(:success)
- expect(project.users).to include project_user
- expect(Namespaces::OnboardingUserAddedWorker.jobs.last['args'][0]).to eq(project.id)
+ expect(source.users).to include member
+ expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(true)
+ end
+
+ context 'when executing on a group' do
+ let_it_be(:source) { create(:group) }
+
+ it 'adds a user to members' do
+ expect(execute_service[:status]).to eq(:success)
+ expect(source.users).to include member
+ expect(OnboardingProgress.completed?(source, :user_added)).to be(true)
+ end
end
end
@@ -31,8 +46,8 @@ RSpec.describe Members::CreateService do
it 'does not add a member' do
expect(execute_service[:status]).to eq(:error)
expect(execute_service[:message]).to be_present
- expect(project.users).not_to include project_user
- expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0)
+ expect(source.users).not_to include member
+ expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(false)
end
end
@@ -42,8 +57,8 @@ RSpec.describe Members::CreateService do
it 'limits the number of users to 100' do
expect(execute_service[:status]).to eq(:error)
expect(execute_service[:message]).to be_present
- expect(project.users).not_to include project_user
- expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0)
+ expect(source.users).not_to include member
+ expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(false)
end
end
@@ -52,19 +67,19 @@ RSpec.describe Members::CreateService do
it 'does not add a member' do
expect(execute_service[:status]).to eq(:error)
- expect(execute_service[:message]).to include("#{project_user.username}: Access level is not included in the list")
- expect(project.users).not_to include project_user
- expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0)
+ expect(execute_service[:message]).to include("#{member.username}: Access level is not included in the list")
+ expect(source.users).not_to include member
+ expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(false)
end
end
context 'when passing an existing invite user id' do
- let(:user_ids) { create(:project_member, :invited, project: project).invite_email }
+ let(:user_ids) { create(:project_member, :invited, project: source).invite_email }
it 'does not add a member' do
expect(execute_service[:status]).to eq(:error)
expect(execute_service[:message]).to eq('Invite email has already been taken')
- expect(Namespaces::OnboardingUserAddedWorker.jobs.size).to eq(0)
+ expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(false)
end
end
end
diff --git a/spec/services/merge_requests/after_create_service_spec.rb b/spec/services/merge_requests/after_create_service_spec.rb
index 69bab3b1ea4..9ae310d8cee 100644
--- a/spec/services/merge_requests/after_create_service_spec.rb
+++ b/spec/services/merge_requests/after_create_service_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
RSpec.describe MergeRequests::AfterCreateService do
+ include AfterNextHelpers
+
let_it_be(:merge_request) { create(:merge_request) }
subject(:after_create_service) do
@@ -27,6 +29,14 @@ RSpec.describe MergeRequests::AfterCreateService do
execute_service
end
+ it 'calls the merge request activity counter' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .to receive(:track_create_mr_action)
+ .with(user: merge_request.author)
+
+ execute_service
+ end
+
it 'creates a new merge request notification' do
expect(notification_service)
.to receive(:new_merge_request).with(merge_request, merge_request.author)
@@ -56,11 +66,15 @@ RSpec.describe MergeRequests::AfterCreateService do
execute_service
end
- it 'records a namespace onboarding progress action' do
- expect(NamespaceOnboardingAction).to receive(:create_action)
- .with(merge_request.target_project.namespace, :merge_request_created).and_call_original
+ it 'registers an onboarding progress action' do
+ OnboardingProgress.onboard(merge_request.target_project.namespace)
+
+ expect_next(OnboardingProgressService, merge_request.target_project.namespace)
+ .to receive(:execute).with(action: :merge_request_created).and_call_original
+
+ execute_service
- expect { execute_service }.to change(NamespaceOnboardingAction, :count).by(1)
+ expect(OnboardingProgress.completed?(merge_request.target_project.namespace, :merge_request_created)).to be(true)
end
end
end
diff --git a/spec/services/merge_requests/cleanup_refs_service_spec.rb b/spec/services/merge_requests/cleanup_refs_service_spec.rb
index 38c0e204e54..a1822a4d5ba 100644
--- a/spec/services/merge_requests/cleanup_refs_service_spec.rb
+++ b/spec/services/merge_requests/cleanup_refs_service_spec.rb
@@ -91,6 +91,26 @@ RSpec.describe MergeRequests::CleanupRefsService do
it_behaves_like 'service that does not clean up merge request refs'
end
+ context 'when a git error is raised' do
+ context 'Gitlab::Git::Repository::GitError' do
+ before do
+ allow(merge_request.project.repository).to receive(:delete_refs).and_raise(Gitlab::Git::Repository::GitError)
+ end
+
+ it_behaves_like 'service that does not clean up merge request refs'
+ end
+
+ context 'Gitlab::Git::CommandError' do
+ before do
+ allow_next_instance_of(Gitlab::Git::KeepAround) do |keep_around|
+ expect(keep_around).to receive(:kept_around?).and_raise(Gitlab::Git::CommandError)
+ end
+ end
+
+ it_behaves_like 'service that does not clean up merge request refs'
+ end
+ end
+
context 'when cleanup schedule fails to update' do
before do
allow(merge_request.cleanup_schedule).to receive(:update).and_return(false)
diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb
index 67fb4eaade5..48f56b3ec68 100644
--- a/spec/services/merge_requests/close_service_spec.rb
+++ b/spec/services/merge_requests/close_service_spec.rb
@@ -18,6 +18,7 @@ RSpec.describe MergeRequests::CloseService do
describe '#execute' do
it_behaves_like 'cache counters invalidator'
+ it_behaves_like 'merge request reviewers cache counters invalidator'
context 'valid params' do
let(:service) { described_class.new(project, user, {}) }
@@ -75,6 +76,14 @@ RSpec.describe MergeRequests::CloseService do
described_class.new(project, user, {}).execute(merge_request)
end
+ it 'calls the merge request activity counter' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .to receive(:track_close_mr_action)
+ .with(user: user)
+
+ described_class.new(project, user, {}).execute(merge_request)
+ end
+
it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do
service = described_class.new(project, user, {})
diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb
index 86e49fe601c..be8c41bc4a1 100644
--- a/spec/services/merge_requests/create_from_issue_service_spec.rb
+++ b/spec/services/merge_requests/create_from_issue_service_spec.rb
@@ -66,10 +66,11 @@ RSpec.describe MergeRequests::CreateFromIssueService do
expect { service.execute }.to change(target_project.merge_requests, :count).by(1)
end
- it 'sets the merge request author to current user', :sidekiq_might_not_need_inline do
+ it 'sets the merge request author to current user and assigns them', :sidekiq_might_not_need_inline do
result = service.execute
expect(result[:merge_request].author).to eq(user)
+ expect(result[:merge_request].assignees).to eq([user])
end
it 'sets the merge request source branch to the new issue branch', :sidekiq_might_not_need_inline do
diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb
index d042b318d02..4f47a22b07c 100644
--- a/spec/services/merge_requests/create_service_spec.rb
+++ b/spec/services/merge_requests/create_service_spec.rb
@@ -155,6 +155,12 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
expect(Todo.where(attributes).count).to eq 1
end
+
+ it 'invalidates counter cache for reviewers', :use_clean_rails_memory_store_caching do
+ expect { merge_request }
+ .to change { user2.review_requested_open_merge_requests_count }
+ .by(1)
+ end
end
context 'when head pipelines already exist for merge request source branch', :sidekiq_inline do
diff --git a/spec/services/merge_requests/export_csv_service_spec.rb b/spec/services/merge_requests/export_csv_service_spec.rb
index ecb17b3fe77..4ce032c396e 100644
--- a/spec/services/merge_requests/export_csv_service_spec.rb
+++ b/spec/services/merge_requests/export_csv_service_spec.rb
@@ -27,11 +27,11 @@ RSpec.describe MergeRequests::ExportCsvService do
let_it_be(:merge_request) { create(:merge_request, assignees: create_list(:user, 2)) }
it 'contains the names of assignees' do
- expect(csv['Assignees']).to eq(merge_request.assignees.map(&:name).join(', '))
+ expect(csv['Assignees'].split(', ')).to match_array(merge_request.assignees.map(&:name))
end
it 'contains the usernames of assignees' do
- expect(csv['Assignee Usernames']).to eq(merge_request.assignees.map(&:username).join(', '))
+ expect(csv['Assignee Usernames'].split(', ')).to match_array(merge_request.assignees.map(&:username))
end
end
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index d0e3102f157..dd37d87e3f5 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -37,6 +37,10 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request.in_progress_merge_commit_sha).to be_nil
end
+ it 'does not update squash_commit_sha if it is not a squash' do
+ expect(merge_request.squash_commit_sha).to be_nil
+ end
+
it 'sends email to user2 about merge of new merge_request' do
email = ActionMailer::Base.deliveries.last
expect(email.to.first).to eq(user2.email)
@@ -76,6 +80,12 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_commit.message).to eq('Merge commit message')
expect(squash_commit.message).to eq("Squash commit message\n")
end
+
+ it 'persists squash_commit_sha' do
+ squash_commit = merge_request.merge_commit.parents.last
+
+ expect(merge_request.squash_commit_sha).to eq(squash_commit.id)
+ end
end
end
@@ -361,6 +371,7 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
+ expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
@@ -381,6 +392,7 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
+ expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
@@ -395,10 +407,27 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
+ expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
+ it 'logs and saves error if there is an PreReceiveError exception' do
+ error_message = 'error message'
+
+ allow(service).to receive(:repository).and_raise(Gitlab::Git::PreReceiveError, "GitLab: #{error_message}")
+ allow(service).to receive(:execute_hooks)
+ merge_request.update!(squash: true)
+
+ service.execute(merge_request)
+
+ expect(merge_request).to be_open
+ expect(merge_request.merge_commit_sha).to be_nil
+ expect(merge_request.squash_commit_sha).to be_nil
+ expect(merge_request.merge_error).to include('Something went wrong during merge pre-receive hook')
+ expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
+ end
+
context 'when fast-forward merge is not allowed' do
before do
allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil)
@@ -415,6 +444,7 @@ RSpec.describe MergeRequests::MergeService do
expect(merge_request).to be_open
expect(merge_request.merge_commit_sha).to be_nil
+ expect(merge_request.squash_commit_sha).to be_nil
expect(merge_request.merge_error).to include(error_message)
expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message))
end
diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb
index 725fc16fa7c..17bfa9d7368 100644
--- a/spec/services/merge_requests/mergeability_check_service_spec.rb
+++ b/spec/services/merge_requests/mergeability_check_service_spec.rb
@@ -124,14 +124,6 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
it_behaves_like 'mergeable merge request'
- context 'when lock is disabled' do
- before do
- stub_feature_flags(merge_ref_auto_sync_lock: false)
- end
-
- it_behaves_like 'mergeable merge request'
- end
-
context 'when concurrent calls' do
it 'waits first lock and returns "cached" result in subsequent calls' do
threads = execute_within_threads(amount: 3)
@@ -167,25 +159,6 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
end
end
- context 'disabled merge ref sync feature flag' do
- before do
- stub_feature_flags(merge_ref_auto_sync: false)
- end
-
- it 'returns error and no payload' do
- result = subject
-
- expect(result).to be_a(ServiceResponse)
- expect(result.error?).to be(true)
- expect(result.message).to eq('Merge ref is outdated due to disabled feature')
- expect(result.payload).to be_empty
- end
-
- it 'ignores merge-ref and updates merge status' do
- expect { subject }.to change(merge_request, :merge_status).from('unchecked').to('can_be_merged')
- end
- end
-
context 'when broken' do
before do
expect(merge_request).to receive(:broken?) { true }
@@ -305,28 +278,6 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar
context 'recheck enforced' do
subject { described_class.new(merge_request).execute(recheck: true) }
- context 'when MR is mergeable and merge-ref auto-sync is disabled' do
- before do
- stub_feature_flags(merge_ref_auto_sync: false)
- merge_request.mark_as_mergeable!
- end
-
- it 'returns ServiceResponse.error' do
- result = subject
-
- expect(result).to be_a(ServiceResponse)
- expect(result.error?).to be(true)
- expect(result.message).to eq('Merge ref is outdated due to disabled feature')
- expect(result.payload).to be_empty
- end
-
- it 'merge status is not changed' do
- subject
-
- expect(merge_request.merge_status).to eq('can_be_merged')
- end
- end
-
context 'when MR is marked as mergeable, but repo is not mergeable and MR is not opened' do
before do
# Making sure that we don't touch the merge-status after
diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb
index 402f753c0af..6523b5a158c 100644
--- a/spec/services/merge_requests/post_merge_service_spec.rb
+++ b/spec/services/merge_requests/post_merge_service_spec.rb
@@ -15,6 +15,7 @@ RSpec.describe MergeRequests::PostMergeService do
describe '#execute' do
it_behaves_like 'cache counters invalidator'
+ it_behaves_like 'merge request reviewers cache counters invalidator'
it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do
# Cache the counter before the MR changed state.
@@ -37,6 +38,14 @@ RSpec.describe MergeRequests::PostMergeService do
subject
end
+ it 'calls the merge request activity counter' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .to receive(:track_merge_mr_action)
+ .with(user: user)
+
+ subject
+ end
+
it 'deletes non-latest diffs' do
diff_removal_service = instance_double(MergeRequests::DeleteNonLatestDiffsService, execute: nil)
diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb
index 2bd83dc36a8..8541d597581 100644
--- a/spec/services/merge_requests/reopen_service_spec.rb
+++ b/spec/services/merge_requests/reopen_service_spec.rb
@@ -17,6 +17,7 @@ RSpec.describe MergeRequests::ReopenService do
describe '#execute' do
it_behaves_like 'cache counters invalidator'
+ it_behaves_like 'merge request reviewers cache counters invalidator'
context 'valid params' do
let(:service) { described_class.new(project, user, {}) }
@@ -80,6 +81,14 @@ RSpec.describe MergeRequests::ReopenService do
described_class.new(project, user, {}).execute(merge_request)
end
+ it 'calls the merge request activity counter' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
+ .to receive(:track_reopen_mr_action)
+ .with(user: user)
+
+ described_class.new(project, user, {}).execute(merge_request)
+ end
+
it 'refreshes the number of open merge requests for a valid MR' do
service = described_class.new(project, user, {})
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index ed8872b71f7..9eb82dcd0ad 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -431,14 +431,6 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
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
@@ -529,6 +521,19 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
should_email(user2)
should_email(user3)
end
+
+ it 'updates open merge request counter for reviewers', :use_clean_rails_memory_store_caching do
+ merge_request.reviewers = [user3]
+
+ # Cache them to ensure the cache gets invalidated on update
+ expect(user2.review_requested_open_merge_requests_count).to eq(0)
+ expect(user3.review_requested_open_merge_requests_count).to eq(1)
+
+ update_merge_request(reviewer_ids: [user2.id])
+
+ expect(user2.review_requested_open_merge_requests_count).to eq(1)
+ expect(user3.review_requested_open_merge_requests_count).to eq(0)
+ end
end
context 'when the milestone is removed' do
@@ -837,20 +842,20 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
it 'does not allow a maintainer of the target project to set `allow_collaboration`' do
target_project.add_developer(user)
- update_merge_request(allow_collaboration: true, title: 'Updated title')
+ update_merge_request(allow_collaboration: false, title: 'Updated title')
expect(merge_request.title).to eq('Updated title')
- expect(merge_request.allow_collaboration).to be_falsy
+ expect(merge_request.allow_collaboration).to be_truthy
end
it 'is allowed by a user that can push to the source and can update the merge request' do
merge_request.update!(assignees: [user])
source_project.add_developer(user)
- update_merge_request(allow_collaboration: true, title: 'Updated title')
+ update_merge_request(allow_collaboration: false, title: 'Updated title')
expect(merge_request.title).to eq('Updated title')
- expect(merge_request.allow_collaboration).to be_truthy
+ expect(merge_request.allow_collaboration).to be_falsy
end
end
diff --git a/spec/services/namespaces/package_settings/update_service_spec.rb b/spec/services/namespaces/package_settings/update_service_spec.rb
new file mode 100644
index 00000000000..fa0c58e4c9b
--- /dev/null
+++ b/spec/services/namespaces/package_settings/update_service_spec.rb
@@ -0,0 +1,102 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Namespaces::PackageSettings::UpdateService do
+ using RSpec::Parameterized::TableSyntax
+
+ let_it_be_with_reload(:namespace) { create(:group) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:params) { {} }
+
+ describe '#execute' do
+ subject { described_class.new(container: namespace, current_user: user, params: params).execute }
+
+ shared_examples 'returning a success' do
+ it 'returns a success' do
+ result = subject
+
+ expect(result.payload[:package_settings]).to be_present
+ expect(result.success?).to be_truthy
+ end
+ end
+
+ shared_examples 'returning an error' do |message, http_status|
+ it 'returns an error' do
+ result = subject
+
+ expect(result.message).to eq(message)
+ expect(result.status).to eq(:error)
+ expect(result.http_status).to eq(http_status)
+ end
+ end
+
+ shared_examples 'updating the namespace package setting' do
+ it_behaves_like 'updating the namespace package setting attributes', from: { maven_duplicates_allowed: true, maven_duplicate_exception_regex: 'SNAPSHOT' }, to: { maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'RELEASE' }
+
+ it_behaves_like 'returning a success'
+
+ context 'with invalid params' do
+ let_it_be(:params) { { maven_duplicates_allowed: nil } }
+
+ it_behaves_like 'not creating the namespace package setting'
+
+ it "doesn't update the maven_duplicates_allowed" do
+ expect { subject }
+ .not_to change { package_settings.reload.maven_duplicates_allowed }
+ end
+
+ it_behaves_like 'returning an error', 'Maven duplicates allowed is not included in the list', 400
+ end
+ end
+
+ shared_examples 'denying access to namespace package setting' do
+ context 'with existing namespace package setting' do
+ it_behaves_like 'not creating the namespace package setting'
+
+ it_behaves_like 'returning an error', 'Access Denied', 403
+ end
+ end
+
+ context 'with existing namespace package setting' do
+ let_it_be(:package_settings) { create(:namespace_package_setting, namespace: namespace) }
+ let_it_be(:params) { { maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'RELEASE' } }
+
+ where(:user_role, :shared_examples_name) do
+ :maintainer | 'updating the namespace package setting'
+ :developer | 'updating the namespace package setting'
+ :reporter | 'denying access to namespace package setting'
+ :guest | 'denying access to namespace package setting'
+ :anonymous | 'denying access to namespace package setting'
+ end
+
+ with_them do
+ before do
+ namespace.send("add_#{user_role}", user) unless user_role == :anonymous
+ end
+
+ it_behaves_like params[:shared_examples_name]
+ end
+ end
+
+ context 'without existing namespace package setting' do
+ let_it_be(:package_settings) { namespace.package_settings }
+
+ where(:user_role, :shared_examples_name) do
+ :maintainer | 'creating the namespace package setting'
+ :developer | 'creating the namespace package setting'
+ :reporter | 'denying access to namespace package setting'
+ :guest | 'denying access to namespace package setting'
+ :anonymous | 'denying access to namespace package setting'
+ end
+
+ with_them do
+ before do
+ namespace.send("add_#{user_role}", user) unless user_role == :anonymous
+ end
+
+ it_behaves_like params[:shared_examples_name]
+ end
+ end
+ end
+end
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 3118956951e..20f06619e02 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -78,6 +78,12 @@ RSpec.describe Notes::CreateService do
end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1)
end
+ it 'does not track merge request usage data' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).not_to receive(:track_create_comment_action)
+
+ described_class.new(project, user, opts).execute
+ end
+
context 'in a merge request' do
let_it_be(:project_with_repo) { create(:project, :repository) }
let_it_be(:merge_request) do
@@ -85,18 +91,6 @@ RSpec.describe Notes::CreateService do
target_project: project_with_repo)
end
- context 'issue comment usage data' do
- let(:opts) do
- { note: 'Awesome comment', noteable_type: 'MergeRequest', noteable_id: merge_request.id }
- end
-
- it 'does not track' do
- expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_added_action)
-
- described_class.new(project, user, opts).execute
- end
- end
-
context 'noteable highlight cache clearing' do
let(:position) do
Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
@@ -119,6 +113,18 @@ RSpec.describe Notes::CreateService do
.to receive(:unfolded_diff?) { true }
end
+ it 'does not track issue comment usage data' do
+ expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_added_action)
+
+ described_class.new(project_with_repo, user, new_opts).execute
+ end
+
+ it 'tracks merge request usage data' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_create_comment_action).with(note: kind_of(Note))
+
+ described_class.new(project_with_repo, user, new_opts).execute
+ end
+
it 'clears noteable diff cache when it was unfolded for the note position' do
expect_any_instance_of(Gitlab::Diff::HighlightCache).to receive(:clear)
diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb
index 0859c28cbe7..eebbdcc33b8 100644
--- a/spec/services/notes/destroy_service_spec.rb
+++ b/spec/services/notes/destroy_service_spec.rb
@@ -35,6 +35,14 @@ RSpec.describe Notes::DestroyService do
end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1)
end
+ it 'tracks merge request usage data' do
+ mr = create(:merge_request, source_project: project)
+ note = create(:note, project: project, noteable: mr)
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_remove_comment_action).with(note: note)
+
+ described_class.new(project, user).execute(note)
+ end
+
context 'in a merge request' do
let_it_be(:repo_project) { create(:project, :repository) }
let_it_be(:merge_request) do
diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb
index 64aa845841b..c098500b78a 100644
--- a/spec/services/notes/quick_actions_service_spec.rb
+++ b/spec/services/notes/quick_actions_service_spec.rb
@@ -153,8 +153,8 @@ RSpec.describe Notes::QuickActionsService do
expect(execute(note)).to be_empty
end
- it 'does not assign the milestone' do
- expect { execute(note) }.not_to change { issue.reload.milestone }
+ it 'assigns the milestone' do
+ expect { execute(note) }.to change { issue.reload.milestone }.from(nil).to(milestone)
end
end
@@ -195,8 +195,8 @@ RSpec.describe Notes::QuickActionsService do
expect(execute(note)).to be_empty
end
- it 'does not remove the milestone' do
- expect { execute(note) }.not_to change { issue.reload.milestone }
+ it 'removes the milestone' do
+ expect { execute(note) }.to change { issue.reload.milestone }.from(milestone).to(nil)
end
end
diff --git a/spec/services/notes/update_service_spec.rb b/spec/services/notes/update_service_spec.rb
index e2f51c9af67..902fd9958f8 100644
--- a/spec/services/notes/update_service_spec.rb
+++ b/spec/services/notes/update_service_spec.rb
@@ -49,11 +49,12 @@ RSpec.describe Notes::UpdateService do
it 'does not track usage data when params is blank' do
expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_edited_action)
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).not_to receive(:track_edit_comment_action)
update_note({})
end
- it 'tracks usage data', :clean_gitlab_redis_shared_state do
+ it 'tracks issue usage data', :clean_gitlab_redis_shared_state do
event = Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_EDITED
counter = Gitlab::UsageDataCounters::HLLRedisCounter
@@ -63,6 +64,17 @@ RSpec.describe Notes::UpdateService do
end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1)
end
+ context 'when the notable is a merge request' do
+ let(:merge_request) { create(:merge_request, source_project: project) }
+ let(:note) { create(:note, project: project, noteable: merge_request, author: user, note: "Old note #{user2.to_reference}") }
+
+ it 'tracks merge request usage data' do
+ expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter).to receive(:track_edit_comment_action).with(note: note)
+
+ update_note(note: 'new text')
+ end
+ end
+
context 'with system note' do
before do
note.update_column(:system, true)
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 9431c023850..85234077b1f 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -2455,6 +2455,18 @@ RSpec.describe NotificationService, :mailer do
let(:notification_trigger) { group.add_guest(added_user) }
end
end
+
+ describe '#updated_group_member_expiration' do
+ let_it_be(:group_member) { create(:group_member) }
+
+ it 'emails the user that their group membership expiry has changed' do
+ expect_next_instance_of(NotificationService) do |notification|
+ allow(notification).to receive(:updated_group_member_expiration).with(group_member)
+ end
+
+ group_member.update!(expires_at: 5.days.from_now)
+ end
+ end
end
describe 'ProjectMember', :deliver_mails_inline do
diff --git a/spec/services/onboarding_progress_service_spec.rb b/spec/services/onboarding_progress_service_spec.rb
index 59b6083d38a..340face4ae8 100644
--- a/spec/services/onboarding_progress_service_spec.rb
+++ b/spec/services/onboarding_progress_service_spec.rb
@@ -5,28 +5,44 @@ require 'spec_helper'
RSpec.describe OnboardingProgressService do
describe '#execute' do
let(:namespace) { create(:namespace, parent: root_namespace) }
+ let(:root_namespace) { nil }
+ let(:action) { :namespace_action }
subject(:execute_service) { described_class.new(namespace).execute(action: :subscription_created) }
context 'when the namespace is a root' do
- let(:root_namespace) { nil }
+ before do
+ OnboardingProgress.onboard(namespace)
+ end
- it 'records a namespace onboarding progress action for the given namespace' do
- expect(NamespaceOnboardingAction).to receive(:create_action)
- .with(namespace, :subscription_created).and_call_original
+ it 'registers a namespace onboarding progress action for the given namespace' do
+ execute_service
- expect { execute_service }.to change(NamespaceOnboardingAction, :count).by(1)
+ expect(OnboardingProgress.completed?(namespace, :subscription_created)).to eq(true)
end
end
context 'when the namespace is not the root' do
- let_it_be(:root_namespace) { build(:namespace) }
+ let(:root_namespace) { build(:namespace) }
+
+ before do
+ OnboardingProgress.onboard(root_namespace)
+ end
+
+ it 'registers a namespace onboarding progress action for the root namespace' do
+ execute_service
+
+ expect(OnboardingProgress.completed?(root_namespace, :subscription_created)).to eq(true)
+ end
+ end
+
+ context 'when no namespace is passed' do
+ let(:namespace) { nil }
- it 'records a namespace onboarding progress action for the root namespace' do
- expect(NamespaceOnboardingAction).to receive(:create_action)
- .with(root_namespace, :subscription_created).and_call_original
+ it 'does not register a namespace onboarding progress action' do
+ execute_service
- expect { execute_service }.to change(NamespaceOnboardingAction, :count).by(1)
+ expect(OnboardingProgress.completed?(root_namespace, :subscription_created)).to be(nil)
end
end
end
diff --git a/spec/services/packages/create_event_service_spec.rb b/spec/services/packages/create_event_service_spec.rb
index f581d704087..f7bab0e5a9f 100644
--- a/spec/services/packages/create_event_service_spec.rb
+++ b/spec/services/packages/create_event_service_spec.rb
@@ -56,7 +56,7 @@ RSpec.describe Packages::CreateEventService do
end
end
- shared_examples 'redis package event creation' do |originator_type, expected_scope|
+ shared_examples 'redis package unique event creation' do |originator_type, expected_scope|
context 'with feature flag disable' do
before do
stub_feature_flags(collect_package_events_redis: false)
@@ -70,29 +70,27 @@ RSpec.describe Packages::CreateEventService do
end
it 'tracks the event' do
- expect(::Gitlab::UsageDataCounters::GuestPackageEventCounter).not_to receive(:count)
- expect(::Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(user.id, Packages::Event.allowed_event_name(expected_scope, event_name, originator_type))
+ expect(::Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(/package/, values: user.id)
subject
end
end
- shared_examples 'redis package guest event creation' do |originator_type, expected_scope|
+ shared_examples 'redis package count event creation' do |originator_type, expected_scope|
context 'with feature flag disabled' do
before do
stub_feature_flags(collect_package_events_redis: false)
end
it 'does not track the event' do
- expect(::Gitlab::UsageDataCounters::GuestPackageEventCounter).not_to receive(:count)
+ expect(::Gitlab::UsageDataCounters::PackageEventCounter).not_to receive(:count)
subject
end
end
it 'tracks the event' do
- expect(::Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event)
- expect(::Gitlab::UsageDataCounters::GuestPackageEventCounter).to receive(:count).with(Packages::Event.allowed_event_name(expected_scope, event_name, originator_type))
+ expect(::Gitlab::UsageDataCounters::PackageEventCounter).to receive(:count).at_least(:once)
subject
end
@@ -102,21 +100,23 @@ RSpec.describe Packages::CreateEventService do
let(:user) { create(:user) }
it_behaves_like 'db package event creation', 'user', 'container'
- it_behaves_like 'redis package event creation', 'user', 'container'
+ it_behaves_like 'redis package unique event creation', 'user', 'container'
+ it_behaves_like 'redis package count event creation', 'user', 'container'
end
context 'with a deploy token' do
let(:user) { create(:deploy_token) }
it_behaves_like 'db package event creation', 'deploy_token', 'container'
- it_behaves_like 'redis package event creation', 'deploy_token', 'container'
+ it_behaves_like 'redis package unique event creation', 'deploy_token', 'container'
+ it_behaves_like 'redis package count event creation', 'deploy_token', 'container'
end
context 'with no user' do
let(:user) { nil }
it_behaves_like 'db package event creation', 'guest', 'container'
- it_behaves_like 'redis package guest event creation', 'guest', 'container'
+ it_behaves_like 'redis package count event creation', 'guest', 'container'
end
context 'with a package as scope' do
@@ -126,14 +126,15 @@ RSpec.describe Packages::CreateEventService do
let(:user) { nil }
it_behaves_like 'db package event creation', 'guest', 'npm'
- it_behaves_like 'redis package guest event creation', 'guest', 'npm'
+ it_behaves_like 'redis package count event creation', 'guest', 'npm'
end
context 'with user' do
let(:user) { create(:user) }
it_behaves_like 'db package event creation', 'user', 'npm'
- it_behaves_like 'redis package event creation', 'user', 'npm'
+ it_behaves_like 'redis package unique event creation', 'user', 'npm'
+ it_behaves_like 'redis package count event creation', 'user', 'npm'
end
end
end
diff --git a/spec/services/packages/debian/create_package_file_service_spec.rb b/spec/services/packages/debian/create_package_file_service_spec.rb
new file mode 100644
index 00000000000..74b97a4f941
--- /dev/null
+++ b/spec/services/packages/debian/create_package_file_service_spec.rb
@@ -0,0 +1,106 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Debian::CreatePackageFileService do
+ include WorkhorseHelpers
+
+ let_it_be(:package) { create(:debian_incoming, without_package_files: true) }
+
+ describe '#execute' do
+ let(:file_name) { 'libsample0_1.2.3~alpha2_amd64.deb' }
+ let(:fixture_path) { "spec/fixtures/packages/debian/#{file_name}" }
+
+ let(:params) do
+ {
+ file: file,
+ file_name: file_name,
+ file_sha1: '54321',
+ file_md5: '12345'
+ }.with_indifferent_access
+ end
+
+ let(:service) { described_class.new(package, params) }
+
+ subject(:package_file) { service.execute }
+
+ shared_examples 'a valid deb' do
+ it 'creates a new package file', :aggregate_failures do
+ expect(package_file).to be_valid
+ expect(package_file.file.read).to start_with('!<arch>')
+ expect(package_file.size).to eq(1124)
+ expect(package_file.file_name).to eq(file_name)
+ expect(package_file.file_sha1).to eq('54321')
+ expect(package_file.file_sha256).to eq('543212345')
+ expect(package_file.file_md5).to eq('12345')
+ expect(package_file.debian_file_metadatum).to be_valid
+ expect(package_file.debian_file_metadatum.file_type).to eq('unknown')
+ expect(package_file.debian_file_metadatum.architecture).to be_nil
+ expect(package_file.debian_file_metadatum.fields).to be_nil
+ end
+ end
+
+ context 'with temp file' do
+ let!(:file) do
+ upload_path = ::Packages::PackageFileUploader.workhorse_local_upload_path
+ file_path = upload_path + '/' + file_name
+
+ FileUtils.mkdir_p(upload_path)
+ File.write(file_path, File.read(fixture_path))
+
+ UploadedFile.new(file_path, filename: File.basename(file_path), sha256: '543212345')
+ end
+
+ it_behaves_like 'a valid deb'
+ end
+
+ context 'with remote file' do
+ let!(:fog_connection) do
+ stub_package_file_object_storage(direct_upload: true)
+ end
+
+ before do
+ allow_next_instance_of(UploadedFile) do |uploaded_file|
+ allow(uploaded_file).to receive(:sha256).and_return('543212345')
+ end
+ end
+
+ let(:tmp_object) do
+ fog_connection.directories.new(key: 'packages').files.create( # rubocop:disable Rails/SaveBang
+ key: "tmp/uploads/#{file_name}",
+ body: File.read(fixture_path)
+ )
+ end
+
+ let!(:file) { fog_to_uploaded_file(tmp_object) }
+
+ it_behaves_like 'a valid deb'
+ end
+
+ context 'package is missing' do
+ let(:package) { nil }
+ let(:params) { {} }
+
+ it 'raises an error' do
+ expect { subject.execute }.to raise_error(ArgumentError, 'Invalid package')
+ end
+ end
+
+ context 'params is empty' do
+ let(:params) { {} }
+
+ it 'raises an error' do
+ expect { subject.execute }.to raise_error(ActiveRecord::RecordInvalid)
+ end
+ end
+
+ context 'file is missing' do
+ let(:file_name) { 'libsample0_1.2.3~alpha2_amd64.deb' }
+ let(:file) { nil }
+
+ it 'raises an error' do
+ expect { subject.execute }.to raise_error(ActiveRecord::RecordInvalid)
+ end
+ end
+ end
+end
diff --git a/spec/services/packages/debian/extract_metadata_service_spec.rb b/spec/services/packages/debian/extract_metadata_service_spec.rb
new file mode 100644
index 00000000000..0aa9a67b263
--- /dev/null
+++ b/spec/services/packages/debian/extract_metadata_service_spec.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Packages::Debian::ExtractMetadataService do
+ let(:service) { described_class.new(package_file) }
+
+ subject { service.execute }
+
+ RSpec.shared_context 'Debian ExtractMetadata Service' do |trait|
+ let(:package_file) { create(:debian_package_file, trait) }
+ end
+
+ RSpec.shared_examples 'Test Debian ExtractMetadata Service' do |expected_file_type, expected_architecture, expected_fields|
+ it "returns file_type #{expected_file_type.inspect}" do
+ expect(subject[:file_type]).to eq(expected_file_type)
+ end
+
+ it "returns architecture #{expected_architecture.inspect}" do
+ expect(subject[:architecture]).to eq(expected_architecture)
+ end
+
+ it "returns fields #{expected_fields.nil? ? '' : 'including '}#{expected_fields.inspect}" do
+ if expected_fields.nil?
+ expect(subject[:fields]).to be_nil
+ else
+ expect(subject[:fields]).to include(**expected_fields)
+ end
+ end
+ end
+
+ using RSpec::Parameterized::TableSyntax
+
+ where(:case_name, :trait, :expected_file_type, :expected_architecture, :expected_fields) do
+ 'with invalid' | :invalid | :unknown | nil | nil
+ 'with source' | :source | :source | nil | nil
+ 'with dsc' | :dsc | :dsc | nil | { 'Binary': 'sample-dev, libsample0, sample-udeb' }
+ 'with deb' | :deb | :deb | 'amd64' | { 'Multi-Arch': 'same' }
+ 'with udeb' | :udeb | :udeb | 'amd64' | { 'Package': 'sample-udeb' }
+ 'with buildinfo' | :buildinfo | :buildinfo | nil | { 'Architecture': 'amd64 source', 'Build-Architecture': 'amd64' }
+ 'with changes' | :changes | :changes | nil | { 'Architecture': 'source amd64', 'Binary': 'libsample0 sample-dev sample-udeb' }
+ end
+
+ with_them do
+ include_context 'Debian ExtractMetadata Service', params[:trait] do
+ it_behaves_like 'Test Debian ExtractMetadata Service',
+ params[:expected_file_type],
+ params[:expected_architecture],
+ params[:expected_fields]
+ end
+ end
+
+ context 'with invalid package file' do
+ let(:package_file) { create(:conan_package_file) }
+
+ it 'raise error' do
+ expect { subject }.to raise_error(described_class::ExtractionError, 'invalid package file')
+ end
+ end
+end
diff --git a/spec/services/packages/debian/get_or_create_incoming_service_spec.rb b/spec/services/packages/debian/get_or_create_incoming_service_spec.rb
new file mode 100644
index 00000000000..ab99b091246
--- /dev/null
+++ b/spec/services/packages/debian/get_or_create_incoming_service_spec.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Debian::GetOrCreateIncomingService do
+ let_it_be(:project) { create(:project) }
+ let_it_be(:user) { create(:user) }
+
+ subject(:service) { described_class.new(project, user) }
+
+ describe '#execute' do
+ subject(:package) { service.execute }
+
+ context 'run once' do
+ it 'creates a new package', :aggregate_failures do
+ expect(package).to be_valid
+ expect(package.project_id).to eq(project.id)
+ expect(package.creator_id).to eq(user.id)
+ expect(package.name).to eq('incoming')
+ expect(package.version).to be_nil
+ expect(package.package_type).to eq('debian')
+ expect(package.debian_incoming?).to be_truthy
+ end
+
+ it_behaves_like 'assigns the package creator'
+ end
+
+ context 'run twice' do
+ let!(:package2) { service.execute }
+
+ it 'returns the same object' do
+ expect(package2.id).to eq(package.id)
+ end
+ end
+ end
+end
diff --git a/spec/services/packages/maven/find_or_create_package_service_spec.rb b/spec/services/packages/maven/find_or_create_package_service_spec.rb
index 2eaad7db445..82dffeefcde 100644
--- a/spec/services/packages/maven/find_or_create_package_service_spec.rb
+++ b/spec/services/packages/maven/find_or_create_package_service_spec.rb
@@ -11,29 +11,36 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do
let(:file_name) { 'test.jar' }
let(:param_path) { "#{path}/#{version}" }
let(:params) { { path: param_path, file_name: file_name } }
+ let(:service) { described_class.new(project, user, params) }
describe '#execute' do
using RSpec::Parameterized::TableSyntax
- subject { described_class.new(project, user, params).execute }
+ subject { service.execute }
- RSpec.shared_examples 'reuse existing package' do
- it { expect { subject}.not_to change { Packages::Package.count } }
+ shared_examples 'reuse existing package' do
+ it { expect { subject }.not_to change { Packages::Package.count } }
- it { is_expected.to eq(existing_package) }
+ it 'returns the existing package' do
+ expect(subject.payload).to eq(package: existing_package)
+ end
end
- RSpec.shared_examples 'create package' do
+ shared_examples 'create package' do
it { expect { subject }.to change { Packages::Package.count }.by(1) }
- it 'sets the proper name and version' do
- pkg = subject
+ it 'sets the proper name and version', :aggregate_failures do
+ pkg = subject.payload[:package]
expect(pkg.name).to eq(path)
expect(pkg.version).to eq(version)
end
- it_behaves_like 'assigns build to package'
+ context 'with a build' do
+ subject { service.execute.payload[:package] }
+
+ it_behaves_like 'assigns build to package'
+ end
end
context 'path with version' do
@@ -90,5 +97,36 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do
expect { subject }.to change { Packages::BuildInfo.count }.by(1)
end
end
+
+ context 'when package duplicates are not allowed' do
+ let_it_be_with_refind(:package_settings) { create(:namespace_package_setting, :group, maven_duplicates_allowed: false) }
+ let_it_be_with_refind(:group) { package_settings.namespace }
+ let_it_be_with_refind(:project) { create(:project, group: group) }
+ let!(:existing_package) { create(:maven_package, name: path, version: version, project: project) }
+
+ it { expect { subject }.not_to change { project.package_files.count } }
+
+ it 'returns an error', :aggregate_failures do
+ expect(subject.payload).to be_empty
+ expect(subject.errors).to include('Duplicate package is not allowed')
+ end
+
+ context 'when uploading different non-duplicate files to the same package' do
+ before do
+ package_file = existing_package.package_files.find_by(file_name: 'my-app-1.0-20180724.124855-1.jar')
+ package_file.destroy!
+ end
+
+ it_behaves_like 'reuse existing package'
+ end
+
+ context 'when the package name matches the exception regex' do
+ before do
+ package_settings.update!(maven_duplicate_exception_regex: '.*')
+ end
+
+ it_behaves_like 'reuse existing package'
+ end
+ end
end
end
diff --git a/spec/services/packages/nuget/search_service_spec.rb b/spec/services/packages/nuget/search_service_spec.rb
index d163e7087e4..db758dc6672 100644
--- a/spec/services/packages/nuget/search_service_spec.rb
+++ b/spec/services/packages/nuget/search_service_spec.rb
@@ -1,8 +1,12 @@
# frozen_string_literal: true
+
require 'spec_helper'
RSpec.describe Packages::Nuget::SearchService do
- let_it_be(:project) { create(:project) }
+ let_it_be(:user) { create(:user) }
+ let_it_be(:group) { create(:group) }
+ let_it_be(:subgroup) { create(:group, parent: group) }
+ let_it_be(:project) { create(:project, namespace: subgroup) }
let_it_be(:package_a) { create(:nuget_package, project: project, name: 'DummyPackageA') }
let_it_be(:packages_b) { create_list(:nuget_package, 5, project: project, name: 'DummyPackageB') }
let_it_be(:packages_c) { create_list(:nuget_package, 5, project: project, name: 'DummyPackageC') }
@@ -16,94 +20,126 @@ RSpec.describe Packages::Nuget::SearchService do
let(:options) { { include_prerelease_versions: include_prerelease_versions, per_page: per_page, padding: padding } }
describe '#execute' do
- subject { described_class.new(project, search_term, options).execute }
+ subject { described_class.new(user, target, search_term, options).execute }
- it { expect_search_results 3, package_a, packages_b, packages_c }
+ shared_examples 'handling all the conditions' do
+ it { expect_search_results 3, package_a, packages_b, packages_c }
- context 'with a smaller per page count' do
- let(:per_page) { 2 }
+ context 'with a smaller per page count' do
+ let(:per_page) { 2 }
- it { expect_search_results 3, package_a, packages_b }
- end
+ it { expect_search_results 3, package_a, packages_b }
+ end
- context 'with 0 per page count' do
- let(:per_page) { 0 }
+ context 'with 0 per page count' do
+ let(:per_page) { 0 }
- it { expect_search_results 3, [] }
- end
+ it { expect_search_results 3, [] }
+ end
- context 'with a negative per page count' do
- let(:per_page) { -1 }
+ context 'with a negative per page count' do
+ let(:per_page) { -1 }
- it { expect { subject }.to raise_error(ArgumentError, 'negative per_page') }
- end
+ it { expect { subject }.to raise_error(ArgumentError, 'negative per_page') }
+ end
- context 'with a padding' do
- let(:padding) { 2 }
+ context 'with a padding' do
+ let(:padding) { 2 }
- it { expect_search_results 3, packages_c }
- end
+ it { expect_search_results 3, packages_c }
+ end
- context 'with a too big padding' do
- let(:padding) { 5 }
+ context 'with a too big padding' do
+ let(:padding) { 5 }
- it { expect_search_results 3, [] }
- end
+ it { expect_search_results 3, [] }
+ end
- context 'with a negative padding' do
- let(:padding) { -1 }
+ context 'with a negative padding' do
+ let(:padding) { -1 }
- it { expect { subject }.to raise_error(ArgumentError, 'negative padding') }
- end
+ it { expect { subject }.to raise_error(ArgumentError, 'negative padding') }
+ end
- context 'with search term' do
- let(:search_term) { 'umm' }
+ context 'with search term' do
+ let(:search_term) { 'umm' }
- it { expect_search_results 3, package_a, packages_b, packages_c }
- end
+ it { expect_search_results 3, package_a, packages_b, packages_c }
+ end
- context 'with nil search term' do
- let(:search_term) { nil }
+ context 'with nil search term' do
+ let(:search_term) { nil }
- it { expect_search_results 4, package_a, packages_b, packages_c, package_d }
- end
+ it { expect_search_results 4, package_a, packages_b, packages_c, package_d }
+ end
- context 'with empty search term' do
- let(:search_term) { '' }
+ context 'with empty search term' do
+ let(:search_term) { '' }
- it { expect_search_results 4, package_a, packages_b, packages_c, package_d }
- end
+ it { expect_search_results 4, package_a, packages_b, packages_c, package_d }
+ end
- context 'with prefix search term' do
- let(:search_term) { 'dummy' }
+ context 'with prefix search term' do
+ let(:search_term) { 'dummy' }
- it { expect_search_results 3, package_a, packages_b, packages_c }
- end
+ it { expect_search_results 3, package_a, packages_b, packages_c }
+ end
+
+ context 'with suffix search term' do
+ let(:search_term) { 'packagec' }
+
+ it { expect_search_results 1, packages_c }
+ end
+
+ context 'with pre release packages' do
+ let_it_be(:package_e) { create(:nuget_package, project: project, name: 'DummyPackageE', version: '3.2.1-alpha') }
+
+ context 'including them' do
+ it { expect_search_results 4, package_a, packages_b, packages_c, package_e }
+ end
+
+ context 'excluding them' do
+ let(:include_prerelease_versions) { false }
- context 'with suffix search term' do
- let(:search_term) { 'packagec' }
+ it { expect_search_results 3, package_a, packages_b, packages_c }
- it { expect_search_results 1, packages_c }
+ context 'when mixed with release versions' do
+ let_it_be(:package_e_release) { create(:nuget_package, project: project, name: 'DummyPackageE', version: '3.2.1') }
+
+ it { expect_search_results 4, package_a, packages_b, packages_c, package_e_release }
+ end
+ end
+ end
end
- context 'with pre release packages' do
- let_it_be(:package_e) { create(:nuget_package, project: project, name: 'DummyPackageE', version: '3.2.1-alpha') }
+ context 'with project' do
+ let(:target) { project }
- context 'including them' do
- it { expect_search_results 4, package_a, packages_b, packages_c, package_e }
+ before do
+ project.add_developer(user)
end
- context 'excluding them' do
- let(:include_prerelease_versions) { false }
+ it_behaves_like 'handling all the conditions'
+ end
- it { expect_search_results 3, package_a, packages_b, packages_c }
+ context 'with subgroup' do
+ let(:target) { subgroup }
- context 'when mixed with release versions' do
- let_it_be(:package_e_release) { create(:nuget_package, project: project, name: 'DummyPackageE', version: '3.2.1') }
+ before do
+ subgroup.add_developer(user)
+ end
- it { expect_search_results 4, package_a, packages_b, packages_c, package_e_release }
- end
+ it_behaves_like 'handling all the conditions'
+ end
+
+ context 'with group' do
+ let(:target) { group }
+
+ before do
+ group.add_developer(user)
end
+
+ it_behaves_like 'handling all the conditions'
end
def expect_search_results(total_count, *results)
diff --git a/spec/services/pages/migrate_legacy_storage_to_deployment_service_spec.rb b/spec/services/pages/migrate_legacy_storage_to_deployment_service_spec.rb
new file mode 100644
index 00000000000..29023621413
--- /dev/null
+++ b/spec/services/pages/migrate_legacy_storage_to_deployment_service_spec.rb
@@ -0,0 +1,118 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Pages::MigrateLegacyStorageToDeploymentService do
+ let(:project) { create(:project, :repository) }
+ let(:service) { described_class.new(project) }
+
+ it 'marks pages as not deployed if public directory is absent' do
+ project.mark_pages_as_deployed
+
+ expect(project.pages_metadatum.reload.deployed).to eq(true)
+
+ expect(service.execute).to(
+ eq(status: :error,
+ message: "Can't create zip archive: Can not find valid public dir in #{project.pages_path}")
+ )
+
+ expect(project.pages_metadatum.reload.deployed).to eq(false)
+ end
+
+ it 'does not mark pages as not deployed if public directory is absent but pages_deployment exists' do
+ deployment = create(:pages_deployment, project: project)
+ project.update_pages_deployment!(deployment)
+ project.mark_pages_as_deployed
+
+ expect(project.pages_metadatum.reload.deployed).to eq(true)
+
+ expect(service.execute).to(
+ eq(status: :error,
+ message: "Can't create zip archive: Can not find valid public dir in #{project.pages_path}")
+ )
+
+ expect(project.pages_metadatum.reload.deployed).to eq(true)
+ end
+
+ it 'does not mark pages as not deployed if public directory is absent but feature is disabled' do
+ stub_feature_flags(pages_migration_mark_as_not_deployed: false)
+
+ project.mark_pages_as_deployed
+
+ expect(project.pages_metadatum.reload.deployed).to eq(true)
+
+ expect(service.execute).to(
+ eq(status: :error,
+ message: "Can't create zip archive: Can not find valid public dir in #{project.pages_path}")
+ )
+
+ expect(project.pages_metadatum.reload.deployed).to eq(true)
+ end
+
+ it 'removes pages archive when can not save deployment' do
+ archive = fixture_file_upload("spec/fixtures/pages.zip")
+ expect_next_instance_of(::Pages::ZipDirectoryService) do |zip_service|
+ expect(zip_service).to receive(:execute).and_return(status: :success,
+ archive_path: archive.path,
+ entries_count: 3)
+ end
+
+ expect_next_instance_of(PagesDeployment) do |deployment|
+ expect(deployment).to receive(:save!).and_raise("Something")
+ end
+
+ expect do
+ service.execute
+ end.to raise_error("Something")
+
+ expect(File.exist?(archive.path)).to eq(false)
+ end
+
+ context 'when pages site is deployed to legacy storage' do
+ before do
+ FileUtils.mkdir_p File.join(project.pages_path, "public")
+ File.open(File.join(project.pages_path, "public/index.html"), "w") do |f|
+ f.write("Hello!")
+ end
+ end
+
+ it 'creates pages deployment' do
+ expect do
+ expect(described_class.new(project).execute).to eq(status: :success)
+ end.to change { project.reload.pages_deployments.count }.by(1)
+
+ deployment = project.pages_metadatum.pages_deployment
+
+ Zip::File.open(deployment.file.path) do |zip_file|
+ expect(zip_file.glob("public").first.ftype).to eq(:directory)
+ expect(zip_file.glob("public/index.html").first.get_input_stream.read).to eq("Hello!")
+ end
+
+ expect(deployment.file_count).to eq(2)
+ expect(deployment.file_sha256).to eq(Digest::SHA256.file(deployment.file.path).hexdigest)
+ end
+
+ it 'removes tmp pages archive' do
+ described_class.new(project).execute
+
+ expect(File.exist?(File.join(project.pages_path, '@migrated.zip'))).to eq(false)
+ end
+
+ it 'does not change pages deployment if it is set' do
+ old_deployment = create(:pages_deployment, project: project)
+ project.update_pages_deployment!(old_deployment)
+
+ expect do
+ described_class.new(project).execute
+ end.not_to change { project.pages_metadatum.reload.pages_deployment_id }.from(old_deployment.id)
+ end
+
+ it 'raises exception if exclusive lease is taken' do
+ described_class.new(project).try_obtain_lease do
+ expect do
+ described_class.new(project).execute
+ end.to raise_error(described_class::ExclusiveLeaseTakenError)
+ end
+ end
+ end
+end
diff --git a/spec/services/pages/zip_directory_service_spec.rb b/spec/services/pages/zip_directory_service_spec.rb
index 1568103d102..dcab6b2dada 100644
--- a/spec/services/pages/zip_directory_service_spec.rb
+++ b/spec/services/pages/zip_directory_service_spec.rb
@@ -14,19 +14,35 @@ RSpec.describe Pages::ZipDirectoryService do
described_class.new(@work_dir).execute
end
- let(:archive) { result.first }
- let(:entries_count) { result.second }
+ let(:status) { result[:status] }
+ let(:message) { result[:message] }
+ let(:archive) { result[:archive_path] }
+ let(:entries_count) { result[:entries_count] }
+
+ it 'returns error if project pages dir does not exist' do
+ expect(
+ described_class.new("/tmp/not/existing/dir").execute
+ ).to eq(status: :error, message: "Can not find valid public dir in /tmp/not/existing/dir")
+ end
+
+ it 'returns nils if there is no public directory and does not leave archive' do
+ expect(status).to eq(:error)
+ expect(message).to eq("Can not find valid public dir in #{@work_dir}")
+ expect(archive).to eq(nil)
+ expect(entries_count).to eq(nil)
- it 'raises error if there is no public directory' do
- expect { archive }.to raise_error(described_class::InvalidArchiveError)
+ expect(File.exist?(File.join(@work_dir, '@migrated.zip'))).to eq(false)
end
- it 'raises error if public directory is a symlink' do
+ it 'returns nils if public directory is a symlink' do
create_dir('target')
create_file('./target/index.html', 'hello')
create_link("public", "./target")
- expect { archive }.to raise_error(described_class::InvalidArchiveError)
+ expect(status).to eq(:error)
+ expect(message).to eq("Can not find valid public dir in #{@work_dir}")
+ expect(archive).to eq(nil)
+ expect(entries_count).to eq(nil)
end
context 'when there is a public directory' do
diff --git a/spec/services/post_receive_service_spec.rb b/spec/services/post_receive_service_spec.rb
index 4e303bfc20a..6e2cd7edf04 100644
--- a/spec/services/post_receive_service_spec.rb
+++ b/spec/services/post_receive_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe PostReceiveService do
include Gitlab::Routing
+ include AfterNextHelpers
let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :repository, :wiki_repo, namespace: user.namespace) }
@@ -46,8 +47,8 @@ RSpec.describe PostReceiveService do
expect(subject).to be_empty
end
- it 'does not record a namespace onboarding progress action' do
- expect(NamespaceOnboardingAction).not_to receive(:create_action)
+ it 'does not record an onboarding progress action' do
+ expect_next(OnboardingProgressService).not_to receive(:execute)
subject
end
@@ -87,9 +88,9 @@ RSpec.describe PostReceiveService do
expect(response.reference_counter_decreased).to be(true)
end
- it 'records a namespace onboarding progress action' do
- expect(NamespaceOnboardingAction).to receive(:create_action)
- .with(project.namespace, :git_write)
+ it 'records an onboarding progress action' do
+ expect_next(OnboardingProgressService, project.namespace)
+ .to receive(:execute).with(action: :git_write)
subject
end
diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb
index a109348ea19..a16aec891a9 100644
--- a/spec/services/projects/after_import_service_spec.rb
+++ b/spec/services/projects/after_import_service_spec.rb
@@ -14,7 +14,7 @@ RSpec.describe Projects::AfterImportService do
describe '#execute' do
before do
- allow(Projects::HousekeepingService)
+ allow(Repositories::HousekeepingService)
.to receive(:new).with(project).and_return(housekeeping_service)
allow(housekeeping_service)
@@ -73,10 +73,10 @@ RSpec.describe Projects::AfterImportService do
end
context 'when housekeeping service lease is taken' do
- let(:exception) { Projects::HousekeepingService::LeaseTaken.new }
+ let(:exception) { Repositories::HousekeepingService::LeaseTaken.new }
it 'logs the error message' do
- allow_next_instance_of(Projects::HousekeepingService) do |instance|
+ allow_next_instance_of(Repositories::HousekeepingService) do |instance|
expect(instance).to receive(:execute).and_raise(exception)
end
diff --git a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb
index 8ddcb8ce660..17c2f0f6c17 100644
--- a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb
+++ b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb
@@ -3,11 +3,14 @@
require 'spec_helper'
RSpec.describe Projects::ContainerRepository::CleanupTagsService do
+ using RSpec::Parameterized::TableSyntax
+
let_it_be(:user) { create(:user) }
let_it_be(:project, reload: true) { create(:project, :private) }
let_it_be(:repository) { create(:container_repository, :root, project: project) }
let(:service) { described_class.new(project, user, params) }
+ let(:tags) { %w[latest A Ba Bb C D E] }
before do
project.add_maintainer(user)
@@ -16,7 +19,8 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
stub_container_registry_tags(
repository: repository.path,
- tags: %w(latest A Ba Bb C D E))
+ tags: tags
+ )
stub_tag_digest('latest', 'sha256:configA')
stub_tag_digest('A', 'sha256:configA')
@@ -30,6 +34,8 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
stub_digest_config('sha256:configB', 5.days.ago)
stub_digest_config('sha256:configC', 1.month.ago)
stub_digest_config('sha256:configD', nil)
+
+ stub_feature_flags(container_registry_expiration_policies_throttling: false)
end
describe '#execute' do
@@ -42,7 +48,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
expect_any_instance_of(Projects::ContainerRepository::DeleteTagsService)
.not_to receive(:execute)
- is_expected.to include(status: :success, deleted: [])
+ is_expected.to eq(expected_service_response(before_truncate_size: 0, after_truncate_size: 0, before_delete_size: 0))
end
end
@@ -51,7 +57,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
it 'does remove all tags except latest' do
expect_delete(%w(A Ba Bb C D E))
- is_expected.to include(status: :success, deleted: %w(A Ba Bb C D E))
+ is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C D E)))
end
end
@@ -78,12 +84,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
subject
end
- it 'returns an error' do
- response = subject
-
- expect(response[:status]).to eq(:error)
- expect(response[:message]).to eq('invalid regex')
- end
+ it { is_expected.to eq(status: :error, message: 'invalid regex') }
it 'calls error tracking service' do
expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original
@@ -119,7 +120,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
it 'does remove C and D' do
expect_delete(%w(C D))
- is_expected.to include(status: :success, deleted: %w(C D))
+ is_expected.to eq(expected_service_response(deleted: %w(C D), before_truncate_size: 2, after_truncate_size: 2, before_delete_size: 2))
end
context 'with overriding allow regex' do
@@ -131,7 +132,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
it 'does not remove C' do
expect_delete(%w(D))
- is_expected.to include(status: :success, deleted: %w(D))
+ is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1))
end
end
@@ -144,7 +145,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
it 'does not remove C' do
expect_delete(%w(D))
- is_expected.to include(status: :success, deleted: %w(D))
+ is_expected.to eq(expected_service_response(deleted: %w(D), before_truncate_size: 1, after_truncate_size: 1, before_delete_size: 1))
end
end
end
@@ -158,7 +159,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
it 'does not remove B*' do
expect_delete(%w(A C D E))
- is_expected.to include(status: :success, deleted: %w(A C D E))
+ is_expected.to eq(expected_service_response(deleted: %w(A C D E), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4))
end
end
@@ -173,7 +174,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
expect(service).to receive(:order_by_date).and_call_original
- is_expected.to include(status: :success, deleted: %w(Bb Ba C))
+ is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 3))
end
end
@@ -187,7 +188,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
expect(service).not_to receive(:order_by_date)
- is_expected.to include(status: :success, deleted: %w(A Ba Bb C))
+ is_expected.to eq(expected_service_response(deleted: %w(A Ba Bb C), before_truncate_size: 4, after_truncate_size: 4, before_delete_size: 4))
end
end
@@ -200,7 +201,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
it 'does remove B* and C as they are the oldest' do
expect_delete(%w(Bb Ba C))
- is_expected.to include(status: :success, deleted: %w(Bb Ba C))
+ is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3))
end
end
@@ -213,7 +214,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
it 'does remove B* and C as they are older than 1 day' do
expect_delete(%w(Ba Bb C))
- is_expected.to include(status: :success, deleted: %w(Ba Bb C))
+ is_expected.to eq(expected_service_response(deleted: %w(Ba Bb C), before_delete_size: 3))
end
end
@@ -227,7 +228,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
it 'does remove B* and C' do
expect_delete(%w(Bb Ba C))
- is_expected.to include(status: :success, deleted: %w(Bb Ba C))
+ is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3))
end
end
@@ -245,7 +246,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
it 'succeeds without a user' do
expect_delete(%w(Bb Ba C), container_expiration_policy: true)
- is_expected.to include(status: :success, deleted: %w(Bb Ba C))
+ is_expected.to eq(expected_service_response(deleted: %w(Bb Ba C), before_delete_size: 3))
end
end
@@ -257,10 +258,73 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
end
it 'fails' do
- is_expected.to include(status: :error, message: 'access denied')
+ is_expected.to eq(status: :error, message: 'access denied')
end
end
end
+
+ context 'truncating the tags list' do
+ let(:params) do
+ {
+ 'name_regex_delete' => '.*',
+ 'keep_n' => 1
+ }
+ end
+
+ shared_examples 'returning the response' do |status:, original_size:, before_truncate_size:, after_truncate_size:, before_delete_size:|
+ it 'returns the response' do
+ result = subject
+
+ service_response = expected_service_response(
+ status: status,
+ original_size: original_size,
+ before_truncate_size: before_truncate_size,
+ after_truncate_size: after_truncate_size,
+ before_delete_size: before_delete_size,
+ deleted: nil
+ )
+
+ expect(result).to eq(service_response.compact)
+ end
+ end
+
+ where(:feature_flag_enabled, :max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do
+ false | 10 | :success | :success | false
+ false | 10 | :error | :error | false
+ false | 3 | :success | :success | false
+ false | 3 | :error | :error | false
+ false | 0 | :success | :success | false
+ false | 0 | :error | :error | false
+ true | 10 | :success | :success | false
+ true | 10 | :error | :error | false
+ true | 3 | :success | :error | true
+ true | 3 | :error | :error | true
+ true | 0 | :success | :success | false
+ true | 0 | :error | :error | false
+ end
+
+ with_them do
+ before do
+ stub_feature_flags(container_registry_expiration_policies_throttling: feature_flag_enabled)
+ stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size)
+ allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service|
+ expect(service).to receive(:execute).and_return(status: delete_tags_service_status)
+ end
+ end
+
+ original_size = 7
+ keep_n = 1
+
+ it_behaves_like(
+ 'returning the response',
+ status: params[:expected_status],
+ original_size: original_size,
+ before_truncate_size: original_size - keep_n,
+ after_truncate_size: params[:expected_truncated] ? params[:max_list_size] + keep_n : original_size - keep_n,
+ before_delete_size: params[:expected_truncated] ? params[:max_list_size] : original_size - keep_n - 1 # one tag is filtered out with older_than filter
+ )
+ end
+ end
end
private
@@ -295,4 +359,16 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
.to receive(:execute)
.with(repository) { { status: :success, deleted: tags } }
end
+
+ # all those -1 because the default tags on L13 have a "latest" that will be filtered out
+ def expected_service_response(status: :success, deleted: [], original_size: tags.size, before_truncate_size: tags.size - 1, after_truncate_size: tags.size - 1, before_delete_size: tags.size - 1)
+ {
+ status: status,
+ deleted: deleted,
+ original_size: original_size,
+ before_truncate_size: before_truncate_size,
+ after_truncate_size: after_truncate_size,
+ before_delete_size: before_delete_size
+ }.compact
+ end
end
diff --git a/spec/services/projects/container_repository/delete_tags_service_spec.rb b/spec/services/projects/container_repository/delete_tags_service_spec.rb
index 4da416d9698..94037d6de1e 100644
--- a/spec/services/projects/container_repository/delete_tags_service_spec.rb
+++ b/spec/services/projects/container_repository/delete_tags_service_spec.rb
@@ -119,9 +119,9 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do
end
end
- it { is_expected.to include(status: :error, message: 'timeout while deleting tags') }
+ it { is_expected.to include(status: :error, message: 'error while deleting tags') }
- it_behaves_like 'logging an error response', message: 'timeout while deleting tags', extra_log: { deleted_tags_count: 0 }
+ it_behaves_like 'logging an error response', message: 'error while deleting tags', extra_log: { deleted_tags_count: 0 }
end
end
end
diff --git a/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb b/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb
index 988971171fc..74f782538c5 100644
--- a/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb
+++ b/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb
@@ -67,7 +67,7 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do
stub_delete_reference_requests('A' => 200)
end
- it { is_expected.to eq(status: :error, message: 'timeout while deleting tags', deleted: ['A']) }
+ it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: ['A'], exception_class_name: Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError.name) }
it 'tracks the exception' do
expect(::Gitlab::ErrorTracking)
@@ -89,6 +89,21 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do
it_behaves_like 'deleting tags'
end
end
+
+ context 'with a network error' do
+ before do
+ expect(service).to receive(:delete_tags).and_raise(::Faraday::TimeoutError)
+ end
+
+ it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: [], exception_class_name: ::Faraday::TimeoutError.name) }
+
+ it 'tracks the exception' do
+ expect(::Gitlab::ErrorTracking)
+ .to receive(:track_exception).with(::Faraday::TimeoutError, tags_count: tags.size, container_repository_id: repository.id)
+
+ subject
+ end
+ end
end
end
diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb
index 60dfee820ca..a11f16573f5 100644
--- a/spec/services/projects/fork_service_spec.rb
+++ b/spec/services/projects/fork_service_spec.rb
@@ -116,6 +116,24 @@ RSpec.describe Projects::ForkService do
expect(to_project.fork_network_member.forked_from_project).to eq(from_forked_project)
end
+ context 'when the forked project has higher visibility than the root project' do
+ let(:root_project) { create(:project, :public) }
+
+ it 'successfully creates a fork of the fork with correct visibility' do
+ forked_project = fork_project(root_project, @to_user, using_service: true)
+
+ root_project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
+
+ # Forked project visibility is not affected by root project visibility change
+ expect(forked_project).to have_attributes(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+
+ fork_of_the_fork = fork_project(forked_project, @to_user, namespace: other_namespace, using_service: true)
+
+ expect(fork_of_the_fork).to be_valid
+ expect(fork_of_the_fork).to have_attributes(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+ end
+ end
+
it_behaves_like 'forks count cache refresh' do
let(:from_project) { from_forked_project }
let(:to_user) { @to_user }
diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb
index 18871f010f8..0aa4a1cd312 100644
--- a/spec/services/projects/housekeeping_service_spec.rb
+++ b/spec/services/projects/housekeeping_service_spec.rb
@@ -2,127 +2,19 @@
require 'spec_helper'
+# This is a compatibility class to avoid calling a non-existent
+# class from sidekiq during deployment.
+#
+# We're deploying the name of the referenced class in 13.9. Nevertheless,
+# we cannot remove the class entirely because there can be jobs
+# referencing it. We still need this specs to ensure that the old
+# class still has the old behavior.
+#
+# We can get rid of this class in 13.10
+# https://gitlab.com/gitlab-org/gitlab/-/issues/297580
+#
RSpec.describe Projects::HousekeepingService do
- subject { described_class.new(project) }
-
- let_it_be(:project) { create(:project, :repository) }
-
- before do
- project.reset_pushes_since_gc
- end
-
- after do
- project.reset_pushes_since_gc
- end
-
- describe '#execute' do
- it 'enqueues a sidekiq job' do
- expect(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
- expect(subject).to receive(:lease_key).and_return(:the_lease_key)
- expect(subject).to receive(:task).and_return(:incremental_repack)
- expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :incremental_repack, :the_lease_key, :the_uuid).and_call_original
-
- Sidekiq::Testing.fake! do
- expect { subject.execute }.to change(GitGarbageCollectWorker.jobs, :size).by(1)
- end
- end
-
- it 'yields the block if given' do
- expect do |block|
- subject.execute(&block)
- end.to yield_with_no_args
- end
-
- it 'resets counter after execution' do
- expect(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
- allow(subject).to receive(:gc_period).and_return(1)
- project.increment_pushes_since_gc
-
- perform_enqueued_jobs do
- expect { subject.execute }.to change { project.pushes_since_gc }.to(0)
- end
- end
-
- context 'when no lease can be obtained' do
- before do
- expect(subject).to receive(:try_obtain_lease).and_return(false)
- end
-
- it 'does not enqueue a job' do
- expect(GitGarbageCollectWorker).not_to receive(:perform_async)
-
- expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
- end
-
- it 'does not reset pushes_since_gc' do
- expect do
- expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
- end.not_to change { project.pushes_since_gc }
- end
-
- it 'does not yield' do
- expect do |block|
- expect { subject.execute(&block) }
- .to raise_error(Projects::HousekeepingService::LeaseTaken)
- end.not_to yield_with_no_args
- end
- end
-
- context 'task type' do
- it 'goes through all three housekeeping tasks, executing only the highest task when there is overlap' do
- allow(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
- allow(subject).to receive(:lease_key).and_return(:the_lease_key)
-
- # At push 200
- expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :gc, :the_lease_key, :the_uuid)
- .once
- # At push 50, 100, 150
- expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :full_repack, :the_lease_key, :the_uuid)
- .exactly(3).times
- # At push 10, 20, ... (except those above)
- expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :incremental_repack, :the_lease_key, :the_uuid)
- .exactly(16).times
- # At push 6, 12, 18, ... (except those above)
- expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :pack_refs, :the_lease_key, :the_uuid)
- .exactly(27).times
-
- 201.times do
- subject.increment!
- subject.execute if subject.needed?
- end
-
- expect(project.pushes_since_gc).to eq(1)
- end
- end
-
- it 'runs the task specifically requested' do
- housekeeping = described_class.new(project, :gc)
-
- allow(housekeeping).to receive(:try_obtain_lease).and_return(:gc_uuid)
- allow(housekeeping).to receive(:lease_key).and_return(:gc_lease_key)
-
- expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :gc, :gc_lease_key, :gc_uuid).twice
-
- 2.times do
- housekeeping.execute
- end
- end
- end
-
- describe '#needed?' do
- it 'when the count is low enough' do
- expect(subject.needed?).to eq(false)
- end
-
- it 'when the count is high enough' do
- allow(project).to receive(:pushes_since_gc).and_return(10)
- expect(subject.needed?).to eq(true)
- end
- end
-
- describe '#increment!' do
- it 'increments the pushes_since_gc counter' do
- expect { subject.increment! }.to change { project.pushes_since_gc }.by(1)
- end
+ it_behaves_like 'housekeeps repository' do
+ let_it_be(:resource) { create(:project, :repository) }
end
end
diff --git a/spec/services/projects/schedule_bulk_repository_shard_moves_service_spec.rb b/spec/services/projects/schedule_bulk_repository_shard_moves_service_spec.rb
index 5b76386bfab..15c9d1e5925 100644
--- a/spec/services/projects/schedule_bulk_repository_shard_moves_service_spec.rb
+++ b/spec/services/projects/schedule_bulk_repository_shard_moves_service_spec.rb
@@ -3,45 +3,10 @@
require 'spec_helper'
RSpec.describe Projects::ScheduleBulkRepositoryShardMovesService do
- before do
- stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' })
- end
-
- let!(:project) { create(:project, :repository).tap { |project| project.track_project_repository } }
- let(:source_storage_name) { 'default' }
- let(:destination_storage_name) { 'test_second_storage' }
-
- describe '#execute' do
- it 'schedules project repository storage moves' do
- expect { subject.execute(source_storage_name, destination_storage_name) }
- .to change(ProjectRepositoryStorageMove, :count).by(1)
-
- storage_move = project.repository_storage_moves.last!
-
- expect(storage_move).to have_attributes(
- source_storage_name: source_storage_name,
- destination_storage_name: destination_storage_name,
- state_name: :scheduled
- )
- end
-
- context 'read-only repository' do
- let!(:project) { create(:project, :repository, :read_only).tap { |project| project.track_project_repository } }
-
- it 'does not get scheduled' do
- expect(subject).to receive(:log_info)
- .with("Project #{project.full_path} (#{project.id}) was skipped: Project is read only")
- expect { subject.execute(source_storage_name, destination_storage_name) }
- .to change(ProjectRepositoryStorageMove, :count).by(0)
- end
- end
- end
-
- describe '.enqueue' do
- it 'defers to the worker' do
- expect(::ProjectScheduleBulkRepositoryShardMovesWorker).to receive(:perform_async).with(source_storage_name, destination_storage_name)
+ it_behaves_like 'moves repository shard in bulk' do
+ let_it_be_with_reload(:container) { create(:project, :repository).tap { |project| project.track_project_repository } }
- described_class.enqueue(source_storage_name, destination_storage_name)
- end
+ let(:move_service_klass) { ProjectRepositoryStorageMove }
+ let(:bulk_worker_klass) { ::ProjectScheduleBulkRepositoryShardMovesWorker }
end
end
diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb
index a15f6bdbe2c..a6730c5de52 100644
--- a/spec/services/projects/update_pages_service_spec.rb
+++ b/spec/services/projects/update_pages_service_spec.rb
@@ -79,6 +79,19 @@ RSpec.describe Projects::UpdatePagesService do
end
end
+ it 'fails if sha on branch was updated before deployment was uploaded' do
+ expect(subject).to receive(:create_pages_deployment).and_wrap_original do |m, *args|
+ build.update!(ref: 'feature')
+ m.call(*args)
+ end
+
+ expect(execute).not_to eq(:success)
+ expect(project.pages_metadatum).not_to be_deployed
+
+ expect(deploy_status).to be_failed
+ expect(deploy_status.description).to eq('build SHA is outdated for this ref')
+ end
+
it 'does not fail if pages_metadata is absent' do
project.pages_metadatum.destroy!
project.reload
diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb
index 760cd85bf71..a59b6adf346 100644
--- a/spec/services/projects/update_service_spec.rb
+++ b/spec/services/projects/update_service_spec.rb
@@ -15,13 +15,6 @@ RSpec.describe Projects::UpdateService do
let(:admin) { create(:admin) }
context 'when changing visibility level' do
- def expect_to_call_unlink_fork_service
- service = Projects::UnlinkForkService.new(project, user)
-
- expect(Projects::UnlinkForkService).to receive(:new).with(project, user).and_return(service)
- expect(service).to receive(:execute).and_call_original
- end
-
context 'when visibility_level changes to INTERNAL' do
it 'updates the project to internal' do
expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in)
@@ -31,18 +24,6 @@ RSpec.describe Projects::UpdateService do
expect(result).to eq({ status: :success })
expect(project).to be_internal
end
-
- context 'and project is PUBLIC' do
- before do
- project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
- end
-
- it 'unlinks project from fork network' do
- expect_to_call_unlink_fork_service
-
- update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL)
- end
- end
end
context 'when visibility_level changes to PUBLIC' do
@@ -78,30 +59,6 @@ RSpec.describe Projects::UpdateService do
expect(result).to eq({ status: :success })
expect(project).to be_private
end
-
- context 'and project is PUBLIC' do
- before do
- project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
- end
-
- it 'unlinks project from fork network' do
- expect_to_call_unlink_fork_service
-
- update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE)
- end
- end
-
- context 'and project is INTERNAL' do
- before do
- project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
- end
-
- it 'unlinks project from fork network' do
- expect_to_call_unlink_fork_service
-
- update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE)
- end
- end
end
context 'when visibility levels are restricted to PUBLIC only' do
diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb
index e6d1d0e90a7..21e294418a1 100644
--- a/spec/services/quick_actions/interpret_service_spec.rb
+++ b/spec/services/quick_actions/interpret_service_spec.rb
@@ -8,6 +8,7 @@ RSpec.describe QuickActions::InterpretService do
let_it_be(:project) { public_project }
let_it_be(:developer) { create(:user) }
let_it_be(:developer2) { create(:user) }
+ let_it_be(:developer3) { create(:user) }
let_it_be_with_reload(:issue) { create(:issue, project: project) }
let(:milestone) { create(:milestone, project: project, title: '9.10') }
let(:commit) { create(:commit, project: project) }
@@ -23,6 +24,7 @@ RSpec.describe QuickActions::InterpretService do
before do
stub_licensed_features(multiple_issue_assignees: false,
+ multiple_merge_request_reviewers: false,
multiple_merge_request_assignees: false)
end
@@ -665,6 +667,24 @@ RSpec.describe QuickActions::InterpretService do
end
end
+ shared_examples 'assign_reviewer command' do
+ it 'assigns a reviewer to a single user' do
+ _, updates, message = service.execute(content, issuable)
+
+ expect(updates).to eq(reviewer_ids: [developer.id])
+ expect(message).to eq("Assigned #{developer.to_reference} as reviewer.")
+ end
+ end
+
+ shared_examples 'unassign_reviewer command' do
+ it 'removes a single reviewer' do
+ _, updates, message = service.execute(content, issuable)
+
+ expect(updates).to eq(reviewer_ids: [])
+ expect(message).to eq("Removed reviewer #{developer.to_reference}.")
+ end
+ end
+
it_behaves_like 'reopen command' do
let(:content) { '/reopen' }
let(:issuable) { issue }
@@ -779,6 +799,11 @@ RSpec.describe QuickActions::InterpretService do
it_behaves_like 'assign command' do
let(:content) { "/assign @#{developer.username}" }
+ let(:issuable) { create(:incident, project: project) }
+ end
+
+ it_behaves_like 'assign command' do
+ let(:content) { "/assign @#{developer.username}" }
let(:issuable) { merge_request }
end
end
@@ -789,12 +814,32 @@ RSpec.describe QuickActions::InterpretService do
project.add_developer(developer2)
end
- it_behaves_like 'assign command' do
+ # There's no guarantee that the reference extractor will preserve
+ # the order of the mentioned users since this is dependent on the
+ # order in which rows are returned. We just ensure that at least
+ # one of the mentioned users is assigned.
+ shared_examples 'assigns to one of the two users' do
+ let(:content) { "/assign @#{developer.username} @#{developer2.username}" }
+
+ it 'assigns to a single user' do
+ _, updates, message = service.execute(content, issuable)
+
+ expect(updates[:assignee_ids].count).to eq(1)
+ assignee = updates[:assignee_ids].first
+ expect([developer.id, developer2.id]).to include(assignee)
+
+ user = assignee == developer.id ? developer : developer2
+
+ expect(message).to match("Assigned #{user.to_reference}.")
+ end
+ end
+
+ it_behaves_like 'assigns to one of the two users' do
let(:content) { "/assign @#{developer.username} @#{developer2.username}" }
let(:issuable) { issue }
end
- it_behaves_like 'assign command', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/27989' do
+ it_behaves_like 'assigns to one of the two users' do
let(:content) { "/assign @#{developer.username} @#{developer2.username}" }
let(:issuable) { merge_request }
end
@@ -834,6 +879,142 @@ RSpec.describe QuickActions::InterpretService do
let(:issuable) { issue }
end
+ context 'when the merge_request_reviewers flag is enabled' do
+ describe 'assign_reviewer command' do
+ let(:content) { "/assign_reviewer @#{developer.username}" }
+ let(:issuable) { merge_request }
+
+ context 'with one user' do
+ it_behaves_like 'assign_reviewer command'
+ end
+
+ context 'with an issue instead of a merge request' do
+ let(:issuable) { issue }
+
+ it_behaves_like 'empty command'
+ end
+
+ # CE does not have multiple reviewers
+ context 'assign command with multiple assignees' do
+ before do
+ project.add_developer(developer2)
+ end
+
+ # There's no guarantee that the reference extractor will preserve
+ # the order of the mentioned users since this is dependent on the
+ # order in which rows are returned. We just ensure that at least
+ # one of the mentioned users is assigned.
+ context 'assigns to one of the two users' do
+ let(:content) { "/assign_reviewer @#{developer.username} @#{developer2.username}" }
+
+ it 'assigns to a single reviewer' do
+ _, updates, message = service.execute(content, issuable)
+
+ expect(updates[:reviewer_ids].count).to eq(1)
+ reviewer = updates[:reviewer_ids].first
+ expect([developer.id, developer2.id]).to include(reviewer)
+
+ user = reviewer == developer.id ? developer : developer2
+
+ expect(message).to match("Assigned #{user.to_reference} as reviewer.")
+ end
+ end
+ end
+
+ context 'with "me" alias' do
+ let(:content) { '/assign_reviewer me' }
+
+ it_behaves_like 'assign_reviewer command'
+ end
+
+ context 'with an alias and whitespace' do
+ let(:content) { '/assign_reviewer me ' }
+
+ it_behaves_like 'assign_reviewer command'
+ end
+
+ context 'with an incorrect user' do
+ let(:content) { '/assign_reviewer @abcd1234' }
+
+ it_behaves_like 'empty command', "Failed to assign a reviewer because no user was found."
+ end
+
+ context 'with the "reviewer" alias' do
+ let(:content) { "/reviewer @#{developer.username}" }
+
+ it_behaves_like 'assign_reviewer command'
+ end
+
+ context 'with no user' do
+ let(:content) { '/assign_reviewer' }
+
+ it_behaves_like 'empty command', "Failed to assign a reviewer because no user was found."
+ end
+
+ context 'includes only the user reference with extra text' do
+ let(:content) { "/assign_reviewer @#{developer.username} do it!" }
+
+ it_behaves_like 'assign_reviewer command'
+ end
+ end
+
+ describe 'unassign_reviewer command' do
+ # CE does not have multiple reviewers, so basically anything
+ # after /unassign_reviewer (including whitespace) will remove
+ # all the current reviewers.
+ let(:issuable) { create(:merge_request, reviewers: [developer]) }
+ let(:content) { "/unassign_reviewer @#{developer.username}" }
+
+ context 'with one user' do
+ it_behaves_like 'unassign_reviewer command'
+ end
+
+ context 'with an issue instead of a merge request' do
+ let(:issuable) { issue }
+
+ it_behaves_like 'empty command'
+ end
+
+ context 'with anything after the command' do
+ let(:content) { '/unassign_reviewer supercalifragilisticexpialidocious' }
+
+ it_behaves_like 'unassign_reviewer command'
+ end
+
+ context 'with the "remove_reviewer" alias' do
+ let(:content) { "/remove_reviewer @#{developer.username}" }
+
+ it_behaves_like 'unassign_reviewer command'
+ end
+
+ context 'with no user' do
+ let(:content) { '/unassign_reviewer' }
+
+ it_behaves_like 'unassign_reviewer command'
+ end
+ end
+ end
+
+ context 'when the merge_request_reviewers flag is disabled' do
+ before do
+ stub_feature_flags(merge_request_reviewers: false)
+ end
+
+ describe 'assign_reviewer command' do
+ it_behaves_like 'empty command' do
+ let(:content) { "/assign_reviewer @#{developer.username}" }
+ let(:issuable) { merge_request }
+ end
+ end
+
+ describe 'unassign_reviewer command' do
+ it_behaves_like 'empty command' do
+ let(:content) { "/unassign_reviewer @#{developer.username}" }
+ let(:issuable) { merge_request }
+ end
+ end
+ end
+
context 'unassign command' do
let(:content) { '/unassign' }
@@ -1117,6 +1298,11 @@ RSpec.describe QuickActions::InterpretService do
let(:issuable) { issue }
end
+ it_behaves_like 'confidential command' do
+ let(:content) { '/confidential' }
+ let(:issuable) { create(:incident, project: project) }
+ end
+
it_behaves_like 'lock command' do
let(:content) { '/lock' }
let(:issuable) { issue }
@@ -1819,6 +2005,28 @@ RSpec.describe QuickActions::InterpretService do
end
end
+ describe 'unassign_reviewer command' do
+ let(:content) { '/unassign_reviewer' }
+ let(:merge_request) { create(:merge_request, source_project: project, reviewers: [developer]) }
+
+ it 'includes current assignee reference' do
+ _, explanations = service.explain(content, merge_request)
+
+ expect(explanations).to eq(["Removes reviewer @#{developer.username}."])
+ end
+ end
+
+ describe 'assign_reviewer command' do
+ let(:content) { "/assign_reviewer #{developer.to_reference}" }
+ let(:merge_request) { create(:merge_request, source_project: project, assignees: [developer]) }
+
+ it 'includes only the user reference' do
+ _, explanations = service.explain(content, merge_request)
+
+ expect(explanations).to eq(["Assigns #{developer.to_reference} as reviewer."])
+ end
+ end
+
describe 'milestone command' do
let(:content) { '/milestone %wrong-milestone' }
let!(:milestone) { create(:milestone, project: project, title: '9.10') }
diff --git a/spec/services/repositories/housekeeping_service_spec.rb b/spec/services/repositories/housekeeping_service_spec.rb
new file mode 100644
index 00000000000..fbd9affb33c
--- /dev/null
+++ b/spec/services/repositories/housekeeping_service_spec.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Repositories::HousekeepingService do
+ it_behaves_like 'housekeeps repository' do
+ let_it_be(:resource) { create(:project, :repository) }
+ end
+
+ it_behaves_like 'housekeeps repository' do
+ let_it_be(:project) { create(:project, :wiki_repo) }
+ let_it_be(:resource) { project.wiki }
+ end
+end
diff --git a/spec/services/resource_events/change_state_service_spec.rb b/spec/services/resource_events/change_state_service_spec.rb
index 5b5379b241b..255ee9eca57 100644
--- a/spec/services/resource_events/change_state_service_spec.rb
+++ b/spec/services/resource_events/change_state_service_spec.rb
@@ -30,6 +30,15 @@ RSpec.describe ResourceEvents::ChangeStateService do
expect_event_source(event, source)
end
+
+ it "sets the created_at timestamp from the system_note_timestamp" do
+ resource.system_note_timestamp = Time.at(43).utc
+
+ described_class.new(user: user, resource: resource).execute(status: state, mentionable_source: source)
+ event = resource.resource_state_events.last
+
+ expect(event.created_at).to eq(Time.at(43).utc)
+ end
end
end
diff --git a/spec/services/service_desk_settings/update_service_spec.rb b/spec/services/service_desk_settings/update_service_spec.rb
index fbef587365d..72134af1369 100644
--- a/spec/services/service_desk_settings/update_service_spec.rb
+++ b/spec/services/service_desk_settings/update_service_spec.rb
@@ -16,19 +16,6 @@ RSpec.describe ServiceDeskSettings::UpdateService do
expect(settings.reload.outgoing_name).to eq 'some name'
expect(settings.reload.project_key).to eq 'foo'
end
-
- context 'when service_desk_custom_address is disabled' do
- before do
- stub_feature_flags(service_desk_custom_address: false)
- end
-
- it 'ignores project_key parameter' do
- result = described_class.new(settings.project, user, params).execute
-
- expect(result[:status]).to eq :success
- expect(settings.reload.project_key).to be_nil
- end
- end
end
context 'when project_key is an empty string' do
diff --git a/spec/services/snippets/schedule_bulk_repository_shard_moves_service_spec.rb b/spec/services/snippets/schedule_bulk_repository_shard_moves_service_spec.rb
new file mode 100644
index 00000000000..764c7f94a46
--- /dev/null
+++ b/spec/services/snippets/schedule_bulk_repository_shard_moves_service_spec.rb
@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Snippets::ScheduleBulkRepositoryShardMovesService do
+ it_behaves_like 'moves repository shard in bulk' do
+ let_it_be_with_reload(:container) { create(:snippet, :repository) }
+
+ let(:move_service_klass) { SnippetRepositoryStorageMove }
+ let(:bulk_worker_klass) { ::SnippetScheduleBulkRepositoryShardMovesWorker }
+ end
+end
diff --git a/spec/services/snippets/update_repository_storage_service_spec.rb b/spec/services/snippets/update_repository_storage_service_spec.rb
new file mode 100644
index 00000000000..b2bcd620d76
--- /dev/null
+++ b/spec/services/snippets/update_repository_storage_service_spec.rb
@@ -0,0 +1,137 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Snippets::UpdateRepositoryStorageService do
+ include Gitlab::ShellAdapter
+
+ subject { described_class.new(repository_storage_move) }
+
+ describe "#execute" do
+ let_it_be_with_reload(:snippet) { create(:snippet, :repository) }
+ let_it_be(:destination) { 'test_second_storage' }
+ let_it_be(:checksum) { snippet.repository.checksum }
+
+ let(:repository_storage_move_state) { :scheduled }
+ let(:repository_storage_move) { create(:snippet_repository_storage_move, repository_storage_move_state, container: snippet, destination_storage_name: destination) }
+ let(:snippet_repository_double) { double(:repository) }
+ let(:original_snippet_repository_double) { double(:repository) }
+
+ before do
+ allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(%w[default test_second_storage])
+ allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original
+ allow(Gitlab::GitalyClient).to receive(:filesystem_id).with(destination).and_return(SecureRandom.uuid)
+ allow(Gitlab::Git::Repository).to receive(:new).and_call_original
+ allow(Gitlab::Git::Repository).to receive(:new)
+ .with(destination, snippet.repository.raw.relative_path, snippet.repository.gl_repository, snippet.repository.full_path)
+ .and_return(snippet_repository_double)
+ allow(Gitlab::Git::Repository).to receive(:new)
+ .with('default', snippet.repository.raw.relative_path, nil, nil)
+ .and_return(original_snippet_repository_double)
+ end
+
+ context 'when the move succeeds' do
+ it 'moves the repository to the new storage and unmarks the repository as read only' do
+ old_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+ snippet.repository.path_to_repo
+ end
+
+ expect(snippet_repository_double).to receive(:replicate)
+ .with(snippet.repository.raw)
+ expect(snippet_repository_double).to receive(:checksum)
+ .and_return(checksum)
+ expect(original_snippet_repository_double).to receive(:remove)
+
+ result = subject.execute
+ snippet.reload
+
+ expect(result).to be_success
+ expect(snippet).not_to be_repository_read_only
+ expect(snippet.repository_storage).to eq(destination)
+ expect(gitlab_shell.repository_exists?('default', old_path)).to be(false)
+ expect(snippet.snippet_repository.shard_name).to eq(destination)
+ end
+ end
+
+ context 'when the filesystems are the same' do
+ let(:destination) { snippet.repository_storage }
+
+ it 'bails out and does nothing' do
+ result = subject.execute
+
+ expect(result).to be_error
+ expect(result.message).to match(/SameFilesystemError/)
+ end
+ end
+
+ context 'when the move fails' do
+ it 'unmarks the repository as read-only without updating the repository storage' do
+ expect(snippet_repository_double).to receive(:replicate)
+ .with(snippet.repository.raw)
+ .and_raise(Gitlab::Git::CommandError)
+
+ result = subject.execute
+
+ expect(result).to be_error
+ expect(snippet).not_to be_repository_read_only
+ expect(snippet.repository_storage).to eq('default')
+ expect(repository_storage_move).to be_failed
+ end
+ end
+
+ context 'when the cleanup fails' do
+ it 'sets the correct state' do
+ expect(snippet_repository_double).to receive(:replicate)
+ .with(snippet.repository.raw)
+ expect(snippet_repository_double).to receive(:checksum)
+ .and_return(checksum)
+ expect(original_snippet_repository_double).to receive(:remove)
+ .and_raise(Gitlab::Git::CommandError)
+
+ result = subject.execute
+
+ expect(result).to be_error
+ expect(repository_storage_move).to be_cleanup_failed
+ end
+ end
+
+ context 'when the checksum does not match' do
+ it 'unmarks the repository as read-only without updating the repository storage' do
+ expect(snippet_repository_double).to receive(:replicate)
+ .with(snippet.repository.raw)
+ expect(snippet_repository_double).to receive(:checksum)
+ .and_return('not matching checksum')
+
+ result = subject.execute
+
+ expect(result).to be_error
+ expect(snippet).not_to be_repository_read_only
+ expect(snippet.repository_storage).to eq('default')
+ end
+ end
+
+ context 'when the repository move is finished' do
+ let(:repository_storage_move_state) { :finished }
+
+ it 'is idempotent' do
+ expect do
+ result = subject.execute
+
+ expect(result).to be_success
+ end.not_to change(repository_storage_move, :state)
+ end
+ end
+
+ context 'when the repository move is failed' do
+ let(:repository_storage_move_state) { :failed }
+
+ it 'is idempotent' do
+ expect do
+ result = subject.execute
+
+ expect(result).to be_success
+ end.not_to change(repository_storage_move, :state)
+ end
+ end
+ end
+end
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 90325c564bc..83d233a8112 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -100,17 +100,18 @@ RSpec.describe TodoService do
end
describe 'Issues' do
- let(:issue) { create(:issue, project: project, assignees: [john_doe], author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
- let(:addressed_issue) { create(:issue, project: project, assignees: [john_doe], author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") }
+ let(:issue) { create(:issue, project: project, author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
+ let(:addressed_issue) { create(:issue, project: project, author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") }
+ let(:assigned_issue) { create(:issue, project: project, assignees: [john_doe]) }
let(:unassigned_issue) { create(:issue, project: project, assignees: []) }
let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: mentions) }
let(:addressed_confident_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee], description: directly_addressed) }
describe '#new_issue' do
it 'creates a todo if assigned' do
- service.new_issue(issue, author)
+ service.new_issue(assigned_issue, author)
- should_create_todo(user: john_doe, target: issue, action: Todo::ASSIGNED)
+ should_create_todo(user: john_doe, target: assigned_issue, action: Todo::ASSIGNED)
end
it 'does not create a todo if unassigned' do
@@ -130,7 +131,7 @@ RSpec.describe TodoService do
should_create_todo(user: member, target: issue, action: Todo::MENTIONED)
should_create_todo(user: guest, target: issue, action: Todo::MENTIONED)
should_create_todo(user: author, target: issue, action: Todo::MENTIONED)
- should_not_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
+ should_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
end
@@ -140,7 +141,7 @@ RSpec.describe TodoService do
should_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
should_create_todo(user: guest, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
should_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+ should_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
end
@@ -244,6 +245,8 @@ RSpec.describe TodoService do
end
it 'does not create a todo if user was already mentioned and todo is pending' do
+ stub_feature_flags(multiple_todos: false)
+
create(:todo, :mentioned, user: member, project: project, target: issue, author: author)
expect { service.update_issue(issue, author, skip_users) }.not_to change(member.todos, :count)
@@ -256,6 +259,8 @@ RSpec.describe TodoService do
end
it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do
+ stub_feature_flags(multiple_todos: false)
+
create(:todo, :directly_addressed, user: member, project: project, target: addressed_issue, author: author)
expect { service.update_issue(addressed_issue, author, skip_users) }.not_to change(member.todos, :count)
@@ -622,6 +627,26 @@ RSpec.describe TodoService do
expect(service.todo_exist?(unassigned_issue, author)).to be_truthy
end
end
+
+ context 'when multiple_todos are enabled' do
+ before do
+ stub_feature_flags(multiple_todos: true)
+ end
+
+ it 'creates a todo even if user already has a pending todo' do
+ create(:todo, :mentioned, user: member, project: project, target: issue, author: author)
+
+ expect { service.update_issue(issue, author) }.to change(member.todos, :count)
+ end
+
+ it 'creates multiple todos if a user is assigned and mentioned in a new issue' do
+ assigned_issue.description = mentions
+ service.new_issue(assigned_issue, author)
+
+ should_create_todo(user: john_doe, target: assigned_issue, action: Todo::ASSIGNED)
+ should_create_todo(user: john_doe, target: assigned_issue, action: Todo::MENTIONED)
+ end
+ end
end
describe '#reassigned_assignable' do
@@ -664,154 +689,161 @@ RSpec.describe TodoService do
end
describe 'Merge Requests' do
- let(:mr_assigned) { create(:merge_request, source_project: project, author: author, assignees: [john_doe], description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
- let(:addressed_mr_assigned) { create(:merge_request, source_project: project, author: author, assignees: [john_doe], description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") }
- let(:mr_unassigned) { create(:merge_request, source_project: project, author: author, assignees: []) }
+ let(:mentioned_mr) { create(:merge_request, source_project: project, author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
+ let(:addressed_mr) { create(:merge_request, source_project: project, author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") }
+ let(:assigned_mr) { create(:merge_request, source_project: project, author: author, assignees: [john_doe]) }
+ let(:unassigned_mr) { create(:merge_request, source_project: project, author: author, assignees: []) }
describe '#new_merge_request' do
it 'creates a pending todo if assigned' do
- service.new_merge_request(mr_assigned, author)
+ service.new_merge_request(assigned_mr, author)
- should_create_todo(user: john_doe, target: mr_assigned, action: Todo::ASSIGNED)
+ should_create_todo(user: john_doe, target: assigned_mr, action: Todo::ASSIGNED)
end
it 'does not create a todo if unassigned' do
- should_not_create_any_todo { service.new_merge_request(mr_unassigned, author) }
+ should_not_create_any_todo { service.new_merge_request(unassigned_mr, author) }
end
- it 'does not create a todo if assignee is the current user' do
- should_not_create_any_todo { service.new_merge_request(mr_unassigned, john_doe) }
+ it 'creates a todo if assignee is the current user' do
+ service.new_merge_request(assigned_mr, john_doe)
+
+ should_create_todo(user: john_doe, target: assigned_mr, author: john_doe, action: Todo::ASSIGNED)
end
it 'creates a todo for each valid mentioned user' do
- service.new_merge_request(mr_assigned, author)
+ service.new_merge_request(mentioned_mr, author)
- should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
- should_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
+ should_create_todo(user: member, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
+ should_create_todo(user: author, target: mentioned_mr, action: Todo::MENTIONED)
+ should_create_todo(user: john_doe, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: non_member, target: mentioned_mr, action: Todo::MENTIONED)
end
it 'creates a todo for each valid user based on the type of mention' do
- mr_assigned.update!(description: directly_addressed_and_mentioned)
+ mentioned_mr.update!(description: directly_addressed_and_mentioned)
- service.new_merge_request(mr_assigned, author)
+ service.new_merge_request(mentioned_mr, author)
- should_create_todo(user: member, target: mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED)
+ should_create_todo(user: member, target: mentioned_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: admin, target: mentioned_mr, action: Todo::MENTIONED)
end
it 'creates a directly addressed todo for each valid addressed user' do
- service.new_merge_request(addressed_mr_assigned, author)
+ service.new_merge_request(addressed_mr, author)
- should_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+ should_create_todo(user: member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: guest, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_create_todo(user: author, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_create_todo(user: john_doe, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: non_member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
end
end
describe '#update_merge_request' do
it 'creates a todo for each valid mentioned user not included in skip_users' do
- service.update_merge_request(mr_assigned, author, skip_users)
+ service.update_merge_request(mentioned_mr, author, skip_users)
- should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
- should_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
- should_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: skipped, target: mr_assigned, action: Todo::MENTIONED)
+ should_create_todo(user: member, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
+ should_create_todo(user: john_doe, target: mentioned_mr, action: Todo::MENTIONED)
+ should_create_todo(user: author, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: non_member, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: skipped, target: mentioned_mr, action: Todo::MENTIONED)
end
it 'creates a todo for each valid user not included in skip_users based on the type of mention' do
- mr_assigned.update!(description: directly_addressed_and_mentioned)
+ mentioned_mr.update!(description: directly_addressed_and_mentioned)
- service.update_merge_request(mr_assigned, author, skip_users)
+ service.update_merge_request(mentioned_mr, author, skip_users)
- should_create_todo(user: member, target: mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: skipped, target: mr_assigned)
+ should_create_todo(user: member, target: mentioned_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: admin, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: skipped, target: mentioned_mr)
end
it 'creates a directly addressed todo for each valid addressed user not included in skip_users' do
- service.update_merge_request(addressed_mr_assigned, author, skip_users)
+ service.update_merge_request(addressed_mr, author, skip_users)
- should_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: skipped, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+ should_create_todo(user: member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: guest, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_create_todo(user: john_doe, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_create_todo(user: author, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: non_member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: skipped, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
end
it 'does not create a todo if user was already mentioned and todo is pending' do
- create(:todo, :mentioned, user: member, project: project, target: mr_assigned, author: author)
+ stub_feature_flags(multiple_todos: false)
+
+ create(:todo, :mentioned, user: member, project: project, target: mentioned_mr, author: author)
- expect { service.update_merge_request(mr_assigned, author) }.not_to change(member.todos, :count)
+ expect { service.update_merge_request(mentioned_mr, author) }.not_to change(member.todos, :count)
end
it 'does not create a todo if user was already mentioned and todo is done' do
- create(:todo, :mentioned, :done, user: skipped, project: project, target: mr_assigned, author: author)
+ create(:todo, :mentioned, :done, user: skipped, project: project, target: mentioned_mr, author: author)
- expect { service.update_merge_request(mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count)
+ expect { service.update_merge_request(mentioned_mr, author, skip_users) }.not_to change(skipped.todos, :count)
end
it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do
- create(:todo, :directly_addressed, user: member, project: project, target: addressed_mr_assigned, author: author)
+ stub_feature_flags(multiple_todos: false)
+
+ create(:todo, :directly_addressed, user: member, project: project, target: addressed_mr, author: author)
- expect { service.update_merge_request(addressed_mr_assigned, author) }.not_to change(member.todos, :count)
+ expect { service.update_merge_request(addressed_mr, author) }.not_to change(member.todos, :count)
end
it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is done' do
- create(:todo, :directly_addressed, user: skipped, project: project, target: addressed_mr_assigned, author: author)
+ create(:todo, :directly_addressed, user: skipped, project: project, target: addressed_mr, author: author)
- expect { service.update_merge_request(addressed_mr_assigned, author, skip_users) }.not_to change(skipped.todos, :count)
+ expect { service.update_merge_request(addressed_mr, author, skip_users) }.not_to change(skipped.todos, :count)
end
context 'with a task list' do
it 'does not create todo when tasks are marked as completed' do
- mr_assigned.update!(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
+ mentioned_mr.update!(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
- service.update_merge_request(mr_assigned, author)
+ service.update_merge_request(mentioned_mr, author)
- should_not_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: assignee, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
- should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
+ should_not_create_todo(user: admin, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: assignee, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: author, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: john_doe, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: member, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: non_member, target: mentioned_mr, action: Todo::MENTIONED)
+ should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
end
it 'does not create directly addressed todo when tasks are marked as completed' do
- addressed_mr_assigned.update!(description: "#{directly_addressed}\n- [x] Task 1\n- [X] Task 2")
+ addressed_mr.update!(description: "#{directly_addressed}\n- [x] Task 1\n- [X] Task 2")
- service.update_merge_request(addressed_mr_assigned, author)
+ service.update_merge_request(addressed_mr, author)
- should_not_create_todo(user: admin, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: assignee, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
- should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: admin, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: assignee, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: author, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: john_doe, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: non_member, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
+ should_not_create_todo(user: guest, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
end
it 'does not raise an error when description not change' do
- mr_assigned.update!(title: 'Sample')
+ mentioned_mr.update!(title: 'Sample')
- expect { service.update_merge_request(mr_assigned, author) }.not_to raise_error
+ expect { service.update_merge_request(mentioned_mr, author) }.not_to raise_error
end
end
end
describe '#close_merge_request' do
it 'marks related pending todos to the target for the user as done' do
- first_todo = create(:todo, :assigned, user: john_doe, project: project, target: mr_assigned, author: author)
- second_todo = create(:todo, :assigned, user: john_doe, project: project, target: mr_assigned, author: author)
- service.close_merge_request(mr_assigned, john_doe)
+ first_todo = create(:todo, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author)
+ second_todo = create(:todo, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author)
+ service.close_merge_request(mentioned_mr, john_doe)
expect(first_todo.reload).to be_done
expect(second_todo.reload).to be_done
@@ -820,55 +852,55 @@ RSpec.describe TodoService do
describe '#merge_merge_request' do
it 'marks related pending todos to the target for the user as done' do
- first_todo = create(:todo, :assigned, user: john_doe, project: project, target: mr_assigned, author: author)
- second_todo = create(:todo, :assigned, user: john_doe, project: project, target: mr_assigned, author: author)
- service.merge_merge_request(mr_assigned, john_doe)
+ first_todo = create(:todo, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author)
+ second_todo = create(:todo, :assigned, user: john_doe, project: project, target: mentioned_mr, author: author)
+ service.merge_merge_request(mentioned_mr, john_doe)
expect(first_todo.reload).to be_done
expect(second_todo.reload).to be_done
end
it 'does not create todo for guests' do
- service.merge_merge_request(mr_assigned, john_doe)
- should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
+ service.merge_merge_request(mentioned_mr, john_doe)
+ should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
end
it 'does not create directly addressed todo for guests' do
- service.merge_merge_request(addressed_mr_assigned, john_doe)
- should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+ service.merge_merge_request(addressed_mr, john_doe)
+ should_not_create_todo(user: guest, target: addressed_mr, action: Todo::DIRECTLY_ADDRESSED)
end
end
describe '#new_award_emoji' do
it 'marks related pending todos to the target for the user as done' do
- todo = create(:todo, user: john_doe, project: project, target: mr_assigned, author: author)
- service.new_award_emoji(mr_assigned, john_doe)
+ todo = create(:todo, user: john_doe, project: project, target: mentioned_mr, author: author)
+ service.new_award_emoji(mentioned_mr, john_doe)
expect(todo.reload).to be_done
end
end
describe '#merge_request_build_failed' do
- let(:merge_participants) { [mr_unassigned.author, admin] }
+ let(:merge_participants) { [unassigned_mr.author, admin] }
before do
- allow(mr_unassigned).to receive(:merge_participants).and_return(merge_participants)
+ allow(unassigned_mr).to receive(:merge_participants).and_return(merge_participants)
end
it 'creates a pending todo for each merge_participant' do
- service.merge_request_build_failed(mr_unassigned)
+ service.merge_request_build_failed(unassigned_mr)
merge_participants.each do |participant|
- should_create_todo(user: participant, author: participant, target: mr_unassigned, action: Todo::BUILD_FAILED)
+ should_create_todo(user: participant, author: participant, target: unassigned_mr, action: Todo::BUILD_FAILED)
end
end
end
describe '#merge_request_push' do
it 'marks related pending todos to the target for the user as done' do
- first_todo = create(:todo, :build_failed, user: author, project: project, target: mr_assigned, author: john_doe)
- second_todo = create(:todo, :build_failed, user: john_doe, project: project, target: mr_assigned, author: john_doe)
- service.merge_request_push(mr_assigned, author)
+ first_todo = create(:todo, :build_failed, user: author, project: project, target: mentioned_mr, author: john_doe)
+ second_todo = create(:todo, :build_failed, user: john_doe, project: project, target: mentioned_mr, author: john_doe)
+ service.merge_request_push(mentioned_mr, author)
expect(first_todo.reload).to be_done
expect(second_todo.reload).not_to be_done
@@ -879,24 +911,24 @@ RSpec.describe TodoService do
let(:merge_participants) { [admin, create(:user)] }
before do
- allow(mr_unassigned).to receive(:merge_participants).and_return(merge_participants)
+ allow(unassigned_mr).to receive(:merge_participants).and_return(merge_participants)
end
it 'creates a pending todo for each merge_participant' do
- mr_unassigned.update!(merge_when_pipeline_succeeds: true, merge_user: admin)
- service.merge_request_became_unmergeable(mr_unassigned)
+ unassigned_mr.update!(merge_when_pipeline_succeeds: true, merge_user: admin)
+ service.merge_request_became_unmergeable(unassigned_mr)
merge_participants.each do |participant|
- should_create_todo(user: participant, author: participant, target: mr_unassigned, action: Todo::UNMERGEABLE)
+ should_create_todo(user: participant, author: participant, target: unassigned_mr, action: Todo::UNMERGEABLE)
end
end
end
describe '#mark_todo' do
it 'creates a todo from a merge request' do
- service.mark_todo(mr_unassigned, author)
+ service.mark_todo(unassigned_mr, author)
- should_create_todo(user: author, target: mr_unassigned, action: Todo::MARKED)
+ should_create_todo(user: author, target: unassigned_mr, action: Todo::MARKED)
end
end
@@ -913,33 +945,33 @@ RSpec.describe TodoService do
end
let(:mention) { john_doe.to_reference }
- let(:diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "Hey #{mention}") }
- let(:addressed_diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "#{mention}, hey!") }
- let(:legacy_diff_note_on_merge_request) { create(:legacy_diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "Hey #{mention}") }
+ let(:diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: unassigned_mr, author: author, note: "Hey #{mention}") }
+ let(:addressed_diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: unassigned_mr, author: author, note: "#{mention}, hey!") }
+ let(:legacy_diff_note_on_merge_request) { create(:legacy_diff_note_on_merge_request, project: project, noteable: unassigned_mr, author: author, note: "Hey #{mention}") }
it 'creates a todo for mentioned user on new diff note' do
service.new_note(diff_note_on_merge_request, author)
- should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::MENTIONED, note: diff_note_on_merge_request)
+ should_create_todo(user: john_doe, target: unassigned_mr, author: author, action: Todo::MENTIONED, note: diff_note_on_merge_request)
end
it 'creates a directly addressed todo for addressed user on new diff note' do
service.new_note(addressed_diff_note_on_merge_request, author)
- should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::DIRECTLY_ADDRESSED, note: addressed_diff_note_on_merge_request)
+ should_create_todo(user: john_doe, target: unassigned_mr, author: author, action: Todo::DIRECTLY_ADDRESSED, note: addressed_diff_note_on_merge_request)
end
it 'creates a todo for mentioned user on legacy diff note' do
service.new_note(legacy_diff_note_on_merge_request, author)
- should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::MENTIONED, note: legacy_diff_note_on_merge_request)
+ should_create_todo(user: john_doe, target: unassigned_mr, author: author, action: Todo::MENTIONED, note: legacy_diff_note_on_merge_request)
end
it 'does not create todo for guests' do
- note_on_merge_request = create :note_on_merge_request, project: project, noteable: mr_assigned, note: mentions
+ note_on_merge_request = create :note_on_merge_request, project: project, noteable: mentioned_mr, note: mentions
service.new_note(note_on_merge_request, author)
- should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
+ should_not_create_todo(user: guest, target: mentioned_mr, action: Todo::MENTIONED)
end
end
end
@@ -1013,6 +1045,8 @@ RSpec.describe TodoService do
end
it 'does not create a todo if user was already mentioned and todo is pending' do
+ stub_feature_flags(multiple_todos: false)
+
create(:todo, :mentioned, user: member, project: project, target: noteable, author: author)
expect { service.update_note(note, author, skip_users) }.not_to change(member.todos, :count)
@@ -1025,6 +1059,8 @@ RSpec.describe TodoService do
end
it 'does not create a directly addressed todo if user was already mentioned or addressed and todo is pending' do
+ stub_feature_flags(multiple_todos: false)
+
create(:todo, :directly_addressed, user: member, project: project, target: noteable, author: author)
expect { service.update_note(addressed_note, author, skip_users) }.not_to change(member.todos, :count)
@@ -1038,7 +1074,7 @@ RSpec.describe TodoService do
end
it 'updates cached counts when a todo is created' do
- issue = create(:issue, project: project, assignees: [john_doe], author: author, description: mentions)
+ issue = create(:issue, project: project, assignees: [john_doe], author: author)
expect(john_doe.todos_pending_count).to eq(0)
expect(john_doe).to receive(:update_todos_count_cache).and_call_original
diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb
index 274c44394f3..b30b7e6eb56 100644
--- a/spec/services/users/update_service_spec.rb
+++ b/spec/services/users/update_service_spec.rb
@@ -31,7 +31,7 @@ RSpec.describe Users::UpdateService do
result = update_user(user, { username: 'taken' })
end.not_to change { user.reload.username }
expect(result[:status]).to eq(:error)
- expect(result[:message]).to eq('Username has already been taken')
+ expect(result[:message]).to eq('A user, alias, or group already exists with that username.')
end
it 'updates the status if status params were given' do
diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb
index a607a6734b0..2fe72ab31c2 100644
--- a/spec/services/web_hook_service_spec.rb
+++ b/spec/services/web_hook_service_spec.rb
@@ -131,6 +131,15 @@ RSpec.describe WebHookService do
end
end
+ context 'when url is not encoded' do
+ let(:project_hook) { create(:project_hook, url: 'http://server.com/my path/') }
+
+ it 'handles exceptions' do
+ expect(service_instance.execute).to eq(status: :error, message: 'bad URI(is not URI?): "http://server.com/my path/"')
+ expect { service_instance.execute }.not_to raise_error
+ end
+ end
+
context 'when request body size is too big' do
it 'does not perform the request' do
stub_const("#{described_class}::REQUEST_BODY_SIZE_LIMIT", 10.bytes)