Skip to content

Guard against updates with no type changes#56443

Merged
davidwengier merged 1 commit intodotnet:mainfrom
davidwengier:FixChangedTypesCheck
Sep 22, 2021
Merged

Guard against updates with no type changes#56443
davidwengier merged 1 commit intodotnet:mainfrom
davidwengier:FixChangedTypesCheck

Conversation

@davidwengier
Copy link
Copy Markdown
Member

Fixes #56431

@davidwengier davidwengier requested a review from a team as a code owner September 16, 2021 08:02
@ghost ghost added the Area-IDE label Sep 16, 2021
@davidwengier davidwengier requested a review from tmat September 21, 2021 21:50
*/
";

var generator = new TestSourceGenerator() { ExecuteImpl = GenerateSource };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the usage of code generators necessary to repro?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was the only case in our existing battery of tests that resulted in an empty updated types list, but there could be other repros, I didn't look too hard.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't just adding the comment directly (w/o source generator) do?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, it does. I could have sworn I tried it, but maybe I had the comment in the wrong place ¯\_(ツ)_/¯

@davidwengier davidwengier merged commit a10a372 into dotnet:main Sep 22, 2021
@davidwengier davidwengier deleted the FixChangedTypesCheck branch September 22, 2021 00:44
@ghost ghost added this to the Next milestone Sep 22, 2021
var delta = updates.Updates.Single();
Assert.Empty(delta.UpdatedTypes);

EndDebuggingSession(debuggingSession);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@davidwengier This line throws an System.InvalidOperationException : Pending update has not been committed or discarded. in the current main branch. See the CI tags in the readme https://github.com/dotnet/roslyn#desktop-unit-tests
The failure is reproducible locally on my machine. A detailed log can be found here https://dev.azure.com/dnceng/public/_build/results?buildId=1377343&view=ms.vss-test-web.build-test-results-tab&runId=40162926&resultId=210614&paneView=debug

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the heads up!

Fix in #56605

@davidwengier davidwengier mentioned this pull request Sep 22, 2021
@Cosifne Cosifne modified the milestones: Next, 17.0.P5 Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Sequence contains no elements" thrown from EmitSolutionUpdateAsync

4 participants