dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.96k stars 4.02k forks source link

Improve docs for testing source generators #58945

Open giggio opened 2 years ago

giggio commented 2 years ago

The current docs for tests for source generators is missing some important information about testing. For example, how to add references to the compilation. The current example is a good quick start but is not enough.

Docs link: https://github.com/dotnet/roslyn/blob/main/docs/features/source-generators.cookbook.md#unit-testing-of-generators

AlbertoMonteiro commented 2 years ago

When I was working with a Source Generator that requires AspNetCore 6.0 I stuck for hours to find how to make the CSharpSourceGeneratorTest be able to know AspNetCore assemblies/types.

new ReferenceAssemblies("net6.0", new("Microsoft.NETCore.App.Ref", "6.0.0"), @"ref\net6.0")
     .WithPackages(ImmutableArray.Create(new PackageIdentity("Microsoft.AspNetCore.App.Ref", "6.0.0")));

After some hours and many tries, I tried "Go To Definition" of the WebApplication class and I ended up getting the package name from that code below image

Being stuck to write that small piece of code(new PackageIdentity("Microsoft.AspNetCore.App.Ref", "6.0.0")) is really annoying

Youssef1313 commented 2 years ago

new ReferenceAssemblies("net6.0", new("Microsoft.NETCore.App.Ref", "6.0.0"), @"ref\net6.0")

You don't need to do that I believe. You can use the already existing property Microsoft.CodeAnalysis.Testing.ReferenceAssemblies.Net.Net60.

Youssef1313 commented 2 years ago

I think this is not source generators specific. It's a question about the testing library in general. It might worth documenting in dotnet/docs repository for wider visibility. @BillWagner @IEvangelist What do you think?

AlbertoMonteiro commented 2 years ago

new ReferenceAssemblies("net6.0", new("Microsoft.NETCore.App.Ref", "6.0.0"), @"ref\net6.0")

You don't need to do that I believe. You can use the already existing property Microsoft.CodeAnalysis.Testing.ReferenceAssemblies.Net.Net60.

There is not Net60 image

Youssef1313 commented 2 years ago

@AlbertoMonteiro You're probably consuming an old version of the testing library.

AlbertoMonteiro commented 2 years ago

@Youssef1313 actually I am using the latest version

https://www.nuget.org/packages/Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.XUnit/

image

image

Youssef1313 commented 2 years ago

Hmm, it looks like the version with Net60 available is only released in the Azure DevOps feed, but not on nuget.org feed.

@jmarolf @sharwell Can a next version be released on nuget.org?

sharwell commented 2 years ago

@AlbertoMonteiro the version on nuget.org will only be "current" for a few weeks at the very most. I recommend using the one from this feed:

https://github.com/dotnet/roslyn/blob/259a9e27ab8c6a1d8d3a3445b079fd5e3a13ec0a/NuGet.config#L7

giggio commented 2 years ago

Why not just publish the new version to Nuget.org? Is it unstable or not finished?

giggio commented 2 years ago

The problem with docs remain, BTW, even when the new lib version is published to Nuget.org.

AlbertoMonteiro commented 2 years ago

I agree with @giggio, I spent more time trying to figure out how to reference aspnet in the test context than anything else, so basically almost 2~3 hours to discover that code new PackageIdentity("Microsoft.AspNetCore.App.Ref", "6.0.0"). So yeah, a new nuget package version won't fix that issue of lacking information about how to do those things.

sharwell commented 2 years ago

Why not just publish the new version to Nuget.org? Is it unstable or not finished?

It's not an automated process, and it's always going to lag behind the daily builds to some degree. Since this dependency is only consumed by test projects, there's no significant consumer downside for referencing the dotnet-tools feed (aside from the one-time setup) or referencing a beta build.

AraHaan commented 2 years ago

CSharpSourceGeneratorTest also lacks being able to test incremental generators which I think is a must have.

Note: It's easy to create a special CSharpIncrementalGeneratorTest class to handle incremental generators. 😄

juwens commented 2 years ago

CSharpSourceGeneratorTest also lacks being able to test incremental generators which I think is a must have.

Note: It's easy to create a special CSharpIncrementalGeneratorTest class to handle incremental generators. 😄

can you please provide this "simple" implementation on a gist (or so). Other people not writing source generators every day, might not find it as easy as you.

grosch-intl commented 12 months ago

CSharpSourceGeneratorTest also lacks being able to test incremental generators which I think is a must have.

Note: It's easy to create a special CSharpIncrementalGeneratorTest class to handle incremental generators. 😄

@AraHaan Can you please provide an example? The cookbook says to use that class, but nothing says how to create it or what it should contain.

AraHaan commented 12 months ago

Sure, I will see if I can dig that code back up again from one of my repositories. @grosch-intl.

AraHaan commented 12 months ago

I found the code, it's all in this folder: https://github.com/Elskom/IDisposableGenerator/tree/main/tests

It covers both C# and Visual Basic as well.

sharwell commented 12 months ago

CSharpSourceGeneratorTest also lacks being able to test incremental generators

This is outdated information (the ISourceGenerator constraint was removed in https://github.com/dotnet/roslyn-sdk/pull/1084), so I'm resolving these comments as outdated to collapse them in this view.