Skip to content
Snippets Groups Projects
Commit db73a228 authored by Max Woolf's avatar Max Woolf
Browse files

Add danger comment for new sidekiq args

MRs that change existing sidekiq jobs and
add additional arguments should remind engineers
of the process to do so.
parent 2e1b6541
No related merge requests found
# frozen_string_literal: true
require_relative '../../tooling/danger/sidekiq_args'
module Danger
class SidekiqArgs < ::Danger::Plugin
# Put the helper code somewhere it can be tested
include Tooling::Danger::SidekiqArgs
end
end
# frozen_string_literal: true
sidekiq_args.changed_worker_files.each do |filename|
sidekiq_args.add_comment_for_matched_line(filename)
end
# frozen_string_literal: true
require 'rspec-parameterized'
require 'gitlab-dangerfiles'
require 'danger'
require 'danger/plugins/internal/helper'
require 'gitlab/dangerfiles/spec_helper'
require_relative '../../../tooling/danger/sidekiq_args'
require_relative '../../../tooling/danger/project_helper'
RSpec.describe Tooling::Danger::SidekiqArgs do
include_context "with dangerfile"
let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
let(:fake_project_helper) { Tooling::Danger::ProjectHelper }
let(:file_lines) do
[
" describe 'foo' do",
" def perform(hello, world)",
" def hello(world, gitlab)",
" def perform",
" ",
" def perform(a, b, c, d, e)",
" def perform(opts = {})" # hashes of options can be excluded
]
end
let(:matching_lines) do
[
"+ def perform(hello, world)",
"+ def perform(a, b, c, d, e)"
]
end
subject(:specs) { fake_danger.new(helper: fake_helper) }
before do
allow(specs).to receive(:project_helper).and_return(fake_project_helper)
end
describe '#add_comment_for_matched_line' do
let(:filename) { 'app/workers/hello_worker.rb' }
before do
expect(specs).to receive(:added_line_matching_args).and_return(matching_lines)
allow(specs.project_helper).to receive(:file_lines).and_return(file_lines)
end
it 'adds suggestions at the correct lines' do
expect(specs).to receive(:markdown).with(format(described_class::SUGGEST_MR_COMMENT), file: filename, line: 2)
expect(specs).to receive(:markdown).with(format(described_class::SUGGEST_MR_COMMENT), file: filename, line: 6)
specs.add_comment_for_matched_line(filename)
end
end
describe '#changed_worker_files' do
let(:base_expected_files) { %w[app/workers/a.rb app/workers/b.rb ee/app/workers/e.rb] }
before do
all_changed_files = %w[
app/workers/a.rb
app/workers/b.rb
ee/app/workers/e.rb
spec/foo_spec.rb
ee/spec/foo_spec.rb
spec/bar_spec.rb
ee/spec/bar_spec.rb
spec/zab_spec.rb
ee/spec/zab_spec.rb
]
allow(specs.helper).to receive(:all_changed_files).and_return(all_changed_files)
end
it 'returns added, modified, and renamed_after files by default' do
expect(specs.changed_worker_files).to match_array(base_expected_files)
end
context 'with include_ee: :exclude' do
it 'returns spec files without EE-specific files' do
expect(specs.changed_worker_files(ee: :exclude)).not_to include(%w[ee/app/workers/e.rb])
end
end
context 'with include_ee: :only' do
it 'returns EE-specific spec files only' do
expect(specs.changed_worker_files(ee: :only)).to match_array(%w[ee/app/workers/e.rb])
end
end
end
end
# frozen_string_literal: true
module Tooling
module Danger
module SidekiqArgs
include ::Danger::Helpers
WORKER_FILES_REGEX = 'app/workers'
EE_PREFIX = 'ee/'
MATCH_PERFORM_REGEX = /def perform\(\w+,.*\)/.freeze
SUGGEST_MR_COMMENT = "Be aware that adding new arguments to a sidekiq worker is [a multi-stage process](https://docs.gitlab.com/ee/development/sidekiq/compatibility_across_updates.html#add-an-argument)."
def changed_worker_files(ee: :include)
changed_files = helper.all_changed_files
folder_prefix =
case ee
when :include
"(#{EE_PREFIX})?"
when :only
EE_PREFIX
when :exclude
nil
end
changed_files.grep(%r{\A#{folder_prefix}#{WORKER_FILES_REGEX}})
end
def add_comment_for_matched_line(filename)
added_lines = added_line_matching_args(filename)
return if added_lines.empty?
spec_file_lines = project_helper.file_lines(filename)
added_lines.each_with_object([]) do |added_line, processed_line_numbers|
line_number = find_line_number(spec_file_lines,
added_line.delete_prefix('+'),
exclude_indexes: processed_line_numbers)
processed_line_numbers << line_number
markdown(format(SUGGEST_MR_COMMENT), file: filename, line: line_number.succ)
end
end
def added_line_matching_args(filename)
helper.changed_lines(filename).grep(/\A\+ /).grep(MATCH_PERFORM_REGEX)
end
private
def find_line_number(file_lines, searched_line, exclude_indexes: [])
file_lines.each_with_index do |file_line, index|
next if exclude_indexes.include?(index)
break index if file_line == searched_line
end
end
end
end
end
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment