From 6edb7e9bb152d919c215f35bd6cb7d52fd3d99be Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 8 Nov 2021 10:33:01 +0000 Subject: Add latest changes from gitlab-org/gitlab@14-4-stable-ee --- .../fix_first_mentioned_in_commit_at_spec.rb | 74 +++++++++++++------ .../database/load_balancing/load_balancer_spec.rb | 18 +++++ .../size_limiter/validator_spec.rb | 86 ++++++++++++++++++++-- spec/models/legacy_diff_note_spec.rb | 42 +++++++++++ spec/models/protected_branch_spec.rb | 17 +++-- 5 files changed, 200 insertions(+), 37 deletions(-) (limited to 'spec') diff --git a/spec/lib/gitlab/background_migration/fix_first_mentioned_in_commit_at_spec.rb b/spec/lib/gitlab/background_migration/fix_first_mentioned_in_commit_at_spec.rb index d2bfa86f0d1..7f15aceca42 100644 --- a/spec/lib/gitlab/background_migration/fix_first_mentioned_in_commit_at_spec.rb +++ b/spec/lib/gitlab/background_migration/fix_first_mentioned_in_commit_at_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20211004110500_add_temporary_index_to_issue_metrics.rb') RSpec.describe Gitlab::BackgroundMigration::FixFirstMentionedInCommitAt, :migration, schema: 20211004110500 do let(:namespaces) { table(:namespaces) } @@ -99,42 +100,67 @@ RSpec.describe Gitlab::BackgroundMigration::FixFirstMentionedInCommitAt, :migrat .perform(issue_metrics.minimum(:issue_id), issue_metrics.maximum(:issue_id)) end - it "marks successful slices as completed" do - min_issue_id = issue_metrics.minimum(:issue_id) - max_issue_id = issue_metrics.maximum(:issue_id) + shared_examples 'fixes first_mentioned_in_commit_at' do + it "marks successful slices as completed" do + min_issue_id = issue_metrics.minimum(:issue_id) + max_issue_id = issue_metrics.maximum(:issue_id) - expect(subject).to receive(:mark_job_as_succeeded).with(min_issue_id, max_issue_id) + expect(subject).to receive(:mark_job_as_succeeded).with(min_issue_id, max_issue_id) - subject.perform(min_issue_id, max_issue_id) - end + subject.perform(min_issue_id, max_issue_id) + end - context 'when the persisted first_mentioned_in_commit_at is later than the first commit authored_date' do - it 'updates the issue_metrics record' do - record1 = issue_metrics.create!(issue_id: issue1.id, first_mentioned_in_commit_at: Time.current) - record2 = issue_metrics.create!(issue_id: issue2.id, first_mentioned_in_commit_at: Time.current) + context 'when the persisted first_mentioned_in_commit_at is later than the first commit authored_date' do + it 'updates the issue_metrics record' do + record1 = issue_metrics.create!(issue_id: issue1.id, first_mentioned_in_commit_at: Time.current) + record2 = issue_metrics.create!(issue_id: issue2.id, first_mentioned_in_commit_at: Time.current) - run_migration - record1.reload - record2.reload + run_migration + record1.reload + record2.reload - expect(record1.first_mentioned_in_commit_at).to be_within(2.seconds).of(commit2.authored_date) - expect(record2.first_mentioned_in_commit_at).to be_within(2.seconds).of(commit3.authored_date) + expect(record1.first_mentioned_in_commit_at).to be_within(2.seconds).of(commit2.authored_date) + expect(record2.first_mentioned_in_commit_at).to be_within(2.seconds).of(commit3.authored_date) + end + end + + context 'when the persisted first_mentioned_in_commit_at is earlier than the first commit authored_date' do + it 'does not update the issue_metrics record' do + record = issue_metrics.create!(issue_id: issue1.id, first_mentioned_in_commit_at: 20.days.ago) + + expect { run_migration }.not_to change { record.reload.first_mentioned_in_commit_at } + end end - end - context 'when the persisted first_mentioned_in_commit_at is earlier than the first commit authored_date' do - it 'does not update the issue_metrics record' do - record = issue_metrics.create!(issue_id: issue1.id, first_mentioned_in_commit_at: 20.days.ago) + context 'when the first_mentioned_in_commit_at is null' do + it 'does nothing' do + record = issue_metrics.create!(issue_id: issue1.id, first_mentioned_in_commit_at: nil) - expect { run_migration }.not_to change { record.reload.first_mentioned_in_commit_at } + expect { run_migration }.not_to change { record.reload.first_mentioned_in_commit_at } + end end end - context 'when the first_mentioned_in_commit_at is null' do - it 'does nothing' do - record = issue_metrics.create!(issue_id: issue1.id, first_mentioned_in_commit_at: nil) + describe 'running the migration when first_mentioned_in_commit_at is timestamp without time zone' do + it_behaves_like 'fixes first_mentioned_in_commit_at' + end + + describe 'running the migration when first_mentioned_in_commit_at is timestamp with time zone' do + around do |example| + AddTemporaryIndexToIssueMetrics.new.down + + ActiveRecord::Base.connection.execute "ALTER TABLE issue_metrics ALTER first_mentioned_in_commit_at type timestamp with time zone" + Gitlab::BackgroundMigration::FixFirstMentionedInCommitAt::TmpIssueMetrics.reset_column_information + AddTemporaryIndexToIssueMetrics.new.up - expect { run_migration }.not_to change { record.reload.first_mentioned_in_commit_at } + example.run + + AddTemporaryIndexToIssueMetrics.new.down + ActiveRecord::Base.connection.execute "ALTER TABLE issue_metrics ALTER first_mentioned_in_commit_at type timestamp without time zone" + Gitlab::BackgroundMigration::FixFirstMentionedInCommitAt::TmpIssueMetrics.reset_column_information + AddTemporaryIndexToIssueMetrics.new.up end + + it_behaves_like 'fixes first_mentioned_in_commit_at' end end diff --git a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb index f3ce5563e38..f824d4cefdf 100644 --- a/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb +++ b/spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb @@ -140,6 +140,24 @@ RSpec.describe Gitlab::Database::LoadBalancing::LoadBalancer, :request_store do lb.read { raise conflict_error } end + context 'only primary is configured' do + let(:lb) do + config = Gitlab::Database::LoadBalancing::Configuration.new(ActiveRecord::Base) + allow(config).to receive(:load_balancing_enabled?).and_return(false) + + described_class.new(config) + end + + it 'does not retry a query on connection error if only the primary is configured' do + host = double(:host, query_cache_enabled: true) + + allow(lb).to receive(:host).and_return(host) + allow(host).to receive(:connection).and_raise(PG::UnableToSend) + + expect { lb.read }.to raise_error(PG::UnableToSend) + end + end + it 'uses the primary if no secondaries are available' do allow(lb).to receive(:connection_error?).and_return(true) diff --git a/spec/lib/gitlab/sidekiq_middleware/size_limiter/validator_spec.rb b/spec/lib/gitlab/sidekiq_middleware/size_limiter/validator_spec.rb index abbfb9cd9fa..3a6fdd7642c 100644 --- a/spec/lib/gitlab/sidekiq_middleware/size_limiter/validator_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/size_limiter/validator_spec.rb @@ -187,37 +187,51 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai context 'when size limit is 0' do let(:size_limit) { 0 } + let(:job) { job_payload(a: 'a' * 300) } it 'does not track jobs' do expect(Gitlab::ErrorTracking).not_to receive(:track_exception) - validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) + validate.call(TestSizeLimiterWorker, job) end it 'does not raise exception' do expect do - validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) + validate.call(TestSizeLimiterWorker, job) end.not_to raise_error end + + it 'marks the job as validated' do + validate.call(TestSizeLimiterWorker, job) + + expect(job['size_limiter']).to eq('validated') + end end context 'when job size is bigger than size limit' do let(:size_limit) { 50 } + let(:job) { job_payload(a: 'a' * 300) } it 'tracks job' do expect(Gitlab::ErrorTracking).to receive(:track_exception).with( be_a(Gitlab::SidekiqMiddleware::SizeLimiter::ExceedLimitError) ) - validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 100)) + validate.call(TestSizeLimiterWorker, job) end it 'does not raise an exception' do expect do - validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) + validate.call(TestSizeLimiterWorker, job) end.not_to raise_error end + it 'marks the job as tracked' do + validate.call(TestSizeLimiterWorker, job) + + expect(job['size_limiter']).to eq('tracked') + end + context 'when the worker has big_payload attribute' do before do worker_class.big_payload! @@ -238,20 +252,33 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai validate.call('TestSizeLimiterWorker', job_payload(a: 'a' * 300)) end.not_to raise_error end + + it 'marks the job as validated' do + validate.call(TestSizeLimiterWorker, job) + + expect(job['size_limiter']).to eq('validated') + end end end context 'when job size is less than size limit' do let(:size_limit) { 50 } + let(:job) { job_payload(a: 'a') } it 'does not track job' do expect(Gitlab::ErrorTracking).not_to receive(:track_exception) - validate.call(TestSizeLimiterWorker, job_payload(a: 'a')) + validate.call(TestSizeLimiterWorker, job) end it 'does not raise an exception' do - expect { validate.call(TestSizeLimiterWorker, job_payload(a: 'a')) }.not_to raise_error + expect { validate.call(TestSizeLimiterWorker, job) }.not_to raise_error + end + + it 'marks the job as validated' do + validate.call(TestSizeLimiterWorker, job) + + expect(job['size_limiter']).to eq('validated') end end end @@ -266,7 +293,13 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai it 'does not raise an exception' do expect(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor).not_to receive(:compress) - expect { validate.call(TestSizeLimiterWorker, job_payload(a: 'a')) }.not_to raise_error + expect { validate.call(TestSizeLimiterWorker, job) }.not_to raise_error + end + + it 'marks the job as validated' do + validate.call(TestSizeLimiterWorker, job) + + expect(job['size_limiter']).to eq('validated') end end @@ -283,6 +316,12 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai validate.call(TestSizeLimiterWorker, job) end.not_to raise_error end + + it 'marks the job as validated' do + validate.call(TestSizeLimiterWorker, job) + + expect(job['size_limiter']).to eq('validated') + end end context 'when job size is bigger than compression threshold and size limit is 0' do @@ -299,6 +338,12 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai validate.call(TestSizeLimiterWorker, job) end.not_to raise_error end + + it 'marks the job as validated' do + validate.call(TestSizeLimiterWorker, job) + + expect(job['size_limiter']).to eq('validated') + end end context 'when the job was already compressed' do @@ -326,6 +371,8 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai expect do validate.call(TestSizeLimiterWorker, job) end.to raise_error(Gitlab::SidekiqMiddleware::SizeLimiter::ExceedLimitError) + + expect(job['size_limiter']).to eq(nil) end it 'does not raise an exception when the worker allows big payloads' do @@ -338,6 +385,8 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai expect do validate.call(TestSizeLimiterWorker, job) end.not_to raise_error + + expect(job['size_limiter']).to eq('validated') end end end @@ -363,6 +412,29 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai validate.call(class_name.constantize, job_payload) end end + + it "skips jobs that are already validated" do + expect(described_class).to receive(:new).once.and_call_original + + job = job_payload + + described_class.validate!(TestSizeLimiterWorker, job) + described_class.validate!(TestSizeLimiterWorker, job) + end + end + + describe '.validated?' do + let(:job) { job_payload } + + it 'returns true when the job is already validated' do + described_class.validate!(TestSizeLimiterWorker, job) + + expect(described_class.validated?(job)).to eq(true) + end + + it 'returns false when job is not yet validated' do + expect(described_class.validated?(job)).to eq(false) + end end describe '#validate!' do diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb index ee3bbf186b9..8934fe6b107 100644 --- a/spec/models/legacy_diff_note_spec.rb +++ b/spec/models/legacy_diff_note_spec.rb @@ -8,4 +8,46 @@ RSpec.describe LegacyDiffNote do it { is_expected.to eq('note') } end + + describe 'callbacks' do + describe '#set_diff' do + let(:note) do + build(:legacy_diff_note_on_merge_request, st_diff: '_st_diff_').tap do |record| + record.instance_variable_set(:@diff, {}) + end + end + + context 'when not importing' do + it 'updates st_diff' do + note.save!(validate: false) + + expect(note.st_diff).to eq({}) + end + end + + context 'when importing' do + before do + note.importing = true + end + + it 'does not update st_diff' do + note.save!(validate: false) + + expect(note.st_diff).to eq('_st_diff_') + end + + context 'when st_diff is blank' do + before do + note.st_diff = nil + end + + it 'updates st_diff' do + note.save!(validate: false) + + expect(note.st_diff).to eq({}) + end + end + end + end + end end diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 587a9683a8e..f7c723cd134 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -163,27 +163,32 @@ RSpec.describe ProtectedBranch do expect(described_class.protected?(project, 'staging/some-branch')).to eq(false) end + it 'returns false when branch name is nil' do + expect(described_class.protected?(project, nil)).to eq(false) + end + context 'with caching', :use_clean_rails_memory_store_caching do let_it_be(:project) { create(:project, :repository) } - let_it_be(:protected_branch) { create(:protected_branch, project: project, name: "jawn") } + let_it_be(:protected_branch) { create(:protected_branch, project: project, name: "“jawn”") } before do - allow(described_class).to receive(:matching).once.and_call_original + allow(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).once.and_call_original + # the original call works and warms the cache - described_class.protected?(project, 'jawn') + described_class.protected?(project, protected_branch.name) end it 'correctly invalidates a cache' do - expect(described_class).to receive(:matching).once.and_call_original + expect(described_class).to receive(:matching).with(protected_branch.name, protected_refs: anything).once.and_call_original create(:protected_branch, project: project, name: "bar") # the cache is invalidated because the project has been "updated" - expect(described_class.protected?(project, 'jawn')).to eq(true) + expect(described_class.protected?(project, protected_branch.name)).to eq(true) end it 'correctly uses the cached version' do expect(described_class).not_to receive(:matching) - expect(described_class.protected?(project, 'jawn')).to eq(true) + expect(described_class.protected?(project, protected_branch.name)).to eq(true) end end end -- cgit v1.2.1