Add latest changes from gitlab-org/gitlab@master

4 files changed, 475 insertions, 1 deletions

diff --git a/spec/rubocop/check_graceful_task_spec.rb b/spec/rubocop/check_graceful_task_spec.rb
new file mode 100644
index 00000000000..0364820a602
--- /dev/null
+++ b/spec/rubocop/check_graceful_task_spec.rb
@@ -0,0 +1,125 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'stringio'
+
+require_relative '../support/helpers/next_instance_of'
+require_relative '../../rubocop/check_graceful_task'
+
+RSpec.describe RuboCop::CheckGracefulTask do
+  include NextInstanceOf
+
+  let(:output) { StringIO.new }
+
+  subject(:task) { described_class.new(output) }
+
+  describe '#run' do
+    let(:status_success) { RuboCop::CLI::STATUS_SUCCESS }
+    let(:status_offenses) { RuboCop::CLI::STATUS_OFFENSES }
+    let(:rubocop_status) { status_success }
+    let(:adjusted_rubocop_status) { rubocop_status }
+
+    subject { task.run(args) }
+
+    before do
+      # Don't notify Slack accidentally.
+      allow(Gitlab::Popen).to receive(:popen).and_raise('Notifications forbidden.')
+      stub_const('ENV', ENV.to_hash.delete_if { |key, _| key.start_with?('CI_') })
+
+      allow_next_instance_of(RuboCop::CLI) do |cli|
+        allow(cli).to receive(:run).and_return(rubocop_status)
+      end
+
+      allow(RuboCop::Formatter::GracefulFormatter)
+        .to receive(:adjusted_exit_status).and_return(adjusted_rubocop_status)
+    end
+
+    shared_examples 'rubocop scan' do |rubocop_args:|
+      it 'invokes a RuboCop scan' do
+        rubocop_options = %w[--parallel --format RuboCop::Formatter::GracefulFormatter]
+        rubocop_options.concat(rubocop_args)
+
+        expect_next_instance_of(RuboCop::CLI) do |cli|
+          expect(cli).to receive(:run).with(rubocop_options).and_return(rubocop_status)
+        end
+
+        subject
+
+        expect(output.string)
+          .to include('Running RuboCop in graceful mode:')
+          .and include("rubocop #{rubocop_options.join(' ')}")
+          .and include('This might take a while...')
+      end
+    end
+
+    context 'without args' do
+      let(:args) { [] }
+
+      it_behaves_like 'rubocop scan', rubocop_args: []
+
+      context 'with adjusted rubocop status' do
+        let(:rubocop_status) { status_offenses }
+        let(:adjusted_rubocop_status) { status_success }
+
+        context 'with sufficient environment variables' do
+          let(:channel) { 'f_rubocop' }
+
+          before do
+            env = {
+              'CI_SLACK_WEBHOOK_URL' => 'webhook_url',
+              'CI_JOB_NAME' => 'job_name',
+              'CI_JOB_URL' => 'job_url'
+            }
+
+            stub_const('ENV', ENV.to_hash.update(env))
+          end
+
+          it 'notifies slack' do
+            popen_args = ['scripts/slack', channel, kind_of(String), 'rubocop', kind_of(String)]
+            popen_result = ['', 0]
+            expect(Gitlab::Popen).to receive(:popen).with(popen_args).and_return(popen_result)
+
+            subject
+
+            expect(output.string).to include("Notifying Slack ##{channel}.")
+          end
+
+          context 'with when notification fails' do
+            it 'prints that notification failed' do
+              popen_result = ['', 1]
+              expect(Gitlab::Popen).to receive(:popen).and_return(popen_result)
+
+              subject
+
+              expect(output.string).to include("Failed to notify Slack channel ##{channel}.")
+            end
+          end
+        end
+
+        context 'with missing environment variables' do
+          it 'skips slack notification' do
+            expect(Gitlab::Popen).not_to receive(:popen)
+
+            subject
+
+            expect(output.string).to include('Skipping Slack notification.')
+          end
+        end
+      end
+    end
+
+    context 'with args' do
+      let(:args) { %w[a.rb Lint/EmptyFile b.rb Lint/Syntax] }
+
+      it_behaves_like 'rubocop scan', rubocop_args: %w[--only Lint/EmptyFile,Lint/Syntax a.rb b.rb]
+
+      it 'does not notify slack' do
+        expect(Gitlab::Popen).not_to receive(:popen)
+
+        subject
+
+        expect(output.string).not_to include('Skipping Slack notification.')
+      end
+    end
+  end
+end
diff --git a/spec/rubocop/cop_todo_spec.rb b/spec/rubocop/cop_todo_spec.rb
index 490f55a8bb6..3f9c378b303 100644
--- a/spec/rubocop/cop_todo_spec.rb
+++ b/spec/rubocop/cop_todo_spec.rb
@@ -14,7 +14,8 @@ RSpec.describe RuboCop::CopTodo do
         cop_name: cop_name,
         files: be_empty,
         offense_count: 0,
