-
Notifications
You must be signed in to change notification settings - Fork 256
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
Add an experimental API for testing incremental steps. #1094
base: main
Are you sure you want to change the base?
Conversation
src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs
Outdated
Show resolved
Hide resolved
…emental change on the test sources.
src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs
Outdated
Show resolved
Hide resolved
@@ -70,10 +70,12 @@ | |||
<MicrosoftVisualStudioProjectSystemSDKToolsVersion>17.3.195-pre</MicrosoftVisualStudioProjectSystemSDKToolsVersion> | |||
<!-- Libs --> | |||
<DiffPlexVersion>1.5.0</DiffPlexVersion> | |||
<HumanizerCoreVersion>2.14.1</HumanizerCoreVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 I would prefer to remove this dependency before merge, but that's further down the line. I'm mostly focusing on the API itself right now.
src/Microsoft.CodeAnalysis.Testing/Microsoft.CodeAnalysis.Analyzer.Testing/AnalyzerTest`1.cs
Show resolved
Hide resolved
@@ -27,5 +28,19 @@ protected override CompilationOptions CreateCompilationOptions() | |||
|
|||
protected override ParseOptions CreateParseOptions() | |||
=> new CSharpParseOptions(DefaultLanguageVersion, DocumentationMode.Diagnose); | |||
|
|||
public IncrementalGeneratorExpectedState IncrementalGeneratorState |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 I think we can remove this in favor of changing the type of IncrementalGeneratorStepStates
to a new type derived from Dictionary<Type, IncrementalGeneratorExpectedState>
that provides a new indexer that creates the value on demand. In practice in means you'd have to type this, but it seems acceptable:
IncrementalGeneratorStepStates =
{
[typeof(TSourceGenerator)] =
{
ExpectedStepStates =
{
// ...
},
},
},
var secondRunProject = await CreateProjectAsync(new EvaluatedProjectState(incrementalTestState, ReferenceAssemblies), incrementalTestState.AdditionalProjects.Values.Select(additionalProject => new EvaluatedProjectState(additionalProject, ReferenceAssemblies)).ToImmutableArray(), cancellationToken); | ||
|
||
driver = driver | ||
.ReplaceAdditionalTexts(secondRunProject.AnalyzerOptions.AdditionalFiles) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❔ Will the driver recognize cases where a file did not change? For example, consider the case of two additional files. If you change the content of just one of them, will it treat the other one as unchanged?
if (!TestState.GeneratedSources.Any()) | ||
{ | ||
// Verify the test state has at least one source, which may or may not be generated | ||
Verify.NotEmpty($"{nameof(TestState)}.{nameof(SolutionState.Sources)}", TestState.Sources); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it okay to have empty sources but non-empty generated sources?
Add a .NET 6.0 test target that references Roslyn 4.4 so we have a test suite that references Roslyn 4.X.
Introduce a new API to enable developers to define post-initial-run solution transforms to test incrementality.
Add a Humanizer reference at the same version as Roslyn's dependency to improve the error message wording of the new validation section (can be removed if we so desire).