From fbdd9343bdaa08c198f43d7df59c06b87fba0cb9 Mon Sep 17 00:00:00 2001 From: Peter Leitzen <pleitzen@gitlab.com> Date: Tue, 18 Apr 2023 14:14:05 +0200 Subject: [PATCH] Resolve all RuboCop offenses for danger specs related files Also remove these file from RuboCop TODOs --- .rubocop_todo/layout/line_length.yml | 1 - .rubocop_todo/lint/unused_block_argument.yml | 1 - .rubocop_todo/style/if_unless_modifier.yml | 1 - .rubocop_todo/style/redundant_freeze.yml | 1 - danger/specs/Dangerfile | 4 +--- .../specs/feature_category_suggestion_spec.rb | 16 ++++++++-------- spec/tooling/danger/specs_spec.rb | 13 ++++++++++--- .../danger/specs/feature_category_suggestion.rb | 2 +- .../danger/specs/project_factory_suggestion.rb | 2 +- 9 files changed, 21 insertions(+), 20 deletions(-) diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index 9f11c94f24268..05e4f96e660b6 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -5478,7 +5478,6 @@ Layout/LineLength: - 'spec/tooling/danger/product_intelligence_spec.rb' - 'spec/tooling/danger/project_helper_spec.rb' - 'spec/tooling/danger/sidekiq_queues_spec.rb' - - 'spec/tooling/danger/specs_spec.rb' - 'spec/tooling/lib/tooling/kubernetes_client_spec.rb' - 'spec/tooling/lib/tooling/test_map_generator_spec.rb' - 'spec/tooling/quality/test_level_spec.rb' diff --git a/.rubocop_todo/lint/unused_block_argument.yml b/.rubocop_todo/lint/unused_block_argument.yml index a70c3823c1dbb..c09dc939ef4c5 100644 --- a/.rubocop_todo/lint/unused_block_argument.yml +++ b/.rubocop_todo/lint/unused_block_argument.yml @@ -438,5 +438,4 @@ Lint/UnusedBlockArgument: - 'spec/tooling/lib/tooling/find_codeowners_spec.rb' - 'spec/tooling/rspec_flaky/config_spec.rb' - 'spec/workers/projects/git_garbage_collect_worker_spec.rb' - - 'tooling/danger/specs.rb' - 'tooling/lib/tooling/find_codeowners.rb' diff --git a/.rubocop_todo/style/if_unless_modifier.yml b/.rubocop_todo/style/if_unless_modifier.yml index 00e7957ed9eba..a4bd4a18854ee 100644 --- a/.rubocop_todo/style/if_unless_modifier.yml +++ b/.rubocop_todo/style/if_unless_modifier.yml @@ -385,7 +385,6 @@ Style/IfUnlessModifier: - 'config/routes.rb' - 'danger/database/Dangerfile' - 'danger/pipeline/Dangerfile' - - 'danger/specs/Dangerfile' - 'danger/z_metadata/Dangerfile' - 'db/migrate/20210909184349_add_index_package_id_id_on_package_files.rb' - 'db/migrate/20220324175325_add_key_data_to_secure_files.rb' diff --git a/.rubocop_todo/style/redundant_freeze.yml b/.rubocop_todo/style/redundant_freeze.yml index 07027d7dd3d9a..3a0f099fd24c4 100644 --- a/.rubocop_todo/style/redundant_freeze.yml +++ b/.rubocop_todo/style/redundant_freeze.yml @@ -235,7 +235,6 @@ Style/RedundantFreeze: - 'tooling/danger/config_files.rb' - 'tooling/danger/customer_success.rb' - 'tooling/danger/datateam.rb' - - 'tooling/danger/specs.rb' - 'tooling/danger/stable_branch.rb' - 'tooling/lib/tooling/kubernetes_client.rb' - 'tooling/lib/tooling/mappings/view_to_js_mappings.rb' diff --git a/danger/specs/Dangerfile b/danger/specs/Dangerfile index bd5bd5078e2d2..ff766cf688ee9 100644 --- a/danger/specs/Dangerfile +++ b/danger/specs/Dangerfile @@ -50,9 +50,7 @@ if has_ee_app_changes && has_spec_changes && !(has_app_changes || has_ee_spec_ch end # Forbidding a new file addition under `/spec/controllers` or `/ee/spec/controllers` -if helper.changes.added.files.grep(%r{^(ee/)?spec/controllers/}).any? - warn CONTROLLER_SPEC_DEPRECATION_MESSAGE -end +warn CONTROLLER_SPEC_DEPRECATION_MESSAGE if helper.changes.added.files.grep(%r{^(ee/)?spec/controllers/}).any? specs.changed_specs_files.each do |filename| specs.add_suggestions_for(filename) diff --git a/spec/tooling/danger/specs/feature_category_suggestion_spec.rb b/spec/tooling/danger/specs/feature_category_suggestion_spec.rb index 4d965e4bc3670..3956553f48848 100644 --- a/spec/tooling/danger/specs/feature_category_suggestion_spec.rb +++ b/spec/tooling/danger/specs/feature_category_suggestion_spec.rb @@ -27,14 +27,14 @@ [ " require 'spec_helper'", " \n", - " RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController, feature_category: :planning_analytics do", + " RSpec.describe Projects::SummaryController, feature_category: :planning_analytics do", " end", - "RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", + "RSpec.describe Projects::SummaryController do", " let_it_be(:user) { create(:user) }", " end", " describe 'GET \"time_summary\"' do", " end", - " RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", + " RSpec.describe Projects::SummaryController do", " let_it_be(:user) { create(:user) }", " end", " describe 'GET \"time_summary\"' do", @@ -57,12 +57,12 @@ let(:changed_lines) do [ - "+ RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController, feature_category: :planning_analytics do", - "+RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", + "+ RSpec.describe Projects::SummaryController, feature_category: :planning_analytics do", + "+RSpec.describe Projects::SummaryController do", "+ let_it_be(:user) { create(:user) }", "- end", "+ describe 'GET \"time_summary\"' do", - "+ RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", + "+ RSpec.describe Projects::SummaryController do", "+RSpec.describe Projects :aggregate_failures,", "+ feature_category: planning_analytics do", "+RSpec.describe Epics :aggregate_failures,", @@ -85,8 +85,8 @@ it 'adds suggestions at the correct lines', :aggregate_failures do [ - { suggested_line: "RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", number: 5 }, - { suggested_line: " RSpec.describe Projects::Analytics::CycleAnalytics::SummaryController do", number: 10 }, + { suggested_line: "RSpec.describe Projects::SummaryController do", number: 5 }, + { suggested_line: " RSpec.describe Projects::SummaryController do", number: 10 }, { suggested_line: "RSpec.describe Epics :aggregate_failures,", number: 19 } ].each do |test_case| diff --git a/spec/tooling/danger/specs_spec.rb b/spec/tooling/danger/specs_spec.rb index c40a99042eb43..b4953858ef7f5 100644 --- a/spec/tooling/danger/specs_spec.rb +++ b/spec/tooling/danger/specs_spec.rb @@ -13,7 +13,12 @@ subject(:specs) { fake_danger.new(helper: fake_helper) } describe '#changed_specs_files' do - let(:base_expected_files) { %w[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] } + let(:base_expected_files) do + %w[ + 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 + ] + end before do all_changed_files = %w[ @@ -37,13 +42,15 @@ context 'with include_ee: :exclude' do it 'returns spec files without EE-specific files' do - expect(specs.changed_specs_files(ee: :exclude)).not_to include(%w[ee/spec/foo_spec.rb ee/spec/bar_spec.rb ee/spec/zab_spec.rb]) + expect(specs.changed_specs_files(ee: :exclude)) + .not_to include(%w[ee/spec/foo_spec.rb ee/spec/bar_spec.rb ee/spec/zab_spec.rb]) end end context 'with include_ee: :only' do it 'returns EE-specific spec files only' do - expect(specs.changed_specs_files(ee: :only)).to match_array(%w[ee/spec/foo_spec.rb ee/spec/bar_spec.rb ee/spec/zab_spec.rb]) + expect(specs.changed_specs_files(ee: :only)) + .to match_array(%w[ee/spec/foo_spec.rb ee/spec/bar_spec.rb ee/spec/zab_spec.rb]) end end end diff --git a/tooling/danger/specs/feature_category_suggestion.rb b/tooling/danger/specs/feature_category_suggestion.rb index bff9421586baf..5acf73c895621 100644 --- a/tooling/danger/specs/feature_category_suggestion.rb +++ b/tooling/danger/specs/feature_category_suggestion.rb @@ -17,7 +17,7 @@ def suggest file_lines = project_helper.file_lines(filename) changed_lines = helper.changed_lines(filename) - changed_lines.each_with_index do |changed_line, i| + changed_lines.each do |changed_line| next unless changed_line =~ RSPEC_TOP_LEVEL_DESCRIBE_REGEX line_number = file_lines.find_index(changed_line.delete_prefix('+')) diff --git a/tooling/danger/specs/project_factory_suggestion.rb b/tooling/danger/specs/project_factory_suggestion.rb index 5fb0c7cbfe2b8..4e5a70ac8e5f7 100644 --- a/tooling/danger/specs/project_factory_suggestion.rb +++ b/tooling/danger/specs/project_factory_suggestion.rb @@ -22,7 +22,7 @@ class ProjectFactorySuggestion < Suggestion \s*\{\s* # Opening curly brace surrounded by 0-many whitespace characters create\( # literal (?:#{PROJECT_FACTORIES.join('|')}) # Any of the project factory names - \W # Non-word character, avoid matching factories like :project_authorization + \W # Non-word character, avoid matching factories like :project_badge ) # end capture group named tail /x -- GitLab