From 2f8630fff49bdba4a849558ff7af5cff9ff4de29 Mon Sep 17 00:00:00 2001 From: Justin Rabiller <38727166+snutij@users.noreply.github.com> Date: Wed, 20 Sep 2023 17:14:35 +0200 Subject: [PATCH] Tests: Use YARP fixtures (#788) * feat: add yarp submodule * feat: update expectation_test_runner to use yarp fixtures * fix: requests with yarp fixtures edge-cases * fix: rescue StandardError for rubocop fail --- .github/workflows/ci.yml | 2 ++ .gitmodules | 3 +++ Rakefile | 2 +- bin/test | 8 +++++++ lib/ruby_lsp/requests/code_action_resolve.rb | 4 +++- .../requests/support/rubocop_runner.rb | 13 ++++++---- test/expectations/expectations_test_runner.rb | 24 +++++++++++++++++-- test/fixtures/yarp | 1 + 8 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 .gitmodules create mode 160000 test/fixtures/yarp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e1b72413d..13f0b5702 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,8 @@ jobs: name: Ruby ${{ matrix.ruby }} on ${{ matrix.os }} steps: - uses: actions/checkout@v3 + with: + submodules: "recursive" - name: Set up Ruby uses: ruby/setup-ruby@v1 diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 000000000..94256e74f --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "test/fixtures/yarp"] + path = test/fixtures/yarp + url = https://github.com/ruby/yarp.git diff --git a/Rakefile b/Rakefile index 238bfa279..f6b60cf7a 100644 --- a/Rakefile +++ b/Rakefile @@ -8,7 +8,7 @@ require "ruby_lsp/check_docs" Rake::TestTask.new(:test) do |t| t.libs << "test" t.libs << "lib" - t.test_files = FileList["test/**/*_test.rb", "lib/ruby_indexer/test/**/*_test.rb"] + t.test_files = FileList["test/**/*_test.rb", "lib/ruby_indexer/test/**/*_test.rb"].exclude("test/fixtures/yarp/**/*") end RDoc::Task.new do |rdoc| diff --git a/bin/test b/bin/test index dfa37a078..850ae5fb6 100755 --- a/bin/test +++ b/bin/test @@ -1,5 +1,13 @@ #!/usr/bin/env bash +YARP_FIXTURES_DIR=test/fixtures/yarp/test/yarp/fixtures + +if [ ! -d "$YARP_FIXTURES_DIR" ]; then + echo "$YARP_FIXTURES_DIR does not exist." + echo "Please run 'git submodule update --init' to pull submodule fixtures." + exit 1 +fi + if [[ 2 -eq $# ]]; then bundle exec rake TEST="$1" TESTOPTS="-n='/$2/'" elif [[ 1 -eq $# ]]; then diff --git a/lib/ruby_lsp/requests/code_action_resolve.rb b/lib/ruby_lsp/requests/code_action_resolve.rb index 757e04f89..258b4128e 100644 --- a/lib/ruby_lsp/requests/code_action_resolve.rb +++ b/lib/ruby_lsp/requests/code_action_resolve.rb @@ -43,6 +43,8 @@ def initialize(document, code_action) sig { override.returns(T.any(Interface::CodeAction, Error)) } def run + return Error::EmptySelection if @document.source.empty? + source_range = @code_action.dig(:data, :range) return Error::EmptySelection if source_range[:start] == source_range[:end] @@ -55,7 +57,7 @@ def run closest_statements, parent_statements = @document .locate(@document.tree, start_index, node_types: [YARP::StatementsNode, YARP::BlockNode]) - return Error::InvalidTargetRange if closest_statements.nil? + return Error::InvalidTargetRange if closest_statements.nil? || closest_statements.child_nodes.compact.empty? # Find the node with the end line closest to the requested position, so that we can place the refactor # immediately after that closest node diff --git a/lib/ruby_lsp/requests/support/rubocop_runner.rb b/lib/ruby_lsp/requests/support/rubocop_runner.rb index b6b7e8c56..4e9d2035f 100644 --- a/lib/ruby_lsp/requests/support/rubocop_runner.rb +++ b/lib/ruby_lsp/requests/support/rubocop_runner.rb @@ -20,14 +20,19 @@ class InternalRuboCopError < StandardError extend T::Sig MESSAGE = <<~EOS - An internal error occurred for the %s cop. + An internal error occurred %s. Updating to a newer version of RuboCop may solve this. For more details, run RuboCop on the command line. EOS - sig { params(rubocop_error: RuboCop::ErrorWithAnalyzedFileLocation).void } + sig { params(rubocop_error: T.any(RuboCop::ErrorWithAnalyzedFileLocation, StandardError)).void } def initialize(rubocop_error) - message = format(MESSAGE, rubocop_error.cop.name) + message = case rubocop_error + when RuboCop::ErrorWithAnalyzedFileLocation + format(MESSAGE, "for the #{rubocop_error.cop.name} cop") + when StandardError + format(MESSAGE, rubocop_error.message) + end super(message) end end @@ -87,7 +92,7 @@ def run(path, contents) raise Formatting::Error, error.message rescue RuboCop::ValidationError => error raise ConfigurationError, error.message - rescue RuboCop::ErrorWithAnalyzedFileLocation => error + rescue StandardError => error raise InternalRuboCopError, error end diff --git a/test/expectations/expectations_test_runner.rb b/test/expectations/expectations_test_runner.rb index c05b9550d..06baf54bc 100644 --- a/test/expectations/expectations_test_runner.rb +++ b/test/expectations/expectations_test_runner.rb @@ -4,7 +4,8 @@ class ExpectationsTestRunner < Minitest::Test TEST_EXP_DIR = "test/expectations" TEST_FIXTURES_DIR = "test/fixtures" - TEST_FIXTURES_GLOB = File.join(TEST_FIXTURES_DIR, "**", "*.rb") + TEST_RUBY_LSP_FIXTURES = File.join(TEST_FIXTURES_DIR, "*.rb") + TEST_YARP_FIXTURES = File.join(TEST_FIXTURES_DIR, "yarp/test/yarp/fixtures/**", "*.txt") class << self def expectations_tests(handler_class, expectation_suffix) @@ -49,7 +50,7 @@ def default_args include ExpectationsRunnerMethods RB - Dir.glob(TEST_FIXTURES_GLOB).each do |path| + Dir.glob(TEST_RUBY_LSP_FIXTURES).each do |path| test_name = File.basename(path, ".rb") expectations_dir = File.join(TEST_EXP_DIR, expectation_suffix) @@ -91,6 +92,25 @@ def test_#{expectation_suffix}__#{test_name}__does_not_raise RB end end + + Dir.glob(TEST_YARP_FIXTURES).each do |path| + class_eval(<<~RB, __FILE__, __LINE__ + 1) + def test_#{expectation_suffix}__#{uniq_name_from_path(path)}__does_not_raise + @_path = "#{path}" + source = File.read(@_path) + run_expectations(source) + rescue RubyLsp::Requests::Support::InternalRuboCopError, RubyLsp::Requests::Formatting::Error + skip "Fixture requires a fix from Rubocop" + end + RB + end + end + + # Ensure that the test name include path context to avoid duplicate + # from test/fixtures/yarp/test/yarp/fixtures/unparser/corpus/semantic/and.txt + # to test_fixtures_yarp_test_yarp_fixtures_unparser_corpus_semantic_and + def uniq_name_from_path(path) + path.gsub("/", "_").gsub('.txt', '') end def ruby_requirement_magic_comment_version(fixture_path) diff --git a/test/fixtures/yarp b/test/fixtures/yarp new file mode 160000 index 000000000..1ba82438f --- /dev/null +++ b/test/fixtures/yarp @@ -0,0 +1 @@ +Subproject commit 1ba82438f2572ead580dc0892358d0ce718565ed