From 9ca027a2b9fff536a5080d40cae79ec7c50d3458 Mon Sep 17 00:00:00 2001 From: Sam Bostock Date: Wed, 11 Jan 2023 14:52:53 -0500 Subject: [PATCH 1/2] Add integration tests to ensure proper config --- test/fixtures/rubocop.integration.yml | 5 ++ test/integration_test.rb | 90 +++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 test/fixtures/rubocop.integration.yml create mode 100644 test/integration_test.rb diff --git a/test/fixtures/rubocop.integration.yml b/test/fixtures/rubocop.integration.yml new file mode 100644 index 00000000..5d6f9834 --- /dev/null +++ b/test/fixtures/rubocop.integration.yml @@ -0,0 +1,5 @@ +inherit_from: + - ../../rubocop.yml + +AllCops: + SuggestExtensions: false # Reduce noise in test output diff --git a/test/integration_test.rb b/test/integration_test.rb new file mode 100644 index 00000000..50a10f2f --- /dev/null +++ b/test/integration_test.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require "test_helper" +require "tempfile" + +# These ensure we have rules configured to behave as expected, especially for tricky cases such as indentation. +class IntegrationTest < Minitest::Test + def test_indentation + assert_autocorrects(<<~'DIFF') + # typed: true + # frozen_string_literal: true + + sig { params(a: Something, b: SomethingElse).void } + def some_method(a, b) + unless b.something + b.something_else + - Constant.call(ANOTHER_CONSTANT, + - (T.must(b.another_thing) - b.some_time.to_i) * 1000.0) + + Constant.call( + + ANOTHER_CONSTANT, + + (T.must(b.another_thing) - b.some_time.to_i) * 1000.0, + + ) + end + end + DIFF + end + + def test_indentation2 + assert_autocorrects(<<~'DIFF') + # typed: true + # frozen_string_literal: true + + class ExampleController < ApplicationController + sig { void } + def create + @object = something + @object.action + + - redirect_to(a_path(@object.to_route_params), + - notice: "success") + + redirect_to( + + a_path(@object.to_route_params), + + notice: "success", + + ) + rescue SomeError + - redirect_to(a_path(@object.to_route_params), + - alert: "failure") + + redirect_to( + + a_path(@object.to_route_params), + + alert: "failure", + + ) + end + end + DIFF + end + + private + + def assert_autocorrects(diff) + initial, expected = split_diff(diff) + normalized_path = "file.rb" + Tempfile.create(normalized_path) do |file| + File.write(file.path, initial) + output, status = run_rubocop_on(file.path, normalized_path:) + actual = File.read(file.path) + + assert(status.success?, "Rubocop test did not exit cleanly!\n#{output}") + assert_equal(expected, actual, "Autocorrection did not produce the expected output!\n#{output}") + end + end + + def run_rubocop_on(path, normalized_path: path) + output, status = Open3.capture2e( + "bundle", "exec", "rubocop", + "-A", + "--config", File.join(__dir__, "fixtures/rubocop.integration.yml"), + path, + ) + output.gsub!(path, normalized_path) unless path == normalized_path + [output, status] + end + + # Splits a naive full file diff into the contents before and the contents after, + # dropping the diff prefixes (" ", "+", or "-"). + def split_diff(diff) + before = diff.each_line.reject { |line| line.start_with?("+") }.map { |line| line.sub(/^[ -]/, '') }.join + after = diff.each_line.reject { |line| line.start_with?("-") }.map { |line| line.sub(/^[ \+]/, '') }.join + [before, after] + end +end From 53af9ac3dfb7183566a1c109ae8097c7f8035237 Mon Sep 17 00:00:00 2001 From: Sam Bostock Date: Thu, 12 Jan 2023 17:48:11 -0500 Subject: [PATCH 2/2] [WIP] Add integration tests for indentation We've found cases where autocorrections are way off. --- test/fixtures/rubocop.integration.yml | 3 + test/integration_test.rb | 136 ++++++++++++++++++++++++-- 2 files changed, 131 insertions(+), 8 deletions(-) diff --git a/test/fixtures/rubocop.integration.yml b/test/fixtures/rubocop.integration.yml index 5d6f9834..7d8cad5f 100644 --- a/test/fixtures/rubocop.integration.yml +++ b/test/fixtures/rubocop.integration.yml @@ -3,3 +3,6 @@ inherit_from: AllCops: SuggestExtensions: false # Reduce noise in test output + +Style/FrozenStringLiteralComment: + Enabled: false # We know this cop works, and having it enabled adds noise to test snippets diff --git a/test/integration_test.rb b/test/integration_test.rb index 50a10f2f..584a457c 100644 --- a/test/integration_test.rb +++ b/test/integration_test.rb @@ -7,9 +7,6 @@ class IntegrationTest < Minitest::Test def test_indentation assert_autocorrects(<<~'DIFF') - # typed: true - # frozen_string_literal: true - sig { params(a: Something, b: SomethingElse).void } def some_method(a, b) unless b.something @@ -27,9 +24,6 @@ def some_method(a, b) def test_indentation2 assert_autocorrects(<<~'DIFF') - # typed: true - # frozen_string_literal: true - class ExampleController < ApplicationController sig { void } def create @@ -54,6 +48,128 @@ def create DIFF end + def test_indentation3 + assert_autocorrects(<<~'DIFF') + def collection + Service.not_deleted.includes( + - { vault_teams: + - [:github_team_vault_teams, :teams] }, + - { app: + - [{ runtimes: + - [:runtime_instances, :app] }, + - :runtimes, + - :repo,] }, + + { + + vault_teams: + + [:github_team_vault_teams, :teams], + + }, + + { + + app: + + [ + + { + + runtimes: + + [:runtime_instances, :app], + + }, + + :runtimes, + + :repo, + + ] + + }, + :repo, + :app, + :monorepo_mobile_app, + :bugsnag_project, + :slack_channels, + :vault_teams, + :vault_users, + ).joins(:app) + end + DIFF + end + + def test_indentation4 + skip + assert_autocorrects(<<~'DIFF') + def collection + Service.not_deleted.includes( + { vault_teams: [:github_team_vault_teams, :teams] }, + - { app:[ + - { runtimes: [:runtime_instances, :app] }, + - :runtimes, + - :repo,] }, + + { + + app: [ + + { runtimes: [:runtime_instances, :app] }, + + :runtimes, + + :repo, + + ], + + }, + :repo, + :app, + :monorepo_mobile_app, + :bugsnag_project, + :slack_channels, + :vault_teams, + :vault_users, + ).joins(:app) + end + DIFF + end + + def test_indentation5 + assert_autocorrects(<<~'DIFF') + # rubocop:disable Layout/LineLength + def pending_merge_response + - { data: + - { repository: + - { issueOrPullRequest: + - { commits: + - { nodes: + - [{ commit: + - { statusCheckRollup: + - { + - state: "PENDING", + - contexts: + - { nodes: + - [{ name: "Shopify/merge-queue", + - status: "QUEUED", }] }, + - } } }] } } } } } + + { + + data: + + { + + repository: + + { + + issueOrPullRequest: + + { + + commits: + + { + + nodes: + + [{ + + commit: + + { + + statusCheckRollup: + + { + + state: "PENDING", + + contexts: + + { + + nodes: + + [{ + + name: "Shopify/merge-queue", + + status: "QUEUED", + + }], + + }, + + }, + + }, + + }], + + }, + + }, + + }, + + }, + + } + end + # rubocop:enable Layout/LineLength + DIFF + end + private def assert_autocorrects(diff) @@ -64,8 +180,8 @@ def assert_autocorrects(diff) output, status = run_rubocop_on(file.path, normalized_path:) actual = File.read(file.path) - assert(status.success?, "Rubocop test did not exit cleanly!\n#{output}") - assert_equal(expected, actual, "Autocorrection did not produce the expected output!\n#{output}") + assert(status.success?, "Rubocop test did not exit cleanly!\n#{indent(output)}") + assert_equal(expected, actual, "Autocorrection did not produce the expected output!") end end @@ -87,4 +203,8 @@ def split_diff(diff) after = diff.each_line.reject { |line| line.start_with?("-") }.map { |line| line.sub(/^[ \+]/, '') }.join [before, after] end + + def indent(string, by: " " * 4 ) + string.gsub(/^/, by) + end end