-
Notifications
You must be signed in to change notification settings - Fork 419
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
LSP: make use of pytest-lsp to start testing some core features #25011
Conversation
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
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.
🎉
pytest==8.2.0 | ||
pytest-lsp==0.4.1 |
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.
This makes me pause a little. Why are we now requiring all users of cls
/chplcheck
to have dependencies that are only used for development testing purposes.
In my opinion, these should be in a separate venv (like test-venv
). Maybe chapel-py-test-venv
?
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Signed-off-by: Danila Fedorin <[email protected]>
Expands the work of @DanilaFe in #25011 to add more tests of CLS, Chapels language server implementation. Infrastructure changes - Moved test-only python dependencies into their own venv - Refactored and split up the monolithic `test_server.py` - all top level python files are considered test files, helper files can be hidden in sub directories - this also allows for tests to be defined in multiple files to logically group them Added tests for the following features - Resolving nested directories - resolution tests - call inlays - type inlays - diagnostics - param inlays - document symbols - end markers - code lenses (show generic, show instantiation) Bugs fixed as a result of writing tests - goto-type-def should use `name_location` when possible - fix type inlay bug with implicit this - fix document symbols inconsistencies - fix off-by-one location error - fix end marker inlays rendering incorrectly - fix type variable locations [Reviewed by @DanilaFe]
The title about says it all. This adds
pytest-lsp
to thechplcheck-requirements.txt
file (which effectively is thechapel-py-requirements.txt
at this point), and adds a newMakefile
target to run the tests. It also locks down go-to-definition, autocompletion, and reference finding.Reviewed by @jabraham17 -- thanks!