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

Add testing suite for LSP server #351

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

marvinborner
Copy link
Member

@marvinborner marvinborner commented Dec 18, 2023

This PR aims to add a test suite for Effekt's LSP server.

Apart from the obvious reasons, LSP testing could also be useful for finding reflection bugs (PR #329).

The implementation relies on ScalaJS and Microsoft's vscode-languageserver-protocol node module. The resulting facade bindings could also be useful if we wanted to replace LSP4J with a ScalaJS implementation (see here for more information on writing facade types).

For now, this PR just consists of a basic proof-of-concept bindings/connection/initialization (I had a lot of trouble with ScalaJS and facade types).

@jiribenes
Copy link
Contributor

jiribenes commented Dec 18, 2023

Hey, looks great so far! :)

Just wondering, have you tried ScalablyTyped?
It could save you lots of time spent on writing wrappers (it uses some advanced magic to get the types via TypeScript).

(I have great experience with it in my ScalaJS projects :))

@marvinborner
Copy link
Member Author

marvinborner commented Dec 18, 2023

Ah yes, I wanted to mention this in the PR. I actually tried it (for a slightly different vscode package though) and it didn't work properly. It gave a lot of warnings about unresolvable/etc. types and the generated Scala files missed several relevant classes. Maybe I'll try again, it would really save a lot of time.

I'd suggest we first evaluate how much time this takes in practical use,
then reconsider running the tests on every push.
This introduces several race condition fixes and improves check
validation (JSON ordering is not determistic, for example)
This is a compromise between test speed and feature usage. I tried to
write some specific tests but found myself rewriting already existing
test code.
once again
Some requests send all diagnostics again and caused some logs (e.g. the
exitClient request)
@marvinborner
Copy link
Member Author

marvinborner commented Feb 7, 2024

Can anyone reproduce the CI failure? It seems to think that hashTriple and main end at a different line/character and I can't find the reason (there also appears to be some general unreproducible symbol flakiness, probably related to #366).

Aside from that, this PR should be mergable soon, so I'm looking for some feedback.

@marvinborner marvinborner marked this pull request as ready for review February 7, 2024 15:39
marvinborner added a commit to marvinborner/effekt that referenced this pull request Feb 8, 2024
@jiribenes jiribenes force-pushed the master branch 3 times, most recently from ee9d209 to 58c8510 Compare October 1, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants