Improve usability of testing framework

Question

Improve usability of testing framework

tom-englert opened this issue a year ago · 12 comments

Writing custom analyzers becomes more and more popular, so testing should be easy to use and to customize out of the box

Functionality of the testing framework is great, however usability sometimes is a bit clumsy, and you have to be an expert to find some of the settings.
As a result many analyzers I've seen don't use tests at all.

Actual workflow:

You need to derive a new Test class from ...AnalyzerTest'1 for every test to get access to many settings.
Since there are already derived classes for every use case like Microsoft.CodeAnalysis.CSharp.CodeFix.Testing.XUnit, you can't extend the base class, but have add some custom logic to every flavor again and again, repeating this for every project that includes an analyzer.
Since AnalyzerTest`1 is generic, you also can't write global usable, verifier independent extension methods to simplify configure your test and avoid duplicated code.

Things that are needed frequently and should be more obvious and user friendly:

Adding an assembly reference to the compilation
Adding a package reference to the compilation
Adding a DiagnosticAnalyzer to the test
Getting the results of the code generators involved
Changing the language version (C# only)

Proposal:
Add some properties to the AnalyzerTest`1 class that ease configuration of the compilation and test

List<Assembly> AdditionalReferences
List<PackageIdentity> AdditionalPackages
List<DiagnosticAnalyzer> AdditionalAnalyzers
ImmutableArray<SyntaxTree> GeneratedTrees
LanguageVersion LanguageVersion (C# tests only)

A test then can be written like this without the need to create a derived class:

await new CSharpAnalyzerTest<MyAnalyzer, MSTestVerifier>
{
    TestCode = source,
    ReferenceAssemblies = ReferenceAssemblies.Net.Net60,
    AdditionalReferences = { GetType().Assembly },
    AdditionalAnalyzers = { new MySuppressor() },
    AdditionalPackages = { PackageReference.TomsToolbox_Essentials },
    LanguageVersion = LanguageVersion.CSharp10,
    ExpectedDiagnostics = { Diagnostics.TextPropertyHasNoDescription.AsResult().WithArguments("BadProperty").WithLocation(0) },
}.RunAsync();

@sharwell Would you accept a PR implementing this?

Answer 1 · 2023-04-01T10:50:54.000Z

Proposal 2
Provide some predefined solution transforms to ease usage of

AddAssemblyReference
LanguageVersion
CompilationOptions

Solution transforms are independent of the test verifier and can be easily shared among all test variations.

public static class SolutionTransform
{
    public static Func<Solution, ProjectId, Solution> Create(Func<Solution, Project, Solution> transform)
    {
        return (solution, projectId) =>
        {
            var project = solution.GetProject(projectId) ?? throw new InvalidOperationException("Project is not part of the solution");
            return transform(solution, project);
        };
    }

    public static Func<Solution, ProjectId, Solution> AddAssemblyReferences(params Assembly[] assemblies) => (solution, projectId) =>
    {
        var metadataReferences = assemblies
            .Distinct()
            .Select(assembly => MetadataReference.CreateFromFile(assembly.Location));

        return solution.AddMetadataReferences(projectId, metadataReferences);
    };

    public static Func<Solution, ProjectId, Solution> UseLanguageVersion(LanguageVersion languageVersion) => Create((solution, project) =>
    {
        var parseOptions = project.ParseOptions as CSharpParseOptions ?? throw new InvalidOperationException("Project does not have CSharpParseOptions");

        return solution.WithProjectParseOptions(project.Id, parseOptions.WithLanguageVersion(languageVersion));
    });

    public static Func<Solution, ProjectId, Solution> WithProjectCompilationOptions(Func<CSharpCompilationOptions, CSharpCompilationOptions> callback) => Create((solution, project) =>
    {
        var compilationOptions = project.CompilationOptions as CSharpCompilationOptions ?? throw new InvalidOperationException("Project does not have CSharpCompilationOptions");

        return solution.WithProjectCompilationOptions(project.Id, callback(compilationOptions));
    });
}

So the test would look like this:

using static SolutionTransform;
...
await new Test
{
    TestState = { Sources = { source, IsExternalInit } },
    SolutionTransforms =
    {
        AddAssemblyReferences(GetType().Assembly), 
        UseLanguageVersion(LanguageVersion.CSharp10), 
        WithProjectCompilationOptions(o => o.WithNullableContextOptions(NullableContextOptions.Enable)),
    },
    AdditionalAnalyzers = { new SuppressNullForgivingWarningAnalyzer() },
    ReferenceAssemblies = ReferenceAssemblies.Net.Net60.AddPackages(PackageReference.TomsToolbox_Essentials),
    ExpectedDiagnostics =
    {
        Nx0002.AsResult().WithLocation(0).WithArguments("InitOnly").WithIsSuppressed(true),
        Nx0002.AsResult().WithLocation(1).WithArguments("Normal").WithIsSuppressed(false)
    }
}
.RunAsync();

Answer 2 · 2023-04-01T14:37:34.000Z

@sharwell Would it be helpful to the project to make it's usage more intuitive?

Answer 3 · 2023-04-01T22:25:57.000Z

You need to derive a new Test class from ...AnalyzerTest'1 for every test to get access to many settings.

I don't understand this. In practice, it's rare to need anything more than the default validation class created by the templates. Can you provide a more specific example?

Answer 4 · 2023-04-01T22:26:55.000Z

Since AnalyzerTest`1 is generic, you also can't write global usable, verifier independent extension methods to simplify configure your test and avoid duplicated code.

Can you provide more information about this? We've had good success implementing extension methods in some projects in the past, although they are rarely needed in general. In most cases where this comes up, the user was trying to do something in a more difficult way than necessary.

Answer 5 · 2023-04-01T22:33:34.000Z

Adding an assembly reference to the compilation

This is available through TestState.AdditionalReferences. However, in most cases this is not what you want. It's extremely difficult to use this property correctly.

Adding a package reference to the compilation

This is available through ReferenceAssemblies. Due to the way caching works for this type, a good way to leverage this can be seen here:
https://github.com/dotnet/roslyn-analyzers/blob/916b9a6842c8d09c385cd25e4f02477e216eb630/src/Test.Utilities/AdditionalMetadataReferences.cs

Adding a DiagnosticAnalyzer to the test

I'd need more information about this one. Normally the using VerifyCS line covers it. I haven't seen many requests for cases where more than one analyzer needs to be added at once, but a custom type could be created to handle this.

Getting the results of the code generators involved

This is exceedingly difficult, because the source generators run multiple times during each test. For analyzer/code fix tests, they can run dozens of times in different contexts.

Changing the language version (C# only)

This seems to be commonly needed, and it would make sense to provide it as part of the base framework (and incorporate it into CreateParseOptions).

Answer 6 · 2023-04-01T22:36:27.000Z

A test then can be written like this without the need to create a derived class:

At this point, the use of the framework without any test-project-owned derivation of the base CSharpAnalyzerTest<,> class seems like a non-goal. It's created by default by all the templates, and all the examples use it in this manner. Considering the amount of functionality provided by the derived type and the relatively small amount of code it needs to contain, it doesn't seem worth it.

Answer 7 · 2023-04-01T22:39:27.000Z

Solution transforms are independent of the test verifier and can be easily shared among all test variations.

Solution transforms are generally a fallback strategy to support cases that aren't provided through better means. Over time, changes to the SDK tend to reduce the amount of times solution transforms are needed. This isn't to say we can't use solution transforms, but rather I'm saying I'd be looking to alternatives first.

Answer 8 · 2023-04-01T22:42:35.000Z

Writing custom analyzers becomes more and more popular, so testing should be easy to use and to customize out of the box

Functionality of the testing framework is great, however usability sometimes is a bit clumsy, and you have to be an expert to find some of the settings.

I agree completely here. We haven't had the resources available that we'd like to have in order to polish the user-facing documentation for the SDK. I'm hoping it can improve over time, and issues like this help especially by showing the parts of the APIs that are proving difficult to locate. Perhaps some better examples might help.

One possible way to build up some examples would be writing a unit test class (or set of classes) specifically designed to highlight features, as opposed to most of the tests in this project where they are more intended for regression prevention. The unit test approach has an advantage over other forms of documentation in that it must be up-to-date (and working) in order for pull requests to merge. Changes to the API would be immediately reflected in the tests if/when necessary.

Answer 9 · 2023-04-02T05:50:41.000Z

Actually I drilled it down to the essentials missing that were hard to find:

https://github.com/tom-englert/RoslynAnalyzerCookbook/blob/main/src/SolutionAnalyzer/SolutionAnalyzer.Test/CSharpAnalyzerTestExtensions.cs

With a sample usage here:
https://github.com/tom-englert/RoslynAnalyzerCookbook/blob/d6fe2d4f3439fb4125f898ecafd4199c1cc6def2/src/SolutionAnalyzer/SolutionAnalyzer.Test/EnforceDescriptionAnalyzerTest.cs#L69-L107

I think all other stuff would be covered by some good samples or documentation

Answer 10 · 2023-04-02T06:31:23.000Z

Since AnalyzerTest`1 is generic, you also can't write global usable, verifier independent extension methods to simplify configure your test and avoid duplicated code.

Can you provide more information about this? We've had good success implementing extension methods in some projects in the past, although they are rarely needed in general. In most cases where this comes up, the user was trying to do something in a more difficult way than necessary.

I could not write extension methods independent of the TVerifier

static class ExtensionMethods
{
    public static T AddSampleCode<T>(T test)
        where T: AnalyzerTest<>
    {
        test.TestCode = "Just a sample";
        return test;
    }
}

Answer 11 · 2023-04-20T14:20:17.000Z

I could not write extension methods independent of the TVerifier

Are you trying to support more than one test framework with the extensions? It seems like it would work to just use one of the specific verifiers like XunitVerifier or MSTestVerifier for the extension methods.

Answer 12 · 2023-04-20T14:55:39.000Z

@sharwell yes, for a public library it should support any verifier.
Actually found a solution by leveraging a code generator that auto-detects the verifier
https://github.com/tom-englert/RoslynAnalyzerTesting.CSharp.Extensions
It addresses all the issues and can be used with the existing 1.1.1 SDK,