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.
|
|
@david-driscoll - would you mind having a look at |
|
BTW, I can't see the AppVeyor build so I'll just have to assume it's working :) |
|
Failing, I meant failing 🤣 |
|
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() |
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.
|
And here's the error fyi: |
|
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 :) |
|
BTW, have you seen Demystifier? Makes those stack traces a million times easier to read... |
|
@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). |
|
I'd seen some stuff on twitter about it. I'm fine if you want to bring it in and make things look better. |
|
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). |
|
Hmm - looks like Will re-test on my mac. Yep, works on Windows and non-Windows. |
tintoy
changed the title
|
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(); |
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
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.