Add request-level tests for LspConnection #47
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@ ## master #47 +/- ## ========================================= + Coverage 55.21% 64.52% +9.3% ========================================= Files 233 234 +1 Lines 3142 3309 +167 ========================================= + Hits 1735 2135 +400 + Misses 1407 1174 -233
Continue to review full report at Codecov.
|
Collaborator Author
| @david-driscoll - would you mind having a look at |
Collaborator Author
| BTW, I can't see the AppVeyor build so I'll just have to assume it's working :) |
Collaborator Author
| Failing, I meant failing 🤣 |
Member
| I'll add you to the appveyor team |
| /// Ensure that the language client can successfully request Hover information. | ||
| /// </summary> | ||
| [Fact(DisplayName = "Language client can successfully request hover info")] | ||
| public async Task Hover_Success() |
Member
There was a problem hiding this comment.
This should makes things so much easier to integration test... and we can finally make sure LanguageServer is working correctly.
Member
| And here's the error fyi: |
Collaborator Author
| Oops! Guess that wasn't so cross-platform after all (was on a mac when I tested it, so I guess the test will need to use a different path depending on OS). Will sort that out shortly :) |
Collaborator Author
| BTW, have you seen Demystifier? Makes those stack traces a million times easier to read... |
Collaborator Author
| @david-driscoll Here's a Demystifier example: https://github.com/DimensionDataResearch/daas-demo/blob/master/src/DaaSDemo.Provisioning.Host/Program.cs#L36 It's only a test-level dependency, but I thought it'd be worth discussing before trying to add it (it's been pretty high-value in my own projects so far). |
Member
| I'd seen some stuff on twitter about it. I'm fine if you want to bring it in and make things look better. |
Collaborator Author
| Cool - I'll look into it once the travis build starts working again (just want to be sure I've knocked the xplat paths issue on the head first). |
Collaborator Author
| Hmm - looks like Will re-test on my mac. Yep, works on Windows and non-Windows. |
tintoy changed the title Collaborator Author
| Ok - if the builds pass then this should be ready for review :) |
david-driscoll approved these changes Nov 16, 2017
| [Fact(DisplayName = "Client connection can handle request from server")] | ||
| public async Task Server_HandleRequest_Success() | ||
| { | ||
| LspConnection clientConnection = await CreateClientConnection(); |
Member
There was a problem hiding this comment.
Minor nitpick, if we make CreateClientConnection return a disposable we can wrap all these calls in a using and remove some boilerplate with disconnect.
However that can be changed later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Hey - I'm starting to add more tests for the LSP client.