Skip to content
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

Re-enable Code Folding tests #6592

Closed
NTaylorMullen opened this issue Jul 14, 2022 · 1 comment
Closed

Re-enable Code Folding tests #6592

NTaylorMullen opened this issue Jul 14, 2022 · 1 comment

Comments

@NTaylorMullen
Copy link
Contributor

In a recent integration test failure formatting tests were falling over because folding ranges weren't available for the document on file open. Basically things ended up timing out because of it.

Technically all features should eventually become available and our "timeout" token represents a long enough period of time where this shouldn't be an issue. Therefore, this makes me believe there may be reliability issues in our code folding language service capabilities.

We should re-enable our CodeFoldingTest's to try and track down these potential reliability issues.

@NTaylorMullen NTaylorMullen added this to the 17.4-Preview1 CC:~7/25 milestone Jul 14, 2022
NTaylorMullen added a commit that referenced this issue Jul 14, 2022
- Originally found in this Integration test CI failure: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=6414782&view=results
- This ended up revealing a few issues:
    1. Prior to this changeset formatting tests would wait for folding ranges to be available before attempting to format anything. In the above CI failure those checks timed out and the screenshots included also showed that no folding ranges were available. This indicates there might be a secondary issue where folding ranges ar ea little flakey. I've filed [this](#6592) to track re-enabling code folding tests and moved our existing formatting tests off of folding ranges being available to waiting for semantic classifications
    2. With the introduction of [this](#6531) change we started accurately formatting content after directive bodies. Updated our baselines for formatting tests to capture this new requirement.
- As part of this work I also found that the existing `WaitForClassificationAsync` was a little misleading because it defaulted to waiting for Razor component classifications. I've gone ahead and renamed the existing method to `WaitForComponentClassificationAsync` to be extra clear what's being waited for. As an extra reaction I added a general `WaitForSemanticClassificationAsync` that's now used in Razor formatting tests so we can wait for semantic Razor transitions.
NTaylorMullen added a commit that referenced this issue Jul 14, 2022
- Originally found in this Integration test CI failure: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=6414782&view=results
- This ended up revealing a few issues:
    1. Prior to this changeset formatting tests would wait for folding ranges to be available before attempting to format anything. In the above CI failure those checks timed out and the screenshots included also showed that no folding ranges were available. This indicates there might be a secondary issue where folding ranges ar ea little flakey. I've filed [this](#6592) to track re-enabling code folding tests and moved our existing formatting tests off of folding ranges being available to waiting for semantic classifications
    2. With the introduction of [this](#6531) change we started accurately formatting content after directive bodies. Updated our baselines for formatting tests to capture this new requirement.
- As part of this work I also found that the existing `WaitForClassificationAsync` was a little misleading because it defaulted to waiting for Razor component classifications. I've gone ahead and renamed the existing method to `WaitForComponentClassificationAsync` to be extra clear what's being waited for. As an extra reaction I added a general `WaitForSemanticClassificationAsync` that's now used in Razor formatting tests so we can wait for semantic Razor transitions.
@ryzngard
Copy link
Contributor

ryzngard commented Aug 9, 2022

Done with #6619

@ryzngard ryzngard closed this as completed Aug 9, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants