diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-05 13:54:15 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-05 13:54:15 +0000 |
| commit | be834a25982746ffd85252ff502df42bb88cb9d5 (patch) | |
| tree | b4d6a8ba0931e12fac08f05abea33a3b8ec2c8a2 /spec/models | |
| parent | ee925a3597f27e92f83a50937a64068109675b3d (diff) | |
| download | gitlab-ce-6e86631ace2b2f57c5e24fc472c890ff4aa7f751.tar.gz | |
Add latest changes from gitlab-org/gitlab@13-5-stable-eev13.5.0-rc32
Diffstat (limited to 'spec/models')
73 files changed, 1806 insertions, 791 deletions
diff --git a/spec/models/alert_management/http_integration_spec.rb b/spec/models/alert_management/http_integration_spec.rb new file mode 100644 index 00000000000..37d67dfe09a --- /dev/null +++ b/spec/models/alert_management/http_integration_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe AlertManagement::HttpIntegration do + let_it_be(:project) { create(:project) } + + subject(:integration) { build(:alert_management_http_integration) } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + it { is_expected.to validate_presence_of(:endpoint_identifier) } + it { is_expected.to validate_length_of(:endpoint_identifier).is_at_most(255) } + + context 'when active' do + # Using `create` instead of `build` the integration so `token` is set. + # Uniqueness spec saves integration with `validate: false` otherwise. + subject { create(:alert_management_http_integration) } + + it { is_expected.to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id, :active) } + end + + context 'when inactive' do + subject { create(:alert_management_http_integration, :inactive) } + + it { is_expected.not_to validate_uniqueness_of(:endpoint_identifier).scoped_to(:project_id, :active) } + end + end + + describe '#token' do + subject { integration.token } + + shared_context 'assign token' do |token| + let!(:previous_token) { integration.token } + + before do + integration.token = token + integration.valid? + end + end + + shared_examples 'valid token' do + it { is_expected.to match(/\A\h{32}\z/) } + end + + context 'when unsaved' do + context 'when unassigned' do + before do + integration.valid? + end + + it_behaves_like 'valid token' + end + + context 'when assigned' do + include_context 'assign token', 'random_token' + + it_behaves_like 'valid token' + it { is_expected.not_to eq('random_token') } + end + end + + context 'when persisted' do + before do + integration.save! + integration.reload + end + + it_behaves_like 'valid token' + + context 'when resetting' do + include_context 'assign token', '' + + it_behaves_like 'valid token' + it { is_expected.not_to eq(previous_token) } + end + + context 'when reassigning' do + include_context 'assign token', 'random_token' + + it_behaves_like 'valid token' + it { is_expected.to eq(previous_token) } + end + end + end +end diff --git a/spec/models/analytics/instance_statistics/measurement_spec.rb b/spec/models/analytics/instance_statistics/measurement_spec.rb index 4df847ea524..379272cfcb9 100644 --- a/spec/models/analytics/instance_statistics/measurement_spec.rb +++ b/spec/models/analytics/instance_statistics/measurement_spec.rb @@ -20,7 +20,11 @@ RSpec.describe Analytics::InstanceStatistics::Measurement, type: :model do issues: 3, merge_requests: 4, groups: 5, - pipelines: 6 + pipelines: 6, + pipelines_succeeded: 7, + pipelines_failed: 8, + pipelines_canceled: 9, + pipelines_skipped: 10 }.with_indifferent_access) end end @@ -42,4 +46,28 @@ RSpec.describe Analytics::InstanceStatistics::Measurement, type: :model do it { is_expected.to match_array([measurement_1, measurement_2]) } end end + + describe '#measurement_identifier_values' do + subject { described_class.measurement_identifier_values.count } + + context 'when the `store_ci_pipeline_counts_by_status` feature flag is off' do + let(:expected_count) { Analytics::InstanceStatistics::Measurement.identifiers.size - Analytics::InstanceStatistics::Measurement::EXPERIMENTAL_IDENTIFIERS.size } + + before do + stub_feature_flags(store_ci_pipeline_counts_by_status: false) + end + + it { is_expected.to eq(expected_count) } + end + + context 'when the `store_ci_pipeline_counts_by_status` feature flag is on' do + let(:expected_count) { Analytics::InstanceStatistics::Measurement.identifiers.size } + + before do + stub_feature_flags(store_ci_pipeline_counts_by_status: true) + end + + it { is_expected.to eq(expected_count) } + end + end end diff --git a/spec/models/application_setting/term_spec.rb b/spec/models/application_setting/term_spec.rb index 82347453437..51a6027698f 100644 --- a/spec/models/application_setting/term_spec.rb +++ b/spec/models/application_setting/term_spec.rb @@ -17,6 +17,7 @@ RSpec.describe ApplicationSetting::Term do describe '#accepted_by_user?' do let(:user) { create(:user) } + let(:project_bot) { create(:user, :project_bot) } let(:term) { create(:term) } it 'is true when the user accepted the terms' do @@ -25,6 +26,10 @@ RSpec.describe ApplicationSetting::Term do expect(term.accepted_by_user?(user)).to be(true) end + it 'is true when user is a bot' do + expect(term.accepted_by_user?(project_bot)).to be(true) + end + it 'is false when the user declined the terms' do decline_terms(term, user) diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 9f76fb3330d..f572e6ffade 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -320,7 +320,7 @@ RSpec.describe ApplicationSetting do end end - it_behaves_like 'an object with email-formated attributes', :admin_notification_email do + it_behaves_like 'an object with email-formated attributes', :abuse_notification_email do subject { setting } end diff --git a/spec/models/audit_event_spec.rb b/spec/models/audit_event_spec.rb index a1ed48c57f4..5c87c2e68db 100644 --- a/spec/models/audit_event_spec.rb +++ b/spec/models/audit_event_spec.rb @@ -6,6 +6,13 @@ RSpec.describe AuditEvent do let_it_be(:audit_event) { create(:project_audit_event) } subject { audit_event } + describe 'validations' do + include_examples 'validates IP address' do + let(:attribute) { :ip_address } + let(:object) { create(:audit_event) } + end + end + describe '#as_json' do context 'ip_address' do subject { build(:group_audit_event, ip_address: '192.168.1.1').as_json } diff --git a/spec/models/authentication_event_spec.rb b/spec/models/authentication_event_spec.rb index 56b0111f2c7..483d45c08be 100644 --- a/spec/models/authentication_event_spec.rb +++ b/spec/models/authentication_event_spec.rb @@ -11,5 +11,41 @@ RSpec.describe AuthenticationEvent do it { is_expected.to validate_presence_of(:provider) } it { is_expected.to validate_presence_of(:user_name) } it { is_expected.to validate_presence_of(:result) } + + include_examples 'validates IP address' do + let(:attribute) { :ip_address } + let(:object) { create(:authentication_event) } + end + end + + describe 'scopes' do + let_it_be(:ldap_event) { create(:authentication_event, provider: :ldapmain, result: :failed) } + let_it_be(:google_oauth2) { create(:authentication_event, provider: :google_oauth2, result: :success) } + + describe '.for_provider' do + it 'returns events only for the specified provider' do + expect(described_class.for_provider(:ldapmain)).to match_array ldap_event + end + end + + describe '.ldap' do + it 'returns all events for an LDAP provider' do + expect(described_class.ldap).to match_array ldap_event + end + end + end + + describe '.providers' do + before do + create(:authentication_event, provider: :ldapmain) + create(:authentication_event, provider: :google_oauth2) + create(:authentication_event, provider: :standard) + create(:authentication_event, provider: :standard) + create(:authentication_event, provider: :standard) + end + + it 'returns an array of distinct providers' do + expect(described_class.providers).to match_array %w(ldapmain google_oauth2 standard) + end end end diff --git a/spec/models/ci/build_pending_state_spec.rb b/spec/models/ci/build_pending_state_spec.rb new file mode 100644 index 00000000000..a546d2aff65 --- /dev/null +++ b/spec/models/ci/build_pending_state_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::BuildPendingState do + describe '#crc32' do + context 'when checksum does not exist' do + let(:pending_state) do + build(:ci_build_pending_state, trace_checksum: nil) + end + + it 'returns nil' do + expect(pending_state.crc32).to be_nil + end + end + + context 'when checksum is in hexadecimal' do + let(:pending_state) do + build(:ci_build_pending_state, trace_checksum: 'crc32:75bcd15') + end + + it 'returns decimal representation of the checksum' do + expect(pending_state.crc32).to eq 123456789 + end + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 1e551d9ee33..cb29cbcbb72 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -4652,4 +4652,24 @@ RSpec.describe Ci::Build do it { is_expected.to be_nil } end end + + describe '#run_on_status_commit' do + it 'runs provided hook after status commit' do + action = spy('action') + + build.run_on_status_commit { action.perform! } + build.success! + + expect(action).to have_received(:perform!).once + end + + it 'does not run hooks when status has not changed' do + action = spy('action') + + build.run_on_status_commit { action.perform! } + build.save! + + expect(action).not_to have_received(:perform!) + end + end end diff --git a/spec/models/ci/build_trace_chunk_spec.rb b/spec/models/ci/build_trace_chunk_spec.rb index fefe5e3bfca..57e58fe494f 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -779,4 +779,62 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do it_behaves_like 'deletes all build_trace_chunk and data in redis' end end + + describe 'comparable build trace chunks' do + describe '#<=>' do + context 'when chunks are associated with different builds' do + let(:first) { create(:ci_build_trace_chunk, build: build, chunk_index: 1) } + let(:second) { create(:ci_build_trace_chunk, chunk_index: 1) } + + it 'returns nil' do + expect(first <=> second).to be_nil + end + end + + context 'when there are two chunks with different indexes' do + let(:first) { create(:ci_build_trace_chunk, build: build, chunk_index: 1) } + let(:second) { create(:ci_build_trace_chunk, build: build, chunk_index: 0) } + + it 'indicates the the first one is greater than then second' do + expect(first <=> second).to eq 1 + end + end + + context 'when there are two chunks with the same index within the same build' do + let(:chunk) { create(:ci_build_trace_chunk) } + + it 'indicates the these are equal' do + expect(chunk <=> chunk).to be_zero # rubocop:disable Lint/UselessComparison + end + end + end + + describe '#==' do + context 'when chunks have the same index' do + let(:chunk) { create(:ci_build_trace_chunk) } + + it 'indicates that the chunks are equal' do + expect(chunk).to eq chunk + end + end + + context 'when chunks have different indexes' do + let(:first) { create(:ci_build_trace_chunk, build: build, chunk_index: 1) } + let(:second) { create(:ci_build_trace_chunk, build: build, chunk_index: 0) } + + it 'indicates that the chunks are not equal' do + expect(first).not_to eq second + end + end + + context 'when chunks are associated with different builds' do + let(:first) { create(:ci_build_trace_chunk, build: build, chunk_index: 1) } + let(:second) { create(:ci_build_trace_chunk, chunk_index: 1) } + + it 'indicates that the chunks are not equal' do + expect(first).not_to eq second + end + end + end + end end diff --git a/spec/models/ci/freeze_period_status_spec.rb b/spec/models/ci/freeze_period_status_spec.rb index 831895cb528..f51381f7a5f 100644 --- a/spec/models/ci/freeze_period_status_spec.rb +++ b/spec/models/ci/freeze_period_status_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Ci::FreezePeriodStatus do shared_examples 'within freeze period' do |time| it 'is frozen' do - Timecop.freeze(time) do + travel_to(time) do expect(subject).to be_truthy end end @@ -19,7 +19,7 @@ RSpec.describe Ci::FreezePeriodStatus do shared_examples 'outside freeze period' do |time| it 'is not frozen' do - Timecop.freeze(time) do + travel_to(time) do expect(subject).to be_falsy end end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 779839df670..26851c93ac3 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Ci::JobArtifact do it_behaves_like 'having unique enum values' - it_behaves_like 'UpdateProjectStatistics' do + it_behaves_like 'UpdateProjectStatistics', :with_counter_attribute do let_it_be(:job, reload: true) { create(:ci_build) } subject { build(:ci_job_artifact, :archive, job: job, size: 107464) } @@ -44,7 +44,7 @@ RSpec.describe Ci::JobArtifact do let!(:metrics_report) { create(:ci_job_artifact, :junit) } let!(:codequality_report) { create(:ci_job_artifact, :codequality) } - it { is_expected.to eq([metrics_report, codequality_report]) } + it { is_expected.to match_array([metrics_report, codequality_report]) } end end diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 949d5f7bd04..cec3b544e50 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -56,7 +56,7 @@ RSpec.describe Ci::PipelineSchedule do subject { described_class.runnable_schedules } let!(:pipeline_schedule) do - Timecop.freeze(1.day.ago) do + travel_to(1.day.ago) do create(:ci_pipeline_schedule, :hourly) end end @@ -118,7 +118,7 @@ RSpec.describe Ci::PipelineSchedule do let(:pipeline_schedule) { create(:ci_pipeline_schedule, :every_minute) } it "updates next_run_at to the sidekiq worker's execution time" do - Timecop.freeze(Time.zone.parse("2019-06-01 12:18:00+0000")) do + travel_to(Time.zone.parse("2019-06-01 12:18:00+0000")) do expect(pipeline_schedule.next_run_at).to eq(cron_worker_next_run_at) end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 228a1e8f7a2..d33ccf0e6f2 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2436,7 +2436,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#retry_failed' do - let(:latest_status) { pipeline.statuses.latest.pluck(:status) } + let(:latest_status) { pipeline.latest_statuses.pluck(:status) } before do stub_not_protect_default_branch @@ -3628,6 +3628,16 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do expect(builds).to include(rspec, jest) expect(builds).not_to include(karma) end + + it 'returns only latest builds' do + obsolete = create(:ci_build, name: "jest", coverage: 10.12, pipeline: pipeline, retried: true) + retried = create(:ci_build, name: "jest", coverage: 20.11, pipeline: pipeline) + + builds = pipeline.builds_with_coverage + + expect(builds).to include(retried) + expect(builds).not_to include(obsolete) + end end describe '#base_and_ancestors' do diff --git a/spec/models/ci_platform_metric_spec.rb b/spec/models/ci_platform_metric_spec.rb index 0b00875df43..f73db713791 100644 --- a/spec/models/ci_platform_metric_spec.rb +++ b/spec/models/ci_platform_metric_spec.rb @@ -45,7 +45,7 @@ RSpec.describe CiPlatformMetric do let(:tomorrow) { today + 1.day } it 'inserts platform target counts for that day' do - Timecop.freeze(today) do + travel_to(today) do create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'ECS') create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'ECS') create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'FARGATE') @@ -53,7 +53,7 @@ RSpec.describe CiPlatformMetric do create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'FARGATE') described_class.insert_auto_devops_platform_targets! end - Timecop.freeze(tomorrow) do + travel_to(tomorrow) do create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'FARGATE') described_class.insert_auto_devops_platform_targets! end @@ -69,7 +69,7 @@ RSpec.describe CiPlatformMetric do let(:today) { Time.zone.local(1982, 4, 24) } it 'ignores those values' do - Timecop.freeze(today) do + travel_to(today) do create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'ECS') create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'FOO') create(:ci_variable, key: described_class::CI_VARIABLE_KEY, value: 'BAR') diff --git a/spec/models/clusters/applications/runner_spec.rb b/spec/models/clusters/applications/runner_spec.rb index fbabfd25b2f..ef916c73e0b 100644 --- a/spec/models/clusters/applications/runner_spec.rb +++ b/spec/models/clusters/applications/runner_spec.rb @@ -69,8 +69,8 @@ RSpec.describe Clusters::Applications::Runner do expect(values).to include('privileged: true') expect(values).to include('image: ubuntu:16.04') expect(values).to include('resources') - expect(values).to match(/runnerToken: '?#{Regexp.escape(ci_runner.token)}/) - expect(values).to match(/gitlabUrl: '?#{Regexp.escape(Gitlab::Routing.url_helpers.root_url)}/) + expect(values).to match(/runnerToken: ['"]?#{Regexp.escape(ci_runner.token)}/) + expect(values).to match(/gitlabUrl: ['"]?#{Regexp.escape(Gitlab::Routing.url_helpers.root_url)}/) end context 'without a runner' do diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 024539e34ec..dd9b96f39ad 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -47,6 +47,7 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to delegate_method(:external_hostname).to(:application_ingress).with_prefix } it { is_expected.to respond_to :project } + it { is_expected.to be_namespace_per_environment } describe 'applications have inverse_of: :cluster option' do let(:cluster) { create(:cluster) } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 6e23f95af03..877188097fd 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -493,47 +493,104 @@ RSpec.describe CommitStatus do end end - describe '#group_name' do - let(:commit_status) do - build(:commit_status, pipeline: pipeline, stage: 'test') - end - - subject { commit_status.group_name } + context 'with the one_dimensional_matrix feature flag disabled' do + describe '#group_name' do + before do + stub_feature_flags(one_dimensional_matrix: false) + end - tests = { - 'rspec:windows' => 'rspec:windows', - 'rspec:windows 0' => 'rspec:windows 0', - 'rspec:windows 0 test' => 'rspec:windows 0 test', - 'rspec:windows 0 1' => 'rspec:windows', - 'rspec:windows 0 1 name' => 'rspec:windows name', - 'rspec:windows 0/1' => 'rspec:windows', - 'rspec:windows 0/1 name' => 'rspec:windows name', - 'rspec:windows 0:1' => 'rspec:windows', - 'rspec:windows 0:1 name' => 'rspec:windows name', - 'rspec:windows 10000 20000' => 'rspec:windows', - 'rspec:windows 0 : / 1' => 'rspec:windows', - 'rspec:windows 0 : / 1 name' => 'rspec:windows name', - '0 1 name ruby' => 'name ruby', - '0 :/ 1 name ruby' => 'name ruby', - 'rspec: [aws]' => 'rspec: [aws]', - 'rspec: [aws] 0/1' => 'rspec: [aws]', - 'rspec: [aws, max memory]' => 'rspec', - 'rspec:linux: [aws, max memory, data]' => 'rspec:linux', - 'rspec: [inception: [something, other thing], value]' => 'rspec', - 'rspec:windows 0/1: [name, other]' => 'rspec:windows', - 'rspec:windows: [name, other] 0/1' => 'rspec:windows', - 'rspec:windows: [name, 0/1] 0/1' => 'rspec:windows', - 'rspec:windows: [0/1, name]' => 'rspec:windows', - 'rspec:windows: [, ]' => 'rspec:windows', - 'rspec:windows: [name]' => 'rspec:windows: [name]', - 'rspec:windows: [name,other]' => 'rspec:windows: [name,other]' - } + let(:commit_status) do + build(:commit_status, pipeline: pipeline, stage: 'test') + end + + subject { commit_status.group_name } + + tests = { + 'rspec:windows' => 'rspec:windows', + 'rspec:windows 0' => 'rspec:windows 0', + 'rspec:windows 0 test' => 'rspec:windows 0 test', + 'rspec:windows 0 1' => 'rspec:windows', + 'rspec:windows 0 1 name' => 'rspec:windows name', + 'rspec:windows 0/1' => 'rspec:windows', + 'rspec:windows 0/1 name' => 'rspec:windows name', + 'rspec:windows 0:1' => 'rspec:windows', + 'rspec:windows 0:1 name' => 'rspec:windows name', + 'rspec:windows 10000 20000' => 'rspec:windows', + 'rspec:windows 0 : / 1' => 'rspec:windows', + 'rspec:windows 0 : / 1 name' => 'rspec:windows name', + '0 1 name ruby' => 'name ruby', + '0 :/ 1 name ruby' => 'name ruby', + 'rspec: [aws]' => 'rspec: [aws]', + 'rspec: [aws] 0/1' => 'rspec: [aws]', + 'rspec: [aws, max memory]' => 'rspec', + 'rspec:linux: [aws, max memory, data]' => 'rspec:linux', + 'rspec: [inception: [something, other thing], value]' => 'rspec', + 'rspec:windows 0/1: [name, other]' => 'rspec:windows', + 'rspec:windows: [name, other] 0/1' => 'rspec:windows', + 'rspec:windows: [name, 0/1] 0/1' => 'rspec:windows', + 'rspec:windows: [0/1, name]' => 'rspec:windows', + 'rspec:windows: [, ]' => 'rspec:windows', + 'rspec:windows: [name]' => 'rspec:windows: [name]', + 'rspec:windows: [name,other]' => 'rspec:windows: [name,other]' + } + + tests.each do |name, group_name| + it "'#{name}' puts in '#{group_name}'" do + commit_status.name = name + + is_expected.to eq(group_name) + end + end + end + end - tests.each do |name, group_name| - it "'#{name}' puts in '#{group_name}'" do - commit_status.name = name + context 'with one_dimensional_matrix feature flag enabled' do + describe '#group_name' do + before do + stub_feature_flags(one_dimensional_matrix: true) + end - is_expected.to eq(group_name) + let(:commit_status) do + build(:commit_status, pipeline: pipeline, stage: 'test') + end + + subject { commit_status.group_name } + + tests = { + 'rspec:windows' => 'rspec:windows', + 'rspec:windows 0' => 'rspec:windows 0', + 'rspec:windows 0 test' => 'rspec:windows 0 test', + 'rspec:windows 0 1' => 'rspec:windows', + 'rspec:windows 0 1 name' => 'rspec:windows name', + 'rspec:windows 0/1' => 'rspec:windows', + 'rspec:windows 0/1 name' => 'rspec:windows name', + 'rspec:windows 0:1' => 'rspec:windows', + 'rspec:windows 0:1 name' => 'rspec:windows name', + 'rspec:windows 10000 20000' => 'rspec:windows', + 'rspec:windows 0 : / 1' => 'rspec:windows', + 'rspec:windows 0 : / 1 name' => 'rspec:windows name', + '0 1 name ruby' => 'name ruby', + '0 :/ 1 name ruby' => 'name ruby', + 'rspec: [aws]' => 'rspec', + 'rspec: [aws] 0/1' => 'rspec', + 'rspec: [aws, max memory]' => 'rspec', + 'rspec:linux: [aws, max memory, data]' => 'rspec:linux', + 'rspec: [inception: [something, other thing], value]' => 'rspec', + 'rspec:windows 0/1: [name, other]' => 'rspec:windows', + 'rspec:windows: [name, other] 0/1' => 'rspec:windows', + 'rspec:windows: [name, 0/1] 0/1' => 'rspec:windows', + 'rspec:windows: [0/1, name]' => 'rspec:windows', + 'rspec:windows: [, ]' => 'rspec:windows', + 'rspec:windows: [name]' => 'rspec:windows', + 'rspec:windows: [name,other]' => 'rspec:windows' + } + + tests.each do |name, group_name| + it "'#{name}' puts in '#{group_name}'" do + commit_status.name = name + + is_expected.to eq(group_name) + end end end end diff --git a/spec/models/concerns/avatarable_spec.rb b/spec/models/concerns/avatarable_spec.rb index 8a8eeea39dc..5bed2cb9a14 100644 --- a/spec/models/concerns/avatarable_spec.rb +++ b/spec/models/concerns/avatarable_spec.rb @@ -21,7 +21,7 @@ RSpec.describe Avatarable do it 'validates the file size' do expect(validator).to receive(:validate_each).and_call_original - project.update(avatar: 'uploads/avatar.png') + project.update!(avatar: 'uploads/avatar.png') end end @@ -29,7 +29,7 @@ RSpec.describe Avatarable do it 'skips validation of file size' do expect(validator).not_to receive(:validate_each) - project.update(name: 'Hello world') + project.update!(name: 'Hello world') end end end diff --git a/spec/models/concerns/bulk_insertable_associations_spec.rb b/spec/models/concerns/bulk_insertable_associations_spec.rb index 5a40639e493..25b13c8233d 100644 --- a/spec/models/concerns/bulk_insertable_associations_spec.rb +++ b/spec/models/concerns/bulk_insertable_associations_spec.rb @@ -187,7 +187,7 @@ RSpec.describe BulkInsertableAssociations do it 'invalidates the parent and returns false' do build_invalid_items(parent: parent) - expect(save_with_bulk_inserts(parent, bangify: false)).to be false + expect(BulkInsertableAssociations.with_bulk_insert { parent.save }).to be false # rubocop:disable Rails/SaveBang expect(parent.errors[:bulk_foos].size).to eq(1) expect(BulkFoo.count).to eq(0) @@ -211,8 +211,8 @@ RSpec.describe BulkInsertableAssociations do private - def save_with_bulk_inserts(entity, bangify: true) - BulkInsertableAssociations.with_bulk_insert { bangify ? entity.save! : entity.save } + def save_with_bulk_inserts(entity) + BulkInsertableAssociations.with_bulk_insert { entity.save! } end def build_items(parent:, relation: :bulk_foos, count: 10) diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 440943171c3..37e2f5fb8d4 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -285,7 +285,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do it_behaves_like 'a class with cached markdown fields' describe '#attribute_invalidated?' do - let(:thing) { klass.create(description: markdown, description_html: html, cached_markdown_version: cache_version) } + let(:thing) { klass.create!(description: markdown, description_html: html, cached_markdown_version: cache_version) } it 'returns true when cached_markdown_version is different' do thing.cached_markdown_version += 1 @@ -318,7 +318,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do let(:thing) do # This forces the record to have outdated HTML. We can't use `create` because the `before_create` hook # would re-render the HTML to the latest version - klass.create.tap do |thing| + klass.create!.tap do |thing| thing.update_columns(description: markdown, description_html: old_html, cached_markdown_version: old_version) end end @@ -326,7 +326,7 @@ RSpec.describe CacheMarkdownField, :clean_gitlab_redis_cache do it 'correctly updates cached HTML even if refresh_markdown_cache is called before updating the attribute' do thing.refresh_markdown_cache - thing.update(description: updated_markdown) + thing.update!(description: updated_markdown) expect(thing.description_html).to eq(updated_html) end diff --git a/spec/models/concerns/case_sensitivity_spec.rb b/spec/models/concerns/case_sensitivity_spec.rb index 521b47c63fd..5fb7cdb4443 100644 --- a/spec/models/concerns/case_sensitivity_spec.rb +++ b/spec/models/concerns/case_sensitivity_spec.rb @@ -12,8 +12,8 @@ RSpec.describe CaseSensitivity do end end - let!(:model_1) { model.create(path: 'mOdEl-1', name: 'mOdEl 1') } - let!(:model_2) { model.create(path: 'mOdEl-2', name: 'mOdEl 2') } + let!(:model_1) { model.create!(path: 'mOdEl-1', name: 'mOdEl 1') } + let!(:model_2) { model.create!(path: 'mOdEl-2', name: 'mOdEl 2') } it 'finds a single instance by a single attribute regardless of case' do expect(model.iwhere(path: 'MODEL-1')).to contain_exactly(model_1) diff --git a/spec/models/concerns/checksummable_spec.rb b/spec/models/concerns/checksummable_spec.rb index b469b2e5c18..3a0387333e8 100644 --- a/spec/models/concerns/checksummable_spec.rb +++ b/spec/models/concerns/checksummable_spec.rb @@ -3,17 +3,21 @@ require 'spec_helper' RSpec.describe Checksummable do - describe ".hexdigest" do - let(:fake_class) do - Class.new do - include Checksummable - end + subject do + Class.new { include Checksummable } + end + + describe ".crc32" do + it 'returns the CRC32 of data' do + expect(subject.crc32('abcd')).to eq 3984772369 end + end + describe ".hexdigest" do it 'returns the SHA256 sum of the file' do expected = Digest::SHA256.file(__FILE__).hexdigest - expect(fake_class.hexdigest(__FILE__)).to eq(expected) + expect(subject.hexdigest(__FILE__)).to eq(expected) end end end diff --git a/spec/models/concerns/counter_attribute_spec.rb b/spec/models/concerns/counter_attribute_spec.rb index f23865a5dbb..a19fbae3cfb 100644 --- a/spec/models/concerns/counter_attribute_spec.rb +++ b/spec/models/concerns/counter_attribute_spec.rb @@ -12,6 +12,36 @@ RSpec.describe CounterAttribute, :counter_attribute, :clean_gitlab_redis_shared_ let(:model) { CounterAttributeModel.find(project_statistics.id) } end + describe 'after_flush callbacks' do + let(:attribute) { model.class.counter_attributes.first} + + subject { model.flush_increments_to_database!(attribute) } + + it 'has registered callbacks' do # defined in :counter_attribute RSpec tag + expect(model.class.after_flush_callbacks.size).to eq(1) + end + + context 'when there are increments to flush' do + before do + model.delayed_increment_counter(attribute, 10) + end + + it 'executes the callbacks' do + subject + + expect(model.flushed).to be_truthy + end + end + + context 'when there are no increments to flush' do + it 'does not execute the callbacks' do + subject + + expect(model.flushed).to be_nil + end + end + end + describe '.steal_increments' do let(:increment_key) { 'counters:Model:123:attribute' } let(:flushed_key) { 'counter:Model:123:attribute:flushed' } diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb index 31186b5fc77..99acc563950 100644 --- a/spec/models/concerns/featurable_spec.rb +++ b/spec/models/concerns/featurable_spec.rb @@ -180,6 +180,6 @@ RSpec.describe Featurable do def update_all_project_features(project, features, value) project_feature_attributes = features.map { |f| ["#{f}_access_level", value] }.to_h - project.project_feature.update(project_feature_attributes) + project.project_feature.update!(project_feature_attributes) end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 44561e2e55a..ff5b270cf33 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -69,7 +69,7 @@ RSpec.describe Issuable do it 'returns nil when author is nil' do issue.author_id = nil - issue.save(validate: false) + issue.save!(validate: false) expect(issue.author_name).to eq nil end @@ -361,13 +361,13 @@ RSpec.describe Issuable do end it 'returns true when a subcription exists and subscribed is true' do - issue.subscriptions.create(user: user, project: project, subscribed: true) + issue.subscriptions.create!(user: user, project: project, subscribed: true) expect(issue.subscribed?(user, project)).to be_truthy end it 'returns false when a subcription exists and subscribed is false' do - issue.subscriptions.create(user: user, project: project, subscribed: false) + issue.subscriptions.create!(user: user, project: project, subscribed: false) expect(issue.subscribed?(user, project)).to be_falsey end @@ -383,13 +383,13 @@ RSpec.describe Issuable do end it 'returns true when a subcription exists and subscribed is true' do - issue.subscriptions.create(user: user, project: project, subscribed: true) + issue.subscriptions.create!(user: user, project: project, subscribed: true) expect(issue.subscribed?(user, project)).to be_truthy end it 'returns false when a subcription exists and subscribed is false' do - issue.subscriptions.create(user: user, project: project, subscribed: false) + issue.subscriptions.create!(user: user, project: project, subscribed: false) expect(issue.subscribed?(user, project)).to be_falsey end @@ -437,7 +437,7 @@ RSpec.describe Issuable do let(:labels) { create_list(:label, 2) } before do - issue.update(labels: [labels[1]]) + issue.update!(labels: [labels[1]]) expect(Gitlab::HookData::IssuableBuilder) .to receive(:new).with(issue).and_return(builder) end @@ -456,7 +456,7 @@ RSpec.describe Issuable do context 'total_time_spent is updated' do before do issue.spend_time(duration: 2, user_id: user.id, spent_at: Time.current) - issue.save + issue.save! expect(Gitlab::HookData::IssuableBuilder) .to receive(:new).with(issue).and_return(builder) end @@ -497,8 +497,8 @@ RSpec.describe Issuable do let(:user2) { create(:user) } before do - merge_request.update(assignees: [user]) - merge_request.update(assignees: [user, user2]) + merge_request.update!(assignees: [user]) + merge_request.update!(assignees: [user, user2]) expect(Gitlab::HookData::IssuableBuilder) .to receive(:new).with(merge_request).and_return(builder) end @@ -554,7 +554,7 @@ RSpec.describe Issuable do before do label_link = issue.label_links.find_by(label_id: second_label.id) label_link.label_id = nil - label_link.save(validate: false) + label_link.save!(validate: false) end it 'filters out bad labels' do @@ -824,7 +824,7 @@ RSpec.describe Issuable do where(:issuable_type, :supports_time_tracking) do :issue | true - :incident | false + :incident | true :merge_request | true end @@ -926,58 +926,4 @@ RSpec.describe Issuable do end end end - - describe '#update_severity' do - let(:severity) { 'low' } - - subject(:update_severity) { issuable.update_severity(severity) } - - context 'when issuable not an incident' do - %i(issue merge_request).each do |issuable_type| - let(:issuable) { build_stubbed(issuable_type) } - - it { is_expected.to be_nil } - - it 'does not set severity' do - expect { subject }.not_to change(IssuableSeverity, :count) - end - end - end - - context 'when issuable is an incident' do - let!(:issuable) { create(:incident) } - - context 'when issuable does not have issuable severity yet' do - it 'creates new record' do - expect { update_severity }.to change { IssuableSeverity.where(issue: issuable).count }.to(1) - end - - it 'sets severity to specified value' do - expect { update_severity }.to change { issuable.severity }.to('low') - end - end - - context 'when issuable has an issuable severity' do - let!(:issuable_severity) { create(:issuable_severity, issue: issuable, severity: 'medium') } - - it 'does not create new record' do - expect { update_severity }.not_to change(IssuableSeverity, :count) - end - - it 'updates existing issuable severity' do - expect { update_severity }.to change { issuable_severity.severity }.to(severity) - end - end - - context 'when severity value is unsupported' do - let(:severity) { 'unsupported-severity' } - - it 'sets the severity to default value' do - update_severity - - expect(issuable.issuable_severity.severity).to eq(IssuableSeverity::DEFAULT) - end - end - end - end end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 758b5aa2ce4..516c0fd75bc 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -177,7 +177,7 @@ RSpec.describe Issue, "Mentionable" do expect(SystemNoteService).not_to receive(:cross_reference) - issue.update(description: 'New description') + issue.update!(description: 'New description') issue.create_new_cross_references! end @@ -186,7 +186,7 @@ RSpec.describe Issue, "Mentionable" do expect(SystemNoteService).to receive(:cross_reference).with(issues[1], any_args) - issue.update(description: issues[1].to_reference) + issue.update!(description: issues[1].to_reference) issue.create_new_cross_references! end @@ -196,7 +196,7 @@ RSpec.describe Issue, "Mentionable" do expect(SystemNoteService).to receive(:cross_reference).with(issues[1], any_args) - note.update(note: issues[1].to_reference) + note.update!(note: issues[1].to_reference) note.create_new_cross_references! end end diff --git a/spec/models/concerns/milestoneable_spec.rb b/spec/models/concerns/milestoneable_spec.rb index f5b82e42ad4..c37582cb65d 100644 --- a/spec/models/concerns/milestoneable_spec.rb +++ b/spec/models/concerns/milestoneable_spec.rb @@ -61,7 +61,7 @@ RSpec.describe Milestoneable do it 'returns true with a milestone from the the parent of the issue project group' do parent = create(:group) - group.update(parent: parent) + group.update!(parent: parent) milestone = create(:milestone, group: parent) expect(build_milestoneable(milestone.id).milestone_available?).to be_truthy diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 58cd054efd5..3b8fc465421 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -102,7 +102,7 @@ RSpec.describe Milestone, 'Milestoneish' do with_them do before do - project.update(visibility_level: project_visibility_levels[visibility]) + project.update!(visibility_level: project_visibility_levels[visibility]) end it 'returns the proper participants' do @@ -139,7 +139,7 @@ RSpec.describe Milestone, 'Milestoneish' do with_them do before do - project.update(visibility_level: project_visibility_levels[visibility]) + project.update!(visibility_level: project_visibility_levels[visibility]) end it 'returns the proper participants' do @@ -171,7 +171,7 @@ RSpec.describe Milestone, 'Milestoneish' do context 'when project is private' do before do - project.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) end it 'does not return any merge request for a non member' do @@ -195,7 +195,7 @@ RSpec.describe Milestone, 'Milestoneish' do context 'when merge requests are available to project members' do before do - project.project_feature.update(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) end it 'does not return any merge request for a non member' do diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index b12ad82920f..7e031bdd263 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -14,6 +14,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do self.reactive_cache_lifetime = 5.minutes self.reactive_cache_refresh_interval = 15.seconds + self.reactive_cache_work_type = :no_dependency attr_reader :id @@ -372,4 +373,14 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do it { expect(subject.reactive_cache_hard_limit).to be_nil } it { expect(subject.reactive_cache_worker_finder).to respond_to(:call) } end + + describe 'classes including this concern' do + it 'sets reactive_cache_work_type' do + classes = ObjectSpace.each_object(Class).select do |klass| + klass < described_class && klass.name + end + + expect(classes).to all(have_attributes(reactive_cache_work_type: be_in(described_class::WORK_TYPE.keys))) + end + end end diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb index c91ddfee944..c0e5ddc23b1 100644 --- a/spec/models/concerns/resolvable_discussion_spec.rb +++ b/spec/models/concerns/resolvable_discussion_spec.rb @@ -553,13 +553,13 @@ RSpec.describe Discussion, ResolvableDiscussion do let(:time) { Time.current.utc } before do - Timecop.freeze(time - 1.second) do + travel_to(time - 1.second) do first_note.resolve!(current_user) end - Timecop.freeze(time) do + travel_to(time) do third_note.resolve!(current_user) end - Timecop.freeze(time + 1.second) do + travel_to(time + 1.second) do second_note.resolve!(current_user) end end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 15d754861b2..e4cf68663ef 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Group, 'Routable' do end it 'updates route record on path change' do - group.update(path: 'wow', name: 'much') + group.update!(path: 'wow', name: 'much') expect(group.route.path).to eq('wow') expect(group.route.name).to eq('much') diff --git a/spec/models/concerns/schedulable_spec.rb b/spec/models/concerns/schedulable_spec.rb index 875c2d80e55..62acd12e267 100644 --- a/spec/models/concerns/schedulable_spec.rb +++ b/spec/models/concerns/schedulable_spec.rb @@ -35,7 +35,7 @@ RSpec.describe Schedulable do context 'for a pipeline_schedule' do # let! is used to reset the next_run_at value before each spec let(:object) do - Timecop.freeze(1.day.ago) do + travel_to(1.day.ago) do create(:ci_pipeline_schedule, :hourly) end end diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb index 2a43e748e58..3e52ca5cf63 100644 --- a/spec/models/concerns/subscribable_spec.rb +++ b/spec/models/concerns/subscribable_spec.rb @@ -20,13 +20,13 @@ RSpec.describe Subscribable, 'Subscribable' do end it 'returns true when a subcription exists and subscribed is true' do - resource.subscriptions.create(user: user_1, subscribed: true) + resource.subscriptions.create!(user: user_1, subscribed: true) expect(resource.subscribed?(user_1)).to be_truthy end it 'returns false when a subcription exists and subscribed is false' do - resource.subscriptions.create(user: user_1, subscribed: false) + resource.subscriptions.create!(user: user_1, subscribed: false) expect(resource.subscribed?(user_1)).to be_falsey end @@ -38,13 +38,13 @@ RSpec.describe Subscribable, 'Subscribable' do end it 'returns true when a subcription exists and subscribed is true' do - resource.subscriptions.create(user: user_1, project: project, subscribed: true) + resource.subscriptions.create!(user: user_1, project: project, subscribed: true) expect(resource.subscribed?(user_1, project)).to be_truthy end it 'returns false when a subcription exists and subscribed is false' do - resource.subscriptions.create(user: user_1, project: project, subscribed: false) + resource.subscriptions.create!(user: user_1, project: project, subscribed: false) expect(resource.subscribed?(user_1, project)).to be_falsey end @@ -58,9 +58,9 @@ RSpec.describe Subscribable, 'Subscribable' do it 'returns the subscribed users' do user_2 = create(:user) - resource.subscriptions.create(user: user_1, subscribed: true) - resource.subscriptions.create(user: user_2, project: project, subscribed: true) - resource.subscriptions.create(user: create(:user), project: project, subscribed: false) + resource.subscriptions.create!(user: user_1, subscribed: true) + resource.subscriptions.create!(user: user_2, project: project, subscribed: true) + resource.subscriptions.create!(user: create(:user), project: project, subscribed: false) expect(resource.subscribers(project)).to contain_exactly(user_1, user_2) end @@ -113,7 +113,7 @@ RSpec.describe Subscribable, 'Subscribable' do describe '#unsubscribe' do context 'without project' do it 'unsubscribes the given current user' do - resource.subscriptions.create(user: user_1, subscribed: true) + resource.subscriptions.create!(user: user_1, subscribed: true) expect(resource.subscribed?(user_1)).to be_truthy resource.unsubscribe(user_1) @@ -124,7 +124,7 @@ RSpec.describe Subscribable, 'Subscribable' do context 'with project' do it 'unsubscribes the given current user' do - resource.subscriptions.create(user: user_1, project: project, subscribed: true) + resource.subscriptions.create!(user: user_1, project: project, subscribed: true) expect(resource.subscribed?(user_1, project)).to be_truthy resource.unsubscribe(user_1, project) @@ -139,7 +139,7 @@ RSpec.describe Subscribable, 'Subscribable' do context 'when desired_state is set to true' do context 'when a user is subscribed to the resource' do it 'keeps the user subscribed' do - resource.subscriptions.create(user: user_1, subscribed: true, project: resource_project) + resource.subscriptions.create!(user: user_1, subscribed: true, project: resource_project) resource.set_subscription(user_1, true, resource_project) @@ -159,7 +159,7 @@ RSpec.describe Subscribable, 'Subscribable' do context 'when desired_state is set to false' do context 'when a user is subscribed to the resource' do it 'unsubscribes the user from the resource' do - resource.subscriptions.create(user: user_1, subscribed: true, project: resource_project) + resource.subscriptions.create!(user: user_1, subscribed: true, project: resource_project) expect { resource.set_subscription(user_1, false, resource_project) } .to change { resource.subscribed?(user_1, resource_project) } diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index e0e764fc63c..90e94b5dca9 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -137,7 +137,7 @@ RSpec.describe PersonalAccessToken, 'TokenAuthenticatable' do subject { PersonalAccessToken.find_by_token(token_value) } it 'finds the token' do - personal_access_token.save + personal_access_token.save! expect(subject).to eq(personal_access_token) end diff --git a/spec/models/container_repository_spec.rb b/spec/models/container_repository_spec.rb index 953f92d103b..2a7aaed5204 100644 --- a/spec/models/container_repository_spec.rb +++ b/spec/models/container_repository_spec.rb @@ -184,6 +184,33 @@ RSpec.describe ContainerRepository do end end + describe '#start_expiration_policy!' do + subject { repository.start_expiration_policy! } + + it 'sets the expiration policy started at to now' do + Timecop.freeze do + expect { subject } + .to change { repository.expiration_policy_started_at }.from(nil).to(Time.zone.now) + end + end + end + + describe '#reset_expiration_policy_started_at!' do + subject { repository.reset_expiration_policy_started_at! } + + before do + repository.start_expiration_policy! + end + + it 'resets the expiration policy started at' do + started_at = repository.expiration_policy_started_at + + expect(started_at).not_to be_nil + expect { subject } + .to change { repository.expiration_policy_started_at }.from(started_at).to(nil) + end + end + describe '.build_from_path' do let(:registry_path) do ContainerRegistry::Path.new(project.full_path + '/some/image') diff --git a/spec/models/design_management/design_collection_spec.rb b/spec/models/design_management/design_collection_spec.rb index 8575cc80b5b..bc8330c7dd3 100644 --- a/spec/models/design_management/design_collection_spec.rb +++ b/spec/models/design_management/design_collection_spec.rb @@ -101,6 +101,18 @@ RSpec.describe DesignManagement::DesignCollection do end end + describe "#empty?" do + it "is true when the design collection has no designs" do + expect(collection).to be_empty + end + + it "is false when the design collection has designs" do + create(:design, issue: issue) + + expect(collection).not_to be_empty + end + end + describe "#versions" do it "includes versions for all designs" do version_1 = create(:design_version) diff --git a/spec/models/design_management/design_spec.rb b/spec/models/design_management/design_spec.rb index d4adc0d42d0..2ce9f00a056 100644 --- a/spec/models/design_management/design_spec.rb +++ b/spec/models/design_management/design_spec.rb @@ -206,6 +206,15 @@ RSpec.describe DesignManagement::Design do end end + describe ".build_full_path" do + it "builds the full path for a design" do + design = build(:design, issue: issue, filename: "hello.jpg") + expected_path = "#{DesignManagement.designs_directory}/issue-#{design.issue.iid}/hello.jpg" + + expect(described_class.build_full_path(issue, design)).to eq(expected_path) + end + end + describe '#visible_in?' do let_it_be(:issue) { create(:issue, project: issue.project) } diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index bafcb7a3741..47492715c11 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -918,6 +918,56 @@ RSpec.describe Event do expect(destroyed).to eq('deleted') expect(archived).to eq('archived') end + + it 'handles correct push_action' do + project = create(:project) + user = create(:user) + project.add_developer(user) + push_event = create_push_event(project, user) + + expect(push_event.push_action?).to be true + expect(push_event.action_name).to eq('pushed to') + end + + context 'handles correct base actions' do + using RSpec::Parameterized::TableSyntax + + where(:trait, :action_name) do + :created | 'created' + :updated | 'opened' + :closed | 'closed' + :reopened | 'opened' + :commented | 'commented on' + :merged | 'accepted' + :joined | 'joined' + :left | 'left' + :destroyed | 'destroyed' + :expired | 'removed due to membership expiration from' + :approved | 'approved' + end + + with_them do + it 'with correct name and method' do + event = build(:event, trait) + + expect(event.action_name).to eq(action_name) + end + end + end + + context 'for created_project_action?' do + it 'returns created for created event' do + action = build(:project_created_event) + + expect(action.action_name).to eq('created') + end + + it 'returns imported for imported event' do + action = build(:project_imported_event) + + expect(action.action_name).to eq('imported') + end + end end def create_push_event(project, user) diff --git a/spec/models/group_import_state_spec.rb b/spec/models/group_import_state_spec.rb index 4404ef64966..469b5c96ac9 100644 --- a/spec/models/group_import_state_spec.rb +++ b/spec/models/group_import_state_spec.rb @@ -6,6 +6,7 @@ RSpec.describe GroupImportState do describe 'validations' do let_it_be(:group) { create(:group) } + it { is_expected.to belong_to(:user).required } it { is_expected.to validate_presence_of(:group) } it { is_expected.to validate_presence_of(:status) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 15972f66fd6..ed027d02b5b 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -224,6 +224,20 @@ RSpec.describe Group do end end + describe '.without_integration' do + let(:another_group) { create(:group) } + let(:instance_integration) { build(:jira_service, :instance) } + + before do + create(:jira_service, group: group, project: nil) + create(:slack_service, group: another_group, project: nil) + end + + it 'returns groups without integration' do + expect(Group.without_integration(instance_integration)).to contain_exactly(another_group) + end + end + describe '.public_or_visible_to_user' do let!(:private_group) { create(:group, :private) } let!(:internal_group) { create(:group, :internal) } @@ -1330,229 +1344,134 @@ RSpec.describe Group do end end - describe '#shared_runners_allowed?' do - using RSpec::Parameterized::TableSyntax - - where(:shared_runners_enabled, :allow_descendants_override, :expected_shared_runners_allowed) do - true | false | true - true | true | true - false | false | false - false | true | true - end - - with_them do - let!(:group) { create(:group, shared_runners_enabled: shared_runners_enabled, allow_descendants_override_disabled_shared_runners: allow_descendants_override) } - - it 'returns the expected result' do - expect(group.shared_runners_allowed?).to eq(expected_shared_runners_allowed) - end - end + def subject_and_reload(*models) + subject + models.map(&:reload) end - describe '#parent_allows_shared_runners?' do - context 'when parent group is present' do - using RSpec::Parameterized::TableSyntax - - where(:shared_runners_enabled, :allow_descendants_override, :expected_shared_runners_allowed) do - true | false | true - true | true | true - false | false | false - false | true | true - end - - with_them do - let!(:parent_group) { create(:group, shared_runners_enabled: shared_runners_enabled, allow_descendants_override_disabled_shared_runners: allow_descendants_override) } - let!(:group) { create(:group, parent: parent_group) } - - it 'returns the expected result' do - expect(group.parent_allows_shared_runners?).to eq(expected_shared_runners_allowed) + describe '#update_shared_runners_setting!' do + context 'enabled' do + subject { group.update_shared_runners_setting!('enabled') } + + context 'group that its ancestors have shared runners disabled' do + let_it_be(:parent) { create(:group, :shared_runners_disabled) } + let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } + let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } + + it 'raises error and does not enable shared Runners' do + expect { subject_and_reload(parent, group, project) } + .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Shared runners enabled cannot be enabled because parent group has shared Runners disabled') + .and not_change { parent.shared_runners_enabled } + .and not_change { group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } end end - end - - context 'when parent group is missing' do - let!(:group) { create(:group) } - - it 'returns true' do - expect(group.parent_allows_shared_runners?).to be_truthy - end - end - end - - describe '#parent_enabled_shared_runners?' do - subject { group.parent_enabled_shared_runners? } - - context 'when parent group is present' do - context 'When shared Runners are disabled' do - let!(:parent_group) { create(:group, :shared_runners_disabled) } - let!(:group) { create(:group, parent: parent_group) } - - it { is_expected.to be_falsy } - end - - context 'When shared Runners are enabled' do - let!(:parent_group) { create(:group) } - let!(:group) { create(:group, parent: parent_group) } - - it { is_expected.to be_truthy } - end - end - - context 'when parent group is missing' do - let!(:group) { create(:group) } - - it { is_expected.to be_truthy } - end - end - - describe '#enable_shared_runners!' do - subject { group.enable_shared_runners! } - context 'group that its ancestors have shared runners disabled' do - let_it_be(:parent) { create(:group, :shared_runners_disabled) } - let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } + context 'root group with shared runners disabled' do + let_it_be(:group) { create(:group, :shared_runners_disabled) } + let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } + let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } - it 'raises error and does not enable shared Runners' do - expect { subject } - .to raise_error(described_class::UpdateSharedRunnersError, 'Shared Runners disabled for the parent group') - .and not_change { parent.reload.shared_runners_enabled } - .and not_change { group.reload.shared_runners_enabled } - .and not_change { project.reload.shared_runners_enabled } + it 'enables shared Runners only for itself' do + expect { subject_and_reload(group, sub_group, project) } + .to change { group.shared_runners_enabled }.from(false).to(true) + .and not_change { sub_group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } + end end end - context 'root group with shared runners disabled' do - let_it_be(:group) { create(:group, :shared_runners_disabled) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } - - it 'enables shared Runners only for itself' do - expect { subject } - .to change { group.reload.shared_runners_enabled }.from(false).to(true) - .and not_change { sub_group.reload.shared_runners_enabled } - .and not_change { project.reload.shared_runners_enabled } + context 'disabled_and_unoverridable' do + let_it_be(:group) { create(:group) } + let_it_be(:sub_group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent: group) } + let_it_be(:sub_group_2) { create(:group, parent: group) } + let_it_be(:project) { create(:project, group: group, shared_runners_enabled: true) } + let_it_be(:project_2) { create(:project, group: sub_group_2, shared_runners_enabled: true) } + + subject { group.update_shared_runners_setting!('disabled_and_unoverridable') } + + it 'disables shared Runners for all descendant groups and projects' do + expect { subject_and_reload(group, sub_group, sub_group_2, project, project_2) } + .to change { group.shared_runners_enabled }.from(true).to(false) + .and not_change { group.allow_descendants_override_disabled_shared_runners } + .and not_change { sub_group.shared_runners_enabled } + .and change { sub_group.allow_descendants_override_disabled_shared_runners }.from(true).to(false) + .and change { sub_group_2.shared_runners_enabled }.from(true).to(false) + .and not_change { sub_group_2.allow_descendants_override_disabled_shared_runners } + .and change { project.shared_runners_enabled }.from(true).to(false) + .and change { project_2.shared_runners_enabled }.from(true).to(false) + end + + context 'with override on self' do + let_it_be(:group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } + + it 'disables it' do + expect { subject_and_reload(group) } + .to not_change { group.shared_runners_enabled } + .and change { group.allow_descendants_override_disabled_shared_runners }.from(true).to(false) + end end end - end - - describe '#disable_shared_runners!' do - let_it_be(:group) { create(:group) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent: group) } - let_it_be(:sub_group_2) { create(:group, parent: group) } - let_it_be(:project) { create(:project, group: group, shared_runners_enabled: true) } - let_it_be(:project_2) { create(:project, group: sub_group_2, shared_runners_enabled: true) } - - subject { group.disable_shared_runners! } - - it 'disables shared Runners for all descendant groups and projects' do - expect { subject } - .to change { group.reload.shared_runners_enabled }.from(true).to(false) - .and not_change { group.reload.allow_descendants_override_disabled_shared_runners } - .and not_change { sub_group.reload.shared_runners_enabled } - .and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners } - .and change { sub_group_2.reload.shared_runners_enabled }.from(true).to(false) - .and not_change { sub_group_2.reload.allow_descendants_override_disabled_shared_runners } - .and change { project.reload.shared_runners_enabled }.from(true).to(false) - .and change { project_2.reload.shared_runners_enabled }.from(true).to(false) - end - end - - describe '#allow_descendants_override_disabled_shared_runners!' do - subject { group.allow_descendants_override_disabled_shared_runners! } - context 'top level group' do - let_it_be(:group) { create(:group, :shared_runners_disabled) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } + context 'disabled_with_override' do + subject { group.update_shared_runners_setting!('disabled_with_override') } - it 'enables allow descendants to override only for itself' do - expect { subject } - .to change { group.reload.allow_descendants_override_disabled_shared_runners }.from(false).to(true) - .and not_change { group.reload.shared_runners_enabled } - .and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners } - .and not_change { sub_group.reload.shared_runners_enabled } - .and not_change { project.reload.shared_runners_enabled } - end - end + context 'top level group' do + let_it_be(:group) { create(:group, :shared_runners_disabled) } + let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } + let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } - context 'group that its ancestors have shared Runners disabled but allows to override' do - let_it_be(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } - let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } - - it 'enables allow descendants to override' do - expect { subject } - .to not_change { parent.reload.allow_descendants_override_disabled_shared_runners } - .and not_change { parent.reload.shared_runners_enabled } - .and change { group.reload.allow_descendants_override_disabled_shared_runners }.from(false).to(true) - .and not_change { group.reload.shared_runners_enabled } - .and not_change { project.reload.shared_runners_enabled } + it 'enables allow descendants to override only for itself' do + expect { subject_and_reload(group, sub_group, project) } + .to change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and not_change { group.shared_runners_enabled } + .and not_change { sub_group.allow_descendants_override_disabled_shared_runners } + .and not_change { sub_group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } + end end - end - context 'when parent does not allow' do - let_it_be(:parent) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false ) } - let_it_be(:group) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } + context 'group that its ancestors have shared Runners disabled but allows to override' do + let_it_be(:parent) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } + let_it_be(:group) { create(:group, :shared_runners_disabled, parent: parent) } + let_it_be(:project) { create(:project, shared_runners_enabled: false, group: group) } - it 'raises error and does not allow descendants to override' do - expect { subject } - .to raise_error(described_class::UpdateSharedRunnersError, 'Group level shared Runners not allowed') - .and not_change { parent.reload.allow_descendants_override_disabled_shared_runners } - .and not_change { parent.reload.shared_runners_enabled } - .and not_change { group.reload.allow_descendants_override_disabled_shared_runners } - .and not_change { group.reload.shared_runners_enabled } + it 'enables allow descendants to override' do + expect { subject_and_reload(parent, group, project) } + .to not_change { parent.allow_descendants_override_disabled_shared_runners } + .and not_change { parent.shared_runners_enabled } + .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and not_change { group.shared_runners_enabled } + .and not_change { project.shared_runners_enabled } + end end - end - context 'top level group that has shared Runners enabled' do - let_it_be(:group) { create(:group, shared_runners_enabled: true) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } + context 'when parent does not allow' do + let_it_be(:parent) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false ) } + let_it_be(:group) { create(:group, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent: parent) } - it 'raises error and does not change config' do - expect { subject } - .to raise_error(described_class::UpdateSharedRunnersError, 'Shared Runners enabled') - .and not_change { group.reload.allow_descendants_override_disabled_shared_runners } - .and not_change { group.reload.shared_runners_enabled } - .and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners } - .and not_change { sub_group.reload.shared_runners_enabled } - .and not_change { project.reload.shared_runners_enabled } + it 'raises error and does not allow descendants to override' do + expect { subject_and_reload(parent, group) } + .to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Allow descendants override disabled shared runners cannot be enabled because parent group does not allow it') + .and not_change { parent.allow_descendants_override_disabled_shared_runners } + .and not_change { parent.shared_runners_enabled } + .and not_change { group.allow_descendants_override_disabled_shared_runners } + .and not_change { group.shared_runners_enabled } + end end - end - end - describe '#disallow_descendants_override_disabled_shared_runners!' do - subject { group.disallow_descendants_override_disabled_shared_runners! } + context 'top level group that has shared Runners enabled' do + let_it_be(:group) { create(:group, shared_runners_enabled: true) } + let_it_be(:sub_group) { create(:group, shared_runners_enabled: true, parent: group) } + let_it_be(:project) { create(:project, shared_runners_enabled: true, group: sub_group) } - context 'top level group' do - let_it_be(:group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners ) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: true, group: sub_group) } - - it 'disables allow project to override for descendants and disables project shared Runners' do - expect { subject } - .to not_change { group.reload.shared_runners_enabled } - .and change { group.reload.allow_descendants_override_disabled_shared_runners }.from(true).to(false) - .and not_change { sub_group.reload.shared_runners_enabled } - .and change { sub_group.reload.allow_descendants_override_disabled_shared_runners }.from(true).to(false) - .and change { project.reload.shared_runners_enabled }.from(true).to(false) - end - end - - context 'top level group that has shared Runners enabled' do - let_it_be(:group) { create(:group, shared_runners_enabled: true) } - let_it_be(:sub_group) { create(:group, :shared_runners_disabled, parent: group) } - let_it_be(:project) { create(:project, shared_runners_enabled: false, group: sub_group) } - - it 'results error and does not change config' do - expect { subject } - .to raise_error(described_class::UpdateSharedRunnersError, 'Shared Runners enabled') - .and not_change { group.reload.allow_descendants_override_disabled_shared_runners } - .and not_change { group.reload.shared_runners_enabled } - .and not_change { sub_group.reload.allow_descendants_override_disabled_shared_runners } - .and not_change { sub_group.reload.shared_runners_enabled } - .and not_change { project.reload.shared_runners_enabled } + it 'enables allow descendants to override & disables shared runners everywhere' do + expect { subject_and_reload(group, sub_group, project) } + .to change { group.shared_runners_enabled }.from(true).to(false) + .and change { group.allow_descendants_override_disabled_shared_runners }.from(false).to(true) + .and change { sub_group.shared_runners_enabled }.from(true).to(false) + .and change { project.shared_runners_enabled }.from(true).to(false) + end end end end diff --git a/spec/models/import_failure_spec.rb b/spec/models/import_failure_spec.rb index cdef125e890..9fee1b0ae7b 100644 --- a/spec/models/import_failure_spec.rb +++ b/spec/models/import_failure_spec.rb @@ -16,7 +16,7 @@ RSpec.describe ImportFailure do it 'orders hard failures by newest first' do older_failure = hard_failure.dup - Timecop.freeze(1.day.before(hard_failure.created_at)) do + travel_to(1.day.before(hard_failure.created_at)) do older_failure.save! expect(ImportFailure.hard_failures_by_correlation_id(correlation_id)).to eq([hard_failure, older_failure]) diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 87ba0f3f7e6..e4bb6522689 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -11,18 +11,18 @@ RSpec.describe Integration do before do create(:jira_service, project: project_1, inherit_from_id: instance_integration.id) create(:jira_service, project: project_2, inherit_from_id: nil) - create(:slack_service, project: project_1, inherit_from_id: nil) + create(:slack_service, project: project_3, inherit_from_id: nil) end - describe '#with_custom_integration_for' do + describe '.with_custom_integration_for' do it 'returns projects with custom integrations' do expect(Project.with_custom_integration_for(instance_integration)).to contain_exactly(project_2) end end - describe '#ids_without_integration' do - it 'returns projects ids without an integration' do - expect(Project.ids_without_integration(instance_integration, 100)).to contain_exactly(project_3.id) + describe '.without_integration' do + it 'returns projects without integration' do + expect(Project.without_integration(instance_integration)).to contain_exactly(project_3) end end end diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb index 966e4321378..1d3c09a48b7 100644 --- a/spec/models/issue/metrics_spec.rb +++ b/spec/models/issue/metrics_spec.rb @@ -38,7 +38,7 @@ RSpec.describe Issue::Metrics do context "milestones" do it "records the first time an issue is associated with a milestone" do time = Time.current - Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) } + travel_to(time) { subject.update(milestone: create(:milestone, project: project)) } metrics = subject.metrics expect(metrics).to be_present @@ -47,9 +47,9 @@ RSpec.describe Issue::Metrics do it "does not record the second time an issue is associated with a milestone" do time = Time.current - Timecop.freeze(time) { subject.update(milestone: create(:milestone, project: project)) } - Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) } - Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone, project: project)) } + travel_to(time) { subject.update(milestone: create(:milestone, project: project)) } + travel_to(time + 2.hours) { subject.update(milestone: nil) } + travel_to(time + 6.hours) { subject.update(milestone: create(:milestone, project: project)) } metrics = subject.metrics expect(metrics).to be_present @@ -61,7 +61,7 @@ RSpec.describe Issue::Metrics do it "records the first time an issue is associated with a list label" do list_label = create(:list).label time = Time.current - Timecop.freeze(time) { subject.update(label_ids: [list_label.id]) } + travel_to(time) { subject.update(label_ids: [list_label.id]) } metrics = subject.metrics expect(metrics).to be_present @@ -71,9 +71,9 @@ RSpec.describe Issue::Metrics do it "does not record the second time an issue is associated with a list label" do time = Time.current first_list_label = create(:list).label - Timecop.freeze(time) { subject.update(label_ids: [first_list_label.id]) } + travel_to(time) { subject.update(label_ids: [first_list_label.id]) } second_list_label = create(:list).label - Timecop.freeze(time + 5.hours) { subject.update(label_ids: [second_list_label.id]) } + travel_to(time + 5.hours) { subject.update(label_ids: [second_list_label.id]) } metrics = subject.metrics expect(metrics).to be_present diff --git a/spec/models/issue_email_participant_spec.rb b/spec/models/issue_email_participant_spec.rb new file mode 100644 index 00000000000..f19e65e31f3 --- /dev/null +++ b/spec/models/issue_email_participant_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe IssueEmailParticipant do + describe "Associations" do + it { is_expected.to belong_to(:issue) } + end + + describe 'Validations' do + subject { build(:issue_email_participant) } + + it { is_expected.to validate_presence_of(:issue) } + it { is_expected.to validate_presence_of(:email) } + it { is_expected.to validate_uniqueness_of(:email).scoped_to([:issue_id]) } + + it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :email + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 283d945157b..16ea2989eda 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -28,10 +28,11 @@ RSpec.describe Issue do it { is_expected.to have_and_belong_to_many(:prometheus_alert_events) } it { is_expected.to have_and_belong_to_many(:self_managed_prometheus_alert_events) } it { is_expected.to have_many(:prometheus_alerts) } + it { is_expected.to have_many(:issue_email_participants) } describe 'versions.most_recent' do it 'returns the most recent version' do - issue = create(:issue) + issue = create(:issue, project: reusable_project) create_list(:design_version, 2, issue: issue) last_version = create(:design_version, issue: issue) @@ -79,19 +80,19 @@ RSpec.describe Issue do end end - subject { create(:issue) } + subject { create(:issue, project: reusable_project) } describe 'callbacks' do describe '#ensure_metrics' do it 'creates metrics after saving' do - issue = create(:issue) + issue = create(:issue, project: reusable_project) expect(issue.metrics).to be_persisted expect(Issue::Metrics.count).to eq(1) end it 'does not create duplicate metrics for an issue' do - issue = create(:issue) + issue = create(:issue, project: reusable_project) issue.close! @@ -102,6 +103,14 @@ RSpec.describe Issue do it 'records current metrics' do expect_any_instance_of(Issue::Metrics).to receive(:record!) + create(:issue, project: reusable_project) + end + end + + describe '#record_create_action' do + it 'records the creation action after saving' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_created_action) + create(:issue) end end @@ -111,8 +120,8 @@ RSpec.describe Issue do subject { described_class.with_alert_management_alerts } it 'gets only issues with alerts' do - alert = create(:alert_management_alert, issue: create(:issue)) - issue = create(:issue) + alert = create(:alert_management_alert, project: reusable_project, issue: create(:issue, project: reusable_project)) + issue = create(:issue, project: reusable_project) expect(subject).to contain_exactly(alert.issue) expect(subject).not_to include(issue) @@ -130,10 +139,9 @@ RSpec.describe Issue do end describe '.with_issue_type' do - let_it_be(:project) { create(:project) } - let_it_be(:issue) { create(:issue, project: project) } - let_it_be(:incident) { create(:incident, project: project) } - let_it_be(:test_case) { create(:quality_test_case, project: project) } + let_it_be(:issue) { create(:issue, project: reusable_project) } + let_it_be(:incident) { create(:incident, project: reusable_project) } + let_it_be(:test_case) { create(:quality_test_case, project: reusable_project) } it 'gives issues with the given issue type' do expect(described_class.with_issue_type('issue')) @@ -146,6 +154,24 @@ RSpec.describe Issue do end end + describe '.order_severity' do + let_it_be(:issue_high_severity) { create(:issuable_severity, severity: :high).issue } + let_it_be(:issue_low_severity) { create(:issuable_severity, severity: :low).issue } + let_it_be(:issue_no_severity) { create(:incident) } + + context 'sorting ascending' do + subject { described_class.order_severity_asc } + + it { is_expected.to eq([issue_no_severity, issue_low_severity, issue_high_severity]) } + end + + context 'sorting descending' do + subject { described_class.order_severity_desc } + + it { is_expected.to eq([issue_high_severity, issue_low_severity, issue_no_severity]) } + end + end + describe '#order_by_position_and_priority' do let(:project) { reusable_project } let(:p1) { create(:label, title: 'P1', project: project, priority: 1) } @@ -195,7 +221,7 @@ RSpec.describe Issue do end describe '#close' do - subject(:issue) { create(:issue, state: 'opened') } + subject(:issue) { create(:issue, project: reusable_project, state: 'opened') } it 'sets closed_at to Time.current when an issue is closed' do expect { issue.close }.to change { issue.closed_at }.from(nil) @@ -210,7 +236,7 @@ RSpec.describe Issue do end describe '#reopen' do - let(:issue) { create(:issue, state: 'closed', closed_at: Time.current, closed_by: user) } + let(:issue) { create(:issue, project: reusable_project, state: 'closed', closed_at: Time.current, closed_by: user) } it 'sets closed_at to nil when an issue is reopend' do expect { issue.reopen }.to change { issue.closed_at }.to(nil) @@ -293,7 +319,7 @@ RSpec.describe Issue do end describe '#assignee_or_author?' do - let(:issue) { create(:issue) } + let(:issue) { create(:issue, project: reusable_project) } it 'returns true for a user that is assigned to an issue' do issue.assignees << user @@ -313,22 +339,21 @@ RSpec.describe Issue do end describe '#related_issues' do - let(:user) { create(:user) } - let(:authorized_project) { create(:project) } - let(:authorized_project2) { create(:project) } - let(:unauthorized_project) { create(:project) } + let_it_be(:authorized_project) { create(:project) } + let_it_be(:authorized_project2) { create(:project) } + let_it_be(:unauthorized_project) { create(:project) } - let(:authorized_issue_a) { create(:issue, project: authorized_project) } - let(:authorized_issue_b) { create(:issue, project: authorized_project) } - let(:authorized_issue_c) { create(:issue, project: authorized_project2) } + let_it_be(:authorized_issue_a) { create(:issue, project: authorized_project) } + let_it_be(:authorized_issue_b) { create(:issue, project: authorized_project) } + let_it_be(:authorized_issue_c) { create(:issue, project: authorized_project2) } - let(:unauthorized_issue) { create(:issue, project: unauthorized_project) } + let_it_be(:unauthorized_issue) { create(:issue, project: unauthorized_project) } - let!(:issue_link_a) { create(:issue_link, source: authorized_issue_a, target: authorized_issue_b) } - let!(:issue_link_b) { create(:issue_link, source: authorized_issue_a, target: unauthorized_issue) } - let!(:issue_link_c) { create(:issue_link, source: authorized_issue_a, target: authorized_issue_c) } + let_it_be(:issue_link_a) { create(:issue_link, source: authorized_issue_a, target: authorized_issue_b) } + let_it_be(:issue_link_b) { create(:issue_link, source: authorized_issue_a, target: unauthorized_issue) } + let_it_be(:issue_link_c) { create(:issue_link, source: authorized_issue_a, target: authorized_issue_c) } - before do + before_all do authorized_project.add_developer(user) authorized_project2.add_developer(user) end @@ -366,17 +391,16 @@ RSpec.describe Issue do end context 'user is reporter in project issue belongs to' do - let(:project) { create(:project) } - let(:issue) { create(:issue, project: project) } + let(:issue) { create(:issue, project: reusable_project) } - before do - project.add_reporter(user) + before_all do + reusable_project.add_reporter(user) end it { is_expected.to eq true } context 'issue not persisted' do - let(:issue) { build(:issue, project: project) } + let(:issue) { build(:issue, project: reusable_project) } it { is_expected.to eq false } end @@ -384,7 +408,7 @@ RSpec.describe Issue do context 'checking destination project also' do subject { issue.can_move?(user, to_project) } - let(:to_project) { create(:project) } + let_it_be(:to_project) { create(:project) } context 'destination project allowed' do before do @@ -420,7 +444,7 @@ RSpec.describe Issue do end describe '#duplicated?' do - let(:issue) { create(:issue) } + let(:issue) { create(:issue, project: reusable_project) } subject { issue.duplicated? } @@ -429,7 +453,7 @@ RSpec.describe Issue do end context 'issue already duplicated' do - let(:duplicated_to_issue) { create(:issue) } + let(:duplicated_to_issue) { create(:issue, project: reusable_project) } let(:issue) { create(:issue, duplicated_to: duplicated_to_issue) } it { is_expected.to eq true } @@ -440,13 +464,13 @@ RSpec.describe Issue do subject { issue.from_service_desk? } context 'when issue author is support bot' do - let(:issue) { create(:issue, author: ::User.support_bot) } + let(:issue) { create(:issue, project: reusable_project, author: ::User.support_bot) } it { is_expected.to be_truthy } end context 'when issue author is not support bot' do - let(:issue) { create(:issue) } + let(:issue) { create(:issue, project: reusable_project) } it { is_expected.to be_falsey } end @@ -495,7 +519,7 @@ RSpec.describe Issue do end describe '#has_related_branch?' do - let(:issue) { create(:issue, title: "Blue Bell Knoll") } + let(:issue) { create(:issue, project: reusable_project, title: "Blue Bell Knoll") } subject { issue.has_related_branch? } @@ -528,7 +552,7 @@ RSpec.describe Issue do end describe "#to_branch_name" do - let(:issue) { create(:issue, title: 'testing-issue') } + let_it_be(:issue) { create(:issue, project: reusable_project, title: 'testing-issue') } it 'starts with the issue iid' do expect(issue.to_branch_name).to match(/\A#{issue.iid}-[A-Za-z\-]+\z/) @@ -539,12 +563,12 @@ RSpec.describe Issue do end it "does not contain the issue title if confidential" do - issue = create(:issue, title: 'testing-issue', confidential: true) + issue = create(:issue, project: reusable_project, title: 'testing-issue', confidential: true) expect(issue.to_branch_name).to match(/confidential-issue\z/) end context 'issue title longer than 100 characters' do - let(:issue) { create(:issue, iid: 999, title: 'Lorem ipsum dolor sit amet consectetur adipiscing elit Mauris sit amet ipsum id lacus custom fringilla convallis') } + let_it_be(:issue) { create(:issue, project: reusable_project, iid: 999, title: 'Lorem ipsum dolor sit amet consectetur adipiscing elit Mauris sit amet ipsum id lacus custom fringilla convallis') } it "truncates branch name to at most 100 characters" do expect(issue.to_branch_name.length).to be <= 100 @@ -581,15 +605,14 @@ RSpec.describe Issue do describe '#participants' do context 'using a public project' do - let(:project) { create(:project, :public) } - let(:issue) { create(:issue, project: project) } + let_it_be(:issue) { create(:issue, project: reusable_project) } let!(:note1) do - create(:note_on_issue, noteable: issue, project: project, note: 'a') + create(:note_on_issue, noteable: issue, project: reusable_project, note: 'a') end let!(:note2) do - create(:note_on_issue, noteable: issue, project: project, note: 'b') + create(:note_on_issue, noteable: issue, project: reusable_project, note: 'b') end it 'includes the issue author' do @@ -604,8 +627,8 @@ RSpec.describe Issue do context 'using a private project' do it 'does not include mentioned users that do not have access to the project' do project = create(:project) - user = create(:user) issue = create(:issue, project: project) + user = create(:user) create(:note_on_issue, noteable: issue, @@ -621,10 +644,9 @@ RSpec.describe Issue do it 'updates when assignees change' do user1 = create(:user) user2 = create(:user) - project = create(:project) - issue = create(:issue, assignees: [user1], project: project) - project.add_developer(user1) - project.add_developer(user2) + issue = create(:issue, assignees: [user1], project: reusable_project) + reusable_project.add_developer(user1) + reusable_project.add_developer(user2) expect(user1.assigned_open_issues_count).to eq(1) expect(user2.assigned_open_issues_count).to eq(0) @@ -638,9 +660,8 @@ RSpec.describe Issue do end describe '#visible_to_user?' do - let(:project) { build(:project) } + let(:project) { reusable_project } let(:issue) { build(:issue, project: project) } - let(:user) { create(:user) } subject { issue.visible_to_user?(user) } @@ -661,6 +682,10 @@ RSpec.describe Issue do context 'without a user' do let(:user) { nil } + before do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PUBLIC) + end + it 'returns true when the issue is publicly visible' do expect(issue).to receive(:publicly_visible?).and_return(true) @@ -995,7 +1020,8 @@ RSpec.describe Issue do with_them do it 'checks for spam on issues that can be seen anonymously' do - project = create(:project, visibility_level: visibility_level) + project = reusable_project + project.update(visibility_level: visibility_level) issue = create(:issue, project: project, confidential: confidential, description: 'original description') issue.assign_attributes(new_attributes) @@ -1016,8 +1042,8 @@ RSpec.describe Issue do describe '.public_only' do it 'only returns public issues' do - public_issue = create(:issue) - create(:issue, confidential: true) + public_issue = create(:issue, project: reusable_project) + create(:issue, project: reusable_project, confidential: true) expect(described_class.public_only).to eq([public_issue]) end @@ -1025,15 +1051,15 @@ RSpec.describe Issue do describe '.confidential_only' do it 'only returns confidential_only issues' do - create(:issue) - confidential_issue = create(:issue, confidential: true) + create(:issue, project: reusable_project) + confidential_issue = create(:issue, project: reusable_project, confidential: true) expect(described_class.confidential_only).to eq([confidential_issue]) end end describe '.by_project_id_and_iid' do - let_it_be(:issue_a) { create(:issue) } + let_it_be(:issue_a) { create(:issue, project: reusable_project) } let_it_be(:issue_b) { create(:issue, iid: issue_a.iid) } let_it_be(:issue_c) { create(:issue, project: issue_a.project) } let_it_be(:issue_d) { create(:issue, project: issue_a.project) } @@ -1050,8 +1076,8 @@ RSpec.describe Issue do describe '.service_desk' do it 'returns the service desk issue' do - service_desk_issue = create(:issue, author: ::User.support_bot) - regular_issue = create(:issue) + service_desk_issue = create(:issue, project: reusable_project, author: ::User.support_bot) + regular_issue = create(:issue, project: reusable_project) expect(described_class.service_desk).to include(service_desk_issue) expect(described_class.service_desk).not_to include(regular_issue) @@ -1064,7 +1090,7 @@ RSpec.describe Issue do describe "#labels_hook_attrs" do let(:label) { create(:label) } - let(:issue) { create(:labeled_issue, labels: [label]) } + let(:issue) { create(:labeled_issue, project: reusable_project, labels: [label]) } it "returns a list of label hook attributes" do expect(issue.labels_hook_attrs).to eq([label.hook_attrs]) @@ -1073,7 +1099,7 @@ RSpec.describe Issue do context "relative positioning" do it_behaves_like "a class that supports relative positioning" do - let_it_be(:project) { create(:project) } + let_it_be(:project) { reusable_project } let(:factory) { :issue } let(:default_params) { { project: project } } end @@ -1083,7 +1109,7 @@ RSpec.describe Issue do describe "#previous_updated_at" do let_it_be(:updated_at) { Time.zone.local(2012, 01, 06) } - let_it_be(:issue) { create(:issue, updated_at: updated_at) } + let_it_be(:issue) { create(:issue, project: reusable_project, updated_at: updated_at) } it 'returns updated_at value if updated_at did not change at all' do allow(issue).to receive(:previous_changes).and_return({}) @@ -1121,7 +1147,7 @@ RSpec.describe Issue do end describe 'current designs' do - let(:issue) { create(:issue) } + let(:issue) { create(:issue, project: reusable_project) } subject { issue.designs.current } @@ -1213,4 +1239,12 @@ RSpec.describe Issue do expect(issue.allows_reviewers?).to be(false) end end + + describe '#issue_type_supports?' do + let_it_be(:issue) { create(:issue) } + + it 'raises error when feature is invalid' do + expect { issue.issue_type_supports?(:unkown_feature) }.to raise_error(ArgumentError) + end + end end diff --git a/spec/models/iteration_spec.rb b/spec/models/iteration_spec.rb index 19a1625aad3..e7ec5de0ef1 100644 --- a/spec/models/iteration_spec.rb +++ b/spec/models/iteration_spec.rb @@ -119,7 +119,7 @@ RSpec.describe Iteration do let(:start_date) { 5.days.from_now } let(:due_date) { 6.days.from_now } - shared_examples_for 'overlapping dates' do + shared_examples_for 'overlapping dates' do |skip_constraint_test: false| context 'when start_date is in range' do let(:start_date) { 5.days.from_now } let(:due_date) { 3.weeks.from_now } @@ -129,9 +129,11 @@ RSpec.describe Iteration do expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations') end - it 'is not valid even if forced' do - subject.validate # to generate iid/etc - expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) + unless skip_constraint_test + it 'is not valid even if forced' do + subject.validate # to generate iid/etc + expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) + end end end @@ -144,9 +146,11 @@ RSpec.describe Iteration do expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations') end - it 'is not valid even if forced' do - subject.validate # to generate iid/etc - expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) + unless skip_constraint_test + it 'is not valid even if forced' do + subject.validate # to generate iid/etc + expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) + end end end @@ -156,9 +160,11 @@ RSpec.describe Iteration do expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations') end - it 'is not valid even if forced' do - subject.validate # to generate iid/etc - expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) + unless skip_constraint_test + it 'is not valid even if forced' do + subject.validate # to generate iid/etc + expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) + end end end end @@ -177,6 +183,14 @@ RSpec.describe Iteration do expect { subject.save! }.not_to raise_exception end end + + context 'sub-group' do + let(:subgroup) { create(:group, parent: group) } + + subject { build(:iteration, group: subgroup, start_date: start_date, due_date: due_date) } + + it_behaves_like 'overlapping dates', skip_constraint_test: true + end end context 'project' do @@ -210,6 +224,17 @@ RSpec.describe Iteration do end end end + + context 'project in a group' do + let_it_be(:project) { create(:project, group: create(:group)) } + let_it_be(:existing_iteration) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) } + + subject { build(:iteration, :skip_project_validation, project: project, start_date: start_date, due_date: due_date) } + + it_behaves_like 'overlapping dates' do + let(:constraint_name) { 'iteration_start_and_due_daterange_project_id_constraint' } + end + end end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 90950d93db4..118b1492cd6 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -212,6 +212,16 @@ RSpec.describe Member do it { expect(described_class.non_request).to include @accepted_request_member } end + describe '.not_accepted_invitations' do + let_it_be(:not_accepted_invitation) { create(:project_member, :invited) } + let_it_be(:accepted_invitation) { create(:project_member, :invited, invite_accepted_at: Date.today) } + + subject { described_class.not_accepted_invitations } + + it { is_expected.to include(not_accepted_invitation) } + it { is_expected.not_to include(accepted_invitation) } + end + describe '.not_accepted_invitations_by_user' do let(:invited_by_user) { create(:project_member, :invited, project: project, created_by: @owner_user) } @@ -225,6 +235,33 @@ RSpec.describe Member do it { is_expected.to contain_exactly(invited_by_user) } end + describe '.not_expired' do + let_it_be(:expiring_yesterday) { create(:group_member, expires_at: 1.day.from_now) } + let_it_be(:expiring_today) { create(:group_member, expires_at: 2.days.from_now) } + let_it_be(:expiring_tomorrow) { create(:group_member, expires_at: 3.days.from_now) } + let_it_be(:not_expiring) { create(:group_member) } + + subject { described_class.not_expired } + + around do |example| + travel_to(2.days.from_now) { example.run } + end + + it { is_expected.not_to include(expiring_yesterday, expiring_today) } + it { is_expected.to include(expiring_tomorrow, not_expiring) } + end + + describe '.last_ten_days_excluding_today' do + let_it_be(:created_today) { create(:group_member, created_at: Date.today.beginning_of_day) } + let_it_be(:created_yesterday) { create(:group_member, created_at: 1.day.ago) } + let_it_be(:created_eleven_days_ago) { create(:group_member, created_at: 11.days.ago) } + + subject { described_class.last_ten_days_excluding_today } + + it { is_expected.to include(created_yesterday) } + it { is_expected.not_to include(created_today, created_eleven_days_ago) } + end + describe '.search_invite_email' do it 'returns only members the matching e-mail' do create(:group_member, :invited) @@ -683,6 +720,45 @@ RSpec.describe Member do end end + describe '#send_invitation_reminder' do + subject { member.send_invitation_reminder(0) } + + context 'an invited group member' do + let!(:member) { create(:group_member, :invited) } + + it 'sends a reminder' do + expect_any_instance_of(NotificationService).to receive(:invite_member_reminder).with(member, member.raw_invite_token, 0) + + subject + end + end + + context 'an invited member without a raw invite token set' do + let!(:member) { create(:group_member, :invited) } + + before do + member.instance_variable_set(:@raw_invite_token, nil) + allow_any_instance_of(NotificationService).to receive(:invite_member_reminder) + end + + it 'generates a new token' do + expect(member).to receive(:generate_invite_token!) + + subject + end + end + + context 'an uninvited member' do + let!(:member) { create(:group_member) } + + it 'does not send a reminder' do + expect_any_instance_of(NotificationService).not_to receive(:invite_member_reminder) + + subject + end + end + end + describe "#invite_to_unknown_user?" do subject { member.invite_to_unknown_user? } diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 2c64201e84d..4ce2b5f0f48 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -180,6 +180,17 @@ RSpec.describe MergeRequestDiff do expect(diff.external_diff_store).to eq(file_store) end + it 'migrates a nil diff file' do + expect(diff).not_to be_stored_externally + MergeRequestDiffFile.where(merge_request_diff_id: diff.id).update_all(diff: nil) + + stub_external_diffs_setting(enabled: true) + + diff.migrate_files_to_external_storage! + + expect(diff).to be_stored_externally + end + it 'safely handles a transaction error when migrating to external storage' do expect(diff).not_to be_stored_externally expect(diff.external_diff).not_to be_exists diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 98f709a0610..5fde9b8661d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2358,48 +2358,43 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - context 'when state event tracking is disabled' do + context 'when no metrics or merge event exists' do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request, :merged) } + before do - stub_feature_flags(track_resource_state_change_events: false) + merge_request.metrics.destroy! end - context 'when merging note is persisted, but no metrics or merge event exists' do - let(:user) { create(:user) } - let(:merge_request) { create(:merge_request, :merged) } - + context 'when resource event for the merge exists' do before do - merge_request.metrics.destroy! - SystemNoteService.change_status(merge_request, merge_request.target_project, user, merge_request.state, nil) end - it 'returns merging note creation date' do + it 'returns the resource event creation date' do expect(merge_request.reload.metrics).to be_nil expect(merge_request.merge_event).to be_nil - expect(merge_request.notes.count).to eq(1) - expect(merge_request.merged_at).to eq(merge_request.notes.first.created_at) + expect(merge_request.resource_state_events.count).to eq(1) + expect(merge_request.merged_at).to eq(merge_request.resource_state_events.first.created_at) end end - end - - context 'when state event tracking is enabled' do - let(:user) { create(:user) } - let(:merge_request) { create(:merge_request, :merged) } - before do - merge_request.metrics.destroy! - - SystemNoteService.change_status(merge_request, - merge_request.target_project, - user, - merge_request.state, nil) - end + context 'when system note for the merge exists' do + before do + # We do not create these system notes anymore but we need this to work for existing MRs + # that used system notes instead of resource state events + create(:note, :system, noteable: merge_request, note: 'merged') + end - it 'does not create a system note' do - expect(merge_request.notes).to be_empty + it 'returns the merging note creation date' do + expect(merge_request.reload.metrics).to be_nil + expect(merge_request.merge_event).to be_nil + expect(merge_request.notes.count).to eq(1) + expect(merge_request.merged_at).to eq(merge_request.notes.first.created_at) + end end end end @@ -4261,24 +4256,6 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe '#allows_reviewers?' do - it 'returns false without merge_request_reviewers feature' do - stub_feature_flags(merge_request_reviewers: false) - - merge_request = build_stubbed(:merge_request) - - expect(merge_request.allows_reviewers?).to be(false) - end - - it 'returns true with merge_request_reviewers feature' do - stub_feature_flags(merge_request_reviewers: true) - - merge_request = build_stubbed(:merge_request) - - expect(merge_request.allows_reviewers?).to be(true) - end - end - describe '#merge_ref_head' do let(:merge_request) { create(:merge_request) } @@ -4304,4 +4281,36 @@ RSpec.describe MergeRequest, factory_default: :keep do end end end + + describe '#allows_reviewers?' do + it 'returns false without merge_request_reviewers feature' do + stub_feature_flags(merge_request_reviewers: false) + + merge_request = build_stubbed(:merge_request) + + expect(merge_request.allows_reviewers?).to be(false) + end + + it 'returns true with merge_request_reviewers feature' do + stub_feature_flags(merge_request_reviewers: true) + + merge_request = build_stubbed(:merge_request) + + expect(merge_request.allows_reviewers?).to be(true) + end + end + + describe '#update_and_mark_in_progress_merge_commit_sha' do + let(:ref) { subject.target_project.repository.commit.id } + + before do + expect(subject.target_project).to receive(:mark_primary_write_location) + end + + it 'updates commit ID' do + expect { subject.update_and_mark_in_progress_merge_commit_sha(ref) } + .to change { subject.in_progress_merge_commit_sha } + .from(nil).to(ref) + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index ca1f06370d4..2e607639e1b 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1320,4 +1320,140 @@ RSpec.describe Namespace do end end end + + describe '#shared_runners_setting' do + using RSpec::Parameterized::TableSyntax + + where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :shared_runners_setting) do + true | true | 'enabled' + true | false | 'enabled' + false | true | 'disabled_with_override' + false | false | 'disabled_and_unoverridable' + end + + with_them do + let(:namespace) { build(:namespace, shared_runners_enabled: shared_runners_enabled, allow_descendants_override_disabled_shared_runners: allow_descendants_override_disabled_shared_runners)} + + it 'returns the result' do + expect(namespace.shared_runners_setting).to eq(shared_runners_setting) + end + end + end + + describe '#shared_runners_setting_higher_than?' do + using RSpec::Parameterized::TableSyntax + + where(:shared_runners_enabled, :allow_descendants_override_disabled_shared_runners, :other_setting, :result) do + true | true | 'enabled' | false + true | true | 'disabled_with_override' | true + true | true | 'disabled_and_unoverridable' | true + false | true | 'enabled' | false + false | true | 'disabled_with_override' | false + false | true | 'disabled_and_unoverridable' | true + false | false | 'enabled' | false + false | false | 'disabled_with_override' | false + false | false | 'disabled_and_unoverridable' | false + end + + with_them do + let(:namespace) { build(:namespace, shared_runners_enabled: shared_runners_enabled, allow_descendants_override_disabled_shared_runners: allow_descendants_override_disabled_shared_runners)} + + it 'returns the result' do + expect(namespace.shared_runners_setting_higher_than?(other_setting)).to eq(result) + end + end + end + + describe 'validation #changing_shared_runners_enabled_is_allowed' do + context 'without a parent' do + let(:namespace) { build(:namespace, shared_runners_enabled: true) } + + it 'is valid' do + expect(namespace).to be_valid + end + end + + context 'with a parent' do + context 'when parent has shared runners disabled' do + let(:parent) { create(:namespace, :shared_runners_disabled) } + let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) } + + it 'is invalid' do + expect(sub_namespace).to be_invalid + expect(sub_namespace.errors[:shared_runners_enabled]).to include('cannot be enabled because parent group has shared Runners disabled') + end + end + + context 'when parent has shared runners disabled but allows override' do + let(:parent) { create(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners) } + let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) } + + it 'is valid' do + expect(sub_namespace).to be_valid + end + end + + context 'when parent has shared runners enabled' do + let(:parent) { create(:namespace, shared_runners_enabled: true) } + let(:sub_namespace) { build(:namespace, shared_runners_enabled: true, parent_id: parent.id) } + + it 'is valid' do + expect(sub_namespace).to be_valid + end + end + end + end + + describe 'validation #changing_allow_descendants_override_disabled_shared_runners_is_allowed' do + context 'without a parent' do + context 'with shared runners disabled' do + let(:namespace) { build(:namespace, :allow_descendants_override_disabled_shared_runners, :shared_runners_disabled) } + + it 'is valid' do + expect(namespace).to be_valid + end + end + + context 'with shared runners enabled' do + let(:namespace) { create(:namespace) } + + it 'is invalid' do + namespace.allow_descendants_override_disabled_shared_runners = true + + expect(namespace).to be_invalid + expect(namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be changed if shared runners are enabled') + end + end + end + + context 'with a parent' do + context 'when parent does not allow shared runners' do + let(:parent) { create(:namespace, :shared_runners_disabled) } + let(:sub_namespace) { build(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } + + it 'is invalid' do + expect(sub_namespace).to be_invalid + expect(sub_namespace.errors[:allow_descendants_override_disabled_shared_runners]).to include('cannot be enabled because parent group does not allow it') + end + end + + context 'when parent allows shared runners and setting to true' do + let(:parent) { create(:namespace, shared_runners_enabled: true) } + let(:sub_namespace) { build(:namespace, :shared_runners_disabled, :allow_descendants_override_disabled_shared_runners, parent_id: parent.id) } + + it 'is valid' do + expect(sub_namespace).to be_valid + end + end + + context 'when parent allows shared runners and setting to false' do + let(:parent) { create(:namespace, shared_runners_enabled: true) } + let(:sub_namespace) { build(:namespace, :shared_runners_disabled, allow_descendants_override_disabled_shared_runners: false, parent_id: parent.id) } + + it 'is valid' do + expect(sub_namespace).to be_valid + end + end + end + end end diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb index 0f765d6b09b..bc50e2af373 100644 --- a/spec/models/notification_setting_spec.rb +++ b/spec/models/notification_setting_spec.rb @@ -175,6 +175,7 @@ RSpec.describe NotificationSetting do :reopen_merge_request, :close_merge_request, :reassign_merge_request, + :change_reviewer_merge_request, :merge_merge_request, :failed_pipeline, :success_pipeline, diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index db432e73355..b4e941f2856 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Operations::FeatureFlag do context 'a version 1 feature flag' do it 'is valid if associated with Operations::FeatureFlagScope models' do project = create(:project) - feature_flag = described_class.create({ name: 'test', project: project, version: 1, + feature_flag = described_class.create!({ name: 'test', project: project, version: 1, scopes_attributes: [{ environment_scope: '*', active: false }] }) expect(feature_flag).to be_valid @@ -33,9 +33,10 @@ RSpec.describe Operations::FeatureFlag do it 'is invalid if associated with Operations::FeatureFlags::Strategy models' do project = create(:project) - feature_flag = described_class.create({ name: 'test', project: project, version: 1, + feature_flag = described_class.new({ name: 'test', project: project, version: 1, strategies_attributes: [{ name: 'default', parameters: {} }] }) + expect(feature_flag.valid?).to eq(false) expect(feature_flag.errors.messages).to eq({ version_associations: ["version 1 feature flags may not have strategies"] }) @@ -45,9 +46,10 @@ RSpec.describe Operations::FeatureFlag do context 'a version 2 feature flag' do it 'is invalid if associated with Operations::FeatureFlagScope models' do project = create(:project) - feature_flag = described_class.create({ name: 'test', project: project, version: 2, + feature_flag = described_class.new({ name: 'test', project: project, version: 2, scopes_attributes: [{ environment_scope: '*', active: false }] }) + expect(feature_flag.valid?).to eq(false) expect(feature_flag.errors.messages).to eq({ version_associations: ["version 2 feature flags may not have scopes"] }) @@ -55,7 +57,7 @@ RSpec.describe Operations::FeatureFlag do it 'is valid if associated with Operations::FeatureFlags::Strategy models' do project = create(:project) - feature_flag = described_class.create({ name: 'test', project: project, version: 2, + feature_flag = described_class.create!({ name: 'test', project: project, version: 2, strategies_attributes: [{ name: 'default', parameters: {} }] }) expect(feature_flag).to be_valid @@ -75,7 +77,7 @@ RSpec.describe Operations::FeatureFlag do it 'defaults to 1 if unspecified' do project = create(:project) - feature_flag = described_class.create(name: 'my_flag', project: project, active: true) + feature_flag = described_class.create!(name: 'my_flag', project: project, active: true) expect(feature_flag).to be_valid expect(feature_flag.version_before_type_cast).to eq(1) @@ -113,14 +115,14 @@ RSpec.describe Operations::FeatureFlag do context 'with a version 1 feature flag' do it 'creates a default scope' do - feature_flag = described_class.create({ name: 'test', project: project, scopes_attributes: [], version: 1 }) + feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 1 }) expect(feature_flag.scopes.count).to eq(1) expect(feature_flag.scopes.first.environment_scope).to eq('*') end it 'allows specifying the default scope in the parameters' do - feature_flag = described_class.create({ name: 'test', project: project, + feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [{ environment_scope: '*', active: false }, { environment_scope: 'review/*', active: true }], version: 1 }) @@ -131,7 +133,7 @@ RSpec.describe Operations::FeatureFlag do context 'with a version 2 feature flag' do it 'does not create a default scope' do - feature_flag = described_class.create({ name: 'test', project: project, scopes_attributes: [], version: 2 }) + feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 2 }) expect(feature_flag.scopes).to eq([]) end diff --git a/spec/models/operations/feature_flags/strategy_spec.rb b/spec/models/operations/feature_flags/strategy_spec.rb index 04e3ef26e9d..0ecb49e75f3 100644 --- a/spec/models/operations/feature_flags/strategy_spec.rb +++ b/spec/models/operations/feature_flags/strategy_spec.rb @@ -4,11 +4,12 @@ require 'spec_helper' RSpec.describe Operations::FeatureFlags::Strategy do let_it_be(:project) { create(:project) } + let_it_be(:feature_flag) { create(:operations_feature_flag, project: project) } describe 'validations' do it do is_expected.to validate_inclusion_of(:name) - .in_array(%w[default gradualRolloutUserId userWithId gitlabUserList]) + .in_array(%w[default gradualRolloutUserId flexibleRollout userWithId gitlabUserList]) .with_message('strategy name is invalid') end @@ -19,7 +20,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'skips parameters validation' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: invalid_name, parameters: { bad: 'params' }) @@ -36,7 +36,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must have valid parameters for the strategy' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: invalid_parameters) @@ -45,7 +44,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'allows the parameters in any order' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { percentage: '10', groupId: 'mygroup' }) @@ -55,13 +53,12 @@ RSpec.describe Operations::FeatureFlags::Strategy do describe 'percentage' do where(:invalid_value) do - [50, 40.0, { key: "value" }, "garbage", "00", "01", "101", "-1", "-10", "0100", - "1000", "10.0", "5%", "25%", "100hi", "e100", "30m", " ", "\r\n", "\n", "\t", - "\n10", "20\n", "\n100", "100\n", "\n ", nil] + [50, 40.0, { key: "value" }, "garbage", "101", "-1", "-10", "1000", "10.0", "5%", "25%", + "100hi", "e100", "30m", " ", "\r\n", "\n", "\t", "\n10", "20\n", "\n100", "100\n", + "\n ", nil] end with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: 'mygroup', percentage: invalid_value }) @@ -75,7 +72,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: 'mygroup', percentage: valid_value }) @@ -92,7 +88,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value of up to 32 lowercase characters' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: invalid_value, percentage: '40' }) @@ -106,7 +101,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be a string value of up to 32 lowercase characters' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: valid_value, percentage: '40' }) @@ -117,13 +111,132 @@ RSpec.describe Operations::FeatureFlags::Strategy do end end + context 'when the strategy name is flexibleRollout' do + valid_parameters = { rollout: '40', groupId: 'mygroup', stickiness: 'DEFAULT' } + where(invalid_parameters: [ + nil, + {}, + *valid_parameters.to_a.combination(1).to_a.map { |p| p.to_h }, + *valid_parameters.to_a.combination(2).to_a.map { |p| p.to_h }, + { **valid_parameters, userIds: '4' }, + { **valid_parameters, extra: nil } + ]) + with_them do + it 'must have valid parameters for the strategy' do + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: invalid_parameters) + + expect(strategy.errors[:parameters]).to eq(['parameters are invalid']) + end + end + + [ + [:rollout, '10'], + [:stickiness, 'DEFAULT'], + [:groupId, 'mygroup'] + ].permutation(3).each do |parameters| + it "allows the parameters in the order #{parameters.map { |p| p.first }.join(', ')}" do + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: Hash[parameters]) + + expect(strategy.errors[:parameters]).to be_empty + end + end + + describe 'rollout' do + where(invalid_value: [50, 40.0, { key: "value" }, "garbage", "101", "-1", " ", "-10", + "1000", "10.0", "5%", "25%", "100hi", "e100", "30m", "\r\n", + "\n", "\t", "\n10", "20\n", "\n100", "100\n", "\n ", nil]) + with_them do + it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do + parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: invalid_value } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([ + 'rollout must be a string between 0 and 100 inclusive' + ]) + end + end + + where(valid_value: %w[0 1 10 38 100 93]) + with_them do + it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do + parameters = { stickiness: 'DEFAULT', groupId: 'mygroup', rollout: valid_value } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([]) + end + end + end + + describe 'groupId' do + where(invalid_value: [nil, 4, 50.0, {}, 'spaces bad', 'bad$', '%bad', '<bad', 'bad>', + '!bad', '.bad', 'Bad', 'bad1', "", " ", "b" * 33, "ba_d", "ba\nd"]) + with_them do + it 'must be a string value of up to 32 lowercase characters' do + parameters = { stickiness: 'DEFAULT', groupId: invalid_value, rollout: '40' } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq(['groupId parameter is invalid']) + end + end + + where(valid_value: ["somegroup", "anothergroup", "okay", "g", "a" * 32]) + with_them do + it 'must be a string value of up to 32 lowercase characters' do + parameters = { stickiness: 'DEFAULT', groupId: valid_value, rollout: '40' } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([]) + end + end + end + + describe 'stickiness' do + where(invalid_value: [nil, " ", "default", "DEFAULT\n", "UserId", "USER", "USERID "]) + with_them do + it 'must be a string representing a supported stickiness setting' do + parameters = { stickiness: invalid_value, groupId: 'mygroup', rollout: '40' } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([ + 'stickiness parameter must be DEFAULT, USERID, SESSIONID, or RANDOM' + ]) + end + end + + where(valid_value: %w[DEFAULT USERID SESSIONID RANDOM]) + with_them do + it 'must be a string representing a supported stickiness setting' do + parameters = { stickiness: valid_value, groupId: 'mygroup', rollout: '40' } + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: parameters) + + expect(strategy.errors[:parameters]).to eq([]) + end + end + end + end + context 'when the strategy name is userWithId' do where(:invalid_parameters) do [nil, { userIds: 'sam', percentage: '40' }, { userIds: 'sam', some: 'param' }, { percentage: '40' }, {}] end with_them do it 'must have valid parameters for the strategy' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', parameters: invalid_parameters) @@ -140,7 +253,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'is valid with a string of comma separated values' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', parameters: { userIds: valid_value }) @@ -155,7 +267,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'is invalid' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', parameters: { userIds: invalid_value }) @@ -173,7 +284,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be empty' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'default', parameters: invalid_value) @@ -183,7 +293,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'must be empty' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'default', parameters: {}) @@ -198,7 +307,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end with_them do it 'must be empty' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', parameters: invalid_value) @@ -208,7 +316,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'must be empty' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', parameters: {}) @@ -221,7 +328,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do describe 'associations' do context 'when name is gitlabUserList' do it 'is valid when associated with a user list' do - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', @@ -232,7 +338,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'is invalid without a user list' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', parameters: {}) @@ -242,7 +347,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do it 'is invalid when associated with a user list from another project' do other_project = create(:project) - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: other_project) strategy = described_class.create(feature_flag: feature_flag, name: 'gitlabUserList', @@ -255,7 +359,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do context 'when name is default' do it 'is invalid when associated with a user list' do - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'default', @@ -266,7 +369,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'is valid without a user list' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'default', parameters: {}) @@ -277,7 +379,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do context 'when name is userWithId' do it 'is invalid when associated with a user list' do - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', @@ -288,7 +389,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'is valid without a user list' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'userWithId', parameters: { userIds: 'user1' }) @@ -299,7 +399,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do context 'when name is gradualRolloutUserId' do it 'is invalid when associated with a user list' do - feature_flag = create(:operations_feature_flag, project: project) user_list = create(:operations_feature_flag_user_list, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', @@ -310,7 +409,6 @@ RSpec.describe Operations::FeatureFlags::Strategy do end it 'is valid without a user list' do - feature_flag = create(:operations_feature_flag, project: project) strategy = described_class.create(feature_flag: feature_flag, name: 'gradualRolloutUserId', parameters: { groupId: 'default', percentage: '10' }) @@ -318,6 +416,30 @@ RSpec.describe Operations::FeatureFlags::Strategy do expect(strategy.errors[:user_list]).to be_empty end end + + context 'when name is flexibleRollout' do + it 'is invalid when associated with a user list' do + user_list = create(:operations_feature_flag_user_list, project: project) + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + user_list: user_list, + parameters: { groupId: 'default', + rollout: '10', + stickiness: 'DEFAULT' }) + + expect(strategy.errors[:user_list]).to eq(['must be blank']) + end + + it 'is valid without a user list' do + strategy = described_class.create(feature_flag: feature_flag, + name: 'flexibleRollout', + parameters: { groupId: 'default', + rollout: '10', + stickiness: 'DEFAULT' }) + + expect(strategy.errors[:user_list]).to be_empty + end + end end end end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index ea1f75d04e7..6a3969802f3 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -108,6 +108,20 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.not_to allow_value('.foobar').for(:name) } it { is_expected.not_to allow_value('%foo%bar').for(:name) } end + + context 'generic package' do + subject { build_stubbed(:generic_package) } + + it { is_expected.to allow_value('123').for(:name) } + it { is_expected.to allow_value('foo').for(:name) } + it { is_expected.to allow_value('foo.bar.baz-2.0-20190901.47283-1').for(:name) } + it { is_expected.not_to allow_value('../../foo').for(:name) } + it { is_expected.not_to allow_value('..\..\foo').for(:name) } + it { is_expected.not_to allow_value('%2f%2e%2e%2f%2essh%2fauthorized_keys').for(:name) } + it { is_expected.not_to allow_value('$foo/bar').for(:name) } + it { is_expected.not_to allow_value('my file name').for(:name) } + it { is_expected.not_to allow_value('!!().for(:name)().for(:name)').for(:name) } + end end describe '#version' do diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb index bc6398de9a4..67fb11f34e0 100644 --- a/spec/models/plan_limits_spec.rb +++ b/spec/models/plan_limits_spec.rb @@ -199,6 +199,7 @@ RSpec.describe PlanLimits do ci_max_artifact_size_secret_detection ci_max_artifact_size_requirements ci_max_artifact_size_coverage_fuzzing + ci_max_artifact_size_api_fuzzing ] end diff --git a/spec/models/project_feature_usage_spec.rb b/spec/models/project_feature_usage_spec.rb index 908b98ee9c2..d55d41fab85 100644 --- a/spec/models/project_feature_usage_spec.rb +++ b/spec/models/project_feature_usage_spec.rb @@ -48,7 +48,7 @@ RSpec.describe ProjectFeatureUsage, type: :model do feature_usage.log_jira_dvcs_integration_usage first_logged_at = feature_usage.jira_dvcs_cloud_last_sync_at - Timecop.freeze(1.hour.from_now) do + travel_to(1.hour.from_now) do ProjectFeatureUsage.new(project_id: project.id).log_jira_dvcs_integration_usage end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fe971832695..20b10be8c3b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -72,6 +72,7 @@ RSpec.describe Project do it { is_expected.to have_one(:last_event).class_name('Event') } it { is_expected.to have_one(:forked_from_project).through(:fork_network_member) } it { is_expected.to have_one(:auto_devops).class_name('ProjectAutoDevops') } + it { is_expected.to have_one(:tracing_setting).class_name('ProjectTracingSetting') } it { is_expected.to have_one(:error_tracking_setting).class_name('ErrorTracking::ProjectErrorTrackingSetting') } it { is_expected.to have_one(:project_setting) } it { is_expected.to have_one(:alerting_setting).class_name('Alerting::ProjectAlertingSetting') } @@ -116,6 +117,7 @@ RSpec.describe Project do it { is_expected.to have_many(:prometheus_alert_events) } it { is_expected.to have_many(:self_managed_prometheus_alert_events) } it { is_expected.to have_many(:alert_management_alerts) } + it { is_expected.to have_many(:alert_management_http_integrations) } it { is_expected.to have_many(:jira_imports) } it { is_expected.to have_many(:metrics_users_starred_dashboards).inverse_of(:project) } it { is_expected.to have_many(:repository_storage_moves) } @@ -123,6 +125,7 @@ RSpec.describe Project do it { is_expected.to have_many(:packages).class_name('Packages::Package') } it { is_expected.to have_many(:package_files).class_name('Packages::PackageFile') } it { is_expected.to have_many(:pipeline_artifacts) } + it { is_expected.to have_many(:terraform_states).class_name('Terraform::State').inverse_of(:project) } # GitLab Pages it { is_expected.to have_many(:pages_domains) } @@ -5812,6 +5815,38 @@ RSpec.describe Project do end end + describe 'validation #changing_shared_runners_enabled_is_allowed' do + using RSpec::Parameterized::TableSyntax + + where(:shared_runners_setting, :project_shared_runners_enabled, :valid_record) do + 'enabled' | true | true + 'enabled' | false | true + 'disabled_with_override' | true | true + 'disabled_with_override' | false | true + 'disabled_and_unoverridable' | true | false + 'disabled_and_unoverridable' | false | true + end + + with_them do + let(:group) { create(:group) } + let(:project) { build(:project, namespace: group, shared_runners_enabled: project_shared_runners_enabled) } + + before do + allow_next_found_instance_of(Group) do |group| + allow(group).to receive(:shared_runners_setting).and_return(shared_runners_setting) + end + end + + it 'validates the configuration' do + expect(project.valid?).to eq(valid_record) + + unless valid_record + expect(project.errors[:shared_runners_enabled]).to contain_exactly('cannot be enabled because parent group does not allow it') + end + end + end + end + describe '#mark_pages_as_deployed' do let(:project) { create(:project) } let(:artifacts_archive) { create(:ci_job_artifact, project: project) } diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 383fabcfffb..9f40dbb3401 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -201,6 +201,23 @@ RSpec.describe ProjectStatistics do statistics.refresh!(only: [:commit_count]) end end + + context 'when the database is read-only' do + it 'does nothing' do + allow(Gitlab::Database).to receive(:read_only?) { true } + + expect(statistics).not_to receive(:update_commit_count) + expect(statistics).not_to receive(:update_repository_size) + expect(statistics).not_to receive(:update_wiki_size) + expect(statistics).not_to receive(:update_lfs_objects_size) + expect(statistics).not_to receive(:update_snippets_size) + expect(statistics).not_to receive(:save!) + expect(Namespaces::ScheduleAggregationWorker) + .not_to receive(:perform_async) + + statistics.refresh! + end + end end describe '#update_commit_count' do @@ -324,22 +341,51 @@ RSpec.describe ProjectStatistics do describe '.increment_statistic' do shared_examples 'a statistic that increases storage_size' do it 'increases the statistic by that amount' do - expect { described_class.increment_statistic(project.id, stat, 13) } + expect { described_class.increment_statistic(project, stat, 13) } .to change { statistics.reload.send(stat) || 0 } .by(13) end it 'increases also storage size by that amount' do - expect { described_class.increment_statistic(project.id, stat, 20) } + expect { described_class.increment_statistic(project, stat, 20) } .to change { statistics.reload.storage_size } .by(20) end end + shared_examples 'a statistic that increases storage_size asynchronously' do + it 'stores the increment temporarily in Redis', :clean_gitlab_redis_shared_state do + described_class.increment_statistic(project, stat, 13) + + Gitlab::Redis::SharedState.with do |redis| + increment = redis.get(statistics.counter_key(stat)) + expect(increment.to_i).to eq(13) + end + end + + it 'schedules a worker to update the statistic and storage_size async' do + expect(FlushCounterIncrementsWorker) + .to receive(:perform_in) + .with(CounterAttribute::WORKER_DELAY, described_class.name, statistics.id, stat) + + expect(FlushCounterIncrementsWorker) + .to receive(:perform_in) + .with(CounterAttribute::WORKER_DELAY, described_class.name, statistics.id, :storage_size) + + described_class.increment_statistic(project, stat, 20) + end + end + context 'when adjusting :build_artifacts_size' do let(:stat) { :build_artifacts_size } - it_behaves_like 'a statistic that increases storage_size' + it_behaves_like 'a statistic that increases storage_size asynchronously' + + it_behaves_like 'a statistic that increases storage_size' do + before do + stub_feature_flags(efficient_counter_attribute: false) + end + end end context 'when adjusting :pipeline_artifacts_size' do diff --git a/spec/models/project_tracing_setting_spec.rb b/spec/models/project_tracing_setting_spec.rb new file mode 100644 index 00000000000..a7e4e557b25 --- /dev/null +++ b/spec/models/project_tracing_setting_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ProjectTracingSetting do + describe '#external_url' do + let_it_be(:project) { create(:project) } + + let(:tracing_setting) { project.build_tracing_setting } + + describe 'Validations' do + describe 'external_url' do + it 'accepts a valid url' do + tracing_setting.external_url = 'https://gitlab.com' + + expect(tracing_setting).to be_valid + end + + it 'fails with an invalid url' do + tracing_setting.external_url = 'gitlab.com' + + expect(tracing_setting).to be_invalid + end + + it 'fails with a blank string' do + tracing_setting.external_url = nil + + expect(tracing_setting).to be_invalid + end + + it 'sanitizes the url' do + tracing_setting.external_url = %{https://replaceme.com/'><script>alert(document.cookie)</script>} + + expect(tracing_setting).to be_valid + expect(tracing_setting.external_url).to eq(%{https://replaceme.com/'>}) + end + end + end + end +end diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb index 960db31d488..da1fe70c891 100644 --- a/spec/models/resource_label_event_spec.rb +++ b/spec/models/resource_label_event_spec.rb @@ -50,26 +50,36 @@ RSpec.describe ResourceLabelEvent, type: :model do end end - describe '#expire_etag_cache' do - def expect_expiration(issue) - expect_next_instance_of(Gitlab::EtagCaching::Store) do |instance| - expect(instance).to receive(:touch) - .with("/#{issue.project.namespace.to_param}/#{issue.project.to_param}/noteable/issue/#{issue.id}/notes") + context 'callbacks' do + describe '#usage_metrics' do + it 'tracks changed labels' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_label_changed_action) + + subject.save! end end - it 'expires resource note etag cache on event save' do - expect_expiration(subject.issuable) + describe '#expire_etag_cache' do + def expect_expiration(issue) + expect_next_instance_of(Gitlab::EtagCaching::Store) do |instance| + expect(instance).to receive(:touch) + .with("/#{issue.project.namespace.to_param}/#{issue.project.to_param}/noteable/issue/#{issue.id}/notes") + end + end - subject.save! - end + it 'expires resource note etag cache on event save' do + expect_expiration(subject.issuable) - it 'expires resource note etag cache on event destroy' do - subject.save! + subject.save! + end + + it 'expires resource note etag cache on event destroy' do + subject.save! - expect_expiration(subject.issuable) + expect_expiration(subject.issuable) - subject.destroy! + subject.destroy! + end end end diff --git a/spec/models/resource_milestone_event_spec.rb b/spec/models/resource_milestone_event_spec.rb index 0a5292b2d16..c1761e5b2e8 100644 --- a/spec/models/resource_milestone_event_spec.rb +++ b/spec/models/resource_milestone_event_spec.rb @@ -11,6 +11,7 @@ RSpec.describe ResourceMilestoneEvent, type: :model do it_behaves_like 'timebox resource event validations' it_behaves_like 'timebox resource event states' it_behaves_like 'timebox resource event actions' + it_behaves_like 'timebox resource tracks issue metrics', :milestone describe 'associations' do it { is_expected.to belong_to(:milestone) } diff --git a/spec/models/resource_state_event_spec.rb b/spec/models/resource_state_event_spec.rb index fc6575b2db8..b8a93bdbe3b 100644 --- a/spec/models/resource_state_event_spec.rb +++ b/spec/models/resource_state_event_spec.rb @@ -39,4 +39,20 @@ RSpec.describe ResourceStateEvent, type: :model do end end end + + context 'callbacks' do + describe '#usage_metrics' do + it 'tracks closed issues' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_closed_action) + + create(described_class.name.underscore.to_sym, issue: issue, state: described_class.states[:closed]) + end + + it 'tracks reopened issues' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_reopened_action) + + create(described_class.name.underscore.to_sym, issue: issue, state: described_class.states[:reopened]) + end + end + end end diff --git a/spec/models/resource_weight_event_spec.rb b/spec/models/resource_weight_event_spec.rb index 8a37883d933..170b302e094 100644 --- a/spec/models/resource_weight_event_spec.rb +++ b/spec/models/resource_weight_event_spec.rb @@ -73,4 +73,14 @@ RSpec.describe ResourceWeightEvent, type: :model do expect(event.discussion_id).to eq('73d167c478') end end + + context 'callbacks' do + describe '#usage_metrics' do + it 'tracks changed weights' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_weight_changed_action).with(author: user1) + + create(:resource_weight_event, issue: issue1, user: user1) + end + end + end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 32e2012e284..42a12a98d63 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -320,18 +320,28 @@ RSpec.describe Service do end it 'sets service to inactive' do - service = described_class.build_from_integration(project.id, integration) + service = described_class.build_from_integration(integration, project_id: project.id) expect(service).to be_valid expect(service.active).to be false end end - context 'when integration is an instance' do + context 'when integration is an instance-level integration' do let(:integration) { create(:jira_service, :instance) } it 'sets inherit_from_id from integration' do - service = described_class.build_from_integration(project.id, integration) + service = described_class.build_from_integration(integration, project_id: project.id) + + expect(service.inherit_from_id).to eq(integration.id) + end + end + + context 'when integration is a group-level integration' do + let(:integration) { create(:jira_service, group: group, project: nil) } + + it 'sets inherit_from_id from integration' do + service = described_class.build_from_integration(integration, project_id: project.id) expect(service.inherit_from_id).to eq(integration.id) end @@ -350,8 +360,22 @@ RSpec.describe Service do end shared_examples 'service creation from an integration' do - it 'creates a correct service' do - service = described_class.build_from_integration(project.id, integration) + it 'creates a correct service for a project integration' do + service = described_class.build_from_integration(integration, project_id: project.id) + + expect(service).to be_active + expect(service.url).to eq(url) + expect(service.api_url).to eq(api_url) + expect(service.username).to eq(username) + expect(service.password).to eq(password) + expect(service.template).to eq(false) + expect(service.instance).to eq(false) + expect(service.project).to eq(project) + expect(service.group).to eq(nil) + end + + it 'creates a correct service for a group integration' do + service = described_class.build_from_integration(integration, group_id: group.id) expect(service).to be_active expect(service.url).to eq(url) @@ -360,6 +384,8 @@ RSpec.describe Service do expect(service.password).to eq(password) expect(service.template).to eq(false) expect(service.instance).to eq(false) + expect(service.project).to eq(nil) + expect(service.group).to eq(group) end end diff --git a/spec/models/snippet_input_action_spec.rb b/spec/models/snippet_input_action_spec.rb index 43dc70bea98..0a9ab47f2f0 100644 --- a/spec/models/snippet_input_action_spec.rb +++ b/spec/models/snippet_input_action_spec.rb @@ -67,7 +67,7 @@ RSpec.describe SnippetInputAction do let(:options) { { action: action, file_path: file_path, content: content, previous_path: previous_path } } let(:expected_options) { options.merge(action: action.to_sym) } - subject { described_class.new(options).to_commit_action } + subject { described_class.new(**options).to_commit_action } it 'transforms attributes to commit action' do expect(subject).to eq(expected_options) diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index 95602a4de0e..30690ce2fa3 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -35,7 +35,7 @@ RSpec.describe SnippetRepository do it 'returns nil when files argument is empty' do expect(snippet.repository).not_to receive(:multi_action) - operation = snippet_repository.multi_files_action(user, [], commit_opts) + operation = snippet_repository.multi_files_action(user, [], **commit_opts) expect(operation).to be_nil end @@ -43,7 +43,7 @@ RSpec.describe SnippetRepository do it 'returns nil when files argument is nil' do expect(snippet.repository).not_to receive(:multi_action) - operation = snippet_repository.multi_files_action(user, nil, commit_opts) + operation = snippet_repository.multi_files_action(user, nil, **commit_opts) expect(operation).to be_nil end @@ -60,7 +60,7 @@ RSpec.describe SnippetRepository do end expect do - snippet_repository.multi_files_action(user, data, commit_opts) + snippet_repository.multi_files_action(user, data, **commit_opts) end.not_to raise_error aggregate_failures do @@ -77,13 +77,13 @@ RSpec.describe SnippetRepository do it 'tries to obtain an exclusive lease' do expect(Gitlab::ExclusiveLease).to receive(:new).with("multi_files_action:#{snippet.id}", anything).and_call_original - snippet_repository.multi_files_action(user, data, commit_opts) + snippet_repository.multi_files_action(user, data, **commit_opts) end it 'cancels the lease when the method has finished' do expect(Gitlab::ExclusiveLease).to receive(:cancel).with("multi_files_action:#{snippet.id}", anything).and_call_original - snippet_repository.multi_files_action(user, data, commit_opts) + snippet_repository.multi_files_action(user, data, **commit_opts) end it 'raises an error if the lease cannot be obtained' do @@ -92,7 +92,7 @@ RSpec.describe SnippetRepository do end expect do - snippet_repository.multi_files_action(user, data, commit_opts) + snippet_repository.multi_files_action(user, data, **commit_opts) end.to raise_error(described_class::CommitError) end @@ -114,7 +114,7 @@ RSpec.describe SnippetRepository do it 'infers the commit action based on the parameters if not present' do expect(repo).to receive(:multi_action).with(user, hash_including(actions: result)) - snippet_repository.multi_files_action(user, data, commit_opts) + snippet_repository.multi_files_action(user, data, **commit_opts) end context 'when commit actions are present' do @@ -128,7 +128,7 @@ RSpec.describe SnippetRepository do user, hash_including(actions: array_including(hash_including(action: expected_action))))) - snippet_repository.multi_files_action(user, data, commit_opts) + snippet_repository.multi_files_action(user, data, **commit_opts) end end @@ -149,7 +149,7 @@ RSpec.describe SnippetRepository do specify do existing_content = blob_at(snippet, previous_path).data - snippet_repository.multi_files_action(user, [move_action], commit_opts) + snippet_repository.multi_files_action(user, [move_action], **commit_opts) blob = blob_at(snippet, new_path) expect(blob).not_to be_nil @@ -177,7 +177,7 @@ RSpec.describe SnippetRepository do specify do last_commit_id = snippet.repository.head_commit.id - snippet_repository.multi_files_action(user, [update_action], commit_opts) + snippet_repository.multi_files_action(user, [update_action], **commit_opts) expect(snippet.repository.head_commit.id).to eq last_commit_id end @@ -214,13 +214,13 @@ RSpec.describe SnippetRepository do before do expect(blob_at(snippet, default_name)).to be_nil - snippet_repository.multi_files_action(user, [new_file], commit_opts) + snippet_repository.multi_files_action(user, [new_file], **commit_opts) expect(blob_at(snippet, default_name)).to be end it 'reuses the existing file name' do - snippet_repository.multi_files_action(user, [existing_file], commit_opts) + snippet_repository.multi_files_action(user, [existing_file], **commit_opts) blob = blob_at(snippet, default_name) expect(blob.data).to eq existing_file[:content] @@ -234,7 +234,7 @@ RSpec.describe SnippetRepository do it 'assigns a new name to the file' do expect(blob_at(snippet, default_name)).to be_nil - snippet_repository.multi_files_action(user, [new_file], commit_opts) + snippet_repository.multi_files_action(user, [new_file], **commit_opts) blob = blob_at(snippet, default_name) expect(blob.data).to eq new_file[:content] @@ -246,7 +246,7 @@ RSpec.describe SnippetRepository do before do expect do - snippet_repository.multi_files_action(user, data, commit_opts) + snippet_repository.multi_files_action(user, data, **commit_opts) end.not_to raise_error end @@ -259,10 +259,10 @@ RSpec.describe SnippetRepository do before do # Pre-populate repository with 9 unnamed snippets. - snippet_repository.multi_files_action(user, pre_populate_data, commit_opts) + snippet_repository.multi_files_action(user, pre_populate_data, **commit_opts) expect do - snippet_repository.multi_files_action(user, data, commit_opts) + snippet_repository.multi_files_action(user, data, **commit_opts) end.not_to raise_error end @@ -274,7 +274,7 @@ RSpec.describe SnippetRepository do it 'raises a path specific error' do expect do - snippet_repository.multi_files_action(user, data, commit_opts) + snippet_repository.multi_files_action(user, data, **commit_opts) end.to raise_error(error) end end diff --git a/spec/models/snippet_statistics_spec.rb b/spec/models/snippet_statistics_spec.rb index 8def6a0bbd4..1fb4ed47169 100644 --- a/spec/models/snippet_statistics_spec.rb +++ b/spec/models/snippet_statistics_spec.rb @@ -75,15 +75,28 @@ RSpec.describe SnippetStatistics do end describe '#refresh!' do - subject { statistics.refresh! } - it 'retrieves and saves statistic data from repository' do expect(statistics).to receive(:update_commit_count) expect(statistics).to receive(:update_file_count) expect(statistics).to receive(:update_repository_size) expect(statistics).to receive(:save!) - subject + statistics.refresh! + end + + context 'when the database is read-only' do + it 'does nothing' do + allow(Gitlab::Database).to receive(:read_only?) { true } + + expect(statistics).not_to receive(:update_commit_count) + expect(statistics).not_to receive(:update_file_count) + expect(statistics).not_to receive(:update_repository_size) + expect(statistics).not_to receive(:save!) + expect(Namespaces::ScheduleAggregationWorker) + .not_to receive(:perform_async) + + statistics.refresh! + end end end diff --git a/spec/models/terraform/state_spec.rb b/spec/models/terraform/state_spec.rb index 01ae80a61d1..1d99d103bb8 100644 --- a/spec/models/terraform/state_spec.rb +++ b/spec/models/terraform/state_spec.rb @@ -15,7 +15,24 @@ RSpec.describe Terraform::State do it { is_expected.to validate_presence_of(:project_id) } before do - stub_terraform_state_object_storage(Terraform::StateUploader) + stub_terraform_state_object_storage + end + + describe 'scopes' do + describe '.ordered_by_name' do + let_it_be(:project) { create(:project) } + let(:names) { %w(state_d state_b state_a state_c) } + + subject { described_class.ordered_by_name } + + before do + names.each do |name| + create(:terraform_state, project: project, name: name) + end + end + + it { expect(subject.map(&:name)).to eq(names.sort) } + end end describe '#file' do @@ -43,7 +60,7 @@ RSpec.describe Terraform::State do context 'when file is stored locally' do before do - stub_terraform_state_object_storage(Terraform::StateUploader, enabled: false) + stub_terraform_state_object_storage(enabled: false) end it_behaves_like 'mounted file in local store' diff --git a/spec/models/terraform/state_version_spec.rb b/spec/models/terraform/state_version_spec.rb index 72dd29e1571..cc5ea87159d 100644 --- a/spec/models/terraform/state_version_spec.rb +++ b/spec/models/terraform/state_version_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Terraform::StateVersion do subject { create(:terraform_state_version) } before do - stub_terraform_state_object_storage(Terraform::StateUploader) + stub_terraform_state_object_storage end describe '#file' do diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 44e81455a67..a9c4c6680cd 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -200,26 +200,42 @@ RSpec.describe Todo do describe '#self_assigned?' do let(:user_1) { build(:user) } - before do - subject.user = user_1 - subject.author = user_1 - subject.action = Todo::ASSIGNED - end + context 'when self_added' do + before do + subject.user = user_1 + subject.author = user_1 + end - it 'is true when todo is ASSIGNED and self_added' do - expect(subject).to be_self_assigned - end + it 'returns true for ASSIGNED' do + subject.action = Todo::ASSIGNED + + expect(subject).to be_self_assigned + end - it 'is false when the todo is not ASSIGNED' do - subject.action = Todo::MENTIONED + it 'returns true for REVIEW_REQUESTED' do + subject.action = Todo::REVIEW_REQUESTED - expect(subject).not_to be_self_assigned + expect(subject).to be_self_assigned + end + + it 'returns false for other action' do + subject.action = Todo::MENTIONED + + expect(subject).not_to be_self_assigned + end end - it 'is false when todo is not self_added' do - subject.author = build(:user) + context 'when todo is not self_added' do + before do + subject.user = user_1 + subject.author = build(:user) + end - expect(subject).not_to be_self_assigned + it 'returns false' do + subject.action = Todo::ASSIGNED + + expect(subject).not_to be_self_assigned + end end end @@ -427,7 +443,7 @@ RSpec.describe Todo do it 'updates updated_at' do create(:todo, :pending) - Timecop.freeze(1.day.from_now) do + travel_to(1.day.from_now) do expected_update_date = Time.current.utc ids = described_class.batch_update(state: :done) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 1841288cd4b..af5614ba85e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3902,7 +3902,7 @@ RSpec.describe User do it 'changes the namespace (just to compare to when username is not changed)' do expect do - Timecop.freeze(1.second.from_now) do + travel_to(1.second.from_now) do user.update!(username: new_username) end end.to change { user.namespace.updated_at } @@ -4330,28 +4330,32 @@ RSpec.describe User do describe '#required_terms_not_accepted?' do let(:user) { build(:user) } + let(:project_bot) { create(:user, :project_bot) } subject { user.required_terms_not_accepted? } context "when terms are not enforced" do - it { is_expected.to be_falsy } + it { is_expected.to be_falsey } end - context "when terms are enforced and accepted by the user" do + context "when terms are enforced" do before do enforce_terms - accept_terms(user) end - it { is_expected.to be_falsy } - end + it "is not accepted by the user" do + expect(subject).to be_truthy + end - context "when terms are enforced but the user has not accepted" do - before do - enforce_terms + it "is accepted by the user" do + accept_terms(user) + + expect(subject).to be_falsey end - it { is_expected.to be_truthy } + it "auto accepts the term for project bots" do + expect(project_bot.required_terms_not_accepted?).to be_falsey + end end end @@ -4895,7 +4899,7 @@ RSpec.describe User do user.block end - it { is_expected.to eq User::BLOCKED_MESSAGE } + it { is_expected.to eq :blocked } end context 'when user is an internal user' do @@ -4903,7 +4907,7 @@ RSpec.describe User do user.update(user_type: :ghost) end - it { is_expected.to be User::LOGIN_FORBIDDEN } + it { is_expected.to be :forbidden } end context 'when user is locked' do diff --git a/spec/models/wiki_directory_spec.rb b/spec/models/wiki_directory_spec.rb index 4cac90786eb..9b6cec99ddb 100644 --- a/spec/models/wiki_directory_spec.rb +++ b/spec/models/wiki_directory_spec.rb @@ -3,43 +3,97 @@ require 'spec_helper' RSpec.describe WikiDirectory do - describe 'validations' do - subject { build(:wiki_directory) } + subject(:directory) { build(:wiki_directory) } + describe 'validations' do it { is_expected.to validate_presence_of(:slug) } end + describe '.group_pages' do + let_it_be(:toplevel1) { build(:wiki_page, title: 'aaa-toplevel1') } + let_it_be(:toplevel2) { build(:wiki_page, title: 'zzz-toplevel2') } + let_it_be(:toplevel3) { build(:wiki_page, title: 'zzz-toplevel3') } + let_it_be(:child1) { build(:wiki_page, title: 'parent1/child1') } + let_it_be(:child2) { build(:wiki_page, title: 'parent1/child2') } + let_it_be(:child3) { build(:wiki_page, title: 'parent2/child3') } + let_it_be(:grandchild1) { build(:wiki_page, title: 'parent1/subparent/grandchild1') } + let_it_be(:grandchild2) { build(:wiki_page, title: 'parent1/subparent/grandchild2') } + + it 'returns a nested array of entries' do + entries = described_class.group_pages( + [toplevel1, toplevel2, toplevel3, child1, child2, child3, grandchild1, grandchild2].sort_by(&:title) + ) + + expect(entries).to match([ + toplevel1, + a_kind_of(WikiDirectory).and( + having_attributes( + slug: 'parent1', entries: [ + child1, + child2, + a_kind_of(WikiDirectory).and( + having_attributes( + slug: 'parent1/subparent', + entries: [grandchild1, grandchild2] + ) + ) + ] + ) + ), + a_kind_of(WikiDirectory).and( + having_attributes( + slug: 'parent2', + entries: [child3] + ) + ), + toplevel2, + toplevel3 + ]) + end + end + describe '#initialize' do - context 'when there are pages' do - let(:pages) { [build(:wiki_page)] } - let(:directory) { described_class.new('/path_up_to/dir', pages) } + context 'when there are entries' do + let(:entries) { [build(:wiki_page)] } + let(:directory) { described_class.new('/path_up_to/dir', entries) } it 'sets the slug attribute' do expect(directory.slug).to eq('/path_up_to/dir') end - it 'sets the pages attribute' do - expect(directory.pages).to eq(pages) + it 'sets the entries attribute' do + expect(directory.entries).to eq(entries) end end - context 'when there are no pages' do + context 'when there are no entries' do let(:directory) { described_class.new('/path_up_to/dir') } it 'sets the slug attribute' do expect(directory.slug).to eq('/path_up_to/dir') end - it 'sets the pages attribute to an empty array' do - expect(directory.pages).to eq([]) + it 'sets the entries attribute to an empty array' do + expect(directory.entries).to eq([]) end end end + describe '#title' do + it 'returns the basename of the directory, with hyphens replaced by spaces' do + directory.slug = 'parent' + expect(directory.title).to eq('parent') + + directory.slug = 'parent/child' + expect(directory.title).to eq('child') + + directory.slug = 'parent/child-foo' + expect(directory.title).to eq('child foo') + end + end + describe '#to_partial_path' do it 'returns the relative path to the partial to be used' do - directory = build(:wiki_directory) - expect(directory.to_partial_path).to eq('../shared/wikis/wiki_directory') end end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index aa8b9ce58b9..be94eca550c 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -4,16 +4,25 @@ require "spec_helper" RSpec.describe WikiPage do let_it_be(:user) { create(:user) } - let(:container) { create(:project, :wiki_repo) } - let(:wiki) { Wiki.for_container(container, user) } - let(:new_page) { build(:wiki_page, wiki: wiki, title: 'test page', content: 'test content') } + let_it_be(:container) { create(:project) } - let(:existing_page) do - create(:wiki_page, wiki: wiki, title: 'test page', content: 'test content', message: 'test commit') - wiki.find_page('test page') + def create_wiki_page(attrs = {}) + page = build_wiki_page(attrs) + + page.create(message: (attrs[:message] || 'test commit')) + + container.wiki.find_page(page.slug) end - subject { new_page } + def build_wiki_page(attrs = {}) + wiki_page_attrs = { container: container, content: 'test content' }.merge(attrs) + + build(:wiki_page, wiki_page_attrs) + end + + def wiki + container.wiki + end def disable_front_matter stub_feature_flags(Gitlab::WikiPages::FrontMatterParser::FEATURE_FLAG => false) @@ -23,92 +32,16 @@ RSpec.describe WikiPage do stub_feature_flags(Gitlab::WikiPages::FrontMatterParser::FEATURE_FLAG => thing) end - describe '.group_by_directory' do - context 'when there are no pages' do - it 'returns an empty array' do - expect(described_class.group_by_directory(nil)).to eq([]) - expect(described_class.group_by_directory([])).to eq([]) - end - end - - context 'when there are pages' do - before do - wiki.create_page('dir_1/dir_1_1/page_3', 'content') - wiki.create_page('page_1', 'content') - wiki.create_page('dir_1/page_2', 'content') - wiki.create_page('dir_2', 'page with dir name') - wiki.create_page('dir_2/page_5', 'content') - wiki.create_page('page_6', 'content') - wiki.create_page('dir_2/page_4', 'content') - end - - let(:page_1) { wiki.find_page('page_1') } - let(:page_6) { wiki.find_page('page_6') } - let(:page_dir_2) { wiki.find_page('dir_2') } - - let(:dir_1) do - WikiDirectory.new('dir_1', [wiki.find_page('dir_1/page_2')]) - end - - let(:dir_1_1) do - WikiDirectory.new('dir_1/dir_1_1', [wiki.find_page('dir_1/dir_1_1/page_3')]) - end - - let(:dir_2) do - pages = [wiki.find_page('dir_2/page_5'), - wiki.find_page('dir_2/page_4')] - WikiDirectory.new('dir_2', pages) - end + # Use for groups of tests that do not modify their `subject`. + # + # include_context 'subject is persisted page', title: 'my title' + shared_context 'subject is persisted page' do |attrs = {}| + let_it_be(:persisted_page) { create_wiki_page(attrs) } - describe "#list_pages" do - context 'sort by title' do - let(:grouped_entries) { described_class.group_by_directory(wiki.list_pages) } - let(:expected_grouped_entries) { [dir_1_1, dir_1, page_dir_2, dir_2, page_1, page_6] } - - it 'returns an array with pages and directories' do - grouped_entries.each_with_index do |page_or_dir, i| - expected_page_or_dir = expected_grouped_entries[i] - expected_slugs = get_slugs(expected_page_or_dir) - slugs = get_slugs(page_or_dir) - - expect(slugs).to match_array(expected_slugs) - end - end - end - - context 'sort by created_at' do - let(:grouped_entries) { described_class.group_by_directory(wiki.list_pages(sort: 'created_at')) } - let(:expected_grouped_entries) { [dir_1_1, page_1, dir_1, page_dir_2, dir_2, page_6] } - - it 'returns an array with pages and directories' do - grouped_entries.each_with_index do |page_or_dir, i| - expected_page_or_dir = expected_grouped_entries[i] - expected_slugs = get_slugs(expected_page_or_dir) - slugs = get_slugs(page_or_dir) - - expect(slugs).to match_array(expected_slugs) - end - end - end - - it 'returns an array with retained order with directories at the top' do - expected_order = ['dir_1/dir_1_1/page_3', 'dir_1/page_2', 'dir_2', 'dir_2/page_4', 'dir_2/page_5', 'page_1', 'page_6'] - - grouped_entries = described_class.group_by_directory(wiki.list_pages) - - actual_order = - grouped_entries.flat_map do |page_or_dir| - get_slugs(page_or_dir) - end - expect(actual_order).to eq(expected_order) - end - end - end + subject { persisted_page } end describe '#front_matter' do - let_it_be(:project) { create(:project) } - let(:container) { project } let(:wiki_page) { create(:wiki_page, container: container, content: content) } shared_examples 'a page without front-matter' do @@ -230,14 +163,14 @@ RSpec.describe WikiPage do describe "#initialize" do context "when initialized with an existing page" do - subject { existing_page } + include_context 'subject is persisted page', title: 'test initialization' it "sets the slug attribute" do - expect(subject.slug).to eq("test-page") + expect(subject.slug).to eq("test-initialization") end it "sets the title attribute" do - expect(subject.title).to eq("test page") + expect(subject.title).to eq("test initialization") end it "sets the formatted content attribute" do @@ -259,6 +192,8 @@ RSpec.describe WikiPage do end describe "validations" do + subject { build_wiki_page } + it "validates presence of title" do subject.attributes.delete(:title) @@ -305,7 +240,7 @@ RSpec.describe WikiPage do end context 'with an existing page exceeding the limit' do - subject { existing_page } + include_context 'subject is persisted page' before do subject @@ -414,18 +349,22 @@ RSpec.describe WikiPage do describe "#create" do let(:attributes) do { - title: "Index", + title: SecureRandom.hex, content: "Home Page", format: "markdown", message: 'Custom Commit Message' } end + let(:title) { attributes[:title] } + + subject { build_wiki_page } + context "with valid attributes" do it "saves the wiki page" do subject.create(attributes) - expect(wiki.find_page("Index")).not_to be_nil + expect(wiki.find_page(title)).not_to be_nil end it "returns true" do @@ -435,7 +374,7 @@ RSpec.describe WikiPage do it 'saves the wiki page with message' do subject.create(attributes) - expect(wiki.find_page("Index").message).to eq 'Custom Commit Message' + expect(wiki.find_page(title).message).to eq 'Custom Commit Message' end it 'if the title is preceded by a / it is removed' do @@ -447,9 +386,7 @@ RSpec.describe WikiPage do context "with invalid attributes" do it 'does not create the page' do - subject.create(title: '') - - expect(wiki.find_page('New Page')).to be_nil + expect { subject.create(title: '') }.not_to change { wiki.list_pages.length } end end end @@ -458,46 +395,40 @@ RSpec.describe WikiPage do let(:title) { 'Index v1.2.3' } describe "#create" do - let(:attributes) { { title: title, content: "Home Page", format: "markdown" } } - - context "with valid attributes" do - it "saves the wiki page" do - subject.create(attributes) + subject { build_wiki_page } - expect(wiki.find_page(title)).not_to be_nil - end + it "saves the wiki page and returns true", :aggregate_failures do + attributes = { title: title, content: "Home Page", format: "markdown" } - it "returns true" do - expect(subject.create(attributes)).to eq(true) - end + expect(subject.create(attributes)).to eq(true) + expect(wiki.find_page(title)).not_to be_nil end end describe '#update' do - subject { create(:wiki_page, wiki: wiki, title: title) } + subject { create_wiki_page(title: title) } + + it 'updates the content of the page and returns true', :aggregate_failures do + expect(subject.update(content: 'new content')).to be_truthy - it 'updates the content of the page' do - subject.update(content: 'new content') page = wiki.find_page(title) expect([subject.content, page.content]).to all(eq('new content')) end - - it "returns true" do - expect(subject.update(content: "more content")).to be_truthy - end end end describe "#update" do - subject { existing_page } + let!(:original_title) { subject.title } + + subject { create_wiki_page } context "with valid attributes" do it "updates the content of the page" do new_content = "new content" subject.update(content: new_content) - page = wiki.find_page('test page') + page = wiki.find_page(original_title) expect([subject.content, page.content]).to all(eq("new content")) end @@ -514,10 +445,9 @@ RSpec.describe WikiPage do describe 'updating front_matter' do shared_examples 'able to update front-matter' do it 'updates the wiki-page front-matter' do - title = subject.title content = subject.content subject.update(front_matter: { slugs: ['x'] }) - page = wiki.find_page(title) + page = wiki.find_page(original_title) expect([subject, page]).to all( have_attributes( @@ -566,10 +496,9 @@ RSpec.describe WikiPage do end it 'updates the wiki-page front-matter and content together' do - title = subject.title content = 'totally new content' subject.update(content: content, front_matter: { slugs: ['x'] }) - page = wiki.find_page(title) + page = wiki.find_page(original_title) expect([subject, page]).to all( have_attributes( @@ -598,11 +527,11 @@ RSpec.describe WikiPage do context 'when renaming a page' do it 'raises an error if the page already exists' do - wiki.create_page('Existing Page', 'content') + existing_page = create_wiki_page - expect { subject.update(title: 'Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError) - expect(subject.title).to eq 'test page' - expect(subject.content).to eq 'new_content' + expect { subject.update(title: existing_page.title, content: 'new_content') }.to raise_error(WikiPage::PageRenameError) + expect(subject.title).to eq original_title + expect(subject.content).to eq 'new_content' # We don't revert the content end it 'updates the content and rename the file' do @@ -623,7 +552,7 @@ RSpec.describe WikiPage do wiki.create_page('foo/Existing Page', 'content') expect { subject.update(title: 'foo/Existing Page', content: 'new_content') }.to raise_error(WikiPage::PageRenameError) - expect(subject.title).to eq 'test page' + expect(subject.title).to eq original_title expect(subject.content).to eq 'new_content' end @@ -639,20 +568,22 @@ RSpec.describe WikiPage do expect(page.content).to eq new_content end - context 'in subdir' do - subject { create(:wiki_page, wiki: wiki, title: 'foo/Existing Page') } - + describe 'in subdir' do it 'moves the page to the root folder if the title is preceded by /' do - expect(subject.slug).to eq 'foo/Existing-Page' - expect(subject.update(title: '/Existing Page', content: 'new_content')).to be_truthy - expect(subject.slug).to eq 'Existing-Page' + page = create_wiki_page(title: 'foo/Existing Page') + + expect(page.slug).to eq 'foo/Existing-Page' + expect(page.update(title: '/Existing Page', content: 'new_content')).to be_truthy + expect(page.slug).to eq 'Existing-Page' end it 'does nothing if it has the same title' do - original_path = subject.slug + page = create_wiki_page(title: 'foo/Another Existing Page') - expect(subject.update(title: 'Existing Page', content: 'new_content')).to be_truthy - expect(subject.slug).to eq original_path + original_path = page.slug + + expect(page.update(title: 'Another Existing Page', content: 'new_content')).to be_truthy + expect(page.slug).to eq original_path end end @@ -660,7 +591,7 @@ RSpec.describe WikiPage do it 'does nothing if the title is preceded by /' do original_path = subject.slug - expect(subject.update(title: '/test page', content: 'new_content')).to be_truthy + expect(subject.update(title: "/#{subject.title}", content: 'new_content')).to be_truthy expect(subject.slug).to eq original_path end end @@ -671,7 +602,7 @@ RSpec.describe WikiPage do expect(subject.update(title: '', content: 'new_content')).to be_falsey expect(subject.content).to eq 'new_content' - page = wiki.find_page('test page') + page = wiki.find_page(original_title) expect(page.content).to eq 'test content' end @@ -679,21 +610,17 @@ RSpec.describe WikiPage do end describe "#delete" do - subject { existing_page } - - it "deletes the page" do - subject.delete - - expect(wiki.list_pages).to be_empty - end + it "deletes the page and returns true", :aggregate_failures do + page = create_wiki_page - it "returns true" do - expect(subject.delete).to eq(true) + expect do + expect(page.delete).to eq(true) + end.to change { wiki.list_pages.length }.by(-1) end end describe "#versions" do - subject { existing_page } + include_context 'subject is persisted page' it "returns an array of all commits for the page" do 3.times { |i| subject.update(content: "content #{i}") } @@ -709,19 +636,21 @@ RSpec.describe WikiPage do describe '#title_changed?' do using RSpec::Parameterized::TableSyntax + let_it_be(:unsaved_page) { build_wiki_page(title: 'test page') } + let_it_be(:existing_page) { create_wiki_page(title: 'test page') } + let_it_be(:directory_page) { create_wiki_page(title: 'parent directory/child page') } + let_it_be(:page_with_special_characters) { create_wiki_page(title: 'test+page') } let(:untitled_page) { described_class.new(wiki) } - let(:directory_page) { create(:wiki_page, title: 'parent directory/child page') } - let(:page_with_special_characters) { create(:wiki_page, title: 'test+page') } where(:page, :title, :changed) do :untitled_page | nil | false :untitled_page | 'new title' | true - :new_page | nil | true - :new_page | 'test page' | true - :new_page | 'test-page' | true - :new_page | 'test+page' | true - :new_page | 'new title' | true + :unsaved_page | nil | true + :unsaved_page | 'test page' | true + :unsaved_page | 'test-page' | true + :unsaved_page | 'test+page' | true + :unsaved_page | 'new title' | true :existing_page | nil | false :existing_page | 'test page' | false @@ -764,7 +693,7 @@ RSpec.describe WikiPage do describe '#content_changed?' do context 'with a new page' do - subject { new_page } + subject { build_wiki_page } it 'returns true if content is set' do subject.attributes[:content] = 'new' @@ -780,7 +709,7 @@ RSpec.describe WikiPage do end context 'with an existing page' do - subject { existing_page } + include_context 'subject is persisted page' it 'returns false' do expect(subject.content_changed?).to be(false) @@ -816,17 +745,21 @@ RSpec.describe WikiPage do describe '#path' do it 'returns the path when persisted' do - expect(existing_page.path).to eq('test-page.md') + existing_page = create_wiki_page(title: 'path test') + + expect(existing_page.path).to eq('path-test.md') end it 'returns nil when not persisted' do - expect(new_page.path).to be_nil + unsaved_page = build_wiki_page(title: 'path test') + + expect(unsaved_page.path).to be_nil end end describe '#directory' do context 'when the page is at the root directory' do - subject { existing_page } + include_context 'subject is persisted page', title: 'directory test' it 'returns an empty string' do expect(subject.directory).to eq('') @@ -834,7 +767,7 @@ RSpec.describe WikiPage do end context 'when the page is inside an actual directory' do - subject { create(:wiki_page, title: 'dir_1/dir_1_1/file') } + include_context 'subject is persisted page', title: 'dir_1/dir_1_1/directory test' it 'returns the full directory hierarchy' do expect(subject.directory).to eq('dir_1/dir_1_1') @@ -843,7 +776,7 @@ RSpec.describe WikiPage do end describe '#historical?' do - subject { existing_page } + include_context 'subject is persisted page' let(:old_version) { subject.versions.last.id } let(:old_page) { wiki.find_page(subject.title, old_version) } @@ -883,22 +816,22 @@ RSpec.describe WikiPage do describe '#persisted?' do it 'returns true for a persisted page' do - expect(existing_page).to be_persisted + expect(create_wiki_page).to be_persisted end it 'returns false for an unpersisted page' do - expect(new_page).not_to be_persisted + expect(build_wiki_page).not_to be_persisted end end describe '#to_partial_path' do it 'returns the relative path to the partial to be used' do - expect(subject.to_partial_path).to eq('../shared/wikis/wiki_page') + expect(build_wiki_page.to_partial_path).to eq('../shared/wikis/wiki_page') end end describe '#==' do - subject { existing_page } + include_context 'subject is persisted page' it 'returns true for identical wiki page' do expect(subject).to eq(subject) @@ -906,7 +839,7 @@ RSpec.describe WikiPage do it 'returns true for updated wiki page' do subject.update(content: "Updated content") - updated_page = wiki.find_page(existing_page.slug) + updated_page = wiki.find_page(subject.slug) expect(updated_page).not_to be_nil expect(updated_page).to eq(subject) @@ -921,7 +854,7 @@ RSpec.describe WikiPage do end it 'returns false for page with different slug on same container' do - other_page = create(:wiki_page, container: subject.container) + other_page = create_wiki_page expect(subject.slug).not_to eq(other_page.slug) expect(subject.container).to eq(other_page.container) @@ -929,7 +862,7 @@ RSpec.describe WikiPage do end it 'returns false for page with the same slug on a different container' do - other_page = create(:wiki_page, title: existing_page.slug) + other_page = create(:wiki_page, title: subject.slug) expect(subject.slug).to eq(other_page.slug) expect(subject.container).not_to eq(other_page.container) @@ -938,7 +871,7 @@ RSpec.describe WikiPage do end describe '#last_commit_sha' do - subject { existing_page } + include_context 'subject is persisted page' it 'returns commit sha' do expect(subject.last_commit_sha).to eq subject.last_version.sha @@ -948,13 +881,15 @@ RSpec.describe WikiPage do last_commit_sha_before_update = subject.last_commit_sha subject.update(content: "new content") - page = wiki.find_page('test page') + page = wiki.find_page(subject.title) expect(page.last_commit_sha).not_to eq last_commit_sha_before_update end end describe '#hook_attrs' do + subject { build_wiki_page } + it 'adds absolute urls for images in the content' do subject.attributes[:content] = 'test' @@ -965,19 +900,21 @@ RSpec.describe WikiPage do describe '#version_commit_timestamp' do context 'for a new page' do it 'returns nil' do - expect(new_page.version_commit_timestamp).to be_nil + expect(build_wiki_page.version_commit_timestamp).to be_nil end end context 'for page that exists' do it 'returns the timestamp of the commit' do + existing_page = create_wiki_page + expect(existing_page.version_commit_timestamp).to eq(existing_page.version.commit.committed_date) end end end describe '#diffs' do - subject { existing_page } + include_context 'subject is persisted page' it 'returns a diff instance' do diffs = subject.diffs(foo: 'bar') @@ -993,14 +930,4 @@ RSpec.describe WikiPage do ) end end - - private - - def get_slugs(page_or_dir) - if page_or_dir.is_a? WikiPage - [page_or_dir.slug] - else - page_or_dir.pages.present? ? page_or_dir.pages.map(&:slug) : [] - end - end end |