Apply correct hierarchy and IDs to Minitest spec items (#3501)

* Add `spec` to load path when running Minitest specs

* Apply correct hierarchy and IDs to Minitest spec items

Author: vinistock

lib/ruby_lsp/listeners/spec_style.rb

@@ -4,12 +4,24 @@
 module RubyLsp
   module Listeners
     class SpecStyle < TestDiscovery
+      class Group
+        #: String
+        attr_reader :id
+
+        #: (String) -> void
+        def initialize(id)
+          @id = id
+        end
+      end
+
+      class ClassGroup < Group; end
+      class DescribeGroup < Group; end
+
       #: (ResponseBuilders::TestCollection, GlobalState, Prism::Dispatcher, URI::Generic) -> void
       def initialize(response_builder, global_state, dispatcher, uri)
         super
 
-        @describe_block_nesting = [] #: Array[String]
-        @spec_class_stack = [] #: Array[bool]
+        @spec_group_id_stack = [] #: Array[Group?]
 
         dispatcher.register(
           self,
@@ -22,21 +34,21 @@ def initialize(response_builder, global_state, dispatcher, uri)
 
       #: (Prism::ClassNode) -> void
       def on_class_node_enter(node)
-        with_test_ancestor_tracking(node) do |_, ancestors|
-          is_spec = ancestors.include?("Minitest::Spec")
-          @spec_class_stack.push(is_spec)
+        with_test_ancestor_tracking(node) do |name, ancestors|
+          @spec_group_id_stack << (ancestors.include?("Minitest::Spec") ? ClassGroup.new(name) : nil)
         end
       end
 
       #: (Prism::ClassNode) -> void
       def on_class_node_leave(node) # rubocop:disable RubyLsp/UseRegisterWithHandlerMethod
         super
-
-        @spec_class_stack.pop
+        @spec_group_id_stack.pop
       end
 
       #: (Prism::CallNode) -> void
       def on_call_node_enter(node)
+        return unless in_spec_context?
+
         case node.name
         when :describe
           handle_describe(node)
@@ -49,85 +61,63 @@ def on_call_node_enter(node)
       def on_call_node_leave(node)
         return unless node.name == :describe && !node.receiver
 
-        @describe_block_nesting.pop
+        @spec_group_id_stack.pop
       end
 
       private
 
       #: (Prism::CallNode) -> void
       def handle_describe(node)
+        # Describes will include the nesting of all classes and all outer describes as part of its ID, unlike classes
+        # that ignore describes
         return if node.block.nil?
-        return unless in_spec_context?
 
         description = extract_description(node)
         return unless description
 
-        if @nesting.empty? && @describe_block_nesting.empty?
-          test_item = Requests::Support::TestItem.new(
-            description,
-            description,
-            @uri,
-            range_from_node(node),
-            framework: :minitest,
-          )
-          @response_builder.add(test_item)
-          @response_builder.add_code_lens(test_item)
+        parent = latest_group
+        id = case parent
+        when Requests::Support::TestItem
+          "#{parent.id}::#{description}"
         else
-          add_to_parent_test_group(description, node)
+          description
         end
 
-        @describe_block_nesting << description
+        test_item = Requests::Support::TestItem.new(
+          id,
+          description,
+          @uri,
+          range_from_node(node),
+          framework: :minitest,
+        )
+
+        parent.add(test_item)
+        @response_builder.add_code_lens(test_item)
+        @spec_group_id_stack << DescribeGroup.new(id)
       end
 
       #: (Prism::CallNode) -> void
       def handle_example(node)
-        return unless in_spec_context?
-        return if @describe_block_nesting.empty? && @nesting.empty?
-
         # Minitest formats the descriptions into test method names by using the count of examples with the description
         # We are not guaranteed to discover examples in the exact order using static analysis, so we use the line number
         # instead. Note that anonymous examples mixed with meta-programming will not be handled correctly
         description = extract_description(node) || "anonymous"
-        line = node.location.start_line
+        line = node.location.start_line - 1
+        parent = latest_group
+        return unless parent.is_a?(Requests::Support::TestItem)
 
-        add_to_parent_test_group(format("test_%04d_%s", line, description), node)
-      end
-
-      #: (String, Prism::CallNode) -> void
-      def add_to_parent_test_group(description, node)
-        parent_test_group = find_parent_test_group
-        return unless parent_test_group
+        id = "#{parent.id}##{format("test_%04d_%s", line, description)}"
 
         test_item = Requests::Support::TestItem.new(
-          description,
+          id,
           description,
           @uri,
           range_from_node(node),
           framework: :minitest,
         )
-        parent_test_group.add(test_item)
-        @response_builder.add_code_lens(test_item)
-      end
-
-      #: -> Requests::Support::TestItem?
-      def find_parent_test_group
-        root_group_name, nested_describe_groups = if @nesting.empty?
-          [@describe_block_nesting.first, @describe_block_nesting[1..]]
-        else
-          [RubyIndexer::Index.actual_nesting(@nesting, nil).join("::"), @describe_block_nesting]
-        end
-        return unless root_group_name
-
-        test_group = @response_builder[root_group_name] #: Requests::Support::TestItem?
-        return unless test_group
 
-        return test_group unless nested_describe_groups
-
-        nested_describe_groups.each do |description|
-          test_group = test_group[description]
-        end
-
-        test_group
+        parent.add(test_item)
+        @response_builder.add_code_lens(test_item)
       end
 
       #: (Prism::CallNode) -> String?
@@ -145,11 +135,36 @@ def extract_description(node)
         end
       end
 
+      #: -> (Requests::Support::TestItem | ResponseBuilders::TestCollection)
+      def latest_group
+        return @response_builder if @spec_group_id_stack.compact.empty?
+
+        first_class_index = @spec_group_id_stack.rindex { |i| i.is_a?(ClassGroup) } || 0
+        first_class = @spec_group_id_stack[0] #: as !nil
+        item = @response_builder[first_class.id] #: as !nil
+
+        # Descend into child items from the beginning all the way to the latest class group, ignoring describes
+        @spec_group_id_stack[1..first_class_index] #: as !nil
+          .each do |group|
+          next unless group.is_a?(ClassGroup)
+
+          item = item[group.id] #: as !nil
+        end
+
+        # From the class forward, we must take describes into account
+        @spec_group_id_stack[first_class_index + 1..] #: as !nil
+          .each do |group|
+          next unless group
+
+          item = item[group.id] #: as !nil
+        end
+
+        item
+      end
+
       #: -> bool
       def in_spec_context?
-        return true if @nesting.empty?
-
-        @spec_class_stack.last #: as !nil
+        @nesting.empty? || @spec_group_id_stack.any? { |id| id }
       end
     end
   end

test/fixtures/minitest_spec_example.rb

@@ -3,17 +3,31 @@
 require "minitest/spec"
 require "minitest/autorun"
 
-Minitest::Test.i_suck_and_my_tests_are_order_dependent!
+Minitest::Spec.i_suck_and_my_tests_are_order_dependent!
 
-class MySpec < Minitest::Spec
-  describe "some scenario" do
-    it "works as expected!" do
-      assert_equal(1, 1)
-    end
+module First
+  module Second
+    module Third
+      class MySpec < Minitest::Spec
+        # Anonymous example
+        it do
+          assert_equal(1, 1)
+        end
+
+        describe "when something is true" do
+          describe "and other thing is false" do
+            it "does what's expected" do
+              assert(true)
+            end
+          end
 
-    # Anonymous example
-    it do
-      assert_equal(2, 2)
+          class NestedSpec < Minitest::Spec
+            it "does something else" do
+              assert(true)
+            end
+          end
+        end
+      end
     end
   end
 end