add proper additional document update change handler in incremental s… #16628
Conversation
heejaechang
added
PR For Personal Review Only
heejaechang
force-pushed
the
additionalDocumentFix
branch
from
0cdc577
to
11662d0
Compare
…olution updater added unit test
heejaechang
force-pushed
the
additionalDocumentFix
branch
from
11662d0
to
ef6a246
Compare
heejaechang
removed
the
PR For Personal Review Only
|
@srivatsn @mavasani @panopticoncentral @dotnet/roslyn-ide can you take a look? |
|
|
||
| var solution = workspace.CurrentSolution; | ||
| var service = await GetSolutionServiceAsync(solution, map); | ||
| private static async Task VerifySolutionUpdate(TestWorkspace workspace, Func<Solution, Solution> newSolutionGetter) |
There was a problem hiding this comment.
this is just extract method. there is actually no change here. not sure why github think there is change here.
There was a problem hiding this comment.
I guess some whitespace changes. Diffing with ?w=1 in the URL looks clean.
|
@aakash-johari can you let us know whether the fix fixed your issue. |
|
@heejaechang The fix works for us. Thanks! |
|
moved bug back to master. |
|
ping? can I get code review? |
|
@MattGertz can I get approval for RTW for monday shiproom? |
|
@jasonmalinowski ya, looks like when I changed OOP to be incrementally updated, I forgot to add additional file change tests and missed issues there. except OOP, I believe additional files are covered by tests pretty well. |
|
@heejaechang Clearly we had a test hole, so are we trying other exploratory testing here to make sure we got everything for realz? |
|
@jasonmalinowski yes, we had testing hole with OOP. can't say covered 100% cases. I made OOP to crash right way if there is any inconsistency. so if there is more issue, we will know :) but so far, all cases reported, when tested no longer reproed. |
|
@heejaechang please redirect this to RC3 |
…olution updater
unit test added
Customer scenario
Customer add or modify additional documents to a C#/VB project then VS crashes.
Bugs this fixes:
https://devdiv.visualstudio.com/web/wi.aspx?pcguid=011b8bdf-6d56-4f87-be0d-0092136884d9&id=370095
Workarounds, if any
add/modify additional file manually and then load solution. or turn off out of proc through internal registry key.
Risk
areas that have changed are covered by unit tests so I would say risk is low.
Performance impact
code change is not related to any area where performance is concerned.
Is this a regression from a previous update?
Yes.
Root cause analysis:
unit test missed this area. only covered initial additional file, not modification of the file once solution is loaded. the regression happened while improving performance of updating solution in Roslyn out of proc
How was the bug found?
Testing and Dogfooding.