Skip to content

Add test to ensure completion won't trigger execution of source generator#56593

Merged
genlu merged 5 commits intodotnet:mainfrom
genlu:CompeltionTest
Sep 22, 2021
Merged

Add test to ensure completion won't trigger execution of source generator#56593
genlu merged 5 commits intodotnet:mainfrom
genlu:CompeltionTest

Conversation

@genlu
Copy link
Copy Markdown
Member

@genlu genlu commented Sep 21, 2021

Final piece of #55578

@jasonmalinowski

@genlu genlu requested a review from a team as a code owner September 21, 2021 23:33
@ghost ghost added the Area-IDE label Sep 21, 2021
@jasonmalinowski jasonmalinowski self-requested a review September 21, 2021 23:48
Copy link
Copy Markdown
Member

@jasonmalinowski jasonmalinowski left a comment

Choose a reason for hiding this comment

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

Looks great other than one tiny concern.

[Fact]
public async Task PartialCompilationsIncludeGeneratedFilesAfterFullGeneration()
{
using var workspace = CreateWorkspaceWithPartalSemantics();
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.

Want to do a static using since we'll probably have this used more in this file down the road?

MarkupTestFile.GetPosition(sourceMarkup.NormalizeLineEndings(), out var source, out int? position);

var generatorRanCount = 0;
var generator = new CallbackGenerator(onInit: _ => { }, onExecute: _ => { generatorRanCount++; });
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.

Do an Interlocked.Increment() here to avoid any potential race. It's not hugely important since you're asserting that the count is simply zero at the end, so even if we were to somehow run in parallel the test would fail; I worry that either it'll get copy/pasted, or we'll have a bug where it's running in parallel and confuse whoever is trying to debug the test.

@genlu genlu enabled auto-merge September 22, 2021 00:04
@genlu genlu merged commit f9bdd61 into dotnet:main Sep 22, 2021
@ghost ghost added this to the Next milestone Sep 22, 2021
@genlu genlu deleted the CompeltionTest branch September 22, 2021 18:22
@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.

Acquiring compilations when source generators are involved can significantly impact language features (i.e. completion)

3 participants