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

Fix non-deterministic ordering of quick actions #68547

Merged
merged 2 commits into from
Jun 12, 2023

Conversation

mavasani
Copy link
Contributor

Note: Recommended to review ignoring whitespace changes

415d510 expanded tracking pending action sets to all priority buckets. However, it switched from using an ArrayBuilder to a MultiDictionary whose values are sets. This means the original order of pending actions for a specific bucket is lost.

This PR switches from a MultiDictionary to a regular Dictionary with ArrayBuilder values. This fixes the non-determinism in the ordering.

Testing: I have also added an integration test to guard against this regression. Verified that this integration test fails prior to the product fix.

dotnet@415d510 expanded tracking pending action sets for all priority buckets. However, it switched from using an ArrayBuilder to a MultiDictionary whose values are sets. This means the original order of pending actions for a specific bucket is lost.

This PR switches from a MultiDictionary to a regular Dictionary with ArrayBuilder values. This fixes the non-determinism in the ordering. I have also added an integration test to guard against this regression
@mavasani mavasani requested a review from CyrusNajmabadi June 12, 2023 11:57
@mavasani mavasani requested review from a team as code owners June 12, 2023 11:57
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Jun 12, 2023

await TestServices.EditorVerifier.CodeActionsAsync(expectedItems, ensureExpectedItemsAreOrdered: true, cancellationToken: HangMitigatingCancellationToken);

await SetUpEditorAsync(@"
Copy link
Contributor Author

@mavasani mavasani Jun 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 2 invocations as it seems the first one might succeed often, but for 9 out of 10 times I saw either first or second one has unexpected order of the items prior to the product fix.

@mavasani mavasani enabled auto-merge June 12, 2023 15:53
@mavasani mavasani merged commit 5bca78f into dotnet:main Jun 12, 2023
@ghost ghost added this to the Next milestone Jun 12, 2023
@mavasani mavasani deleted the FixSuggestedActionsOrdering branch June 13, 2023 02:26
mavasani added a commit to mavasani/roslyn that referenced this pull request Jun 13, 2023
mavasani added a commit that referenced this pull request Jun 13, 2023
@RikkiGibson RikkiGibson modified the milestones: Next, 17.7 P3 Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants