diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-19 21:09:10 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-19 21:09:10 +0300 |
commit | 33795139ea8e72756bee3675b4e16387425e6ab1 (patch) | |
tree | 3ca568fca61482e57810ee30ad5ce4b964a82c4e /spec | |
parent | c7e385e282bcb8505589bce526e692b7bb819ffa (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
22 files changed, 744 insertions, 20 deletions
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 6b698c6da66..ee61ef73b45 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -135,6 +135,10 @@ describe Projects::MilestonesController do end describe "#destroy" do + before do + stub_feature_flags(track_resource_milestone_change_events: false) + end + it "removes milestone" do expect(issue.milestone_id).to eq(milestone.id) diff --git a/spec/factories/resource_milestone_event.rb b/spec/factories/resource_milestone_event.rb new file mode 100644 index 00000000000..86c54f2be68 --- /dev/null +++ b/spec/factories/resource_milestone_event.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :resource_milestone_event do + issue { merge_request.nil? ? create(:issue) : nil } + merge_request { nil } + milestone + action { :add } + state { :opened } + user { issue&.author || merge_request&.author || create(:user) } + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 4cf2553b90d..bb5475130cf 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1893,4 +1893,60 @@ describe Gitlab::Database::MigrationHelpers do end end end + + describe '#migrate_async' do + it 'calls BackgroundMigrationWorker.perform_async' do + expect(BackgroundMigrationWorker).to receive(:perform_async).with("Class", "hello", "world") + + model.migrate_async("Class", "hello", "world") + end + + it 'pushes a context with the current class name as caller_id' do + expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) + + model.migrate_async('Class', 'hello', 'world') + end + end + + describe '#migrate_in' do + it 'calls BackgroundMigrationWorker.perform_in' do + expect(BackgroundMigrationWorker).to receive(:perform_in).with(10.minutes, 'Class', 'Hello', 'World') + + model.migrate_in(10.minutes, 'Class', 'Hello', 'World') + end + + it 'pushes a context with the current class name as caller_id' do + expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) + + model.migrate_in(10.minutes, 'Class', 'Hello', 'World') + end + end + + describe '#bulk_migrate_async' do + it 'calls BackgroundMigrationWorker.bulk_perform_async' do + expect(BackgroundMigrationWorker).to receive(:bulk_perform_async).with([%w(Class hello world)]) + + model.bulk_migrate_async([%w(Class hello world)]) + end + + it 'pushes a context with the current class name as caller_id' do + expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) + + model.bulk_migrate_async([%w(Class hello world)]) + end + end + + describe '#bulk_migrate_in' do + it 'calls BackgroundMigrationWorker.bulk_perform_in_' do + expect(BackgroundMigrationWorker).to receive(:bulk_perform_in).with(10.minutes, [%w(Class hello world)]) + + model.bulk_migrate_in(10.minutes, [%w(Class hello world)]) + end + + it 'pushes a context with the current class name as caller_id' do + expect(Gitlab::ApplicationContext).to receive(:with_context).with(caller_id: model.class.to_s) + + model.bulk_migrate_in(10.minutes, [%w(Class hello world)]) + end + end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 4dadb310029..6c9a4bbfc71 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -9,6 +9,7 @@ issues: - notes - resource_label_events - resource_weight_events +- resource_milestone_events - sentry_issue - label_links - labels @@ -109,6 +110,7 @@ merge_requests: - milestone - notes - resource_label_events +- resource_milestone_events - label_links - labels - last_edited_by diff --git a/spec/lib/gitlab/sidekiq_config/worker_spec.rb b/spec/lib/gitlab/sidekiq_config/worker_spec.rb index 38edd0f5eeb..fcdce8f9432 100644 --- a/spec/lib/gitlab/sidekiq_config/worker_spec.rb +++ b/spec/lib/gitlab/sidekiq_config/worker_spec.rb @@ -12,7 +12,8 @@ describe Gitlab::SidekiqConfig::Worker do get_weight: attributes[:weight], get_worker_resource_boundary: attributes[:resource_boundary], latency_sensitive_worker?: attributes[:latency_sensitive], - worker_has_external_dependencies?: attributes[:has_external_dependencies] + worker_has_external_dependencies?: attributes[:has_external_dependencies], + idempotent?: attributes[:idempotent] ) described_class.new(inner_worker, ee: false) @@ -89,7 +90,8 @@ describe Gitlab::SidekiqConfig::Worker do has_external_dependencies: false, latency_sensitive: false, resource_boundary: :memory, - weight: 2 + weight: 2, + idempotent: true } attributes_b = { @@ -97,7 +99,8 @@ describe Gitlab::SidekiqConfig::Worker do has_external_dependencies: true, latency_sensitive: true, resource_boundary: :unknown, - weight: 1 + weight: 3, + idempotent: false } worker_a = create_worker(queue: 'a', **attributes_a) diff --git a/spec/models/milestone_note_spec.rb b/spec/models/milestone_note_spec.rb new file mode 100644 index 00000000000..9e77ef91bb2 --- /dev/null +++ b/spec/models/milestone_note_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MilestoneNote do + describe '.from_event' do + let(:author) { create(:user) } + let(:project) { create(:project, :repository) } + let(:noteable) { create(:issue, author: author, project: project) } + let(:event) { create(:resource_milestone_event, issue: noteable) } + + subject { described_class.from_event(event, resource: noteable, resource_parent: project) } + + it_behaves_like 'a system note', exclude_project: true do + let(:action) { 'milestone' } + end + end +end diff --git a/spec/models/resource_milestone_event_spec.rb b/spec/models/resource_milestone_event_spec.rb new file mode 100644 index 00000000000..1b0181e3fd2 --- /dev/null +++ b/spec/models/resource_milestone_event_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ResourceMilestoneEvent, type: :model do + it_behaves_like 'a resource event' + it_behaves_like 'a resource event for issues' + it_behaves_like 'a resource event for merge requests' + + it_behaves_like 'having unique enum values' + + describe 'associations' do + it { is_expected.to belong_to(:milestone) } + end + + describe 'validations' do + context 'when issue and merge_request are both nil' do + subject { build(described_class.name.underscore.to_sym, issue: nil, merge_request: nil) } + + it { is_expected.not_to be_valid } + end + + context 'when issue and merge_request are both set' do + subject { build(described_class.name.underscore.to_sym, issue: build(:issue), merge_request: build(:merge_request)) } + + it { is_expected.not_to be_valid } + end + + context 'when issue is set' do + subject { create(described_class.name.underscore.to_sym, issue: create(:issue), merge_request: nil) } + + it { is_expected.to be_valid } + end + + context 'when merge_request is set' do + subject { create(described_class.name.underscore.to_sym, issue: nil, merge_request: create(:merge_request)) } + + it { is_expected.to be_valid } + end + end + + describe 'states' do + [Issue, MergeRequest].each do |klass| + klass.available_states.each do |state| + it "supports state #{state.first} for #{klass.name.underscore}" do + model = create(klass.name.underscore, state: state[0]) + key = model.class.name.underscore + event = build(described_class.name.underscore.to_sym, key => model, state: model.state) + + expect(event.state).to eq(state[0]) + end + end + end + end +end diff --git a/spec/rubocop/cop/migration/schedule_async_spec.rb b/spec/rubocop/cop/migration/schedule_async_spec.rb new file mode 100644 index 00000000000..3453f1c51cc --- /dev/null +++ b/spec/rubocop/cop/migration/schedule_async_spec.rb @@ -0,0 +1,152 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/schedule_async' + +describe RuboCop::Cop::Migration::ScheduleAsync do + include CopHelper + + let(:cop) { described_class.new } + let(:source) do + <<~SOURCE + def up + BackgroundMigrationWorker.perform_async(ClazzName, "Bar", "Baz") + end + SOURCE + end + + shared_examples 'a disabled cop' do + it 'does not register any offenses' do + inspect_source(source) + + expect(cop.offenses).to be_empty + end + end + + context 'outside of a migration' do + it_behaves_like 'a disabled cop' + end + + context 'in a migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + context 'in an old migration' do + before do + allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE - 5) + end + + it_behaves_like 'a disabled cop' + end + + context 'that is recent' do + before do + allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE + 5) + end + + context 'BackgroundMigrationWorker.perform_async' do + it 'adds an offence when calling `BackgroundMigrationWorker.peform_async`' do + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + end + + it 'autocorrects to the right version' do + correct_source = <<~CORRECT + def up + migrate_async(ClazzName, "Bar", "Baz") + end + CORRECT + + expect(autocorrect_source(source)).to eq(correct_source) + end + end + + context 'BackgroundMigrationWorker.perform_in' do + let(:source) do + <<~SOURCE + def up + BackgroundMigrationWorker + .perform_in(delay, ClazzName, "Bar", "Baz") + end + SOURCE + end + + it 'adds an offence' do + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + end + + it 'autocorrects to the right version' do + correct_source = <<~CORRECT + def up + migrate_in(delay, ClazzName, "Bar", "Baz") + end + CORRECT + + expect(autocorrect_source(source)).to eq(correct_source) + end + end + + context 'BackgroundMigrationWorker.bulk_perform_async' do + let(:source) do + <<~SOURCE + def up + BackgroundMigrationWorker + .bulk_perform_async(jobs) + end + SOURCE + end + + it 'adds an offence' do + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + end + + it 'autocorrects to the right version' do + correct_source = <<~CORRECT + def up + bulk_migrate_async(jobs) + end + CORRECT + + expect(autocorrect_source(source)).to eq(correct_source) + end + end + + context 'BackgroundMigrationWorker.bulk_perform_in' do + let(:source) do + <<~SOURCE + def up + BackgroundMigrationWorker + .bulk_perform_in(5.minutes, jobs) + end + SOURCE + end + + it 'adds an offence' do + inspect_source(source) + + expect(cop.offenses.size).to eq(1) + end + + it 'autocorrects to the right version' do + correct_source = <<~CORRECT + def up + bulk_migrate_in(5.minutes, jobs) + end + CORRECT + + expect(autocorrect_source(source)).to eq(correct_source) + end + end + end + end +end diff --git a/spec/rubocop/cop/scalability/idempotent_worker_spec.rb b/spec/rubocop/cop/scalability/idempotent_worker_spec.rb new file mode 100644 index 00000000000..7abd602f8bc --- /dev/null +++ b/spec/rubocop/cop/scalability/idempotent_worker_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require_relative '../../../support/helpers/expect_offense' +require_relative '../../../../rubocop/cop/scalability/idempotent_worker' + +describe RuboCop::Cop::Scalability::IdempotentWorker do + include CopHelper + include ExpectOffense + + subject(:cop) { described_class.new } + + before do + allow(cop) + .to receive(:in_worker?) + .and_return(true) + end + + it 'adds an offense when not defining idempotent method' do + inspect_source(<<~CODE.strip_indent) + class SomeWorker + end + CODE + + expect(cop.offenses.size).to eq(1) + end + + it 'adds an offense when not defining idempotent method' do + inspect_source(<<~CODE.strip_indent) + class SomeWorker + idempotent! + end + CODE + + expect(cop.offenses.size).to be_zero + end +end diff --git a/spec/rubocop/migration_helpers_spec.rb b/spec/rubocop/migration_helpers_spec.rb new file mode 100644 index 00000000000..73ced8c58da --- /dev/null +++ b/spec/rubocop/migration_helpers_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rspec-parameterized' + +require_relative '../../rubocop/migration_helpers' + +describe RuboCop::MigrationHelpers do + using RSpec::Parameterized::TableSyntax + + subject(:fake_cop) { Class.new { include RuboCop::MigrationHelpers }.new } + + let(:node) { double(:node) } + + before do + allow(node).to receive_message_chain('location.expression.source_buffer.name') + .and_return(name) + end + + describe '#in_migration?' do + where(:name, :expected) do + '/gitlab/db/migrate/20200210184420_create_operations_scopes_table.rb' | true + '/gitlab/db/post_migrate/20200210184420_create_operations_scopes_table.rb' | true + '/gitlab/db/geo/migrate/20200210184420_create_operations_scopes_table.rb' | true + '/gitlab/db/geo/post_migrate/20200210184420_create_operations_scopes_table.rb' | true + '/gitlab/db/elsewhere/20200210184420_create_operations_scopes_table.rb' | false + end + + with_them do + it { expect(fake_cop.in_migration?(node)).to eq(expected) } + end + end + + describe '#in_post_deployment_migration?' do + where(:name, :expected) do + '/gitlab/db/migrate/20200210184420_create_operations_scopes_table.rb' | false + '/gitlab/db/post_migrate/20200210184420_create_operations_scopes_table.rb' | true + '/gitlab/db/geo/migrate/20200210184420_create_operations_scopes_table.rb' | false + '/gitlab/db/geo/post_migrate/20200210184420_create_operations_scopes_table.rb' | true + '/gitlab/db/elsewhere/20200210184420_create_operations_scopes_table.rb' | false + end + + with_them do + it { expect(fake_cop.in_post_deployment_migration?(node)).to eq(expected) } + end + end + + describe "#version" do + let(:name) do + '/path/to/gitlab/db/migrate/20200210184420_create_operations_scopes_table.rb' + end + + it { expect(fake_cop.version(node)).to eq(20200210184420) } + end +end diff --git a/spec/services/issuable/clone/attributes_rewriter_spec.rb b/spec/services/issuable/clone/attributes_rewriter_spec.rb index 20bda6984bd..6bc0df8260b 100644 --- a/spec/services/issuable/clone/attributes_rewriter_spec.rb +++ b/spec/services/issuable/clone/attributes_rewriter_spec.rb @@ -75,5 +75,47 @@ describe Issuable::Clone::AttributesRewriter do expect(new_issue.reload.milestone).to eq(milestone) end + + context 'with existing milestone events' do + let!(:milestone1_project1) { create(:milestone, title: 'milestone1', project: project1) } + let!(:milestone2_project1) { create(:milestone, title: 'milestone2', project: project1) } + let!(:milestone3_project1) { create(:milestone, title: 'milestone3', project: project1) } + + let!(:milestone1_project2) { create(:milestone, title: 'milestone1', project: project2) } + let!(:milestone2_project2) { create(:milestone, title: 'milestone2', project: project2) } + + before do + original_issue.update(milestone: milestone2_project1) + + create_event(milestone1_project1) + create_event(milestone2_project1) + create_event(milestone1_project1, 'remove') + create_event(milestone3_project1) + end + + it 'copies existing resource milestone events' do + subject.execute + + new_issue_milestone_events = new_issue.reload.resource_milestone_events + expect(new_issue_milestone_events.count).to eq(3) + + expect_milestone_event(new_issue_milestone_events.first, milestone: milestone1_project2, action: 'add', state: 'opened') + expect_milestone_event(new_issue_milestone_events.second, milestone: milestone2_project2, action: 'add', state: 'opened') + expect_milestone_event(new_issue_milestone_events.third, milestone: milestone1_project2, action: 'remove', state: 'opened') + end + + def create_event(milestone, action = 'add') + create(:resource_milestone_event, issue: original_issue, milestone: milestone, action: action) + end + + def expect_milestone_event(event, expected_attrs) + expect(event.milestone_id).to eq(expected_attrs[:milestone].id) + expect(event.action).to eq(expected_attrs[:action]) + expect(event.state).to eq(expected_attrs[:state]) + + expect(event.reference).to be_nil + expect(event.reference_html).to be_nil + end + end end end diff --git a/spec/services/issuable/common_system_notes_service_spec.rb b/spec/services/issuable/common_system_notes_service_spec.rb index 7e40ac9ff4d..771e7ca42c9 100644 --- a/spec/services/issuable/common_system_notes_service_spec.rb +++ b/spec/services/issuable/common_system_notes_service_spec.rb @@ -3,8 +3,9 @@ require 'spec_helper' describe Issuable::CommonSystemNotesService do - let(:user) { create(:user) } - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let(:issuable) { create(:issue, project: project) } context 'on issuable update' do @@ -35,6 +36,8 @@ describe Issuable::CommonSystemNotesService do before do milestone = create(:milestone, project: project) issuable.milestone_id = milestone.id + + stub_feature_flags(track_resource_milestone_change_events: false) end it_behaves_like 'system note creation', {}, 'changed milestone' @@ -97,12 +100,39 @@ describe Issuable::CommonSystemNotesService do expect(event.user_id).to eq user.id end - it 'creates a system note for milestone set' do - issuable.milestone = create(:milestone, project: project) - issuable.save + context 'when milestone change event tracking is disabled' do + before do + stub_feature_flags(track_resource_milestone_change_events: false) - expect { subject }.to change { issuable.notes.count }.from(0).to(1) - expect(issuable.notes.last.note).to match('changed milestone') + issuable.milestone = create(:milestone, project: project) + issuable.save + end + + it 'creates a system note for milestone set' do + expect { subject }.to change { issuable.notes.count }.from(0).to(1) + expect(issuable.notes.last.note).to match('changed milestone') + end + + it 'does not create a milestone change event' do + expect { subject }.not_to change { ResourceMilestoneEvent.count } + end + end + + context 'when milestone change event tracking is enabled' do + let_it_be(:milestone) { create(:milestone, project: project) } + let_it_be(:issuable) { create(:issue, project: project, milestone: milestone) } + + before do + stub_feature_flags(track_resource_milestone_change_events: true) + end + + it 'does not create a system note for milestone set' do + expect { subject }.not_to change { issuable.notes.count } + end + + it 'creates a milestone change event' do + expect { subject }.to change { ResourceMilestoneEvent.count }.from(0).to(1) + end end it 'creates a system note for due_date set' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 888a63980f6..5da77dd914c 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -385,6 +385,10 @@ describe Issues::UpdateService, :mailer do end context 'when the milestone is removed' do + before do + stub_feature_flags(track_resource_milestone_change_events: false) + end + let!(:non_subscriber) { create(:user) } let!(:subscriber) do @@ -411,6 +415,10 @@ describe Issues::UpdateService, :mailer do end context 'when the milestone is changed' do + before do + stub_feature_flags(track_resource_milestone_change_events: false) + end + let!(:non_subscriber) { create(:user) } let!(:subscriber) do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index f295f3c4a81..dd5d90b2d07 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -367,6 +367,10 @@ describe MergeRequests::UpdateService, :mailer do end context 'when the milestone is removed' do + before do + stub_feature_flags(track_resource_milestone_change_events: false) + end + let!(:non_subscriber) { create(:user) } let!(:subscriber) do @@ -393,6 +397,10 @@ describe MergeRequests::UpdateService, :mailer do end context 'when the milestone is changed' do + before do + stub_feature_flags(track_resource_milestone_change_events: false) + end + let!(:non_subscriber) { create(:user) } let!(:subscriber) do diff --git a/spec/services/resource_events/change_milestone_service_spec.rb b/spec/services/resource_events/change_milestone_service_spec.rb new file mode 100644 index 00000000000..c6b4f8e1b7e --- /dev/null +++ b/spec/services/resource_events/change_milestone_service_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ResourceEvents::ChangeMilestoneService do + shared_examples 'milestone events creator' do + let_it_be(:user) { create(:user) } + + let_it_be(:milestone) { create(:milestone) } + + context 'when milestone is present' do + before do + resource.milestone = milestone + end + + let(:service) { described_class.new(resource: resource, user: user, created_at: created_at_time) } + + it 'creates the expected event record' do + expect { service.execute }.to change { ResourceMilestoneEvent.count }.from(0).to(1) + + events = ResourceMilestoneEvent.all + + expect(events.size).to eq(1) + expect_event_record(events.first, action: 'add', milestone: milestone, state: 'opened') + end + end + + context 'when milestones is not present' do + before do + resource.milestone = nil + end + + let(:service) { described_class.new(resource: resource, user: user, created_at: created_at_time) } + + it 'creates the expected event records' do + expect { service.execute }.to change { ResourceMilestoneEvent.count }.from(0).to(1) + + expect_event_record(ResourceMilestoneEvent.first, action: 'remove', milestone: nil, state: 'opened') + end + end + + def expect_event_record(event, expected_attrs) + expect(event.action).to eq(expected_attrs[:action]) + expect(event.state).to eq(expected_attrs[:state]) + expect(event.user).to eq(user) + expect(event.issue).to eq(resource) if resource.is_a?(Issue) + expect(event.issue).to be_nil unless resource.is_a?(Issue) + expect(event.merge_request).to eq(resource) if resource.is_a?(MergeRequest) + expect(event.merge_request).to be_nil unless resource.is_a?(MergeRequest) + expect(event.milestone).to eq(expected_attrs[:milestone]) + expect(event.created_at).to eq(created_at_time) + end + end + + let_it_be(:merge_request) { create(:merge_request) } + let_it_be(:issue) { create(:issue) } + + let!(:created_at_time) { Time.utc(2019, 12, 30) } + + it_behaves_like 'milestone events creator' do + let(:resource) { issue } + end + + it_behaves_like 'milestone events creator' do + let(:resource) { merge_request } + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 35bf6846ab3..b862fc4bbd9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -119,6 +119,7 @@ RSpec.configure do |config| config.include PolicyHelpers, type: :policy config.include MemoryUsageHelper config.include ExpectRequestWithStatus, type: :request + config.include IdempotentWorkerHelper, type: :worker config.include RailsHelpers if ENV['CI'] || ENV['RETRIES'] diff --git a/spec/support/helpers/idempotent_worker_helper.rb b/spec/support/helpers/idempotent_worker_helper.rb new file mode 100644 index 00000000000..b80758d10bd --- /dev/null +++ b/spec/support/helpers/idempotent_worker_helper.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module IdempotentWorkerHelper + WORKER_EXEC_TIMES = 2 + + def perform_multiple(args = [], worker: described_class.new, exec_times: WORKER_EXEC_TIMES) + Sidekiq::Testing.inline! do + job_args = args.nil? ? [nil] : Array.wrap(args) + + expect(worker).to receive(:perform).exactly(exec_times).and_call_original + + exec_times.times { worker.perform(*job_args) } + end + end +end diff --git a/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb index 897a962fc56..32c46753006 100644 --- a/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/issue/move_quick_action_shared_examples.rb @@ -50,6 +50,8 @@ RSpec.shared_examples 'move quick action' do let(:bug) { create(:label, project: project, title: 'bug') } let(:wontfix) { create(:label, project: project, title: 'wontfix') } + let!(:target_milestone) { create(:milestone, title: '1.0', project: target_project) } + before do target_project.add_maintainer(user) end diff --git a/spec/support/shared_examples/resource_events.rb b/spec/support/shared_examples/resource_events.rb new file mode 100644 index 00000000000..d7e7349ad6c --- /dev/null +++ b/spec/support/shared_examples/resource_events.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'spec_helper' + +shared_examples 'a resource event' do + 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(:user) } + end + + describe 'associations' do + it { is_expected.to belong_to(:user) } + 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(described_class.name.underscore.to_sym, issue: issue1, created_at: created_at1) } + let!(:event2) { create(described_class.name.underscore.to_sym, issue: issue2, created_at: created_at2) } + let!(:event3) { create(described_class.name.underscore.to_sym, issue: issue2, created_at: created_at3) } + + it 'returns the expected events' do + events = described_class.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 = described_class.created_after(1.minute.ago) + + expect(events).to be_empty + end + end +end + +shared_examples 'a resource event for issues' do + 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 'associations' do + it { is_expected.to belong_to(:issue) } + end + + describe '.by_issue' do + let_it_be(:event1) { create(described_class.name.underscore.to_sym, issue: issue1) } + let_it_be(:event2) { create(described_class.name.underscore.to_sym, issue: issue2) } + let_it_be(:event3) { create(described_class.name.underscore.to_sym, issue: issue1) } + + it 'returns the expected records for an issue with events' do + events = described_class.by_issue(issue1) + + expect(events).to contain_exactly(event1, event3) + end + + it 'returns the expected records for an issue with no events' do + events = described_class.by_issue(issue3) + + expect(events).to be_empty + end + end +end + +shared_examples 'a resource event for merge requests' do + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + + let_it_be(:merge_request1) { create(:merge_request, author: user1) } + let_it_be(:merge_request2) { create(:merge_request, author: user1) } + let_it_be(:merge_request3) { create(:merge_request, author: user2) } + + describe 'associations' do + it { is_expected.to belong_to(:merge_request) } + end + + describe '.by_merge_request' do + let_it_be(:event1) { create(described_class.name.underscore.to_sym, merge_request: merge_request1) } + let_it_be(:event2) { create(described_class.name.underscore.to_sym, merge_request: merge_request2) } + let_it_be(:event3) { create(described_class.name.underscore.to_sym, merge_request: merge_request1) } + + it 'returns the expected records for an issue with events' do + events = described_class.by_merge_request(merge_request1) + + expect(events).to contain_exactly(event1, event3) + end + + it 'returns the expected records for an issue with no events' do + events = described_class.by_merge_request(merge_request3) + + expect(events).to be_empty + end + end +end diff --git a/spec/support/shared_examples/workers/idempotency_shared_examples.rb b/spec/support/shared_examples/workers/idempotency_shared_examples.rb new file mode 100644 index 00000000000..19be1fe2c9d --- /dev/null +++ b/spec/support/shared_examples/workers/idempotency_shared_examples.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# This shared_example requires the following variables: +# - job_args (if not given, will fallback to call perform without arguments) +# +# Usage: +# +# include_examples 'an idempotent worker' do +# it 'checks the side-effects for multiple calls' do +# # it'll call the job's perform method 3 times +# # by default. +# subject +# +# expect(model.state).to eq('state') +# end +# end +# +RSpec.shared_examples 'an idempotent worker' do + # Avoid stubbing calls for a more accurate run. + subject do + defined?(job_args) ? perform_multiple(job_args) : perform_multiple + end + + it 'is labeled as idempotent' do + expect(described_class).to be_idempotent + end + + it 'performs multiple times sequentially without raising an exception' do + expect { subject }.not_to raise_error + end +end diff --git a/spec/workers/background_migration_worker_spec.rb b/spec/workers/background_migration_worker_spec.rb index aae6fa02a0c..2b2ffc9f5b9 100644 --- a/spec/workers/background_migration_worker_spec.rb +++ b/spec/workers/background_migration_worker_spec.rb @@ -11,7 +11,7 @@ describe BackgroundMigrationWorker, :clean_gitlab_redis_shared_state do end end - describe '.perform' do + describe '#perform' do it 'performs a background migration' do expect(Gitlab::BackgroundMigration) .to receive(:perform) @@ -52,6 +52,14 @@ describe BackgroundMigrationWorker, :clean_gitlab_redis_shared_state do worker.perform('Foo', [10, 20]) end + + it 'sets the class that will be executed as the caller_id' do + expect(Gitlab::BackgroundMigration).to receive(:perform) do + expect(Labkit::Context.current.to_h).to include('meta.caller_id' => 'Foo') + end + + worker.perform('Foo', [10, 20]) + end end describe '#healthy_database?' do diff --git a/spec/workers/expire_job_cache_worker_spec.rb b/spec/workers/expire_job_cache_worker_spec.rb index eeab304d926..ebca7020ee5 100644 --- a/spec/workers/expire_job_cache_worker_spec.rb +++ b/spec/workers/expire_job_cache_worker_spec.rb @@ -6,20 +6,32 @@ describe ExpireJobCacheWorker do set(:pipeline) { create(:ci_empty_pipeline) } let(:project) { pipeline.project } - subject { described_class.new } - describe '#perform' do context 'with a job in the pipeline' do let(:job) { create(:ci_build, pipeline: pipeline) } + let(:job_args) { job.id } + + include_examples 'an idempotent worker' do + it 'invalidates Etag caching for the job path' do + pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json" + job_path = "/#{project.full_path}/builds/#{job.id}.json" + + spy_store = Gitlab::EtagCaching::Store.new + + allow(Gitlab::EtagCaching::Store).to receive(:new) { spy_store } - it 'invalidates Etag caching for the job path' do - pipeline_path = "/#{project.full_path}/pipelines/#{pipeline.id}.json" - job_path = "/#{project.full_path}/builds/#{job.id}.json" + expect(spy_store).to receive(:touch) + .exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES).times + .with(pipeline_path) + .and_call_original - expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipeline_path) - expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(job_path) + expect(spy_store).to receive(:touch) + .exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES).times + .with(job_path) + .and_call_original - subject.perform(job.id) + subject + end end end @@ -27,7 +39,7 @@ describe ExpireJobCacheWorker do it 'does not change the etag store' do expect(Gitlab::EtagCaching::Store).not_to receive(:new) - subject.perform(9999) + perform_multiple(9999) end end end |