Fix SignatureHelpProviderTests #1949
Conversation
|
TBH, it looks really ugly:
I think it should not duplicate the doc for current parameter, but highlight in bold it. |
|
So what does it mean? Fixing tests or code? |
|
Before:
|
|
Looks the same. |
|
Why are "arg0" and "An object to write using format" on different lines? It should be fixed. |
|
@vasily-kirichenko @forki There's something in Roslyn method displays we can't control here - I'm sure we can get them to change it eventually |
|
@dsyme what I don't like here is that "arg0" and its XML Doc are on different lines, that's all. |
|
ok seems there are more to come. but since I can't compile this it will take some time until I have the tests fixed |
|
ok changed test a bit to actually report all issues. |
forki
force-pushed
the
fixbuild
branch
2 times, most recently
from
cda7c51 to
a73ad3d
Compare
|
this should become green now and therefore fix the build on master. @dsyme / @KevinRansom / @cartermp please take a look and merge if you think it's ok |
|
@forki Thanks. Not sure why this went wrong, but I imagine the document created temporarily by the tests on the server is different than that on my machine when I updated these tests. That would explain the discrepancy in I think that first item in the tuple can just be outright ignored. I don't think that verifying that the correct @dsyme @vasily-kirichenko I'll create an issue about the XML doc being on the newline |
|
@cartermp Right now I think I'm just totally focusing on recovering parity with VS2015 rather than working out what's actually right/wrong :) I think one key post RTM is to implement other relevant navigation aids (go to generated signature, F1 help etc), and cleanup quick info too. |
|
@dsyme I figured it out; it's entirely on our end and has nothing to do with Roslyn. |
|
Tbh most important right now is green build ;-)
Am 06.12.2016 6:25 nachm. schrieb "Phillip Carter" <notifications@github.com
…:
@dsyme <https://github.com/dsyme> I figured it out; it's entirely on our
end and has nothing to do with Roslyn.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1949 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADgNG01TnAN9idTEoWKB5UqLaTNZG4Jks5rFZqRgaJpZM4LFSJp>
.
|
|
@forki Agreed, this is tangential and not related to this PR, minus the first part of this comment, since I think it'll lead to a less flaky test suite if |
|
AppVeyor failures don't seem to be an issue. @KevinRansom can you verify and pull this if it's all good? This fixes a test in CI that's been going red for unrelated PRs. |
|
Please please merge. |
Current master is broken since @cartermp's commit 50e7a82
This is an attempt to fix it, but I'm not sure if the test is broken or the code itself