diff --git a/danger/plugins/todos.rb b/danger/plugins/todos.rb index b31f147f2af28117cbe20617f37d2efa19a4d0a8..06b76321f9597257ac9f42a2af4b9498df76a414 100644 --- a/danger/plugins/todos.rb +++ b/danger/plugins/todos.rb @@ -5,7 +5,11 @@ module Danger class Todos < ::Danger::Plugin def check_outdated_todos(filenames) - Tooling::Danger::OutdatedTodo.new(filenames, context: self).check + Tooling::Danger::OutdatedTodo.new(filenames, context: self, allow_fail: from_lefthook?).check + end + + def from_lefthook? + %w[1 true].include?(ENV['FROM_LEFTHOOK']) end end end diff --git a/lefthook.yml b/lefthook.yml index 7272b64096f3dd33bf776b83f0642f266002fabf..fe8f5dd0466b19ba95781a8bf520897cd71f4fb7 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -4,7 +4,9 @@ pre-push: - ref: master commands: danger: - run: bundle exec rake danger_local + files: git diff --name-only $(git merge-base origin/master HEAD)..HEAD + # We need to specify {files} as part of the command, otherwise it won't execute the hook + run: echo {files} >/dev/null && FROM_LEFTHOOK=1 bundle exec rake danger_local eslint: tags: frontend style files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD diff --git a/spec/tooling/danger/outdated_todo_spec.rb b/spec/tooling/danger/outdated_todo_spec.rb index 3a3909c69ac2cab28b90579041a644a2d84d2828..3079c26bafcb96ce1c27542331b96b188db97bc1 100644 --- a/spec/tooling/danger/outdated_todo_spec.rb +++ b/spec/tooling/danger/outdated_todo_spec.rb @@ -15,69 +15,78 @@ ] end - subject(:plugin) { described_class.new(filenames, context: fake_danger, todos: todos) } - - context 'when the filenames are mentioned in single todo' do - let(:filenames) { ['app/controllers/acme_challenges_controller.rb'] } - - it 'warns about mentions' do - expect(fake_danger) - .to receive(:warn) - .with <<~MESSAGE - `app/controllers/acme_challenges_controller.rb` was removed but is mentioned in: - - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:5` - MESSAGE - - plugin.check - end - end - - context 'when the filenames are mentioned in multiple todos' do - let(:filenames) do - [ - 'app/controllers/application_controller.rb', - 'app/controllers/acme_challenges_controller.rb' - ] - end - - it 'warns about mentions' do - expect(fake_danger) - .to receive(:warn) - .with(<<~FIRSTMESSAGE) - `app/controllers/application_controller.rb` was removed but is mentioned in: - - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:4` - - `spec/fixtures/tooling/danger/rubocop_todo/cop2.yml:4` - FIRSTMESSAGE - - expect(fake_danger) - .to receive(:warn) - .with(<<~SECONDMESSAGE) - `app/controllers/acme_challenges_controller.rb` was removed but is mentioned in: - - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:5` - SECONDMESSAGE - - plugin.check - end - end - - context 'when the filenames are not mentioned in todos' do - let(:filenames) { ['any/inexisting/file.rb'] } - - it 'does not warn' do - expect(fake_danger).not_to receive(:warn) - - plugin.check - end - end - - context 'when there is no todos' do - let(:filenames) { ['app/controllers/acme_challenges_controller.rb'] } - let(:todos) { [] } - - it 'does not warn' do - expect(fake_danger).not_to receive(:warn) - - plugin.check + subject(:plugin) { described_class.new(filenames, context: fake_danger, todos: todos, allow_fail: allow_fail) } + + [true, false].each do |allow_failure| + context "with allow_fail set to #{allow_failure}" do + let(:allow_fail) { allow_failure } + let(:expected_method) do + allow_failure ? :fail : :warn + end + + context 'when the filenames are mentioned in single todo' do + let(:filenames) { ['app/controllers/acme_challenges_controller.rb'] } + + it 'warns about mentions' do + expect(fake_danger) + .to receive(expected_method) + .with <<~MESSAGE + `app/controllers/acme_challenges_controller.rb` was removed but is mentioned in: + - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:5` + MESSAGE + + plugin.check + end + end + + context 'when the filenames are mentioned in multiple todos' do + let(:filenames) do + [ + 'app/controllers/application_controller.rb', + 'app/controllers/acme_challenges_controller.rb' + ] + end + + it 'warns about mentions' do + expect(fake_danger) + .to receive(expected_method) + .with(<<~FIRSTMESSAGE) + `app/controllers/application_controller.rb` was removed but is mentioned in: + - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:4` + - `spec/fixtures/tooling/danger/rubocop_todo/cop2.yml:4` + FIRSTMESSAGE + + expect(fake_danger) + .to receive(expected_method) + .with(<<~SECONDMESSAGE) + `app/controllers/acme_challenges_controller.rb` was removed but is mentioned in: + - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:5` + SECONDMESSAGE + + plugin.check + end + end + + context 'when the filenames are not mentioned in todos' do + let(:filenames) { ['any/inexisting/file.rb'] } + + it 'does not warn' do + expect(fake_danger).not_to receive(expected_method) + + plugin.check + end + end + + context 'when there is no todos' do + let(:filenames) { ['app/controllers/acme_challenges_controller.rb'] } + let(:todos) { [] } + + it 'does not warn' do + expect(fake_danger).not_to receive(expected_method) + + plugin.check + end + end end end end diff --git a/tooling/danger/outdated_todo.rb b/tooling/danger/outdated_todo.rb index a5f5cc897a9ae4f7914ca74d6ef886cb9c02177e..5ae0561010cb02a87d31a48efd8d9c78ff6101cc 100644 --- a/tooling/danger/outdated_todo.rb +++ b/tooling/danger/outdated_todo.rb @@ -8,10 +8,11 @@ class OutdatedTodo spec/support/rspec_order_todo.yml ].freeze - def initialize(filenames, context:, todos: TODOS_GLOBS) + def initialize(filenames, context:, todos: TODOS_GLOBS, allow_fail: false) @filenames = filenames @context = context @todos_globs = todos + @allow_fail = allow_fail end def check @@ -22,17 +23,23 @@ def check private - attr_reader :filenames, :context + attr_reader :filenames, :context, :allow_fail def check_filename(filename) mentions = all_mentions_for(filename) return if mentions.empty? - context.warn <<~MESSAGE + message = <<~MESSAGE `#{filename}` was removed but is mentioned in: #{mentions.join("\n")} MESSAGE + + if allow_fail + context.fail message + else + context.warn message + end end def all_mentions_for(filename)