Skip to content

Add an experimental API for testing incremental steps.#1094

Open
jkoritzinsky wants to merge 8 commits intodotnet:mainfrom
jkoritzinsky:incremental-testing
Open

Add an experimental API for testing incremental steps.#1094
jkoritzinsky wants to merge 8 commits intodotnet:mainfrom
jkoritzinsky:incremental-testing

Conversation

@jkoritzinsky
Copy link
Member

@jkoritzinsky jkoritzinsky commented May 3, 2023

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).

Note

Notes from @sharwell on specific tests we believe would be valuable. These tests could be provided by the API in this pull request, or through some other means.

  • Ability to verify that a change which did not impact outputs only executed specific incremental steps. Since it's easy (per @jkoritzinsky) to unintentionally break equality comparisons for model objects, there is a likelihood of regression in the incremental evaluation due to equivalent items returning not-equal.
  • Ability to verify that a change which only impacted a subset of outputs only executed specific incremental steps that serve as inputs to other outputs which did not change.
  • Ability to test that a source generator did not include Compilation as part of its incremental state. In other words, after running source generators on a compilation and still holding the GeneratorDriver instance, the Compilation passed to it should be eligible for garbage collection.

@jkoritzinsky jkoritzinsky added the Area-MS.CA.Testing Microsoft.CodeAnalysis.Testing label May 3, 2023
@jkoritzinsky jkoritzinsky requested a review from sharwell May 3, 2023 18:40
@jkoritzinsky jkoritzinsky requested a review from a team as a code owner May 3, 2023 18:40
<MicrosoftVisualStudioProjectSystemSDKToolsVersion>17.3.195-pre</MicrosoftVisualStudioProjectSystemSDKToolsVersion>
<!-- Libs -->
<DiffPlexVersion>1.5.0</DiffPlexVersion>
<HumanizerCoreVersion>2.14.1</HumanizerCoreVersion>
Copy link
Contributor

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.

protected override ParseOptions CreateParseOptions()
=> new CSharpParseOptions(DefaultLanguageVersion, DocumentationMode.Diagnose);

public IncrementalGeneratorExpectedState IncrementalGeneratorState
Copy link
Contributor

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)
Copy link
Contributor

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I believe so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-MS.CA.Testing Microsoft.CodeAnalysis.Testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants