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

Only handle requests in _{spec,test}.rb files #6

Merged
merged 4 commits into from
Oct 29, 2023

Conversation

rohitpaulk
Copy link
Contributor

@rohitpaulk rohitpaulk commented Oct 28, 2023

Ran into this issue today where I saw rspec markers in a non-spec file:

Screenshot 2023-10-28 at 21 43 51

This PR fixes that behaviour by restricting the codelens listener to only listen on files that are named _test.rb or _spec.rb.

Potential Issues

Rspec's default file pattern is _spec.rb, but that can be configured.

I vaguely remember coming across codebases that use _test.rb for specs, hence the added pattern. This can backfire if people use both rspec & minitest. Minitest has a spec syntax too, so ruby-lsp-rspec will end up creating incorrect markers on minitest files. Won't affect minitest-only users since I assume they won't have this addon installed.

I suspect the only accurate way to fix these issues would be to get the exact value of rspec's file pattern - either through static analysis, or by having a configuration option passed in. Doubt that either of these options are easy to do, but open to suggestions!

Other approaches

My first instinct was to improve how context is detected, by ignoring usages like context.foo. Thought about this a bit more, and dropped it because even a non-test file could have usages like context "foo" {} if it was using a non-rspec DSL.

@rohitpaulk rohitpaulk marked this pull request as ready for review October 28, 2023 21:43
@rohitpaulk rohitpaulk force-pushed the restrict-file-paths branch 2 times, most recently from 3ce7c08 to 0a56b49 Compare October 28, 2023 22:11
Copy link
Owner

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@st0012 st0012 added the bug Something isn't working label Oct 29, 2023
lib/ruby_lsp/ruby_lsp_rspec/addon.rb Outdated Show resolved Hide resolved
lib/ruby_lsp/ruby_lsp_rspec/code_lens.rb Outdated Show resolved Hide resolved
@st0012 st0012 merged commit ade0a14 into st0012:main Oct 29, 2023
3 checks passed
@rohitpaulk
Copy link
Contributor Author

Thanks for cleaning up the lint errors!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants