diff options
Diffstat (limited to 'spec/models')
107 files changed, 3235 insertions, 1170 deletions
diff --git a/spec/models/alert_management/alert_spec.rb b/spec/models/alert_management/alert_spec.rb index eb9dcca842d..b57062b5fc1 100644 --- a/spec/models/alert_management/alert_spec.rb +++ b/spec/models/alert_management/alert_spec.rb @@ -133,7 +133,7 @@ RSpec.describe AlertManagement::Alert do let(:new_alert) { build(:alert_management_alert, new_status, fingerprint: fingerprint, project: project) } before do - existing_alert.public_send(described_class::STATUS_EVENTS[existing_status]) + existing_alert.change_status_to(existing_status) end if params[:valid] @@ -170,6 +170,12 @@ RSpec.describe AlertManagement::Alert do it { is_expected.to be_valid } end + + context 'nested array' do + let(:hosts) { ['111.111.111.111', ['111.111.111.111']] } + + it { is_expected.not_to be_valid } + end end end @@ -189,14 +195,14 @@ RSpec.describe AlertManagement::Alert do end describe '.for_status' do - let(:status) { AlertManagement::Alert::STATUSES[:resolved] } + let(:status) { :resolved } subject { AlertManagement::Alert.for_status(status) } it { is_expected.to match_array(resolved_alert) } context 'with multiple statuses' do - let(:status) { AlertManagement::Alert::STATUSES.values_at(:resolved, :ignored) } + let(:status) { [:resolved, :ignored] } it { is_expected.to match_array([resolved_alert, ignored_alert]) } end @@ -230,6 +236,35 @@ RSpec.describe AlertManagement::Alert do it { is_expected.to match_array(env_alert) } end + describe '.for_assignee_username' do + let_it_be(:alert) { triggered_alert } + let_it_be(:assignee) { create(:user) } + + subject { AlertManagement::Alert.for_assignee_username(assignee_username) } + + before_all do + alert.update!(assignees: [assignee]) + end + + context 'when matching assignee_username' do + let(:assignee_username) { assignee.username } + + it { is_expected.to contain_exactly(alert) } + end + + context 'when unknown assignee_username' do + let(:assignee_username) { 'unknown username' } + + it { is_expected.to be_empty } + end + + context 'with empty assignee_username' do + let(:assignee_username) { ' ' } + + it { is_expected.to be_empty } + end + end + describe '.order_severity_with_open_prometheus_alert' do subject { described_class.where(project: alert_project).order_severity_with_open_prometheus_alert } @@ -241,19 +276,6 @@ RSpec.describe AlertManagement::Alert do it { is_expected.to eq([triggered_critical_alert, triggered_high_alert]) } end - describe '.counts_by_status' do - subject { described_class.counts_by_status } - - it do - is_expected.to eq( - triggered_alert.status => 1, - acknowledged_alert.status => 1, - resolved_alert.status => 1, - ignored_alert.status => 1 - ) - end - end - describe '.counts_by_project_id' do subject { described_class.counts_by_project_id } @@ -278,6 +300,55 @@ RSpec.describe AlertManagement::Alert do end end + describe '.status_value' do + using RSpec::Parameterized::TableSyntax + + where(:status, :status_value) do + :triggered | 0 + :acknowledged | 1 + :resolved | 2 + :ignored | 3 + :unknown | nil + end + + with_them do + it 'returns status value by its name' do + expect(described_class.status_value(status)).to eq(status_value) + end + end + end + + describe '.status_name' do + using RSpec::Parameterized::TableSyntax + + where(:raw_status, :status) do + 0 | :triggered + 1 | :acknowledged + 2 | :resolved + 3 | :ignored + -1 | nil + end + + with_them do + it 'returns status name by its values' do + expect(described_class.status_name(raw_status)).to eq(status) + end + end + end + + describe '.counts_by_status' do + subject { described_class.counts_by_status } + + it do + is_expected.to eq( + triggered: 1, + acknowledged: 1, + resolved: 1, + ignored: 1 + ) + end + end + describe '.last_prometheus_alert_by_project_id' do subject { described_class.last_prometheus_alert_by_project_id } @@ -363,6 +434,24 @@ RSpec.describe AlertManagement::Alert do end end + describe '.open_status?' do + using RSpec::Parameterized::TableSyntax + + where(:status, :is_open_status) do + :triggered | true + :acknowledged | true + :resolved | false + :ignored | false + nil | false + end + + with_them do + it 'returns true when the status is open status' do + expect(described_class.open_status?(status)).to eq(is_open_status) + end + end + end + describe '#to_reference' do it { expect(triggered_alert.to_reference).to eq("^alert##{triggered_alert.iid}") } end @@ -453,4 +542,54 @@ RSpec.describe AlertManagement::Alert do expect { subject }.to change { alert.events }.by(1) end end + + describe '#status_event_for' do + using RSpec::Parameterized::TableSyntax + + where(:for_status, :event) do + :triggered | :trigger + 'triggered' | :trigger + :acknowledged | :acknowledge + 'acknowledged' | :acknowledge + :resolved | :resolve + 'resolved' | :resolve + :ignored | :ignore + 'ignored' | :ignore + :unknown | nil + nil | nil + '' | nil + 1 | nil + end + + with_them do + let(:alert) { build(:alert_management_alert, project: project) } + + it 'returns event by status name' do + expect(alert.status_event_for(for_status)).to eq(event) + end + end + end + + describe '#change_status_to' do + let_it_be_with_reload(:alert) { create(:alert_management_alert, project: project) } + + context 'with valid statuses' do + it 'changes the status to triggered' do + alert.acknowledge! # change to non-triggered status + expect { alert.change_status_to(:triggered) }.to change { alert.triggered? }.to(true) + end + + %i(acknowledged resolved ignored).each do |status| + it "changes the status to #{status}" do + expect { alert.change_status_to(status) }.to change { alert.public_send(:"#{status}?") }.to(true) + end + end + end + + context 'with invalid status' do + it 'does not change the current status' do + expect { alert.change_status_to(nil) }.not_to change { alert.status } + end + end + end end 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_record_spec.rb b/spec/models/application_record_spec.rb index 5ea1907543a..d080b298e2f 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -90,4 +90,12 @@ RSpec.describe ApplicationRecord do expect(User.at_most(2).count).to eq(2) end end + + describe '.where_exists' do + it 'produces a WHERE EXISTS query' do + user = create(:user) + + expect(User.where_exists(User.limit(1))).to eq([user]) + 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..fb702d10a42 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -112,6 +112,35 @@ RSpec.describe ApplicationSetting do it { is_expected.to allow_value(nil).for(:repository_storages_weighted_default) } it { is_expected.not_to allow_value({ default: 100, shouldntexist: 50 }).for(:repository_storages_weighted) } + context 'help_page_documentation_base_url validations' do + it { is_expected.to allow_value(nil).for(:help_page_documentation_base_url) } + it { is_expected.to allow_value('https://docs.gitlab.com').for(:help_page_documentation_base_url) } + it { is_expected.to allow_value('http://127.0.0.1').for(:help_page_documentation_base_url) } + it { is_expected.not_to allow_value('docs.gitlab.com').for(:help_page_documentation_base_url) } + + context 'when url length validation' do + let(:value) { 'http://'.ljust(length, 'A') } + + context 'when value string length is 255 characters' do + let(:length) { 255 } + + it 'allows the value' do + is_expected.to allow_value(value).for(:help_page_documentation_base_url) + end + end + + context 'when value string length exceeds 255 characters' do + let(:length) { 256 } + + it 'does not allow the value' do + is_expected.not_to allow_value(value) + .for(:help_page_documentation_base_url) + .with_message('is too long (maximum is 255 characters)') + end + end + end + end + context 'grafana_url validations' do before do subject.instance_variable_set(:@parsed_grafana_url, nil) @@ -320,7 +349,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 @@ -778,6 +807,23 @@ RSpec.describe ApplicationSetting do end end + describe '#instance_review_permitted?', :request_store do + subject { setting.instance_review_permitted? } + + before do + RequestStore.store[:current_license] = nil + expect(Rails.cache).to receive(:fetch).and_return( + ::ApplicationSetting::INSTANCE_REVIEW_MIN_USERS + users_over_minimum + ) + end + + where(users_over_minimum: [-1, 0, 1]) + + with_them do + it { is_expected.to be(users_over_minimum >= 0) } + end + end + describe 'email_restrictions' do context 'when email restrictions are enabled' do before do 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/blob_viewer/markup_spec.rb b/spec/models/blob_viewer/markup_spec.rb new file mode 100644 index 00000000000..13b040d62d0 --- /dev/null +++ b/spec/models/blob_viewer/markup_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BlobViewer::Markup do + include FakeBlobHelpers + + let(:project) { create(:project, :repository) } + let(:blob) { fake_blob(path: 'CHANGELOG.md') } + + subject { described_class.new(blob) } + + describe '#banzai_render_context' do + it 'returns context needed for banzai rendering' do + expect(subject.banzai_render_context.keys).to eq([:cache_key]) + end + + context 'when blob does respond to rendered_markup' do + before do + allow(blob).to receive(:rendered_markup).and_return("some rendered markup") + end + + it 'does sets rendered key' do + expect(subject.banzai_render_context.keys).to include(:rendered) + end + end + + context 'when cached_markdown_blob feature flag is disabled' do + before do + stub_feature_flags(cached_markdown_blob: false) + end + + it 'does not set cache_key key' do + expect(subject.banzai_render_context.keys).not_to include(:cache_key) + end + end + end +end diff --git a/spec/models/bulk_import_spec.rb b/spec/models/bulk_import_spec.rb new file mode 100644 index 00000000000..1a7e1ed8119 --- /dev/null +++ b/spec/models/bulk_import_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImport, type: :model do + describe 'associations' do + it { is_expected.to belong_to(:user).required } + it { is_expected.to have_one(:configuration) } + it { is_expected.to have_many(:entities) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:source_type) } + it { is_expected.to validate_presence_of(:status) } + + it { is_expected.to define_enum_for(:source_type).with_values(%i[gitlab]) } + end +end diff --git a/spec/models/bulk_imports/configuration_spec.rb b/spec/models/bulk_imports/configuration_spec.rb new file mode 100644 index 00000000000..1cbfef631ac --- /dev/null +++ b/spec/models/bulk_imports/configuration_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::Configuration, type: :model do + describe 'associations' do + it { is_expected.to belong_to(:bulk_import).required } + end + + describe 'validations' do + it { is_expected.to validate_length_of(:url).is_at_most(255) } + it { is_expected.to validate_length_of(:access_token).is_at_most(255) } + + it { is_expected.to validate_presence_of(:url) } + it { is_expected.to validate_presence_of(:access_token) } + end +end diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb new file mode 100644 index 00000000000..ad6e3ec6f30 --- /dev/null +++ b/spec/models/bulk_imports/entity_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BulkImports::Entity, type: :model do + describe 'associations' do + it { is_expected.to belong_to(:bulk_import).required } + it { is_expected.to belong_to(:parent) } + it { is_expected.to belong_to(:group) } + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:source_type) } + it { is_expected.to validate_presence_of(:source_full_path) } + it { is_expected.to validate_presence_of(:destination_name) } + it { is_expected.to validate_presence_of(:destination_namespace) } + + it { is_expected.to define_enum_for(:source_type).with_values(%i[group_entity project_entity]) } + + context 'when associated with a group and project' do + it 'is invalid' do + entity = build(:bulk_import_entity, group: build(:group), project: build(:project)) + + expect(entity).not_to be_valid + expect(entity.errors).to include(:project, :group) + end + end + + context 'when not associated with a group or project' do + it 'is valid' do + entity = build(:bulk_import_entity, group: nil, project: nil) + + expect(entity).to be_valid + end + end + + context 'when associated with a group and no project' do + it 'is valid as a group_entity' do + entity = build(:bulk_import_entity, :group_entity, group: build(:group), project: nil) + + expect(entity).to be_valid + end + + it 'is invalid as a project_entity' do + entity = build(:bulk_import_entity, :project_entity, group: build(:group), project: nil) + + expect(entity).not_to be_valid + expect(entity.errors).to include(:group) + end + end + + context 'when associated with a project and no group' do + it 'is valid' do + entity = build(:bulk_import_entity, :project_entity, group: nil, project: build(:project)) + + expect(entity).to be_valid + end + + it 'is invalid as a project_entity' do + entity = build(:bulk_import_entity, :group_entity, group: nil, project: build(:project)) + + expect(entity).not_to be_valid + expect(entity.errors).to include(:project) + end + end + + context 'when the parent is a group import' do + it 'is valid' do + entity = build(:bulk_import_entity, parent: build(:bulk_import_entity, :group_entity)) + + expect(entity).to be_valid + end + end + + context 'when the parent is a project import' do + it 'is invalid' do + entity = build(:bulk_import_entity, parent: build(:bulk_import_entity, :project_entity)) + + expect(entity).not_to be_valid + expect(entity.errors).to include(:parent) + end + end + end +end diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 850fc1ec6e6..c464e176c17 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -59,30 +59,20 @@ RSpec.describe Ci::Bridge do describe 'state machine transitions' do context 'when bridge points towards downstream' do - it 'schedules downstream pipeline creation' do - expect(bridge).to receive(:schedule_downstream_pipeline!) + %i[created manual].each do |status| + it "schedules downstream pipeline creation when the status is #{status}" do + bridge.status = status - bridge.enqueue! - end - end - end - - describe 'state machine transitions' do - context 'when bridge points towards downstream' do - it 'schedules downstream pipeline creation' do - expect(bridge).to receive(:schedule_downstream_pipeline!) + expect(bridge).to receive(:schedule_downstream_pipeline!) - bridge.enqueue! + bridge.enqueue! + end end - end - end - describe 'state machine transitions' do - context 'when bridge points towards downstream' do - it 'schedules downstream pipeline creation' do - expect(bridge).to receive(:schedule_downstream_pipeline!) + it 'raises error when the status is failed' do + bridge.status = :failed - bridge.enqueue! + expect { bridge.enqueue! }.to raise_error(StateMachines::InvalidTransition) end end end @@ -304,4 +294,67 @@ RSpec.describe Ci::Bridge do end end end + + describe '#play' do + let(:downstream_project) { create(:project) } + let(:user) { create(:user) } + let(:bridge) { create(:ci_bridge, :playable, pipeline: pipeline, downstream: downstream_project) } + + subject { bridge.play(user) } + + before do + project.add_maintainer(user) + downstream_project.add_maintainer(user) + end + + it 'enqueues the bridge' do + subject + + expect(bridge).to be_pending + end + end + + describe '#playable?' do + context 'when bridge is a manual action' do + subject { build_stubbed(:ci_bridge, :manual).playable? } + + it { is_expected.to be_truthy } + + context 'when FF ci_manual_bridges is disabled' do + before do + stub_feature_flags(ci_manual_bridges: false) + end + + it { is_expected.to be_falsey } + end + end + + context 'when build is not a manual action' do + subject { build_stubbed(:ci_bridge, :created).playable? } + + it { is_expected.to be_falsey } + end + end + + describe '#action?' do + context 'when bridge is a manual action' do + subject { build_stubbed(:ci_bridge, :manual).action? } + + it { is_expected.to be_truthy } + + context 'when FF ci_manual_bridges is disabled' do + before do + stub_feature_flags(ci_manual_bridges: false) + end + + it { is_expected.to be_falsey } + end + end + + context 'when build is not a manual action' do + subject { build_stubbed(:ci_bridge, :created).action? } + + it { is_expected.to be_falsey } + 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..f1d51324bbf 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1109,7 +1109,8 @@ RSpec.describe Ci::Build do let(:environment) { deployment.environment } before do - allow(Deployments::FinishedWorker).to receive(:perform_async) + allow(Deployments::LinkMergeRequestWorker).to receive(:perform_async) + allow(Deployments::ExecuteHooksWorker).to receive(:perform_async) end it 'has deployments record with created status' do @@ -1129,7 +1130,8 @@ RSpec.describe Ci::Build do context 'when transits to success' do before do - allow(Deployments::SuccessWorker).to receive(:perform_async) + allow(Deployments::UpdateEnvironmentWorker).to receive(:perform_async) + allow(Deployments::ExecuteHooksWorker).to receive(:perform_async) build.success! end @@ -2305,6 +2307,54 @@ RSpec.describe Ci::Build do end end + describe '#has_expired_locked_archive_artifacts?' do + subject { build.has_expired_locked_archive_artifacts? } + + context 'when build does not have artifacts' do + it { is_expected.to eq(nil) } + end + + context 'when build has artifacts' do + before do + create(:ci_job_artifact, :archive, job: build) + end + + context 'when artifacts are unlocked' do + before do + build.pipeline.unlocked! + end + + it { is_expected.to eq(false) } + end + + context 'when artifacts are locked' do + before do + build.pipeline.artifacts_locked! + end + + context 'when artifacts do not expire' do + it { is_expected.to eq(false) } + end + + context 'when artifacts expire in the future' do + before do + build.update!(artifacts_expire_at: 1.day.from_now) + end + + it { is_expected.to eq(false) } + end + + context 'when artifacts expired in the past' do + before do + build.update!(artifacts_expire_at: 1.day.ago) + end + + it { is_expected.to eq(true) } + end + end + end + end + describe '#has_expiring_archive_artifacts?' do context 'when artifacts have expiration date set' do before do @@ -2504,94 +2554,6 @@ RSpec.describe Ci::Build do end end - describe 'CHANGED_PAGES variables' do - let(:route_map_yaml) do - <<~ROUTEMAP - - source: 'bar/branch-test.txt' - public: '/bar/branches' - - source: 'with space/README.md' - public: '/README' - ROUTEMAP - end - - before do - allow_any_instance_of(Project) - .to receive(:route_map_for).with(/.+/) - .and_return(Gitlab::RouteMap.new(route_map_yaml)) - end - - context 'with a deployment environment and a merge request' do - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:environment) { create(:environment, project: merge_request.project, name: "foo-#{project.default_branch}") } - let(:build) { create(:ci_build, pipeline: pipeline, environment: environment.name) } - - let(:full_urls) do - [ - File.join(environment.external_url, '/bar/branches'), - File.join(environment.external_url, '/README') - ] - end - - it 'populates CI_MERGE_REQUEST_CHANGED_PAGES_* variables' do - expect(subject).to include( - { - key: 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS', - value: '/bar/branches,/README', - public: true, - masked: false - }, - { - key: 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS', - value: full_urls.join(','), - public: true, - masked: false - } - ) - end - - context 'with a deployment environment and no merge request' do - let(:environment) { create(:environment, project: project, name: "foo-#{project.default_branch}") } - let(:build) { create(:ci_build, pipeline: pipeline, environment: environment.name) } - - it 'does not append CHANGED_PAGES variables' do - ci_variables = subject.select { |var| var[:key] =~ /MERGE_REQUEST_CHANGED_PAGES/ } - - expect(ci_variables).to be_empty - end - end - - context 'with no deployment environment and a present merge request' do - let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline, source_project: project, target_project: project) } - let(:build) { create(:ci_build, pipeline: merge_request.all_pipelines.take) } - - it 'does not append CHANGED_PAGES variables' do - ci_variables = subject.select { |var| var[:key] =~ /MERGE_REQUEST_CHANGED_PAGES/ } - - expect(ci_variables).to be_empty - end - end - - context 'with no deployment environment and no merge request' do - it 'does not append CHANGED_PAGES variables' do - ci_variables = subject.select { |var| var[:key] =~ /MERGE_REQUEST_CHANGED_PAGES/ } - - expect(ci_variables).to be_empty - end - end - end - - context 'with the :modified_path_ci_variables feature flag disabled' do - before do - stub_feature_flags(modified_path_ci_variables: false) - end - - it 'does not set CI_MERGE_REQUEST_CHANGED_PAGES_* variables' do - expect(subject.find { |var| var[:key] == 'CI_MERGE_REQUEST_CHANGED_PAGE_PATHS' }).to be_nil - expect(subject.find { |var| var[:key] == 'CI_MERGE_REQUEST_CHANGED_PAGE_URLS' }).to be_nil - end - end - end - context 'when build has user' do let(:user_variables) do [ @@ -4652,4 +4614,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..cdbdd2b1d20 100644 --- a/spec/models/ci/build_trace_chunk_spec.rb +++ b/spec/models/ci/build_trace_chunk_spec.rb @@ -502,22 +502,12 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do describe '#persist_data!' do let(:build) { create(:ci_build, :running) } - subject { build_trace_chunk.persist_data! } - - shared_examples_for 'Atomic operation' do - context 'when the other process is persisting' do - let(:lease_key) { "trace_write:#{build_trace_chunk.build.id}:chunks:#{build_trace_chunk.chunk_index}" } - - before do - stub_exclusive_lease_taken(lease_key) - end - - it 'raise an error' do - expect { subject }.to raise_error('Failed to obtain a lock') - end - end + before do + build_trace_chunk.save! end + subject { build_trace_chunk.persist_data! } + context 'when data_store is redis' do let(:data_store) { :redis } @@ -548,8 +538,6 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do expect(build_trace_chunk.reload.checksum).to eq '3398914352' end - - it_behaves_like 'Atomic operation' end context 'when data size has not reached CHUNK_SIZE' do @@ -575,6 +563,62 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do expect(build_trace_chunk.fog?).to be_truthy end end + + context 'when the chunk has been modifed by a different worker' do + it 'reloads the chunk before migration' do + described_class + .find(build_trace_chunk.id) + .update!(data_store: :fog) + + build_trace_chunk.persist_data! + end + + it 'verifies the operation using optimistic locking' do + allow(build_trace_chunk) + .to receive(:save!) + .and_raise(ActiveRecord::StaleObjectError) + + expect { build_trace_chunk.persist_data! } + .to raise_error(described_class::FailedToPersistDataError) + end + + it 'does not allow flushing unpersisted chunk' do + build_trace_chunk.checksum = '12345' + + expect { build_trace_chunk.persist_data! } + .to raise_error(described_class::FailedToPersistDataError, + /Modifed build trace chunk detected/) + end + end + + context 'when the chunk is being locked by a different worker' do + let(:metrics) { spy('metrics') } + + it 'does not raise an exception' do + lock_chunk do + expect { build_trace_chunk.persist_data! }.not_to raise_error + end + end + + it 'increments stalled chunk trace metric' do + allow(build_trace_chunk) + .to receive(:metrics) + .and_return(metrics) + + lock_chunk { build_trace_chunk.persist_data! } + + expect(metrics) + .to have_received(:increment_trace_operation) + .with(operation: :stalled) + .once + end + + def lock_chunk(&block) + "trace_write:#{build.id}:chunks:#{chunk_index}".then do |key| + build_trace_chunk.in_lock(key, &block) + end + end + end end end @@ -609,8 +653,6 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) end - - it_behaves_like 'Atomic operation' end context 'when data size has not reached CHUNK_SIZE' do @@ -670,8 +712,6 @@ RSpec.describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do expect(Ci::BuildTraceChunks::Database.new.data(build_trace_chunk)).to be_nil expect(Ci::BuildTraceChunks::Fog.new.data(build_trace_chunk)).to eq(data) end - - it_behaves_like 'Atomic operation' end context 'when data size has not reached CHUNK_SIZE' do @@ -779,4 +819,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/deleted_object_spec.rb b/spec/models/ci/deleted_object_spec.rb new file mode 100644 index 00000000000..cb8911d5027 --- /dev/null +++ b/spec/models/ci/deleted_object_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::DeletedObject, :aggregate_failures do + describe 'attributes' do + it { is_expected.to respond_to(:file) } + it { is_expected.to respond_to(:store_dir) } + it { is_expected.to respond_to(:file_store) } + it { is_expected.to respond_to(:pick_up_at) } + end + + describe '.bulk_import' do + context 'with data' do + let!(:artifact) { create(:ci_job_artifact, :archive, :expired) } + + it 'imports data' do + expect { described_class.bulk_import(Ci::JobArtifact.all) }.to change { described_class.count }.by(1) + + deleted_artifact = described_class.first + + expect(deleted_artifact.file_store).to eq(artifact.file_store) + expect(deleted_artifact.store_dir).to eq(artifact.file.store_dir.to_s) + expect(deleted_artifact.file_identifier).to eq(artifact.file_identifier) + expect(deleted_artifact.pick_up_at).to eq(artifact.expire_at) + end + end + + context 'with invalid data' do + let!(:artifact) { create(:ci_job_artifact) } + + it 'does not import anything' do + expect(artifact.file_identifier).to be_nil + + expect { described_class.bulk_import([artifact]) } + .not_to change { described_class.count } + end + end + + context 'with empty data' do + it 'returns successfully' do + expect { described_class.bulk_import([]) } + .not_to change { described_class.count } + end + end + end + + context 'ActiveRecord scopes' do + let_it_be(:not_ready) { create(:ci_deleted_object, pick_up_at: 1.day.from_now) } + let_it_be(:ready) { create(:ci_deleted_object, pick_up_at: 1.day.ago) } + + describe '.ready_for_destruction' do + it 'returns objects that are ready' do + result = described_class.ready_for_destruction(2) + + expect(result).to contain_exactly(ready) + end + end + + describe '.lock_for_destruction' do + subject(:result) { described_class.lock_for_destruction(10) } + + it 'returns objects that are ready' do + expect(result).to contain_exactly(ready) + end + + it 'selects only the id' do + expect(result.select_values).to contain_exactly(:id) + end + + it 'orders by pick_up_at' do + expect(result.order_values.map(&:to_sql)) + .to contain_exactly("\"ci_deleted_objects\".\"pick_up_at\" ASC") + end + + it 'applies limit' do + expect(result.limit_value).to eq(10) + end + + it 'uses select for update' do + expect(result.locked?).to eq('FOR UPDATE SKIP LOCKED') + end + end + end + + describe '#delete_file_from_storage' do + let(:object) { build(:ci_deleted_object) } + + it 'does not raise errors' do + expect(object.file).to receive(:remove!).and_raise(StandardError) + + expect(object.delete_file_from_storage).to be_falsy + 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..88d08f1ec45 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 @@ -2988,6 +2988,57 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '#builds_in_self_and_descendants' do + subject(:builds) { pipeline.builds_in_self_and_descendants } + + let(:pipeline) { create(:ci_pipeline, project: project) } + let!(:build) { create(:ci_build, pipeline: pipeline) } + + context 'when pipeline is standalone' do + it 'returns the list of builds' do + expect(builds).to contain_exactly(build) + end + end + + context 'when pipeline is parent of another pipeline' do + let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } + let!(:child_build) { create(:ci_build, pipeline: child_pipeline) } + + it 'returns the list of builds' do + expect(builds).to contain_exactly(build, child_build) + end + end + + context 'when pipeline is parent of another parent pipeline' do + let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } + let!(:child_build) { create(:ci_build, pipeline: child_pipeline) } + let(:child_of_child_pipeline) { create(:ci_pipeline, child_of: child_pipeline) } + let!(:child_of_child_build) { create(:ci_build, pipeline: child_of_child_pipeline) } + + it 'returns the list of builds' do + expect(builds).to contain_exactly(build, child_build, child_of_child_build) + end + end + end + + describe '#build_with_artifacts_in_self_and_descendants' do + let!(:build) { create(:ci_build, name: 'test', pipeline: pipeline) } + let(:child_pipeline) { create(:ci_pipeline, child_of: pipeline) } + let!(:child_build) { create(:ci_build, :artifacts, name: 'test', pipeline: child_pipeline) } + + it 'returns the build with a given name, having artifacts' do + expect(pipeline.build_with_artifacts_in_self_and_descendants('test')).to eq(child_build) + end + + context 'when same job name is present in both parent and child pipeline' do + let!(:build) { create(:ci_build, :artifacts, name: 'test', pipeline: pipeline) } + + it 'returns the job in the parent pipeline' do + expect(pipeline.build_with_artifacts_in_self_and_descendants('test')).to eq(build) + end + end + end + describe '#find_job_with_archive_artifacts' do let!(:old_job) { create(:ci_build, name: 'rspec', retried: true, pipeline: pipeline) } let!(:job_without_artifacts) { create(:ci_build, name: 'rspec', pipeline: pipeline) } @@ -3628,6 +3679,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/agent_spec.rb b/spec/models/clusters/agent_spec.rb index 99de0d1ddf7..148bb3cf870 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -13,6 +13,20 @@ RSpec.describe Clusters::Agent do it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } describe 'scopes' do + describe '.ordered_by_name' do + let(:names) { %w(agent-d agent-b agent-a agent-c) } + + subject { described_class.ordered_by_name } + + before do + names.each do |name| + create(:cluster_agent, name: name) + end + end + + it { expect(subject.map(&:name)).to eq(names.sort) } + end + describe '.with_name' do let!(:matching_name) { create(:cluster_agent, name: 'matching-name') } let!(:other_name) { create(:cluster_agent, name: 'other-name') } diff --git a/spec/models/clusters/applications/fluentd_spec.rb b/spec/models/clusters/applications/fluentd_spec.rb index be7b4a87947..3bda3e99ec1 100644 --- a/spec/models/clusters/applications/fluentd_spec.rb +++ b/spec/models/clusters/applications/fluentd_spec.rb @@ -25,7 +25,7 @@ RSpec.describe Clusters::Applications::Fluentd do it 'is initialized with fluentd arguments' do expect(subject.name).to eq('fluentd') - expect(subject.chart).to eq('stable/fluentd') + expect(subject.chart).to eq('fluentd/fluentd') expect(subject.version).to eq('2.4.0') expect(subject).to be_rbac end diff --git a/spec/models/clusters/applications/ingress_spec.rb b/spec/models/clusters/applications/ingress_spec.rb index e029283326f..196d57aff7b 100644 --- a/spec/models/clusters/applications/ingress_spec.rb +++ b/spec/models/clusters/applications/ingress_spec.rb @@ -135,7 +135,7 @@ RSpec.describe Clusters::Applications::Ingress do it 'is initialized with ingress arguments' do expect(subject.name).to eq('ingress') - expect(subject.chart).to eq('stable/nginx-ingress') + expect(subject.chart).to eq('ingress/nginx-ingress') expect(subject.version).to eq('1.40.2') expect(subject).to be_rbac expect(subject.files).to eq(ingress.files) diff --git a/spec/models/clusters/applications/prometheus_spec.rb b/spec/models/clusters/applications/prometheus_spec.rb index 82971596176..b450900bee6 100644 --- a/spec/models/clusters/applications/prometheus_spec.rb +++ b/spec/models/clusters/applications/prometheus_spec.rb @@ -152,7 +152,7 @@ RSpec.describe Clusters::Applications::Prometheus do it 'is initialized with 3 arguments' do expect(subject.name).to eq('prometheus') - expect(subject.chart).to eq('stable/prometheus') + expect(subject.chart).to eq('prometheus/prometheus') expect(subject.version).to eq('10.4.1') expect(subject).to be_rbac expect(subject.files).to eq(prometheus.files) @@ -240,7 +240,7 @@ RSpec.describe Clusters::Applications::Prometheus do it 'is initialized with 3 arguments' do expect(patch_command.name).to eq('prometheus') - expect(patch_command.chart).to eq('stable/prometheus') + expect(patch_command.chart).to eq('prometheus/prometheus') expect(patch_command.version).to eq('10.4.1') expect(patch_command.files).to eq(prometheus.files) end 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/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index c6a2b67a008..e877ba2ac96 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -412,7 +412,7 @@ RSpec.describe Clusters::Platforms::Kubernetes do end let(:namespace) { "project-namespace" } - let(:environment) { instance_double(Environment, deployment_namespace: namespace) } + let(:environment) { instance_double(Environment, deployment_namespace: namespace, project: service.cluster.project) } subject { service.calculate_reactive_cache_for(environment) } @@ -428,6 +428,7 @@ RSpec.describe Clusters::Platforms::Kubernetes do before do stub_kubeclient_pods(namespace) stub_kubeclient_deployments(namespace) + stub_kubeclient_ingresses(namespace) end it { is_expected.to include(pods: [expected_pod_cached_data]) } @@ -437,6 +438,7 @@ RSpec.describe Clusters::Platforms::Kubernetes do before do stub_kubeclient_pods(namespace, status: 500) stub_kubeclient_deployments(namespace, status: 500) + stub_kubeclient_ingresses(namespace, status: 500) end it { expect { subject }.to raise_error(Kubeclient::HttpError) } @@ -446,6 +448,7 @@ RSpec.describe Clusters::Platforms::Kubernetes do before do stub_kubeclient_pods(namespace, status: 404) stub_kubeclient_deployments(namespace, status: 404) + stub_kubeclient_ingresses(namespace, status: 404) end it { is_expected.to include(pods: []) } 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/each_batch_spec.rb b/spec/models/concerns/each_batch_spec.rb index 3c93c8a7a79..8b70753633c 100644 --- a/spec/models/concerns/each_batch_spec.rb +++ b/spec/models/concerns/each_batch_spec.rb @@ -18,13 +18,13 @@ RSpec.describe EachBatch do shared_examples 'each_batch handling' do |kwargs| it 'yields an ActiveRecord::Relation when a block is given' do - model.each_batch(kwargs) do |relation| + model.each_batch(**kwargs) do |relation| expect(relation).to be_a_kind_of(ActiveRecord::Relation) end end it 'yields a batch index as the second argument' do - model.each_batch(kwargs) do |_, index| + model.each_batch(**kwargs) do |_, index| expect(index).to eq(1) end end @@ -32,7 +32,7 @@ RSpec.describe EachBatch do it 'accepts a custom batch size' do amount = 0 - model.each_batch(kwargs.merge({ of: 1 })) { amount += 1 } + model.each_batch(**kwargs.merge({ of: 1 })) { amount += 1 } expect(amount).to eq(5) end 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/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb index 9496bb57b8b..c87bbf24c30 100644 --- a/spec/models/concerns/has_user_type_spec.rb +++ b/spec/models/concerns/has_user_type_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe User do specify 'types consistency checks', :aggregate_failures do expect(described_class::USER_TYPES.keys) - .to match_array(%w[human ghost alert_bot project_bot support_bot service_user visual_review_bot migration_bot]) + .to match_array(%w[human ghost alert_bot project_bot support_bot service_user security_bot visual_review_bot migration_bot]) expect(described_class::USER_TYPES).to include(*described_class::BOT_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::NON_INTERNAL_USER_TYPES) expect(described_class::USER_TYPES).to include(*described_class::INTERNAL_USER_TYPES) @@ -31,6 +31,12 @@ RSpec.describe User do end end + describe '.without_bots' do + it 'includes everyone except bots' do + expect(described_class.without_bots).to match_array(everyone - bots) + end + end + describe '.bots_without_project_bot' do it 'includes all bots except project_bot' do expect(described_class.bots_without_project_bot).to match_array(bots - [project_bot]) 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_expiration_policy_spec.rb b/spec/models/container_expiration_policy_spec.rb index 588685b04bf..1d9dbe8a867 100644 --- a/spec/models/container_expiration_policy_spec.rb +++ b/spec/models/container_expiration_policy_spec.rb @@ -104,6 +104,18 @@ RSpec.describe ContainerExpirationPolicy, type: :model do end end + describe '.executable' do + subject { described_class.executable } + + let_it_be(:policy1) { create(:container_expiration_policy, :runnable) } + let_it_be(:container_repository1) { create(:container_repository, project: policy1.project) } + let_it_be(:policy2) { create(:container_expiration_policy, :runnable) } + let_it_be(:container_repository2) { create(:container_repository, project: policy2.project) } + let_it_be(:policy3) { create(:container_expiration_policy, :runnable) } + + it { is_expected.to contain_exactly(policy1, policy2) } + end + describe '#disable!' do let_it_be(:container_expiration_policy) { create(:container_expiration_policy) } 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/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 9fd3751be13..60a3e3fc0e2 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -353,4 +353,29 @@ RSpec.describe DeployToken do end end end + + describe '#accessible_projects' do + subject { deploy_token.accessible_projects } + + context 'when a deploy token is associated to a project' do + let_it_be(:deploy_token) { create(:deploy_token, :project) } + + it 'returns only projects directly associated with the token' do + expect(deploy_token).to receive(:projects) + + subject + end + end + + context 'when a deploy token is associated to a group' do + let_it_be(:group) { create(:group) } + let_it_be(:deploy_token) { create(:deploy_token, :group, groups: [group]) } + + it 'returns all projects from the group' do + expect(group).to receive(:all_projects) + + subject + end + end + end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 1c7b11257ce..3e855584c38 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -98,16 +98,36 @@ RSpec.describe Deployment do context 'when deployment runs' do let(:deployment) { create(:deployment) } - before do - deployment.run! - end - it 'starts running' do freeze_time do + deployment.run! + expect(deployment).to be_running expect(deployment.finished_at).to be_nil end end + + it 'executes Deployments::ExecuteHooksWorker asynchronously' do + expect(Deployments::ExecuteHooksWorker) + .to receive(:perform_async).with(deployment.id) + + deployment.run! + end + + it 'does not execute Deployments::ExecuteHooksWorker when feature is disabled' do + stub_feature_flags(ci_send_deployment_hook_when_start: false) + expect(Deployments::ExecuteHooksWorker) + .not_to receive(:perform_async).with(deployment.id) + + deployment.run! + end + + it 'executes Deployments::DropOlderDeploymentsWorker asynchronously' do + expect(Deployments::DropOlderDeploymentsWorker) + .to receive(:perform_async).once.with(deployment.id) + + deployment.run! + end end context 'when deployment succeeded' do @@ -122,15 +142,15 @@ RSpec.describe Deployment do end end - it 'executes Deployments::SuccessWorker asynchronously' do - expect(Deployments::SuccessWorker) + it 'executes Deployments::UpdateEnvironmentWorker asynchronously' do + expect(Deployments::UpdateEnvironmentWorker) .to receive(:perform_async).with(deployment.id) deployment.succeed! end - it 'executes Deployments::FinishedWorker asynchronously' do - expect(Deployments::FinishedWorker) + it 'executes Deployments::ExecuteHooksWorker asynchronously' do + expect(Deployments::ExecuteHooksWorker) .to receive(:perform_async).with(deployment.id) deployment.succeed! @@ -149,12 +169,19 @@ RSpec.describe Deployment do end end - it 'executes Deployments::FinishedWorker asynchronously' do - expect(Deployments::FinishedWorker) + it 'executes Deployments::LinkMergeRequestWorker asynchronously' do + expect(Deployments::LinkMergeRequestWorker) .to receive(:perform_async).with(deployment.id) deployment.drop! end + + it 'executes Deployments::ExecuteHooksWorker asynchronously' do + expect(Deployments::ExecuteHooksWorker) + .to receive(:perform_async).with(deployment.id) + + deployment.drop! + end end context 'when deployment was canceled' do @@ -169,12 +196,19 @@ RSpec.describe Deployment do end end - it 'executes Deployments::FinishedWorker asynchronously' do - expect(Deployments::FinishedWorker) + it 'executes Deployments::LinkMergeRequestWorker asynchronously' do + expect(Deployments::LinkMergeRequestWorker) .to receive(:perform_async).with(deployment.id) deployment.cancel! end + + it 'executes Deployments::ExecuteHooksWorker asynchronously' do + expect(Deployments::ExecuteHooksWorker) + .to receive(:perform_async).with(deployment.id) + + deployment.cancel! + end end end @@ -580,9 +614,10 @@ RSpec.describe Deployment do expect(deploy).to be_success end - it 'schedules SuccessWorker and FinishedWorker when finishing a deploy' do - expect(Deployments::SuccessWorker).to receive(:perform_async) - expect(Deployments::FinishedWorker).to receive(:perform_async) + it 'schedules workers when finishing a deploy' do + expect(Deployments::UpdateEnvironmentWorker).to receive(:perform_async) + expect(Deployments::LinkMergeRequestWorker).to receive(:perform_async) + expect(Deployments::ExecuteHooksWorker).to receive(:perform_async) deploy.update_status('success') end diff --git a/spec/models/design_management/design_at_version_spec.rb b/spec/models/design_management/design_at_version_spec.rb index 3c1ff45c53f..220de80a52a 100644 --- a/spec/models/design_management/design_at_version_spec.rb +++ b/spec/models/design_management/design_at_version_spec.rb @@ -274,29 +274,6 @@ RSpec.describe DesignManagement::DesignAtVersion do build(:design_at_version, design: design, version: version).id end - describe '.instantiate' do - context 'when attrs are valid' do - subject do - described_class.instantiate(design: design, version: version) - end - - it { is_expected.to be_a(described_class).and(be_valid) } - end - - context 'when attrs are invalid' do - subject do - described_class.instantiate( - design: create(:design), - version: create(:design_version) - ) - end - - it 'raises a validation error' do - expect { subject }.to raise_error(ActiveModel::ValidationError) - end - end - end - describe '.lazy_find' do let!(:version_a) do create(:design_version, designs: create_list(:design, 3, issue: issue)) 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/environment_spec.rb b/spec/models/environment_spec.rb index 433ede97b82..06d3e9da286 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -312,18 +312,25 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do describe '#update_merge_request_metrics?' do { + 'gprd' => false, + 'prod' => true, + 'prod-test' => false, + 'PROD' => true, 'production' => true, + 'production-test' => false, + 'PRODUCTION' => true, 'production/eu' => true, + 'PRODUCTION/EU' => true, 'production/www.gitlab.com' => true, 'productioneu' => false, - 'Production' => false, - 'Production/eu' => false, + 'Production' => true, + 'Production/eu' => true, 'test-production' => false }.each do |name, expected_value| it "returns #{expected_value} for #{name}" do env = create(:environment, name: name) - expect(env.update_merge_request_metrics?).to eq(expected_value) + expect(env.update_merge_request_metrics?).to eq(expected_value), "Expected the name '#{name}' to result in #{expected_value}, but it didn't." end end end diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index a6954fb5d56..09a73a4cdcb 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -66,18 +66,6 @@ RSpec.describe EnvironmentStatus do end end - describe '#changed_paths' do - subject { environment_status.changed_urls } - - it { is_expected.to contain_exactly("#{environment.external_url}/ruby-style-guide.html", "#{environment.external_url}/html/page.html") } - end - - describe '#changed_urls' do - subject { environment_status.changed_paths } - - it { is_expected.to contain_exactly('ruby-style-guide.html', 'html/page.html') } - end - describe '.for_merge_request' do let(:admin) { create(:admin) } let!(:pipeline) { create(:ci_pipeline, sha: sha, merge_requests_as_head_pipeline: [merge_request]) } 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..cc29e20710a 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -222,6 +222,50 @@ RSpec.describe Group do end end end + + describe '#two_factor_authentication_allowed' do + let_it_be(:group) { create(:group) } + + context 'for a parent group' do + it 'is valid' do + group.require_two_factor_authentication = true + + expect(group).to be_valid + end + end + + context 'for a child group' do + let(:sub_group) { create(:group, parent: group) } + + it 'is valid when parent group allows' do + sub_group.require_two_factor_authentication = true + + expect(sub_group).to be_valid + end + + it 'is invalid when parent group blocks' do + group.namespace_settings.update!(allow_mfa_for_subgroups: false) + sub_group.require_two_factor_authentication = true + + expect(sub_group).to be_invalid + expect(sub_group.errors[:require_two_factor_authentication]).to include('is forbidden by a top-level group') + end + end + 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 @@ -1330,229 +1374,156 @@ 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 + 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 - 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) } + 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 'returns the expected result' do - expect(group.parent_allows_shared_runners?).to eq(expected_shared_runners_allowed) + 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 'when parent group is missing' do - let!(:group) { create(:group) } - - it 'returns true' do - expect(group.parent_allows_shared_runners?).to be_truthy + 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 '#parent_enabled_shared_runners?' do - subject { group.parent_enabled_shared_runners? } + context 'disabled_with_override' do + subject { group.update_shared_runners_setting!('disabled_with_override') } - 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) } + 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) } - it { is_expected.to be_falsy } + 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 - context 'When shared Runners are enabled' do - let!(:parent_group) { create(:group) } - let!(:group) { create(:group, parent: parent_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 { is_expected.to be_truthy } + 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 '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 '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' 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 } - .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 '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 - 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) } + 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) } - 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 } + 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 - 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) } - - 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 '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 } - 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) } - - 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 } - 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 '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 } + describe "#default_branch_name" do + context "group.namespace_settings does not have a default branch name" do + it "returns nil" do + expect(group.default_branch_name).to be_nil end end - end - describe '#disallow_descendants_override_disabled_shared_runners!' do - subject { group.disallow_descendants_override_disabled_shared_runners! } + context "group.namespace_settings has a default branch name" do + let(:example_branch_name) { "example_branch_name" } - 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) + before do + expect(group.namespace_settings) + .to receive(:default_branch_name) + .and_return(example_branch_name) 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 "returns the default branch name" do + expect(group.default_branch_name).to eq(example_branch_name) end end end @@ -1600,4 +1571,24 @@ RSpec.describe Group do end end end + + describe '#parent_allows_two_factor_authentication?' do + it 'returns true for top-level group' do + expect(group.parent_allows_two_factor_authentication?).to eq(true) + end + + context 'for subgroup' do + let(:subgroup) { create(:group, parent: group) } + + it 'returns true if parent group allows two factor authentication for its descendants' do + expect(subgroup.parent_allows_two_factor_authentication?).to eq(true) + end + + it 'returns true if parent group allows two factor authentication for its descendants' do + group.namespace_settings.update!(allow_mfa_for_subgroups: false) + + expect(subgroup.parent_allows_two_factor_authentication?).to eq(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..d89b323f525 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -3,26 +3,26 @@ require 'spec_helper' RSpec.describe Integration do - let!(:project_1) { create(:project) } - let!(:project_2) { create(:project) } - let!(:project_3) { create(:project) } - let(:instance_integration) { create(:jira_service, :instance) } + let_it_be(:project_1) { create(:project) } + let_it_be(:project_2) { create(:project) } + let_it_be(:project_3) { create(:project) } + let_it_be(:instance_integration) { create(:jira_service, :instance) } 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..6706083fd91 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 @@ -646,13 +657,32 @@ RSpec.describe MergeRequestDiff do expect(diff_with_commits.commit_shas).to all(match(/\h{40}/)) end - context 'with limit attribute' do + shared_examples 'limited number of shas' do it 'returns limited number of shas' do expect(diff_with_commits.commit_shas(limit: 2).size).to eq(2) expect(diff_with_commits.commit_shas(limit: 100).size).to eq(29) expect(diff_with_commits.commit_shas.size).to eq(29) end end + + context 'with limit attribute' do + it_behaves_like 'limited number of shas' + end + + context 'with preloaded diff commits' do + before do + # preloads the merge_request_diff_commits association + diff_with_commits.merge_request_diff_commits.to_a + end + + it_behaves_like 'limited number of shas' + + it 'does not trigger any query' do + count = ActiveRecord::QueryRecorder.new { diff_with_commits.commit_shas(limit: 2) }.count + + expect(count).to eq(0) + end + end end describe '#compare_with' do @@ -865,4 +895,25 @@ RSpec.describe MergeRequestDiff do expect(subject.lines_count).to eq 189 end end + + describe '.latest_diff_for_merge_requests' do + let_it_be(:merge_request_1) { create(:merge_request_without_merge_request_diff) } + let_it_be(:merge_request_1_diff_1) { create(:merge_request_diff, merge_request: merge_request_1, created_at: 3.days.ago) } + let_it_be(:merge_request_1_diff_2) { create(:merge_request_diff, merge_request: merge_request_1, created_at: 1.day.ago) } + + let_it_be(:merge_request_2) { create(:merge_request_without_merge_request_diff) } + let_it_be(:merge_request_2_diff_1) { create(:merge_request_diff, merge_request: merge_request_2, created_at: 3.days.ago) } + + let_it_be(:merge_request_3) { create(:merge_request_without_merge_request_diff) } + + subject { described_class.latest_diff_for_merge_requests([merge_request_1, merge_request_2]) } + + it 'loads the latest merge_request_diff record for the given merge requests' do + expect(subject).to match_array([merge_request_1_diff_2, merge_request_2_diff_1]) + end + + it 'loads nothing if the merge request has no diff record' do + expect(described_class.latest_diff_for_merge_requests(merge_request_3)).to be_empty + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 98f709a0610..ddb3ffdda2f 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 @@ -3525,6 +3520,25 @@ RSpec.describe MergeRequest, factory_default: :keep do end end + describe '#merge_base_pipeline' do + let(:merge_request) do + create(:merge_request, :with_merge_request_pipeline) + end + + let(:merge_base_pipeline) do + create(:ci_pipeline, ref: merge_request.target_branch, sha: merge_request.target_branch_sha) + end + + before do + merge_base_pipeline + merge_request.update_head_pipeline + end + + it 'returns a pipeline pointing to a commit on the target ref' do + expect(merge_request.merge_base_pipeline).to eq(merge_base_pipeline) + end + end + describe '#has_commits?' do it 'returns true when merge request diff has commits' do allow(subject.merge_request_diff).to receive(:commits_count) @@ -4214,14 +4228,26 @@ RSpec.describe MergeRequest, factory_default: :keep do it 'returns true' do expect(subject.diffable_merge_ref?).to eq(true) end - end - end - context 'merge request cannot be merged' do - it 'returns false' do - subject.mark_as_unchecked! + context 'merge request cannot be merged' do + before do + subject.mark_as_unchecked! + end + + it 'returns false' do + expect(subject.diffable_merge_ref?).to eq(true) + end + + context 'display_merge_conflicts_in_diff is disabled' do + before do + stub_feature_flags(display_merge_conflicts_in_diff: false) + end - expect(subject.diffable_merge_ref?).to eq(false) + it 'returns false' do + expect(subject.diffable_merge_ref?).to eq(false) + end + end + end end end end @@ -4261,24 +4287,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 +4312,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/milestone_release_spec.rb b/spec/models/milestone_release_spec.rb index 3c781545d8a..b2a174f1d90 100644 --- a/spec/models/milestone_release_spec.rb +++ b/spec/models/milestone_release_spec.rb @@ -10,8 +10,8 @@ RSpec.describe MilestoneRelease do subject { build(:milestone_release, release: release, milestone: milestone) } describe 'associations' do - it { is_expected.to belong_to(:milestone) } it { is_expected.to belong_to(:release) } + it { is_expected.to belong_to(:milestone) } end context 'when trying to create the same record in milestone_releases twice' do diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 257d78dfa2c..c6e8d5b129c 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -3,5 +3,71 @@ require 'spec_helper' RSpec.describe NamespaceSetting, type: :model do - it { is_expected.to belong_to(:namespace) } + # Relationships + # + describe "Associations" do + it { is_expected.to belong_to(:namespace) } + end + + describe "validations" do + describe "#default_branch_name_content" do + let_it_be(:group) { create(:group) } + + let(:namespace_settings) { group.namespace_settings } + + shared_examples "doesn't return an error" do + it "doesn't return an error" do + expect(namespace_settings.valid?).to be_truthy + expect(namespace_settings.errors.full_messages).to be_empty + end + end + + context "when not set" do + it_behaves_like "doesn't return an error" + end + + context "when set" do + before do + namespace_settings.default_branch_name = "example_branch_name" + end + + it_behaves_like "doesn't return an error" + end + + context "when an empty string" do + before do + namespace_settings.default_branch_name = '' + end + + it "returns an error" do + expect(namespace_settings.valid?).to be_falsey + expect(namespace_settings.errors.full_messages).not_to be_empty + end + end + end + + describe '#allow_mfa_for_group' do + let(:settings) { group.namespace_settings } + + context 'group is top-level group' do + let(:group) { create(:group) } + + it 'is valid' do + settings.allow_mfa_for_subgroups = false + + expect(settings).to be_valid + end + end + + context 'group is a subgroup' do + let(:group) { create(:group, parent: create(:group)) } + + it 'is invalid' do + settings.allow_mfa_for_subgroups = false + + expect(settings).to be_invalid + end + end + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index ca1f06370d4..91b18f346c6 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -855,13 +855,49 @@ RSpec.describe Namespace do end describe '#all_projects' do - let(:group) { create(:group) } - let(:child) { create(:group, parent: group) } - let!(:project1) { create(:project_empty_repo, namespace: group) } - let!(:project2) { create(:project_empty_repo, namespace: child) } + shared_examples 'all projects for a namespace' do + let(:namespace) { create(:namespace) } + let(:child) { create(:group, parent: namespace) } + let!(:project1) { create(:project_empty_repo, namespace: namespace) } + let!(:project2) { create(:project_empty_repo, namespace: child) } + + it { expect(namespace.all_projects.to_a).to match_array([project2, project1]) } + it { expect(child.all_projects.to_a).to match_array([project2]) } + end + + shared_examples 'all project examples' do + include_examples 'all projects for a namespace' + + context 'when namespace is a group' do + let_it_be(:namespace) { create(:group) } + + include_examples 'all projects for a namespace' + end - it { expect(group.all_projects.to_a).to match_array([project2, project1]) } - it { expect(child.all_projects.to_a).to match_array([project2]) } + context 'when namespace is a user namespace' do + let_it_be(:user) { create(:user) } + let_it_be(:user_namespace) { create(:namespace, owner: user) } + let_it_be(:project) { create(:project, namespace: user_namespace) } + + it { expect(user_namespace.all_projects.to_a).to match_array([project]) } + end + end + + context 'with recursive approach' do + before do + stub_feature_flags(recursive_approach_for_all_projects: true) + end + + include_examples 'all project examples' + end + + context 'with route path wildcard approach' do + before do + stub_feature_flags(recursive_approach_for_all_projects: false) + end + + include_examples 'all project examples' + end end describe '#all_pipelines' do @@ -1320,4 +1356,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..ca408303524 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 @@ -257,7 +271,12 @@ RSpec.describe Packages::Package, type: :model do end it_behaves_like 'validating version to be SemVer compliant for', :npm_package - it_behaves_like 'validating version to be SemVer compliant for', :nuget_package + + context 'nuget package' do + it_behaves_like 'validating version to be SemVer compliant for', :nuget_package + + it { is_expected.to allow_value('1.2.3.4').for(:version) } + end end describe '#package_already_taken' do @@ -497,6 +516,14 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to match_array([package1, package2]) } end + + describe '.with_normalized_pypi_name' do + let_it_be(:pypi_package) { create(:pypi_package, name: 'Foo.bAr---BAZ_buz') } + + subject { described_class.with_normalized_pypi_name('foo-bar-baz-buz') } + + it { is_expected.to match_array([pypi_package]) } + end end describe '.select_distinct_name' do diff --git a/spec/models/pages_deployment_spec.rb b/spec/models/pages_deployment_spec.rb index eafff1ed59a..5d26ade740e 100644 --- a/spec/models/pages_deployment_spec.rb +++ b/spec/models/pages_deployment_spec.rb @@ -18,4 +18,21 @@ RSpec.describe PagesDeployment do expect(create(:pages_deployment)).to be_valid end end + + describe 'default for file_store' do + it 'uses local store when object storage is not enabled' do + expect(build(:pages_deployment).file_store).to eq(ObjectStorage::Store::LOCAL) + end + + it 'uses remote store when object storage is enabled' do + stub_pages_object_storage(::Pages::DeploymentUploader) + + expect(build(:pages_deployment).file_store).to eq(ObjectStorage::Store::REMOTE) + end + end + + it 'saves size along with the file' do + deployment = create(:pages_deployment) + expect(deployment.size).to eq(deployment.file.size) + end end 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/preloaders/merge_request_diff_preloader_spec.rb b/spec/models/preloaders/merge_request_diff_preloader_spec.rb new file mode 100644 index 00000000000..9a76d42e73f --- /dev/null +++ b/spec/models/preloaders/merge_request_diff_preloader_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::MergeRequestDiffPreloader do + let_it_be(:merge_request_1) { create(:merge_request) } + let_it_be(:merge_request_2) { create(:merge_request) } + let_it_be(:merge_request_3) { create(:merge_request_without_merge_request_diff) } + + let(:merge_requests) { [merge_request_1, merge_request_2, merge_request_3] } + + def trigger(merge_requests) + Array(merge_requests).each(&:merge_request_diff) + end + + def merge_requests_with_preloaded_diff + described_class.new(MergeRequest.where(id: merge_requests.map(&:id)).to_a).preload_all + end + + it 'does not trigger N+1 queries' do + # warmup + trigger(merge_requests_with_preloaded_diff) + + first_merge_request = merge_requests_with_preloaded_diff.first + clean_merge_requests = merge_requests_with_preloaded_diff + + expect { trigger(clean_merge_requests) }.to issue_same_number_of_queries_as { trigger(first_merge_request) } + end +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_repository_spec.rb b/spec/models/project_repository_spec.rb index 6852ca0097d..eba908b0fdb 100644 --- a/spec/models/project_repository_spec.rb +++ b/spec/models/project_repository_spec.rb @@ -8,6 +8,11 @@ RSpec.describe ProjectRepository do it { is_expected.to belong_to(:project) } end + it_behaves_like 'shardable scopes' do + let_it_be(:record_1) { create(:project_repository) } + let_it_be(:record_2, reload: true) { create(:project_repository) } + end + describe '.find_project' do it 'finds project by disk path' do project = create(:project) diff --git a/spec/models/project_repository_storage_move_spec.rb b/spec/models/project_repository_storage_move_spec.rb index 3e679c8af4d..d32867efb39 100644 --- a/spec/models/project_repository_storage_move_spec.rb +++ b/spec/models/project_repository_storage_move_spec.rb @@ -43,6 +43,18 @@ RSpec.describe ProjectRepositoryStorageMove, type: :model do end end + describe 'defaults' do + context 'destination_storage_name' do + subject { build(:project_repository_storage_move) } + + it 'picks storage from ApplicationSetting' do + expect(Gitlab::CurrentSettings).to receive(:pick_repository_storage).and_return('picked').at_least(:once) + + expect(subject.destination_storage_name).to eq('picked') + end + end + end + describe 'state transitions' do let(:project) { create(:project) } diff --git a/spec/models/project_services/chat_message/deployment_message_spec.rb b/spec/models/project_services/chat_message/deployment_message_spec.rb index 9c361f90ae0..6bdf2120b36 100644 --- a/spec/models/project_services/chat_message/deployment_message_spec.rb +++ b/spec/models/project_services/chat_message/deployment_message_spec.rb @@ -70,6 +70,17 @@ RSpec.describe ChatMessage::DeploymentMessage do expect(message.pretext).to eq('Deploy to staging unknown') end + + it 'returns a message for a running deployment' do + data = { + status: 'running', + environment: 'production' + } + + message = described_class.new(data) + + expect(message.pretext).to eq('Starting deploy to production') + end end describe '#attachments' do diff --git a/spec/models/project_services/chat_message/issue_message_spec.rb b/spec/models/project_services/chat_message/issue_message_spec.rb index 051f4780ba4..4701ef3e49e 100644 --- a/spec/models/project_services/chat_message/issue_message_spec.rb +++ b/spec/models/project_services/chat_message/issue_message_spec.rb @@ -44,7 +44,7 @@ RSpec.describe ChatMessage::IssueMessage do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - '[<http://somewhere.com|project_name>] Issue opened by Test User (test.user)') + '[<http://somewhere.com|project_name>] Issue <http://url.com|#100 Issue title> opened by Test User (test.user)') expect(subject.attachments).to eq([ { title: "#100 Issue title", @@ -91,7 +91,7 @@ RSpec.describe ChatMessage::IssueMessage do context 'open' do it 'returns a message regarding opening of issues' do expect(subject.pretext).to eq( - '[[project_name](http://somewhere.com)] Issue opened by Test User (test.user)') + '[[project_name](http://somewhere.com)] Issue [#100 Issue title](http://url.com) opened by Test User (test.user)') expect(subject.attachments).to eq('issue description') expect(subject.activity).to eq({ title: 'Issue opened by Test User (test.user)', diff --git a/spec/models/project_services/prometheus_service_spec.rb b/spec/models/project_services/prometheus_service_spec.rb index 16837e2b93a..76fc5a826c9 100644 --- a/spec/models/project_services/prometheus_service_spec.rb +++ b/spec/models/project_services/prometheus_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching do +RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching, :snowplow do include PrometheusHelpers include ReactiveCachingHelpers @@ -421,18 +421,16 @@ RSpec.describe PrometheusService, :use_clean_rails_memory_store_caching do context "enabling manual_configuration" do it "tracks enable event" do service.update!(manual_configuration: false) - - expect(Gitlab::Tracking).to receive(:event).with('cluster:services:prometheus', 'enabled_manual_prometheus') - service.update!(manual_configuration: true) + + expect_snowplow_event(category: 'cluster:services:prometheus', action: 'enabled_manual_prometheus') end it "tracks disable event" do service.update!(manual_configuration: true) - - expect(Gitlab::Tracking).to receive(:event).with('cluster:services:prometheus', 'disabled_manual_prometheus') - service.update!(manual_configuration: false) + + expect_snowplow_event(category: 'cluster:services:prometheus', action: 'disabled_manual_prometheus') end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fe971832695..53a213891e9 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) } @@ -133,6 +136,7 @@ RSpec.describe Project do let_it_be(:container) { create(:project, :repository, path: 'somewhere') } let(:stubbed_container) { build_stubbed(:project) } let(:expected_full_path) { "#{container.namespace.full_path}/somewhere" } + let(:expected_lfs_enabled) { true } end it_behaves_like 'model with wiki' do @@ -4329,7 +4333,7 @@ RSpec.describe Project do end it 'schedules HashedStorage::ProjectMigrateWorker with delayed start when the wiki repo is in use' do - Gitlab::ReferenceCounter.new(Gitlab::GlRepository::WIKI.identifier_for_container(project)).increase + Gitlab::ReferenceCounter.new(Gitlab::GlRepository::WIKI.identifier_for_container(project.wiki)).increase expect(HashedStorage::ProjectMigrateWorker).to receive(:perform_in) @@ -4975,15 +4979,21 @@ RSpec.describe Project do context "with an empty repository" do let_it_be(:project) { create(:project_empty_repo) } - context "Gitlab::CurrentSettings.default_branch_name is unavailable" do + context "group.default_branch_name is available" do + let(:project_group) { create(:group) } + let(:project) { create(:project, path: 'avatar', namespace: project_group) } + before do expect(Gitlab::CurrentSettings) + .not_to receive(:default_branch_name) + + expect(project.group) .to receive(:default_branch_name) - .and_return(nil) + .and_return('example_branch') end - it "returns that value" do - expect(project.default_branch).to be_nil + it "returns the group default value" do + expect(project.default_branch).to eq("example_branch") end end @@ -4991,11 +5001,23 @@ RSpec.describe Project do before do expect(Gitlab::CurrentSettings) .to receive(:default_branch_name) - .and_return('example_branch') + .and_return(example_branch_name) end - it "returns that value" do - expect(project.default_branch).to eq("example_branch") + context "is missing or nil" do + let(:example_branch_name) { nil } + + it "returns nil" do + expect(project.default_branch).to be_nil + end + end + + context "is present" do + let(:example_branch_name) { "example_branch_name" } + + it "returns the expected branch name" do + expect(project.default_branch).to eq(example_branch_name) + end end end end @@ -5487,12 +5509,13 @@ RSpec.describe Project do describe '#find_or_initialize_services' do it 'returns only enabled services' do allow(Service).to receive(:available_services_names).and_return(%w[prometheus pushover teamcity]) + allow(Service).to receive(:project_specific_services_names).and_return(%w[asana]) allow(subject).to receive(:disabled_services).and_return(%w[prometheus]) services = subject.find_or_initialize_services - expect(services.count).to eq(2) - expect(services.map(&:title)).to eq(['JetBrains TeamCity CI', 'Pushover']) + expect(services.count).to eq(3) + expect(services.map(&:title)).to eq(['Asana', 'JetBrains TeamCity CI', 'Pushover']) end end @@ -5563,32 +5586,6 @@ RSpec.describe Project do end end - describe '.for_repository_storage' do - it 'returns the projects for a given repository storage' do - stub_storage_settings('test_second_storage' => { - 'path' => TestEnv::SECOND_STORAGE_PATH, - 'gitaly_address' => Gitlab.config.repositories.storages.default.gitaly_address - }) - expected_project = create(:project, repository_storage: 'default') - create(:project, repository_storage: 'test_second_storage') - - expect(described_class.for_repository_storage('default')).to eq([expected_project]) - end - end - - describe '.excluding_repository_storage' do - it 'returns the projects excluding the given repository storage' do - stub_storage_settings('test_second_storage' => { - 'path' => TestEnv::SECOND_STORAGE_PATH, - 'gitaly_address' => Gitlab.config.repositories.storages.default.gitaly_address - }) - expected_project = create(:project, repository_storage: 'test_second_storage') - create(:project, repository_storage: 'default') - - expect(described_class.excluding_repository_storage('default')).to eq([expected_project]) - end - end - describe '.deployments' do subject { project.deployments } @@ -5812,6 +5809,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/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 29c3d0e1a73..2e82fcf5511 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -6,6 +6,7 @@ RSpec.describe ProjectWiki do it_behaves_like 'wiki model' do let(:wiki_container) { create(:project, :wiki_repo, namespace: user.namespace) } let(:wiki_container_without_repo) { create(:project, namespace: user.namespace) } + let(:wiki_lfs_enabled) { true } it { is_expected.to delegate_method(:storage).to(:container) } it { is_expected.to delegate_method(:repository_storage).to(:container) } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index a3042d619eb..31211f8ff2c 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2688,7 +2688,7 @@ RSpec.describe Repository do expect(subject).to be_a(Gitlab::Git::Repository) expect(subject.relative_path).to eq(project.disk_path + '.wiki.git') expect(subject.gl_repository).to eq("wiki-#{project.id}") - expect(subject.gl_project_path).to eq(project.full_path) + expect(subject.gl_project_path).to eq(project.wiki.full_path) end end end @@ -2941,12 +2941,19 @@ RSpec.describe Repository do expect(snippet.repository.project).to be_nil end + it 'returns the project for a project wiki' do + wiki = create(:project_wiki) + + expect(wiki.project).to be(wiki.repository.project) + end + it 'returns the container if it is a project' do expect(repository.project).to be(project) end it 'returns nil if the container is not a project' do - expect(repository).to receive(:container).and_return(Group.new) + repository.container = Group.new + expect(repository.project).to be_nil end end @@ -2981,17 +2988,11 @@ RSpec.describe Repository do context 'for a project wiki repository' do let(:repository) { project.wiki.repository } - it 'returns true when LFS is enabled' do - stub_lfs_setting(enabled: true) + it 'delegates to the project' do + expect(project).to receive(:lfs_enabled?).and_return(true) is_expected.to be_truthy end - - it 'returns false when LFS is disabled' do - stub_lfs_setting(enabled: false) - - is_expected.to be_falsy - end end context 'for a project snippet repository' do 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 deleted file mode 100644 index 8a37883d933..00000000000 --- a/spec/models/resource_weight_event_spec.rb +++ /dev/null @@ -1,76 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe ResourceWeightEvent, type: :model do - it_behaves_like 'a resource event' - it_behaves_like 'a resource event for issues' - - let_it_be(:user1) { create(:user) } - let_it_be(:user2) { create(:user) } - - let_it_be(:issue1) { create(:issue, author: user1) } - let_it_be(:issue2) { create(:issue, author: user1) } - let_it_be(:issue3) { create(:issue, author: user2) } - - describe 'validations' do - it { is_expected.not_to allow_value(nil).for(:issue) } - it { is_expected.to allow_value(nil).for(:weight) } - end - - describe 'associations' do - it { is_expected.to belong_to(:issue) } - end - - describe '.by_issue' do - let_it_be(:event1) { create(:resource_weight_event, issue: issue1) } - let_it_be(:event2) { create(:resource_weight_event, issue: issue2) } - let_it_be(:event3) { create(:resource_weight_event, issue: issue1) } - - it 'returns the expected records for an issue with events' do - events = ResourceWeightEvent.by_issue(issue1) - - expect(events).to contain_exactly(event1, event3) - end - - it 'returns the expected records for an issue with no events' do - events = ResourceWeightEvent.by_issue(issue3) - - expect(events).to be_empty - end - end - - describe '.created_after' do - let!(:created_at1) { 1.day.ago } - let!(:created_at2) { 2.days.ago } - let!(:created_at3) { 3.days.ago } - - let!(:event1) { create(:resource_weight_event, issue: issue1, created_at: created_at1) } - let!(:event2) { create(:resource_weight_event, issue: issue2, created_at: created_at2) } - let!(:event3) { create(:resource_weight_event, issue: issue2, created_at: created_at3) } - - it 'returns the expected events' do - events = ResourceWeightEvent.created_after(created_at3) - - expect(events).to contain_exactly(event1, event2) - end - - it 'returns no events if time is after last record time' do - events = ResourceWeightEvent.created_after(1.minute.ago) - - expect(events).to be_empty - end - end - - describe '#discussion_id' do - let_it_be(:event) { create(:resource_weight_event, issue: issue1, created_at: Time.utc(2019, 12, 30)) } - - it 'returns the expected id' do - allow(Digest::SHA1).to receive(:hexdigest) - .with("ResourceWeightEvent-#{event.id}-#{user1.id}") - .and_return('73d167c478') - - expect(event.discussion_id).to eq('73d167c478') - end - end -end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 32e2012e284..db3cf19a03f 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,8 @@ 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) @@ -360,6 +370,22 @@ 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(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) + 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(nil) + expect(service.group).to eq(group) end end @@ -455,13 +481,119 @@ RSpec.describe Service do expect(described_class.default_integration('JiraService', subgroup)).to eq(group_service) end - context 'having a service' do + context 'having a service with custom settings' do let!(:subgroup_service) { create(:jira_service, group_id: subgroup.id, project_id: nil) } it 'returns the closest group service for a project' do expect(described_class.default_integration('JiraService', project)).to eq(subgroup_service) end end + + context 'having a service inheriting settings' do + let!(:subgroup_service) { create(:jira_service, group_id: subgroup.id, project_id: nil, inherit_from_id: group_service.id) } + + it 'returns the closest group service which does not inherit from its parent for a project' do + expect(described_class.default_integration('JiraService', project)).to eq(group_service) + end + end + end + end + end + end + + describe '.create_from_active_default_integrations' do + context 'with an active service template' do + let_it_be(:template_integration) { create(:prometheus_service, :template, api_url: 'https://prometheus.template.com/') } + + it 'creates a service from the template' do + described_class.create_from_active_default_integrations(project, :project_id, with_templates: true) + + expect(project.reload.services.size).to eq(1) + expect(project.reload.services.first.api_url).to eq(template_integration.api_url) + expect(project.reload.services.first.inherit_from_id).to be_nil + end + + context 'with an active instance-level integration' do + let!(:instance_integration) { create(:prometheus_service, :instance, api_url: 'https://prometheus.instance.com/') } + + it 'creates a service from the instance-level integration' do + described_class.create_from_active_default_integrations(project, :project_id, with_templates: true) + + expect(project.reload.services.size).to eq(1) + expect(project.reload.services.first.api_url).to eq(instance_integration.api_url) + expect(project.reload.services.first.inherit_from_id).to eq(instance_integration.id) + end + + context 'passing a group' do + it 'creates a service from the instance-level integration' do + described_class.create_from_active_default_integrations(group, :group_id) + + expect(group.reload.services.size).to eq(1) + expect(group.reload.services.first.api_url).to eq(instance_integration.api_url) + expect(group.reload.services.first.inherit_from_id).to eq(instance_integration.id) + end + end + + context 'with an active group-level integration' do + let!(:group_integration) { create(:prometheus_service, group: group, project: nil, api_url: 'https://prometheus.group.com/') } + + it 'creates a service from the group-level integration' do + described_class.create_from_active_default_integrations(project, :project_id, with_templates: true) + + expect(project.reload.services.size).to eq(1) + expect(project.reload.services.first.api_url).to eq(group_integration.api_url) + expect(project.reload.services.first.inherit_from_id).to eq(group_integration.id) + end + + context 'passing a group' do + let!(:subgroup) { create(:group, parent: group) } + + it 'creates a service from the group-level integration' do + described_class.create_from_active_default_integrations(subgroup, :group_id) + + expect(subgroup.reload.services.size).to eq(1) + expect(subgroup.reload.services.first.api_url).to eq(group_integration.api_url) + expect(subgroup.reload.services.first.inherit_from_id).to eq(group_integration.id) + end + end + + context 'with an active subgroup' do + let!(:subgroup_integration) { create(:prometheus_service, group: subgroup, project: nil, api_url: 'https://prometheus.subgroup.com/') } + let!(:subgroup) { create(:group, parent: group) } + let(:project) { create(:project, group: subgroup) } + + it 'creates a service from the subgroup-level integration' do + described_class.create_from_active_default_integrations(project, :project_id, with_templates: true) + + expect(project.reload.services.size).to eq(1) + expect(project.reload.services.first.api_url).to eq(subgroup_integration.api_url) + expect(project.reload.services.first.inherit_from_id).to eq(subgroup_integration.id) + end + + context 'passing a group' do + let!(:sub_subgroup) { create(:group, parent: subgroup) } + + it 'creates a service from the subgroup-level integration' do + described_class.create_from_active_default_integrations(sub_subgroup, :group_id) + + expect(sub_subgroup.reload.services.size).to eq(1) + expect(sub_subgroup.reload.services.first.api_url).to eq(subgroup_integration.api_url) + expect(sub_subgroup.reload.services.first.inherit_from_id).to eq(subgroup_integration.id) + end + + context 'having a service inheriting settings' do + let!(:subgroup_integration) { create(:prometheus_service, group: subgroup, project: nil, inherit_from_id: group_integration.id, api_url: 'https://prometheus.subgroup.com/') } + + it 'creates a service from the group-level integration' do + described_class.create_from_active_default_integrations(sub_subgroup, :group_id) + + expect(sub_subgroup.reload.services.size).to eq(1) + expect(sub_subgroup.reload.services.first.api_url).to eq(group_integration.api_url) + expect(sub_subgroup.reload.services.first.inherit_from_id).to eq(group_integration.id) + end + end + end + end end 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..cdbc1feefce 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/spec/models/snippet_repository_spec.rb @@ -13,6 +13,11 @@ RSpec.describe SnippetRepository do it { is_expected.to belong_to(:snippet) } end + it_behaves_like 'shardable scopes' do + let_it_be(:record_1) { create(:snippet_repository) } + let_it_be(:record_2, reload: true) { create(:snippet_repository) } + end + describe '.find_snippet' do it 'finds snippet by disk path' do snippet = create(:snippet, author: user) @@ -35,7 +40,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 +48,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 +65,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 +82,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 +97,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 +119,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 +133,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 +154,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 +182,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 +219,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 +239,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 +251,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 +264,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 +279,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_spec.rb b/spec/models/snippet_spec.rb index ab614a6d45c..d74f5faab7f 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -666,16 +666,13 @@ RSpec.describe Snippet do let(:checker) { subject.repository_size_checker } let(:current_size) { 60 } + let(:namespace) { nil } before do allow(subject.repository).to receive(:size).and_return(current_size) end - it 'sets up size checker', :aggregate_failures do - expect(checker.current_size).to eq(current_size.megabytes) - expect(checker.limit).to eq(Gitlab::CurrentSettings.snippet_size_limit) - expect(checker.enabled?).to be_truthy - end + include_examples 'size checker for snippet' end describe '#can_cache_field?' do @@ -717,19 +714,11 @@ RSpec.describe Snippet do end describe '.max_file_limit' do - subject { described_class.max_file_limit(nil) } + subject { described_class.max_file_limit } it "returns #{Snippet::MAX_FILE_COUNT}" do expect(subject).to eq Snippet::MAX_FILE_COUNT end - - context 'when feature flag :snippet_multiple_files is disabled' do - it "returns #{described_class::MAX_SINGLE_FILE_COUNT}" do - stub_feature_flags(snippet_multiple_files: false) - - expect(subject).to eq described_class::MAX_SINGLE_FILE_COUNT - end - end end describe '#list_files' do 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..608c5bdf03a 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' @@ -70,11 +87,17 @@ RSpec.describe Terraform::State do let(:terraform_state) { create(:terraform_state, :with_file) } it { is_expected.to eq terraform_state.file } + + context 'and a version exists (migration to versioned in progress)' do + let!(:migrated_version) { create(:terraform_state_version, terraform_state: terraform_state) } + + it { is_expected.to eq terraform_state.latest_version.file } + end end end describe '#update_file!' do - let(:version) { 2 } + let(:version) { 3 } let(:data) { Hash[terraform_version: '0.12.21'].to_json } subject { terraform_state.update_file!(CarrierWaveStringFile.new(data), version: version) } @@ -98,6 +121,33 @@ RSpec.describe Terraform::State do expect(terraform_state.latest_file.read).to eq(data) end + + context 'and a version exists (migration to versioned in progress)' do + let!(:migrated_version) { create(:terraform_state_version, terraform_state: terraform_state, version: 0) } + + it 'creates a new version, corrects the migrated version number, and marks the state as versioned' do + expect { subject }.to change { Terraform::StateVersion.count } + + expect(migrated_version.reload.version).to eq(1) + expect(migrated_version.file.read).to eq(terraform_state_file) + + expect(terraform_state.reload.latest_version.version).to eq(version) + expect(terraform_state.latest_version.file.read).to eq(data) + expect(terraform_state).to be_versioning_enabled + end + + context 'the current version cannot be determined' do + before do + migrated_version.update!(file: CarrierWaveStringFile.new('invalid-json')) + end + + it 'uses version - 1 to correct the migrated version number' do + expect { subject }.to change { Terraform::StateVersion.count } + + expect(migrated_version.reload.version).to eq(2) + end + end + end end end end 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..64bff5d00aa 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -705,6 +705,34 @@ RSpec.describe User do end describe "scopes" do + context 'blocked users' do + let_it_be(:active_user) { create(:user) } + let_it_be(:blocked_user) { create(:user, :blocked) } + let_it_be(:ldap_blocked_user) { create(:omniauth_user, :ldap_blocked) } + let_it_be(:blocked_pending_approval_user) { create(:user, :blocked_pending_approval) } + + describe '.blocked' do + subject { described_class.blocked } + + it 'returns only blocked users' do + expect(subject).to include( + blocked_user, + ldap_blocked_user + ) + + expect(subject).not_to include(active_user, blocked_pending_approval_user) + end + end + + describe '.blocked_pending_approval' do + subject { described_class.blocked_pending_approval } + + it 'returns only pending approval users' do + expect(subject).to contain_exactly(blocked_pending_approval_user) + end + end + end + describe ".with_two_factor" do it "returns users with 2fa enabled via OTP" do user_with_2fa = create(:user, :two_factor_via_otp) @@ -1694,6 +1722,24 @@ RSpec.describe User do end end + describe 'blocking a user pending approval' do + let(:user) { create(:user) } + + before do + user.block_pending_approval + end + + context 'an active user' do + it 'can be blocked pending approval' do + expect(user.blocked_pending_approval?).to eq(true) + end + + it 'behaves like a blocked user' do + expect(user.blocked?).to eq(true) + end + end + end + describe '.filter_items' do let(:user) { double } @@ -1715,6 +1761,12 @@ RSpec.describe User do expect(described_class.filter_items('blocked')).to include user end + it 'filters by blocked pending approval' do + expect(described_class).to receive(:blocked_pending_approval).and_return([user]) + + expect(described_class.filter_items('blocked_pending_approval')).to include user + end + it 'filters by deactivated' do expect(described_class).to receive(:deactivated).and_return([user]) @@ -2744,6 +2796,14 @@ RSpec.describe User do it_behaves_like 'eligible for deactivation' end + + context 'a user who is internal' do + it 'returns false' do + internal_user = create(:user, :bot) + + expect(internal_user.can_be_deactivated?).to be_falsey + end + end end describe "#contributed_projects" do @@ -3902,7 +3962,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 +4390,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 @@ -4818,7 +4882,8 @@ RSpec.describe User do { state: 'blocked' }, { user_type: :ghost }, { user_type: :alert_bot }, - { user_type: :support_bot } + { user_type: :support_bot }, + { user_type: :security_bot } ] end @@ -4873,6 +4938,7 @@ RSpec.describe User do 'human' | true 'alert_bot' | false 'support_bot' | false + 'security_bot' | false end with_them do @@ -4895,7 +4961,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 +4969,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 @@ -4913,6 +4979,14 @@ RSpec.describe User do it { is_expected.to be :locked } end + + context 'when user is blocked pending approval' do + before do + user.block_pending_approval! + end + + it { is_expected.to be :blocked_pending_approval } + end end describe '#password_required?' do @@ -4976,9 +5050,11 @@ RSpec.describe User do it_behaves_like 'bot users', :alert_bot it_behaves_like 'bot users', :support_bot it_behaves_like 'bot users', :migration_bot + it_behaves_like 'bot users', :security_bot it_behaves_like 'bot users', :ghost it_behaves_like 'bot user avatars', :alert_bot, 'alert-bot.png' it_behaves_like 'bot user avatars', :support_bot, 'support-bot.png' + it_behaves_like 'bot user avatars', :security_bot, 'security-bot.png' end end 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 diff --git a/spec/models/wiki_spec.rb b/spec/models/wiki_spec.rb deleted file mode 100644 index 8dd510a0b98..00000000000 --- a/spec/models/wiki_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Wiki do - describe '.new' do - it 'verifies that the user is a User' do - expect { described_class.new(double, 1) }.to raise_error(ArgumentError) - expect { described_class.new(double, build(:group)) }.to raise_error(ArgumentError) - expect { described_class.new(double, build(:user)) }.not_to raise_error - expect { described_class.new(double, nil) }.not_to raise_error - end - end -end |