Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Build test items in hierarchy if it's supported #914

Merged
merged 1 commit into from
Nov 27, 2023
Merged

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Nov 24, 2023

Motivation

Currently, tests in Test Explorer are assumed to only have 2 levels, which is incorrect, especially with RSpec tests.

With a 2 level nesting test file:

class ParentTest < Minitest::Test
  def test_foo; end

  class FirstChildTest < Minitest::Test
    def test_bar; end
  end

  class SecondChildTest < Minitest::Test
    def test_baz; end
  end

  def test_foobarbaz; end
end

The current implementation doesn't display the nesting nor group tests correctly:

Screenshot 2023-11-24 at 20 40 44

But with Shopify/ruby-lsp#1214 and this PR, we can deliever the correct result:

Screenshot 2023-11-24 at 20 42 40

Closes Shopify/ruby-lsp#1744

Implementation

  • If code lens items has group_id data attribute, we can build items hierarchy with them.
  • Otherwise, fallback to the current approach.

Automated Tests

Manual Tests

@st0012 st0012 added the enhancement New feature or request label Nov 24, 2023
@st0012 st0012 self-assigned this Nov 24, 2023
@st0012 st0012 requested a review from a team as a code owner November 24, 2023 20:43
@st0012 st0012 requested review from andyw8 and KaanOzkan November 24, 2023 20:43
this.testController.items.add(testItem);
// Add test methods as children to the test class so it appears nested in Test explorer
// and running the test class will run all of the test methods
// eslint-disable-next-line no-lonely-if
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel blending the 2 versions of implementation together makes things harder to understand and also harder to remove the fallback. So I want to separate them with simple if/else condition and simply remove the fallback when we're ready to do so.

@st0012 st0012 requested a review from vinistock November 24, 2023 20:48
@vinistock
Copy link
Member

Very clean, the implementation looks good to me. Do you have a project that we can manually test this on? Does IRB use nested test declarations?

@st0012
Copy link
Member Author

st0012 commented Nov 27, 2023

The only minitest/test-unit file that I have to test against is this.

Before (incorrect nesting)

Screenshot 2023-11-27 at 17 36 28

After

Screenshot 2023-11-27 at 17 53 46

@st0012 st0012 merged commit f52c8ba into main Nov 27, 2023
4 checks passed
@st0012 st0012 deleted the implement-#792 branch November 27, 2023 18:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support nested code lens groups
2 participants