-
Notifications
You must be signed in to change notification settings - Fork 420
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
Fix generate file #1143
Fix generate file #1143
Conversation
rchande
commented
Apr 5, 2018
- Use the modified Omnisharp solution, because the apply change operation solution does not have filepaths for new files
- Use the documentId requested by the apply change operation
* Use the modified Omnisharp solution, because the apply change operation solution does not have filepaths for new files * Use the documentId requested by the apply change operation
The new test is failing. |
@@ -0,0 +1,21 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll probably want to add this to build.json so it gets restored and built before the test runs: https://github.com/OmniSharp/omnisharp-roslyn/blob/master/build.json#L40.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do. Thanks!
var changes = response.Changes.ToArray(); | ||
Assert.Equal(2, changes.Length); | ||
Assert.NotNull(changes[0].FileName); | ||
Assert.NotNull(changes[1].FileName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you also assert that the file was generated on disk?
FYI: I've removed cli-deps in my MSBuild PR, which is what is breaking the build here. I have one more fix that I'm testing to ensure Unity projects keep working as expected. Once I'm done, I'll push that change and merge my PR after CI completes. So, if I go first, you can update your PR with my changes and CI will work for you. |
@DustinCampbell Sounds good! |
This is more fallout from the MyGet issues that happened yesterday. 😞 |