Skip to content

Workaround bug in LSP client in 16.10p2 in integration tests#53127

Merged
dibarbet merged 1 commit intorelease/dev16.10from
dev/dibarbet/integration_1610p2
May 4, 2021
Merged

Workaround bug in LSP client in 16.10p2 in integration tests#53127
dibarbet merged 1 commit intorelease/dev16.10from
dev/dibarbet/integration_1610p2

Conversation

@dibarbet
Copy link
Member

@dibarbet dibarbet commented May 4, 2021

The LSP client has a bug in 16.10p2 where server activation tasks that are not completed will never get completed if the solution is closed. Then when a solution is re-opened, these activation tasks are not cleared, so features that wait for LSP to initialize just hang on these never completing tasks.

The fix comes in two parts:

  1. Disable intellicode's LSP server. We don't control this one, so we just disable it to prevent it from creating one of these never completing tasks.
  2. For our roslyn C# server (for cntrl+Q), we now wait for it to completely initialize before we proceed with the test. Therefore we can never close the solution while initialization is in progress.

This is fixed on the LSP client side in 16.10p3, but this should prevent us from breaking when the queue switches over to 16.10p2
https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1322125

TODO:
Target 16.10
Make sure tests pass on 16.10p2 queue (a few times)

  1. https://dev.azure.com/dnceng/public/_build/results?buildId=1119847&view=results
  2. https://dev.azure.com/dnceng/public/_build/results?buildId=1119945&view=results
  3. https://dev.azure.com/dnceng/public/_build/results?buildId=1120134&view=results

@ghost ghost added the Area-IDE label May 4, 2021
@dibarbet dibarbet changed the base branch from main to release/dev16.10 May 4, 2021 02:20
@dibarbet dibarbet changed the base branch from release/dev16.10 to main May 4, 2021 02:21
that do not complete before solution close are never completed or
cleared from the task list.  This manifests as hangs when a new solution
is opened in features that wait for all LSP server activation tasks to
complete.
@dibarbet dibarbet force-pushed the dev/dibarbet/integration_1610p2 branch from 58b3da0 to 56d69fa Compare May 4, 2021 05:59
@dibarbet dibarbet changed the base branch from main to release/dev16.10 May 4, 2021 05:59
@dibarbet dibarbet marked this pull request as ready for review May 4, 2021 05:59
@dibarbet dibarbet requested a review from a team as a code owner May 4, 2021 05:59
@dibarbet dibarbet requested review from JoeRobich and sharwell May 4, 2021 05:59
@dibarbet dibarbet changed the title Workaround bug in LSP client in 16.10p2 where server activation tasks Workaround bug in LSP client in 16.10p2 in integration tests May 4, 2021
@dibarbet
Copy link
Member Author

dibarbet commented May 4, 2021

@jinujoseph or @vatsalyaagrawal - I want to get this test-only integration test fix into the 16.10 branch so that our 16.10 / 16.11 qb changes are able to run integration tests when the queue upgrades. This doesn't need to be inserted though since it's a test only change. Is it good to go in?

@dibarbet dibarbet merged commit 6d41b2f into release/dev16.10 May 4, 2021
@dibarbet dibarbet deleted the dev/dibarbet/integration_1610p2 branch May 4, 2021 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants