Add rubocops to ensure Sidekiq workers include ApplicationWorker and don't manually set their queue

This commit is contained in:
Douwe Maan 2017-12-12 17:21:00 +01:00
parent 0ec81dd5ac
commit 3cad04cd51
7 changed files with 117 additions and 6 deletions

View File

@ -3,7 +3,7 @@ Sidekiq::Worker.extend ActiveSupport::Concern
module ApplicationWorker
extend ActiveSupport::Concern
include Sidekiq::Worker
include Sidekiq::Worker # rubocop:disable Cop/IncludeSidekiqWorker
included do
set_queue
@ -17,7 +17,7 @@ module ApplicationWorker
def set_queue
queue_name = [queue_namespace, base_queue_name].compact.join(':')
sidekiq_options queue: queue_name
sidekiq_options queue: queue_name # rubocop:disable Cop/SidekiqOptionsQueue
end
def base_queue_name

View File

@ -0,0 +1,29 @@
require_relative '../spec_helpers'
module RuboCop
module Cop
# Cop that makes sure workers include `ApplicationWorker`, not `Sidekiq::Worker`.
class IncludeSidekiqWorker < RuboCop::Cop::Cop
include SpecHelpers
MSG = 'Include `ApplicationWorker`, not `Sidekiq::Worker`.'.freeze
def_node_matcher :includes_sidekiq_worker?, <<~PATTERN
(send nil :include (const (const nil :Sidekiq) :Worker))
PATTERN
def on_send(node)
return if in_spec?(node)
return unless includes_sidekiq_worker?(node)
add_offense(node.arguments.first, :expression)
end
def autocorrect(node)
lambda do |corrector|
corrector.replace(node.source_range, 'ApplicationWorker')
end
end
end
end
end

View File

@ -0,0 +1,27 @@
require_relative '../spec_helpers'
module RuboCop
module Cop
# Cop that prevents manually setting a queue in Sidekiq workers.
class SidekiqOptionsQueue < RuboCop::Cop::Cop
include SpecHelpers
MSG = 'Do not manually set a queue; `ApplicationWorker` sets one automatically.'.freeze
def_node_matcher :sidekiq_options?, <<~PATTERN
(send nil :sidekiq_options $...)
PATTERN
def on_send(node)
return if in_spec?(node)
return unless sidekiq_options?(node)
node.arguments.first.each_node(:pair) do |pair|
key_name = pair.key.children[0]
add_offense(pair, :expression) if key_name == :queue
end
end
end
end
end

View File

@ -3,10 +3,12 @@ require_relative 'cop/active_record_serialize'
require_relative 'cop/custom_error_class'
require_relative 'cop/gem_fetcher'
require_relative 'cop/in_batches'
require_relative 'cop/include_sidekiq_worker'
require_relative 'cop/line_break_after_guard_clauses'
require_relative 'cop/polymorphic_associations'
require_relative 'cop/project_path_helper'
require_relative 'cop/redirect_with_status'
require_relative 'cop/sidekiq_options_queue'
require_relative 'cop/migration/add_column'
require_relative 'cop/migration/add_concurrent_foreign_key'
require_relative 'cop/migration/add_concurrent_index'

View File

@ -0,0 +1,31 @@
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/include_sidekiq_worker'
describe RuboCop::Cop::IncludeSidekiqWorker do
include CopHelper
subject(:cop) { described_class.new }
context 'when `Sidekiq::Worker` is included' do
let(:source) { 'include Sidekiq::Worker' }
let(:correct_source) { 'include ApplicationWorker' }
it 'registers an offense ' do
inspect_source(cop, source)
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([1])
expect(cop.highlights).to eq(['Sidekiq::Worker'])
end
end
it 'autocorrects to the right version' do
autocorrected = autocorrect_source(cop, source)
expect(autocorrected).to eq(correct_source)
end
end
end

View File

@ -0,0 +1,26 @@
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/sidekiq_options_queue'
describe RuboCop::Cop::SidekiqOptionsQueue do
include CopHelper
subject(:cop) { described_class.new }
it 'registers an offense when `sidekiq_options` is used with the `queue` option' do
inspect_source(cop, 'sidekiq_options queue: "some_queue"')
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([1])
expect(cop.highlights).to eq(['queue: "some_queue"'])
end
end
it 'does not register an offense when `sidekiq_options` is used with another option' do
inspect_source(cop, 'sidekiq_options retry: false')
expect(cop.offenses).to be_empty
end
end

View File

@ -1,10 +1,6 @@
require 'spec_helper'
describe 'Every Sidekiq worker' do
it 'includes ApplicationWorker' do
expect(Gitlab::SidekiqConfig.workers).to all(include(ApplicationWorker))
end
it 'does not use the default queue' do
expect(Gitlab::SidekiqConfig.workers.map(&:queue)).not_to include('default')
end