From b39512ed755239198a9c294b6a45e65c05900235 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 18 Aug 2022 08:17:02 +0000 Subject: Add latest changes from gitlab-org/gitlab@15-3-stable-ee --- spec/models/concerns/bulk_insert_safe_spec.rb | 4 +- .../concerns/chronic_duration_attribute_spec.rb | 12 ++-- spec/models/concerns/ci/artifactable_spec.rb | 24 +++++++- spec/models/concerns/counter_attribute_spec.rb | 2 +- .../concerns/cross_database_modification_spec.rb | 32 ---------- .../concerns/database_event_tracking_spec.rb | 69 ++++++++++++++++++++++ spec/models/concerns/expirable_spec.rb | 5 ++ spec/models/concerns/issuable_spec.rb | 21 +++++++ spec/models/concerns/nullify_if_blank_spec.rb | 2 +- spec/models/concerns/participable_spec.rb | 26 +++++++- .../project_features_compatibility_spec.rb | 6 +- spec/models/concerns/reactive_caching_spec.rb | 2 +- spec/models/concerns/token_authenticatable_spec.rb | 12 ++-- 13 files changed, 165 insertions(+), 52 deletions(-) create mode 100644 spec/models/concerns/database_event_tracking_spec.rb (limited to 'spec/models/concerns') diff --git a/spec/models/concerns/bulk_insert_safe_spec.rb b/spec/models/concerns/bulk_insert_safe_spec.rb index e6b197f34ca..569dc3a3a3e 100644 --- a/spec/models/concerns/bulk_insert_safe_spec.rb +++ b/spec/models/concerns/bulk_insert_safe_spec.rb @@ -170,7 +170,9 @@ RSpec.describe BulkInsertSafe do all_items = bulk_insert_item_class.valid_list(10) + bulk_insert_item_class.invalid_list(10) expect do - bulk_insert_item_class.bulk_insert!(all_items, batch_size: 2) rescue nil + bulk_insert_item_class.bulk_insert!(all_items, batch_size: 2) + rescue StandardError + nil end.not_to change { bulk_insert_item_class.count } end diff --git a/spec/models/concerns/chronic_duration_attribute_spec.rb b/spec/models/concerns/chronic_duration_attribute_spec.rb index 00e28e19bd5..61b86455840 100644 --- a/spec/models/concerns/chronic_duration_attribute_spec.rb +++ b/spec/models/concerns/chronic_duration_attribute_spec.rb @@ -95,8 +95,8 @@ end RSpec.describe 'ChronicDurationAttribute' do context 'when default value is not set' do - let(:source_field) {:maximum_timeout} - let(:virtual_field) {:maximum_timeout_human_readable} + let(:source_field) { :maximum_timeout } + let(:virtual_field) { :maximum_timeout_human_readable } let(:default_value) { nil } subject { create(:ci_runner) } @@ -106,8 +106,8 @@ RSpec.describe 'ChronicDurationAttribute' do end context 'when default value is set' do - let(:source_field) {:build_timeout} - let(:virtual_field) {:build_timeout_human_readable} + let(:source_field) { :build_timeout } + let(:virtual_field) { :build_timeout_human_readable } let(:default_value) { 3600 } subject { create(:project) } @@ -118,8 +118,8 @@ RSpec.describe 'ChronicDurationAttribute' do end RSpec.describe 'ChronicDurationAttribute - reader' do - let(:source_field) {:timeout} - let(:virtual_field) {:timeout_human_readable} + let(:source_field) { :timeout } + let(:virtual_field) { :timeout_human_readable } subject { create(:ci_build).ensure_metadata } diff --git a/spec/models/concerns/ci/artifactable_spec.rb b/spec/models/concerns/ci/artifactable_spec.rb index 6af244a5a0f..64691165e21 100644 --- a/spec/models/concerns/ci/artifactable_spec.rb +++ b/spec/models/concerns/ci/artifactable_spec.rb @@ -46,8 +46,30 @@ RSpec.describe Ci::Artifactable do end end + context 'when file format is zip' do + context 'when artifact contains one file' do + let(:artifact) { build(:ci_job_artifact, :zip_with_single_file) } + + it 'iterates blob once' do + expect { |b| artifact.each_blob(&b) }.to yield_control.once + end + end + + context 'when artifact contains two files' do + let(:artifact) { build(:ci_job_artifact, :zip_with_multiple_files) } + + it 'iterates blob two times' do + expect { |b| artifact.each_blob(&b) }.to yield_control.exactly(2).times + end + end + end + context 'when there are no adapters for the file format' do - let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) } + let(:artifact) { build(:ci_job_artifact, :junit) } + + before do + allow(artifact).to receive(:file_format).and_return(:unknown) + end it 'raises an error' do expect { |b| artifact.each_blob(&b) }.to raise_error(described_class::NotSupportedAdapterError) diff --git a/spec/models/concerns/counter_attribute_spec.rb b/spec/models/concerns/counter_attribute_spec.rb index a19fbae3cfb..8d32ef14f47 100644 --- a/spec/models/concerns/counter_attribute_spec.rb +++ b/spec/models/concerns/counter_attribute_spec.rb @@ -13,7 +13,7 @@ RSpec.describe CounterAttribute, :counter_attribute, :clean_gitlab_redis_shared_ end describe 'after_flush callbacks' do - let(:attribute) { model.class.counter_attributes.first} + let(:attribute) { model.class.counter_attributes.first } subject { model.flush_increments_to_database!(attribute) } diff --git a/spec/models/concerns/cross_database_modification_spec.rb b/spec/models/concerns/cross_database_modification_spec.rb index 72544536953..c3831b654cf 100644 --- a/spec/models/concerns/cross_database_modification_spec.rb +++ b/spec/models/concerns/cross_database_modification_spec.rb @@ -4,38 +4,6 @@ require 'spec_helper' RSpec.describe CrossDatabaseModification do describe '.transaction' do - context 'feature flag disabled' do - before do - stub_feature_flags(track_gitlab_schema_in_current_transaction: false) - end - - it 'does not add to gitlab_transactions_stack' do - ApplicationRecord.transaction do - expect(ApplicationRecord.gitlab_transactions_stack).to be_empty - - Project.first - end - - expect(ApplicationRecord.gitlab_transactions_stack).to be_empty - end - end - - context 'feature flag is not yet setup' do - before do - allow(Feature::FlipperFeature).to receive(:table_exists?).and_raise(ActiveRecord::NoDatabaseError) - end - - it 'does not add to gitlab_transactions_stack' do - ApplicationRecord.transaction do - expect(ApplicationRecord.gitlab_transactions_stack).to be_empty - - Project.first - end - - expect(ApplicationRecord.gitlab_transactions_stack).to be_empty - end - end - it 'adds the current gitlab schema to gitlab_transactions_stack', :aggregate_failures do ApplicationRecord.transaction do expect(ApplicationRecord.gitlab_transactions_stack).to contain_exactly(:gitlab_main) diff --git a/spec/models/concerns/database_event_tracking_spec.rb b/spec/models/concerns/database_event_tracking_spec.rb new file mode 100644 index 00000000000..976462b4174 --- /dev/null +++ b/spec/models/concerns/database_event_tracking_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe DatabaseEventTracking, :snowplow do + let(:test_class) do + Class.new(ActiveRecord::Base) do + include DatabaseEventTracking + + self.table_name = 'application_setting_terms' + + self::SNOWPLOW_ATTRIBUTES = %w[id].freeze # rubocop:disable RSpec/LeakyConstantDeclaration + end + end + + subject(:create_test_class_record) { test_class.create!(id: 1, terms: "") } + + context 'if event emmiter failed' do + before do + allow(Gitlab::Tracking).to receive(:event).and_raise(StandardError) # rubocop:disable RSpec/ExpectGitlabTracking + end + + it 'tracks the exception' do + expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) + + create_test_class_record + end + end + + context 'if product_intelligence_database_event_tracking FF is off' do + before do + stub_feature_flags(product_intelligence_database_event_tracking: false) + end + + it 'does not track the event' do + create_test_class_record + + expect_no_snowplow_event + end + end + + describe 'event tracking' do + let(:category) { test_class.to_s } + let(:event) { 'database_event' } + + it 'when created' do + create_test_class_record + + expect_snowplow_event(category: category, action: "#{event}_create", label: 'application_setting_terms', + property: 'create', namespace: nil, "id" => 1) + end + + it 'when updated' do + create_test_class_record + test_class.first.update!(id: 3) + + expect_snowplow_event(category: category, action: "#{event}_update", label: 'application_setting_terms', + property: 'update', namespace: nil, "id" => 3) + end + + it 'when destroyed' do + create_test_class_record + test_class.first.destroy! + + expect_snowplow_event(category: category, action: "#{event}_destroy", label: 'application_setting_terms', + property: 'destroy', namespace: nil, "id" => 1) + end + end +end diff --git a/spec/models/concerns/expirable_spec.rb b/spec/models/concerns/expirable_spec.rb index 5eb6530881e..50dfb138ac9 100644 --- a/spec/models/concerns/expirable_spec.rb +++ b/spec/models/concerns/expirable_spec.rb @@ -16,6 +16,11 @@ RSpec.describe Expirable do it { expect(ProjectMember.expired).to match_array([expired]) } end + describe '.not_expired' do + it { expect(ProjectMember.not_expired).to include(no_expire, expire_later) } + it { expect(ProjectMember.not_expired).not_to include(expired) } + end + describe '#expired?' do it { expect(no_expire.expired?).to eq(false) } it { expect(expire_later.expired?).to eq(false) } diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 87821de3cf5..6763cc904b4 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -569,6 +569,27 @@ RSpec.describe Issuable do end end + context 'merge_request update reviewers' do + let(:merge_request) { create(:merge_request) } + let(:user2) { create(:user) } + + before do + merge_request.update!(reviewers: [user]) + merge_request.update!(reviewers: [user, user2]) + expect(Gitlab::DataBuilder::Issuable) + .to receive(:new).with(merge_request).and_return(builder) + end + + it 'delegates to Gitlab::DataBuilder::Issuable#build' do + expect(builder).to receive(:build).with( + user: user, + changes: hash_including( + 'reviewers' => [[user.hook_attrs], [user.hook_attrs, user2.hook_attrs]] + )) + merge_request.to_hook_data(user, old_associations: { reviewers: [user] }) + end + end + context 'incident severity is updated' do let(:issue) { create(:incident) } diff --git a/spec/models/concerns/nullify_if_blank_spec.rb b/spec/models/concerns/nullify_if_blank_spec.rb index 2d1bdba39dd..b0e229f4c91 100644 --- a/spec/models/concerns/nullify_if_blank_spec.rb +++ b/spec/models/concerns/nullify_if_blank_spec.rb @@ -31,7 +31,7 @@ RSpec.describe NullifyIfBlank do context 'attribute is nil' do let(:name) { nil } - it { is_expected.to be_nil} + it { is_expected.to be_nil } end context 'attribute is not blank' do diff --git a/spec/models/concerns/participable_spec.rb b/spec/models/concerns/participable_spec.rb index b92c7c52f0b..f7f68cb38d8 100644 --- a/spec/models/concerns/participable_spec.rb +++ b/spec/models/concerns/participable_spec.rb @@ -124,6 +124,7 @@ RSpec.describe Participable do end let(:readable) { true } + let(:project) { build(:project, :public) } it 'returns the list of participants' do model.participant(:foo) @@ -132,7 +133,6 @@ RSpec.describe Participable do user1 = build(:user) user2 = build(:user) user3 = build(:user) - project = build(:project, :public) instance = model.new allow(instance).to receive_message_chain(:model_name, :element) { 'class' } @@ -155,7 +155,6 @@ RSpec.describe Participable do instance = model.new user1 = build(:user) user2 = build(:user) - project = build(:project, :public) allow(instance).to receive_message_chain(:model_name, :element) { 'class' } allow(instance).to receive(:bar).and_return(user2) @@ -164,6 +163,29 @@ RSpec.describe Participable do expect(instance.visible_participants(user1)).to be_empty end end + + context 'with multiple system notes from the same author and mentioned_users' do + let!(:user1) { create(:user) } + let!(:user2) { create(:user) } + + it 'skips expensive checks if the author is aleady in participants list' do + model.participant(:notes) + + instance = model.new + note1 = create(:system_note, author: user1) + note2 = create(:system_note, author: user1) # only skip system notes with no mentioned users + note3 = create(:system_note, author: user1, note: "assigned to #{user2.to_reference}") + note4 = create(:note, author: user2) + + allow(instance).to receive(:project).and_return(project) + allow(instance).to receive_message_chain(:model_name, :element) { 'class' } + allow(instance).to receive(:notes).and_return([note1, note2, note3, note4]) + + allow(Ability).to receive(:allowed?).with(anything, :read_project, anything).and_return(true) + allow(Ability).to receive(:allowed?).with(anything, :read_note, anything).exactly(3).times.and_return(true) + expect(instance.visible_participants(user1)).to match_array [user1, user2] + end + end end describe '#participant?' do diff --git a/spec/models/concerns/project_features_compatibility_spec.rb b/spec/models/concerns/project_features_compatibility_spec.rb index f2dc8464e86..b49b9ce8a2a 100644 --- a/spec/models/concerns/project_features_compatibility_spec.rb +++ b/spec/models/concerns/project_features_compatibility_spec.rb @@ -5,7 +5,11 @@ require 'spec_helper' RSpec.describe ProjectFeaturesCompatibility do let(:project) { create(:project) } let(:features_enabled) { %w(issues wiki builds merge_requests snippets security_and_compliance) } - let(:features) { features_enabled + %w(repository pages operations container_registry package_registry) } + let(:features) do + features_enabled + %w( + repository pages operations container_registry package_registry environments feature_flags releases + ) + end # We had issues_enabled, snippets_enabled, builds_enabled, merge_requests_enabled and issues_enabled fields on projects table # All those fields got moved to a new table called project_feature and are now integers instead of booleans diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 5468699f9dd..cb9bb676ede 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -320,7 +320,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do stub_reactive_cache(instance, "preexisting") end - let(:calculation) { -> { raise "foo"} } + let(:calculation) { -> { raise "foo" } } it 'leaves the cache untouched' do expect { go! }.to raise_error("foo") diff --git a/spec/models/concerns/token_authenticatable_spec.rb b/spec/models/concerns/token_authenticatable_spec.rb index a2ce02f4661..3f6bbe795cc 100644 --- a/spec/models/concerns/token_authenticatable_spec.rb +++ b/spec/models/concerns/token_authenticatable_spec.rb @@ -102,7 +102,7 @@ RSpec.describe ApplicationSetting, 'TokenAuthenticatable' do subject { described_class.send(:add_authentication_token_field, :runners_registration_token) } it 'raises error' do - expect {subject}.to raise_error(ArgumentError) + expect { subject }.to raise_error(ArgumentError) end end end @@ -126,7 +126,7 @@ RSpec.describe PersonalAccessToken, 'TokenAuthenticatable' do end end - let(:token_value) { 'token' } + let(:token_value) { Devise.friendly_token } let(:token_digest) { Gitlab::CryptoHelper.sha256(token_value) } let(:user) { create(:user) } let(:personal_access_token) do @@ -442,7 +442,7 @@ RSpec.shared_examples 'prefixed token rotation' do context 'token is not set' do it 'generates a new token' do - expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/) + expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/o) expect(instance).not_to be_persisted end end @@ -453,7 +453,7 @@ RSpec.shared_examples 'prefixed token rotation' do end it 'generates a new token' do - expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/) + expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/o) expect(instance).not_to be_persisted end end @@ -475,7 +475,7 @@ RSpec.shared_examples 'prefixed token rotation' do context 'token is not set' do it 'generates a new token' do - expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/) + expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/o) expect(instance).to be_persisted end end @@ -486,7 +486,7 @@ RSpec.shared_examples 'prefixed token rotation' do end it 'generates a new token' do - expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/) + expect(subject).to match(/^#{RunnersTokenPrefixable::RUNNERS_TOKEN_PREFIX}/o) expect(instance).to be_persisted end end -- cgit v1.2.1