Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't generate code lens for nested definitions #2067

Merged
merged 1 commit into from
May 17, 2024

Conversation

thomasmarshall
Copy link
Contributor

@thomasmarshall thomasmarshall commented May 16, 2024

Motivation

When testing a test framework or plugin, it's common to have test methods inside test methods. This change ensures that the code lens is only generated for tests that will actually be run by the test runner, not for nested definitions.

Before After

Implementation

It tracks the depth of nested method definition nodes and returns early if the depth is greater than one level.

Automated Tests

I've added a test that ensures the code lens is generated for the outer test method but not the inner one.

Manual Tests

  1. Write a test with a another test method inside

    def test_foo
      def test_bar
      end
    end
  2. Confirm that code lens appears for test_foo but not test_bar

@thomasmarshall thomasmarshall requested a review from a team as a code owner May 16, 2024 20:21
@thomasmarshall thomasmarshall requested review from andyw8 and st0012 May 16, 2024 20:21
When testing a test framework or plugin, it's common to have test
methods inside test methods [1]. This change ensures that the code lens
is only generated for tests that will actually be run by the test
runner, not for nested definitions.

[1]: https://github.com/minitest/minitest/blob/f60c6f2e29082b404fb6933abb6a1813b2c09c5d/test/minitest/test_minitest_test.rb#L92-L94
@thomasmarshall thomasmarshall force-pushed the code-lens-nested-defs branch from 6016875 to 757e6ca Compare May 17, 2024 14:05
@andyw8 andyw8 added server This pull request should be included in the server gem's release notes bugfix This PR will fix an existing bug labels May 17, 2024
@andyw8 andyw8 enabled auto-merge (squash) May 17, 2024 14:28
@andyw8 andyw8 merged commit 49fa2d7 into Shopify:main May 17, 2024
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants