-
Notifications
You must be signed in to change notification settings - Fork 179
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
Start discovering all available tests lazily #3120
Start discovering all available tests lazily #3120
Conversation
How to use the Graphite Merge QueueAdd the label graphite-merge to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
bb3d40b
to
ed5685c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to expand test/controllers/foo_controller_test.rb
in the test explorer when I opened foo_controller_test.rb
in the editor? Similar to how VS Code would expand the file explorer based on the currently opened file.
Not a blocker for the PR but it'd be a very useful feature IMO.
ed5685c
to
b0f99fc
Compare
b0f99fc
to
a63d395
Compare
Yes! Added that. Note that you need to have clicked on the explorer first, to lazily populate the tests. We don't force the explorer to reveal, but if you're already there, we focus on the current test. |
c2ba939
to
baabe7d
Compare
a63d395
to
077b2cc
Compare
077b2cc
to
a523375
Compare
baabe7d
to
76294b3
Compare
ec861f4
to
c76d5d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested it in Core, IRB, RDoc, and ruby-lsp-rspec. All worked as expected 👍
One non-blocking suggestion, I think the experience would be better if it can automatically expand results to the first level having multiple results.
For example, in IRB I needed to expand test
and then irb
even though both of them are the only item on their level.
![Screenshot 2025-02-07 at 21 32 50](https://private-user-images.githubusercontent.com/5079556/411091254-1def7122-727c-4515-9a9f-587321c7e2a3.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzkwNTY1ODAsIm5iZiI6MTczOTA1NjI4MCwicGF0aCI6Ii81MDc5NTU2LzQxMTA5MTI1NC0xZGVmNzEyMi03MjdjLTQ1MTUtOWE5Zi01ODczMjFjN2UyYTMucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMjMxMTIwWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9Mjg5NmRjMDg0MjNmMTFmNGE2OWZiZjc3MzRhMjUzMzE0MTU4ODAzNDY0NTJmOGRiMTAzZWVlYTkzNWFlMTk0NSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.hZYPwKxhm2A0A1Sj4l9C-B1NpXjw4mMp0qLbd5zIQMo)
Merge activity
|
### Motivation This is the first step in addressing several test explorer related issues that we currently have open. This PR implements lazy test discovery up until the file level. ### Implementation The VS Code API for the explorer supports lazily discovering tests by implementing the `resolveHandler`. I essentially only implement two resolutions: 1. If there's only one workspace folder, we eager discover all test files in the workspace 2. If there are many workspace folders, then the first level in the hierarchy are the workspaces themselves. If you open any of them, then it triggers resolution and we discover all available tests in that workspace The tests are discovered using the VS Code `findFiles` API, which is very efficient and will avoid having us implement an indexing-like mechanism in the server. Since the test explorer is VS Code only anyway, this should be fine. I also considered two levels of hierarchy for tests (in addition to the workspace when there are many). The firsts level is the `test/spec/features` directory. Since there may be many of them in an application split by components, we need to have that level of grouping. The second level is the immediate subdirectory of the `test/spec/features` directory, which in a Rails app normally accounts for `models`, `controllers` and so on. This hierarchy is useful because it will allow users to take actions like running all tests in a workspace, test directory or all tests of a kind (e.g.: models). ### Notes This PR doesn't yet implement the resolution of the test files themselves. The approach we will take for that is asking the server for which tests are available in a file, which gives us the opportunity to include add-on listeners in the discovery. Since the implementation is still partial, I put a new under development feature flag. ### Automated Tests Added tests. ### Manual tests Enable this under development flag ```json { "rubyLsp.featureFlags": { "newTestExperience": true } } ``` Then start the extension on this branch and navigate tests https://github.com/user-attachments/assets/1aaf145a-7b8f-48bf-b30b-61f0eef3e039
c76d5d4
to
37eb419
Compare
Motivation
This is the first step in addressing several test explorer related issues that we currently have open.
This PR implements lazy test discovery up until the file level.
Implementation
The VS Code API for the explorer supports lazily discovering tests by implementing the
resolveHandler
. I essentially only implement two resolutions:The tests are discovered using the VS Code
findFiles
API, which is very efficient and will avoid having us implement an indexing-like mechanism in the server. Since the test explorer is VS Code only anyway, this should be fine.I also considered two levels of hierarchy for tests (in addition to the workspace when there are many). The firsts level is the
test/spec/features
directory. Since there may be many of them in an application split by components, we need to have that level of grouping.The second level is the immediate subdirectory of the
test/spec/features
directory, which in a Rails app normally accounts formodels
,controllers
and so on. This hierarchy is useful because it will allow users to take actions like running all tests in a workspace, test directory or all tests of a kind (e.g.: models).Notes
This PR doesn't yet implement the resolution of the test files themselves. The approach we will take for that is asking the server for which tests are available in a file, which gives us the opportunity to include add-on listeners in the discovery.
Since the implementation is still partial, I put a new under development feature flag.
Automated Tests
Added tests.
Manual tests
Enable this under development flag
Then start the extension on this branch and navigate tests
demo.mov