Skip to content

Add Rails test command resolution #596

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

Merged
merged 4 commits into from
Apr 24, 2025
Merged

Add Rails test command resolution #596

merged 4 commits into from
Apr 24, 2025

Conversation

alexcrocha
Copy link
Contributor

@alexcrocha alexcrocha commented Apr 23, 2025

Closes Shopify/ruby-lsp#3179, closes Shopify/ruby-lsp#3170

This PR builds on #591’s test discovery support by generating test commands for Rails tests discovered in VS Code’s Test Explorer.

Before

Tests defined via test "…" appeared in the explorer, but clicking Run did nothing as no command was emitted.

After

Now clicking Run emits the exact bin/rails test … command Rails uses at runtime.

Added command resolution for:

  • Directories: Dir.glob on *_test.rb/test_*.rb
  • Files: individual test files
  • Groups: named groups via --name "/GroupTest(#|::)/"
  • Methods: single tests as file:line entries

For this to work, we normalize string-based test names (e.g., "should do something") by prefixing test_ and replacing spaces with underscores (test_#{test_name.gsub(/\s+/, '_')}), matching Rails' own ActiveSupport::Testing::Declarative#test helper. This ensures our test IDs line up with the methods Rails actually runs (e.g., test_should_do_something).

Rails allows writing tests with the syntax `test "should do something"`,
but internally converts these to standard Minitest method names
`test_should_do_something`.
This normalization ensures our test identifiers match what Rails uses at
runtime, allowing proper test discovery and execution.
@alexcrocha alexcrocha added the enhancement New feature or request label Apr 23, 2025
@alexcrocha alexcrocha force-pushed the ar/command-resolution branch from 465c4a5 to 9d04fcb Compare April 24, 2025 00:23
@alexcrocha alexcrocha requested review from st0012 and vinistock April 24, 2025 00:36
@alexcrocha alexcrocha marked this pull request as ready for review April 24, 2025 00:36
@alexcrocha alexcrocha requested a review from a team as a code owner April 24, 2025 00:36
))
end
elsif tags.include?("test_file")
full_files << path if children.empty?
Copy link
Member

Choose a reason for hiding this comment

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

What happens when the children is not empty?

Copy link
Member

Choose a reason for hiding this comment

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

We continue to concat the children into the queue. But if the children are empty, that indicates that we are trying to run that entire file and so we must append it to full_files.

))
end
elsif tags.include?("test_file")
full_files << path if children.empty?
Copy link
Member

Choose a reason for hiding this comment

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

We continue to concat the children into the queue. But if the children are empty, that indicates that we are trying to run that entire file and so we must append it to full_files.

@alexcrocha alexcrocha enabled auto-merge April 24, 2025 20:47
@alexcrocha alexcrocha merged commit 7c81248 into main Apr 24, 2025
29 checks passed
@alexcrocha alexcrocha deleted the ar/command-resolution branch April 24, 2025 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for the new API in the Rails add-on New test explorer implementation
3 participants