Add test and fix signature off-by-one #782
Conversation
|
This is a wonderful surprise :) Thank you for taking the time to investigate and write some tests! Yes, the Great Offset Shift is having reprecussions to this day, not the least of which was moving all text reading over to a less-error-prone model! (see ISourceText, etc). You're likely correct about these follow-on places that use Regarding placement/naming of the tests, the name |
|
Regarding the test results here, the only one you really need to worry about is the macos one. There are some core issues we have to address on the Windows tests (a combination of case-sensitive path comparisons as well as some serious flakiness on the windows runners in GitHub Actions), as well as the ubuntu tests (unknown/undiagnosed hangs). |
Use a separate testcase file for this endpoint. Use `FilePathToUri` instead of manually creating the uri. Test all positions of the identifier in a single `testCaseAsync`.
|
Thanks, this is a great change 👍. There are a couple other fixes I want to try and land this weekend, then I'll do a release including this change. |
The
fsharp/signatureextension also seems to be a victim of the 'Great Offset Shift of 2021' (see #767, #774, #771).The tests are basically the first tooltip tests from CoreTests.fs. Not sure what is preferred, to have them there because they share logic (e.g.
server), or if they should be in ExtensionTests because they test a custom endpoint. Currently in the latter, should it have a separate TestCases file instead of reusing the TestCases/Tooltips.fs file?(a quick search shows a few more places where
Lexer.findLongIdents(pos.Column - 1, lineStr)is used, likely they are affected as well).