Wait for design mode before closing a solution

[sharwell]

This change improves integration test reliability by ensuring the debugger is terminated prior to attempting to close a solution.

📝 This pull request builds on #32122. Only the last commit is unique to this pull request.

[jinujoseph]

@sharwell is this good to merge to master ?

[sharwell]

@jinujoseph Yes as soon as it's reviewed.

[jinujoseph]

@ivanbasov take you review this pls

[jasonmalinowski]

This looks like a product bug to me: the debugging session should be updated synchronously, right @tmat?

[sharwell]

@jasonmalinowski It may be a product bug, but it still helped with integration test stability in certain edge cases from the past.

[sharwell]

Ah, I think I remember: if a test related to debugging failed, it could end the test before the debugger terminated. This would lead to a failure to close the solution followed by a cascading failure that impacted all remaining tests, either by causing them to fail or by causing them to just time out.

[jasonmalinowski]

@sharwell If that's the case, then you need the TerminateAll line but not any of the other waiting code, correct?

[sharwell]

@jasonmalinowski No, the handling is asynchronous and I'm not aware of a direct waiter for the sequence.

[jasonmalinowski]

@sharwell Why are we waiting then? Either we need a waiter or it shouldn't be necessary. What breaks?

[sharwell]

@jasonmalinowski I don't remember what specifically broke. I found it during test hardening work in #28173.

[jasonmalinowski]

@sharwell It seems to me then we should either remove the waiting (and leave the "TerminateAll") or debug further. Otherwise this really looks like integration tests are finding an actual bug here, and working around that isn't the way to go (at least not without understanding it!)

[sharwell]

@jasonmalinowski I found the issue. When attempting to fix CSharpImmediate.DumpLocalVariableValue (#25814), the test hangs because the debugger is left running when an assertion fails in the middle of a test and then the test cleanup operations attempt to close the solution with the debugger running.

[sharwell]

@jasonmalinowski @tmat I moved this back to dev16.0. I cannot proceed with 16.0 integration testing without this change (it blocks an upcoming PR).

Dismissing my block, since "we're doing this to cleanup the potential broken state once the test failed" makes good sense.

[sharwell]

📝 The debug test failure was caused by this temporary state issue:

[sharwell]

Release build passed 👍