Replace 'Sidekiq::Testing.inline!' with 'perform_enqueued_jobs'

`perform_enqueued_jobs` is a Sidekiq method.
Using this method violates the Dependency inversion principle[0].

This commit replaces `perform_enqueued_jobs` with ActiveJob's abstract
method `perform_enqueued_jobs` in specs.

[0]: https://en.wikipedia.org/wiki/Dependency_inversion_principle
This commit is contained in:
blackst0ne 2018-07-23 15:34:54 +11:00
parent dc7b4b7bb9
commit ddd2a25679
27 changed files with 49 additions and 44 deletions

View file

@ -0,0 +1,5 @@
---
title: Replace 'Sidekiq::Testing.inline!' with 'perform_enqueued_jobs'
merge_request: 20768
author: "@blackst0ne"
type: other

View file

@ -1,6 +1,6 @@
require './spec/support/sidekiq'
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
Gitlab::Seeder.quiet do
project_urls = [
'https://gitlab.com/gitlab-org/gitlab-test.git',

View file

@ -1,6 +1,6 @@
require './spec/support/sidekiq'
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
Gitlab::Seeder.quiet do
Group.all.each do |group|
User.all.sample(4).each do |user|

View file

@ -78,7 +78,7 @@ class Gitlab::Seeder::CycleAnalytics
def seed!
Sidekiq::Worker.skipping_transaction_check do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
issues = create_issues
puts '.'

View file

@ -1,6 +1,6 @@
require './spec/support/sidekiq'
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
Gitlab::Seeder.quiet do
flag = 'SEED_NESTED_GROUPS'

View file

@ -11,7 +11,7 @@ class Gitlab::Seeder::LabeledIssues
end
def seed!
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
group = create_group
create_projects(group)

View file

@ -7,7 +7,7 @@ describe 'GPG signed commits', :js do
user = create :user, email: 'unrelated.user@example.org'
project.add_maintainer(user)
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
create :gpg_key, key: GpgHelpers::User1.public_key, user: user
end
@ -21,7 +21,7 @@ describe 'GPG signed commits', :js do
end
# user changes his email which makes the gpg key verified
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
user.skip_reconfirmation!
user.update!(email: GpgHelpers::User1.emails.first)
end
@ -48,7 +48,7 @@ describe 'GPG signed commits', :js do
end
# user adds the gpg key which makes the signature valid
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
create :gpg_key, key: GpgHelpers::User1.public_key, user: user
end
@ -66,7 +66,7 @@ describe 'GPG signed commits', :js do
end
let(:user_1_key) do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
create :gpg_key, key: GpgHelpers::User1.public_key, user: user_1
end
end
@ -79,7 +79,7 @@ describe 'GPG signed commits', :js do
end
let(:user_2_key) do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
create :gpg_key, key: GpgHelpers::User2.public_key, user: user_2
end
end

View file

@ -65,7 +65,7 @@ describe Gitlab::HashedStorage::Migrator do
end
it 'migrate project' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
subject.migrate(project)
end

View file

@ -31,7 +31,7 @@ describe ScheduleSetConfidentialNoteEventsOnServices, :migration, :sidekiq do
end
it 'correctly processes services' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
expect(services_table.where(confidential_note_events: nil).count).to eq 4
expect(services_table.where(confidential_note_events: true).count).to eq 1

View file

@ -44,7 +44,7 @@ describe MigrateStageIdReferenceInBackground, :migration, :sidekiq do
end
it 'schedules background migrations' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
expect(jobs.where(stage_id: nil).count).to eq 5
migrate!

View file

@ -34,7 +34,7 @@ describe MigrateStagesStatuses, :sidekiq, :migration do
end
it 'correctly migrates stages statuses' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
expect(stages.where(status: nil).count).to eq 3
migrate!

View file

@ -38,7 +38,7 @@ describe NormalizeLdapExternUids, :migration, :sidekiq do
end
it 'migrates the LDAP identities' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
migrate!
identities.where(id: 1..4).each do |identity|
expect(identity.extern_uid).to eq("uid=foo #{identity.id},ou=people,dc=example,dc=com")
@ -47,7 +47,7 @@ describe NormalizeLdapExternUids, :migration, :sidekiq do
end
it 'does not modify non-LDAP identities' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
migrate!
identity = identities.last
expect(identity.extern_uid).to eq(" uid = foo 5, ou = People, dc = example, dc = com ")

View file

@ -20,7 +20,7 @@ describe ScheduleCreateGpgKeySubkeysFromGpgKeys, :migration, :sidekiq do
end
it 'schedules background migrations' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
expect(GpgKeySubkey.count).to eq(0)
migrate!

View file

@ -33,7 +33,7 @@ describe ScheduleMergeRequestDiffMigrations, :migration, :sidekiq do
end
it 'schedules background migrations' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
non_empty = 'st_commits IS NOT NULL OR st_diffs IS NOT NULL'
expect(merge_request_diffs.where(non_empty).count).to eq 3

View file

@ -33,7 +33,7 @@ describe ScheduleMergeRequestDiffMigrationsTakeTwo, :migration, :sidekiq do
end
it 'migrates the data' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
non_empty = 'st_commits IS NOT NULL OR st_diffs IS NOT NULL'
expect(merge_request_diffs.where(non_empty).count).to eq 3

View file

@ -53,7 +53,7 @@ describe ScheduleMergeRequestLatestMergeRequestDiffIdMigrations, :migration, :si
end
it 'schedules background migrations' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
expect(merge_requests_table.where(latest_merge_request_diff_id: nil).count).to eq 3
migrate!

View file

@ -31,7 +31,7 @@ describe ScheduleSetConfidentialNoteEventsOnWebhooks, :migration, :sidekiq do
end
it 'correctly processes web hooks' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
expect(web_hooks_table.where(confidential_note_events: nil).count).to eq 4
expect(web_hooks_table.where(confidential_note_events: true).count).to eq 1

View file

@ -179,7 +179,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
end
it 'migrates data to object storage' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
subject
build_trace_chunk.reload
@ -201,7 +201,7 @@ describe Ci::BuildTraceChunk, :clean_gitlab_redis_shared_state do
end
it 'does not migrate data to object storage' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
data_store = build_trace_chunk.data_store
subject

View file

@ -22,7 +22,7 @@ describe SpamLog do
spam_log = build(:spam_log)
user = spam_log.user
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
spam_log.remove_user(deleted_by: admin)
end

View file

@ -102,7 +102,7 @@ describe API::ProjectImport do
it 'correctly overrides params during the import' do
override_params = { 'description' => 'Hello world' }
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
post api('/projects/import', user),
path: 'test-import',
file: fixture_file_upload(file),

View file

@ -1067,7 +1067,7 @@ describe API::Users do
end
it "deletes user" do
Sidekiq::Testing.inline! { delete api("/users/#{user.id}", admin) }
perform_enqueued_jobs { delete api("/users/#{user.id}", admin) }
expect(response).to have_gitlab_http_status(204)
expect { User.find(user.id) }.to raise_error ActiveRecord::RecordNotFound
@ -1079,30 +1079,30 @@ describe API::Users do
end
it "does not delete for unauthenticated user" do
Sidekiq::Testing.inline! { delete api("/users/#{user.id}") }
perform_enqueued_jobs { delete api("/users/#{user.id}") }
expect(response).to have_gitlab_http_status(401)
end
it "is not available for non admin users" do
Sidekiq::Testing.inline! { delete api("/users/#{user.id}", user) }
perform_enqueued_jobs { delete api("/users/#{user.id}", user) }
expect(response).to have_gitlab_http_status(403)
end
it "returns 404 for non-existing user" do
Sidekiq::Testing.inline! { delete api("/users/999999", admin) }
perform_enqueued_jobs { delete api("/users/999999", admin) }
expect(response).to have_gitlab_http_status(404)
expect(json_response['message']).to eq('404 User Not Found')
end
it "returns a 404 for invalid ID" do
Sidekiq::Testing.inline! { delete api("/users/ASDF", admin) }
perform_enqueued_jobs { delete api("/users/ASDF", admin) }
expect(response).to have_gitlab_http_status(404)
end
context "hard delete disabled" do
it "moves contributions to the ghost user" do
Sidekiq::Testing.inline! { delete api("/users/#{user.id}", admin) }
perform_enqueued_jobs { delete api("/users/#{user.id}", admin) }
expect(response).to have_gitlab_http_status(204)
expect(issue.reload).to be_persisted
@ -1112,7 +1112,7 @@ describe API::Users do
context "hard delete enabled" do
it "removes contributions" do
Sidekiq::Testing.inline! { delete api("/users/#{user.id}?hard_delete=true", admin) }
perform_enqueued_jobs { delete api("/users/#{user.id}?hard_delete=true", admin) }
expect(response).to have_gitlab_http_status(204)
expect(Issue.exists?(issue.id)).to be_falsy

View file

@ -49,7 +49,7 @@ describe Groups::DestroyService do
context 'Sidekiq inline' do
before do
# Run sidekiq immediately to check that renamed dir will be removed
Sidekiq::Testing.inline! { destroy_group(group, user, async) }
perform_enqueued_jobs { destroy_group(group, user, async) }
end
it 'verifies that paths have been deleted' do

View file

@ -28,7 +28,7 @@ describe Projects::CreateFromTemplateService do
context 'the result project' do
before do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
@project = subject.execute
end

View file

@ -45,18 +45,18 @@ describe Projects::DestroyService do
shared_examples 'handles errors thrown during async destroy' do |error_message|
it 'does not allow the error to bubble up' do
expect do
Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
perform_enqueued_jobs { destroy_project(project, user, {}) }
end.not_to raise_error
end
it 'unmarks the project as "pending deletion"' do
Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
perform_enqueued_jobs { destroy_project(project, user, {}) }
expect(project.reload.pending_delete).to be(false)
end
it 'stores an error message in `projects.delete_error`' do
Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
perform_enqueued_jobs { destroy_project(project, user, {}) }
expect(project.reload.delete_error).to be_present
expect(project.delete_error).to include(error_message)
@ -66,7 +66,7 @@ describe Projects::DestroyService do
context 'Sidekiq inline' do
before do
# Run sidekiq immediatly to check that renamed repository will be removed
Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
perform_enqueued_jobs { destroy_project(project, user, {}) }
end
context 'when has remote mirrors' do
@ -110,7 +110,7 @@ describe Projects::DestroyService do
end
it 'keeps project team intact upon an error' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
begin
destroy_project(project, user, {})
rescue ::Redis::CannotConnectError
@ -128,7 +128,7 @@ describe Projects::DestroyService do
before do
project.project_feature.update_attribute("issues_access_level", ProjectFeature::PRIVATE)
# Run sidekiq immediately to check that renamed repository will be removed
Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
perform_enqueued_jobs { destroy_project(project, user, {}) }
end
it_behaves_like 'deleting the project'
@ -172,7 +172,7 @@ describe Projects::DestroyService do
it 'allows error to bubble up and rolls back project deletion' do
expect do
Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
perform_enqueued_jobs { destroy_project(project, user, {}) }
end.to raise_error(Exception, 'Other error message')
expect(project.reload.pending_delete).to be(false)

View file

@ -35,7 +35,7 @@ describe Projects::HousekeepingService do
allow(subject).to receive(:gc_period).and_return(1)
project.increment_pushes_since_gc
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
expect { subject.execute }.to change { project.pushes_since_gc }.to(0)
end
end

View file

@ -173,7 +173,7 @@ describe Users::DestroyService do
describe "user personal's repository removal" do
before do
Sidekiq::Testing.inline! { service.execute(user) }
perform_enqueued_jobs { service.execute(user) }
end
context 'legacy storage' do

View file

@ -13,7 +13,7 @@ describe StorageMigratorWorker do
end
it 'migrates projects in the specified range' do
Sidekiq::Testing.inline! do
perform_enqueued_jobs do
worker.perform(ids.min, ids.max)
end