-        previously_disabled: false
+        previously_disabled: false,
+        grace_period: false
       )
     end
   end
@@ -102,6 +103,23 @@ RSpec.describe RuboCop::CopTodo do
       end
     end
 
+    context 'with grace period' do
+      specify do
+        cop_todo.record('a.rb', 1)
+        cop_todo.record('b.rb', 2)
+        cop_todo.grace_period = true
+
+        expect(yaml).to eq(<<~YAML)
+          ---
+          #{cop_name}:
+            Details: grace period
+            Exclude:
+              - 'a.rb'
+              - 'b.rb'
+        YAML
+      end
+    end
+
     context 'with multiple files' do
       before do
         cop_todo.record('a.rb', 0)
diff --git a/spec/rubocop/formatter/graceful_formatter_spec.rb b/spec/rubocop/formatter/graceful_formatter_spec.rb
new file mode 100644
index 00000000000..0e0c1d52067
--- /dev/null
+++ b/spec/rubocop/formatter/graceful_formatter_spec.rb
@@ -0,0 +1,239 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rspec-parameterized'
+require 'rubocop/rspec/shared_contexts'
+require 'stringio'
+
+require_relative '../../../rubocop/formatter/graceful_formatter'
+require_relative '../../../rubocop/todo_dir'
+
+RSpec.describe RuboCop::Formatter::GracefulFormatter, :isolated_environment do
+  # Set by :isolated_environment
+  let(:todo_dir) { RuboCop::TodoDir.new("#{Dir.pwd}/.rubocop_todo") }
+  let(:stdout) { StringIO.new }
+
+  subject(:formatter) { described_class.new(stdout) }
+
+  shared_examples 'summary reporting' do |inspected:, offenses: 0, silenced: 0|
+    it "reports summary with #{inspected} inspected, #{offenses} offenses, #{silenced} silenced" do
+      expect(stdout.string)
+        .to match(/Inspecting #{inspected} files/)
+        .and match(/#{inspected} files inspected/)
+
+      if offenses > 0
+        expect(stdout.string).to match(/Offenses:/)
+        expect(stdout.string).to match(/#{offenses} offenses detected/)
+      else
+        expect(stdout.string).not_to match(/Offenses:/)
+        expect(stdout.string).to match(/no offenses detected/)
+      end
+
+      if silenced > 0
+        expect(stdout.string).to match(/Silenced offenses:/)
+        expect(stdout.string).to match(/#{silenced} offenses silenced/)
+      else
+        expect(stdout.string).not_to match(/Silenced offenses:/)
+        expect(stdout.string).not_to match(/offenses silenced/)
+      end
+    end
+  end
+
+  context 'with offenses' do
+    let(:offense1) { fake_offense('Cop1') }
+    let(:offense2) { fake_offense('Cop2') }
+
+    before do
+      FileUtils.touch('.rubocop_todo.yml')
+
+      File.write('.rubocop.yml', <<~YAML)
+        inherit_from:
+          <% Dir.glob('.rubocop_todo/**/*.yml').each do |rubocop_todo_yaml| %>
+          - '<%= rubocop_todo_yaml %>'
+          <% end %>
+          - '.rubocop_todo.yml'
+
+        AllCops:
+          NewCops: enable # Avoiding RuboCop warnings
+      YAML
+
+      # These cops are unknown and would raise an validation error
+      allow(RuboCop::Cop::Registry.global).to receive(:contains_cop_matching?)
+        .and_return(true)
+    end
+
+    context 'with active only' do
+      before do
+        formatter.started(%w[a.rb b.rb])
+        formatter.file_finished('a.rb', [offense1])
+        formatter.file_finished('b.rb', [offense2])
+        formatter.finished(%w[a.rb b.rb])
+      end
+
+      it_behaves_like 'summary reporting', inspected: 2, offenses: 2
+    end
+
+    context 'with silenced only' do
+      before do
+        todo_dir.write('Cop1', <<~YAML)
+        ---
+        Cop1:
+          Details: grace period
+        YAML
+
+        File.write('.rubocop_todo.yml', <<~YAML)
+        ---
+        Cop2:
+          Details: grace period
+        YAML
+
+        formatter.started(%w[a.rb b.rb])
+        formatter.file_finished('a.rb', [offense1])
+        formatter.file_finished('b.rb', [offense2])
+        formatter.finished(%w[a.rb b.rb])
+      end
+
+      it_behaves_like 'summary reporting', inspected: 2, silenced: 2
+    end
+
+    context 'with active and silenced' do
+      before do
+        todo_dir.write('Cop1', <<~YAML)
+        ---
+        Cop1:
+          Details: grace period
+        YAML
+
+        formatter.started(%w[a.rb b.rb])
+        formatter.file_finished('a.rb', [offense1, offense2])
+        formatter.file_finished('b.rb', [offense2, offense1, offense1])
+        formatter.finished(%w[a.rb b.rb])
+      end
+
+      it_behaves_like 'summary reporting', inspected: 2, offenses: 2, silenced: 3
+    end
+  end
+
+  context 'without offenses' do
+    before do
+      formatter.started(%w[a.rb b.rb])
+      formatter.file_finished('a.rb', [])
+      formatter.file_finished('b.rb', [])
+      formatter.finished(%w[a.rb b.rb])
+    end
+
+    it_behaves_like 'summary reporting', inspected: 2
+  end
+
+  context 'without files to inspect' do
+    before do
+      formatter.started([])
+      formatter.finished([])
+    end
+
+    it_behaves_like 'summary reporting', inspected: 0
+  end
+
+  context 'with missing @total_offense_count' do
+    it 'raises an error' do
+      formatter.started(%w[a.rb])
+
+      if formatter.instance_variable_defined?(:@total_offense_count)
+        formatter.remove_instance_variable(:@total_offense_count)
+      end
+
+      expect do
+        formatter.finished(%w[a.rb])
+      end.to raise_error(/RuboCop has changed its internals/)
+    end
+  end
+
+  describe '.adjusted_exit_status' do
+    using RSpec::Parameterized::TableSyntax
+
+    success = RuboCop::CLI::STATUS_SUCCESS
+    offenses = RuboCop::CLI::STATUS_OFFENSES
+    error = RuboCop::CLI::STATUS_ERROR
+
+    subject { described_class.adjusted_exit_status(status) }
+
+    where(:active_offenses, :status, :adjusted_status) do
+      0 | success  | success
+      0 | offenses | success
+      1 | offenses | offenses
+      0 | error    | error
+      1 | error    | error
+      # impossible cases
+      1 | success  | success
+    end
+
+    with_them do
+      around do |example|
+        described_class.active_offenses = active_offenses
+        example.run
+      ensure
+        described_class.active_offenses = 0
+      end
+
+      it { is_expected.to eq(adjusted_status) }
+    end
+  end
+
+  describe '.grace_period?' do
+    let(:cop_name) { 'Cop/Name' }
+
+    subject { described_class.grace_period?(cop_name, config) }
+
+    context 'with Details in config' do
+      let(:config) { { 'Details' => 'grace period' } }
+
+      it { is_expected.to eq(true) }
+    end
+
+    context 'with unknown value for Details in config' do
+      let(:config) { { 'Details' => 'unknown' } }
+
+      specify do
+        expect { is_expected.to eq(false) }
+          .to output(/#{cop_name}: Unhandled value "unknown" for `Details` key./)
+          .to_stderr
+      end
+    end
+
+    context 'with empty config' do
+      let(:config) { {} }
+
+      it { is_expected.to eq(false) }
+    end
+
+    context 'without Details in config' do
+      let(:config) { { 'Exclude' => false } }
+
+      it { is_expected.to eq(false) }
+    end
+  end
+
+  describe '.grace_period_key_value' do
+    subject { described_class.grace_period_key_value }
+
+    it { is_expected.to eq('Details: grace period') }
+  end
+
+  def fake_offense(cop_name)
+    # rubocop:disable RSpec/VerifiedDoubles
+    double(:offense,
+           cop_name: cop_name,
+           corrected?: false,
+           correctable?: false,
+           severity: double(:severity, name: 'convention', code: :C),
+           line: 5,
+           column: 23,
+           real_column: 23,
+           corrected_with_todo?: false,
+           message: "#{cop_name} message",
+           location: double(:location, source_line: 'line', first_line: 1, last_line: 2),
+           highlighted_area: double(:highlighted_area, begin_pos: 1, size: 2)
+          )
+    # rubocop:enable RSpec/VerifiedDoubles
+  end
+end
diff --git a/spec/rubocop/formatter/todo_formatter_spec.rb b/spec/rubocop/formatter/todo_formatter_spec.rb
index 54b40e5b15f..edd84632409 100644
--- a/spec/rubocop/formatter/todo_formatter_spec.rb
+++ b/spec/rubocop/formatter/todo_formatter_spec.rb
@@ -176,6 +176,98 @@ RSpec.describe RuboCop::Formatter::TodoFormatter do
       end
     end
 
+    context 'with grace period' do
+      let(:yaml) do
+        <<~YAML
+          ---
+          B/TooManyOffenses:
+            Details: grace period
+            Exclude:
+              - 'x.rb'
+        YAML
+      end
+
+      shared_examples 'keeps grace period' do
+        it 'keeps Details: grace period' do
+          run_formatter
+
+          expect(todo_yml('B/TooManyOffenses')).to eq(<<~YAML)
+            ---
+            B/TooManyOffenses:
+              Details: grace period
+              Exclude:
+                - 'a.rb'
+                - 'c.rb'
+          YAML
+        end
+      end
+
+      context 'in rubocop_todo/' do
+        before do
+          todo_dir.write('B/TooManyOffenses', yaml)
+          todo_dir.inspect_all
+        end
+
+        it_behaves_like 'keeps grace period'
+      end
+
+      context 'in rubocop_todo.yml' do
+        before do
+          File.write('.rubocop_todo.yml', yaml)
+        end
+
+        it_behaves_like 'keeps grace period'
+      end
+
+      context 'with invalid details value' do
+        let(:yaml) do
+          <<~YAML
+            ---
+            B/TooManyOffenses:
+              Details: something unknown
+              Exclude:
+                - 'x.rb'
+          YAML
+        end
+
+        it 'ignores the details and warns' do
+          File.write('.rubocop_todo.yml', yaml)
+
+          expect { run_formatter }
+            .to output(%r{B/TooManyOffenses: Unhandled value "something unknown" for `Details` key.})
+            .to_stderr
+
+          expect(todo_yml('B/TooManyOffenses')).to eq(<<~YAML)
+            ---
+            B/TooManyOffenses:
+              Exclude:
+                - 'a.rb'
+                - 'c.rb'
+          YAML
+        end
+      end
+
+      context 'and previously disabled' do
+        let(:yaml) do
+          <<~YAML
+            ---
+            B/TooManyOffenses:
+              Enabled: false
+              Details: grace period
+              Exclude:
+                - 'x.rb'
+          YAML
+        end
+
+        it 'raises an exception' do
+          File.write('.rubocop_todo.yml', yaml)
+
+          expect { run_formatter }
+            .to raise_error(RuntimeError, 'B/TooManyOffenses: Cop must be enabled to use `Details: grace period`.')
+        end
+      end
+    end
+
     context 'with cop configuration in both .rubocop_todo/ and .rubocop_todo.yml' do
       before do
         todo_dir.write('B/TooManyOffenses', <<~YAML)