ruby-lsp icon indicating copy to clipboard operation
ruby-lsp copied to clipboard

Fix minitest spec code lens selectors

Open thomasmarshall opened this issue 1 year ago • 0 comments

Motivation

Closes #2648 and #2688.

This PR improves minitest spec code lens in two ways:

  • Fixes the issue I introduced (sorry!) in #2522 whereby the selectors for minitest specs inside classes were incorrect, meaning the commands matched no tests either in the explorer or terminal.
  • Changes the base command to add spec to the load path instead of test if the current path is inside a spec directory (for example tapioca).

Note: This doesn't fix the issue with the code lens above the class in spec files not matching any tests. That has been broken since before my earlier changes and is more tricky to fix because when we generate the code lens we don't yet know if we're in a class that wraps minitest/spec examples or not.

Implementation

To fix the specs inside classes issue, I started tracking the group type (:class, :module, :describe) alongside the group name in @group_stack. It then filters out anything except the :describe groups when constructing the selector.

To fix the load path option, it checks to see whether the current path is inside a spec directory or a test directory and adds an appropriate -I argument to the base command. This changes the default behavior—if the current path is in neither a spec or test directory it no longer adds -Itest.

There are of course weird edge cases here, for example if the path is inside both a spec and test directory (like test/spec/foo_test.rb). In that case both test and spec would be added to the load path, but I don't currently see a better way of guessing which directory should be in the load path.

Automated Tests

I added a new test case to capture the original behavior for minitest spec code lens' and then updated it to match each fix. I also regenerated the expectation JSON files, which have mostly changed just to remove -Itest. I guess we could change the logic so -Itest is added by default, even if it's not in a test directory, and only switch to -Ispec if necessary. That would minimize the expectation JSON changes here.

Manual Tests

Use code lens to run specs in the tapioca codebase:

https://github.com/user-attachments/assets/ba99a00f-fc6c-49bc-a6ec-f81339845c10

Use code lens to make sure non-spec tests still work fine:

https://github.com/user-attachments/assets/5c1bd148-b9f0-40c7-a640-18ea75dcac99

thomasmarshall avatar Oct 10 '24 18:10 thomasmarshall