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

Verify the error type and span only for error tag #64852

Merged
merged 5 commits into from
Oct 24, 2022

Conversation

Cosifne
Copy link
Member

@Cosifne Cosifne commented Oct 20, 2022

Try to fix the test failure
https://dev.azure.com/dnceng-public/public/_build/results?buildId=57273&view=ms.vss-test-web.build-test-results-tab&runId=1175948&resultId=100668&paneView=debug
image

We introduced a new type RoslynErrorTag
https://github.com/dotnet/roslyn/blob/main/src/EditorFeatures/Core/Diagnostics/AbstractDiagnosticsAdornmentTaggerProvider.RoslynErrorTag.cs
so it breaks the integrationt test.

I feel the logic to verify using the fully qualified name is quite fragile, so use the error type + span to do the verification work.

Need to wait for a run to see if test passed in CI

@Cosifne Cosifne marked this pull request as ready for review October 20, 2022 21:30
@Cosifne Cosifne requested a review from a team as a code owner October 20, 2022 21:30
@Cosifne
Copy link
Member Author

Cosifne commented Oct 21, 2022

The original PR introduce the error has been reverted. So this would not be needed right now

@jasonmalinowski
Copy link
Member

@Cosifne Should we close this then?

@Cosifne
Copy link
Member Author

Cosifne commented Oct 21, 2022

@Cosifne Should we close this then?

If possible can we still check this in? I still see some minor race when verify error tag
image

I feel my change here would give us more about the error message, and generally I feel it is more stable than only asserting string
(e.g. I think if this race happens again, we can know the syntax error Id with the message, so it would be easier to locate the problem)

@CyrusNajmabadi CyrusNajmabadi merged commit 1fa161f into dotnet:main Oct 24, 2022
@ghost ghost added this to the Next milestone Oct 24, 2022
@Cosifne Cosifne deleted the dev/shech/FixTags branch October 24, 2022 21:49
@RikkiGibson RikkiGibson modified the milestones: Next, 17.5 P1 Oct 24, 2022
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.

4 participants