Merge pull request rails#46644 from brasic/another-try-at-test-parsing

Replace `method_source` gem with Ripper (again)

Author: byroot

Gemfile.lock

@@ -94,7 +94,6 @@ PATH
     railties (7.1.0.alpha)
       actionpack (= 7.1.0.alpha)
       activesupport (= 7.1.0.alpha)
-      method_source
       rake (>= 12.2)
       thor (~> 1.0)
       zeitwerk (~> 2.6)
@@ -320,7 +319,6 @@ GEM
     marcel (1.0.2)
     matrix (0.4.2)
     memoist (0.16.2)
-    method_source (1.0.0)
     mini_magick (4.11.0)
     mini_mime (1.1.2)
     mini_portile2 (2.8.0)

railties/lib/rails/test_unit/runner.rb

@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
 require "shellwords"
-require "method_source"
 require "rake/file_list"
 require "active_support"
 require "active_support/core_ext/module/attribute_accessors"
+require "rails/test_unit/test_parser"
 
 module Rails
   module TestUnit
@@ -168,10 +168,7 @@ def ===(method)
 
       private
         def definition_for(method)
-          file, start_line = method.source_location
-          end_line = method.source.count("\n") + start_line - 1
-
-          return file, start_line..end_line
+          TestParser.definition_for(method)
         end
     end
   end

railties/lib/rails/test_unit/test_parser.rb

@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+require "ripper"
+
+module Rails
+  module TestUnit
+    # Parse a test file to extract the line ranges of all tests in both
+    # method-style (def test_foo) and declarative-style (test "foo" do)
+    class TestParser < Ripper # :nodoc:
+      # Helper to translate a method object into the path and line range where
+      # the method was defined.
+      def self.definition_for(method_obj)
+        path, begin_line = method_obj.source_location
+        begins_to_ends = new(File.read(path), path).parse
+        return unless end_line = begins_to_ends[begin_line]
+        [path, (begin_line..end_line)]
+      end
+
+      def initialize(*)
+        # A hash mapping the 1-indexed line numbers that tests start on to where they end.
+        @begins_to_ends = {}
+        super
+      end
+
+      def parse
+        super
+        @begins_to_ends
+      end
+
+      # method test e.g. `def test_some_description`
+      # This event's first argument gets the `ident` node containing the method
+      # name, which we have overridden to return the line number of the ident
+      # instead.
+      def on_def(begin_line, *)
+        @begins_to_ends[begin_line] = lineno
+      end
+
+      # Everything past this point is to support declarative tests, which
+      # require more work to get right because of the many different ways
+      # methods can be invoked in ruby, all of which are parsed differently.
+      #
+      # The approach is just to store the current line number when the
+      # "test" method is called and pass it up the tree so it's available at
+      # the point when we also know the line where the associated block ends.
+
+      def on_method_add_block(begin_line, end_line)
+        if begin_line && end_line
+          @begins_to_ends[begin_line] = end_line
+        end
+      end
+
+      def on_command_call(*, begin_lineno, _args)
+        begin_lineno
+      end
+
+      def first_arg(arg, *)
+        arg
+      end
+
+      def just_lineno(*)
+        lineno
+      end
+
+      alias on_method_add_arg first_arg
+      alias on_command first_arg
+      alias on_stmts_add first_arg
+      alias on_arg_paren first_arg
+      alias on_bodystmt first_arg
+
+      alias on_ident just_lineno
+      alias on_do_block just_lineno
+      alias on_stmts_new just_lineno
+      alias on_brace_block just_lineno
+
+      def on_args_new
+        []
+      end
+
+      def on_args_add(parts, part)
+        parts << part
+      end
+
+      def on_args_add_block(args, *rest)
+        args.first
+      end
+    end
+  end
+end

railties/railties.gemspec

@@ -42,7 +42,6 @@ Gem::Specification.new do |s|
 
   s.add_dependency "rake", ">= 12.2"
   s.add_dependency "thor", "~> 1.0"
-  s.add_dependency "method_source"
   s.add_dependency "zeitwerk", "~> 2.6"
 
   s.add_development_dependency "actionview", version

railties/test/application/test_runner_test.rb

@@ -376,7 +376,7 @@ def test_line_filter_does_not_run_this
       end
     end
 
-    def test_more_than_one_line_filter
+    def test_more_than_one_line_filter_macro_syntax
       app_file "test/models/post_test.rb", <<-RUBY
         require "test_helper"
 
@@ -397,10 +397,83 @@ class PostTest < ActiveSupport::TestCase
         end
       RUBY
 
-      run_test_command("test/models/post_test.rb:4:13").tap do |output|
-        assert_match "PostTest:FirstFilter", output
-        assert_match "PostTest:SecondFilter", output
-        assert_match "2 runs, 2 assertions", output
+      pos_cases = {
+        "first line of each test" => "test/models/post_test.rb:4:13",
+        "interior of tests" => "test/models/post_test.rb:5:14",
+        "last line of each test" => "test/models/post_test.rb:7:16"
+      }
+
+      pos_cases.each do |name, cmd|
+        output = run_test_command(cmd)
+        assert_match "PostTest:FirstFilter", output, "for #{cmd} (#{name})"
+        assert_match "PostTest:SecondFilter", output, "for #{cmd} (#{name})"
+        assert_match "2 runs, 2 assertions", output, "for #{cmd} (#{name})"
+      end
+
+      # one past the end of each test matches nothing
+      run_test_command("test/models/post_test.rb:8:17").tap do |output|
+        assert_match "0 runs, 0 assertions", output
+      end
+    end
+
+    def test_more_than_one_line_filter_test_method_syntax
+      app_file "test/models/post_test.rb", <<-RUBY
+        require "test_helper"
+
+        class PostTest < ActiveSupport::TestCase
+          def test_first_filter
+            puts 'PostTest:FirstFilter'
+            assert true
+          end
+
+          def test_second_filter
+            puts 'PostTest:SecondFilter'
+            assert true
+          end
+
+          def test_line_filter_does_not_run_this
+            assert true
+          end
+        end
+      RUBY
+
+      pos_cases = {
+        "first line of each test" => "test/models/post_test.rb:4:9",
+        "interior of tests" => "test/models/post_test.rb:5:10",
+        "last line of each test" => "test/models/post_test.rb:7:12"
+      }
+
+      pos_cases.each do |name, cmd|
+        output = run_test_command(cmd)
+        assert_match "PostTest:FirstFilter", output, "for #{cmd} (#{name})"
+        assert_match "PostTest:SecondFilter", output, "for #{cmd} (#{name})"
+        assert_match "2 runs, 2 assertions", output, "for #{cmd} (#{name})"
+      end
+
+      # one past the end of each test matches nothing
+      run_test_command("test/models/post_test.rb:8:13").tap do |output|
+        assert_match "0 runs, 0 assertions", output
+      end
+    end
+
+    def test_multiple_tests_on_same_line
+      app_file "test/models/account_test.rb", <<-RUBY
+        require "test_helper"
+
+        class AccountTest < ActiveSupport::TestCase
+          test "first" do puts :first; end; def test_second; puts :second; end
+          test "third" do
+            puts :third
+            assert false
+          end
+        end
+      RUBY
+
+      run_test_command("test/models/account_test.rb:4").tap do |output|
+        assert_match "first", output
+        assert_match "second", output
+        assert_no_match "third", output
+        assert_match "2 runs, 0 assertions, 0 failures", output
       end
     end
 

railties/test/test_unit/test_parser_test.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+require "active_support/test_case"
+require "active_support/testing/autorun"
+require "rails/test_unit/test_parser"
+
+class TestParserTest < ActiveSupport::TestCase
+  def test_parser
+    example_test = <<~RUBY
+      require "test_helper"
+
+      class ExampleTest < ActiveSupport::TestCase
+        def test_method
+          assert true
+
+
+        end
+
+        def test_oneline; assert true; end
+
+        test "declarative" do
+          assert true
+        end
+
+        test("declarative w/parens") do
+          assert true
+
+        end
+
+        self.test "declarative explicit receiver" do
+          assert true
+        end
+
+        test("declarative oneline") { assert true }
+
+        test("declarative oneline do") do assert true end
+
+        test("declarative multiline w/ braces") {
+          assert true
+          refute false
+        }
+      end
+    RUBY
+
+    parser = Rails::TestUnit::TestParser.new(example_test, "example_test.rb")
+    expected_map = {
+      4 => 8,   # test_method
+      10 => 10, # test_oneline
+      12 => 14, # declarative
+      16 => 19, # declarative w/parens
+      21 => 23, # declarative explicit receiver
+      25 => 25, # declarative oneline
+      27 => 27, # declarative oneilne do
+      29 => 32  # declarative multiline w/braces
+    }
+    assert_equal expected_map, parser.parse
+  end
+end