dotnet / roslyn-sdk

Roslyn-SDK templates and Syntax Visualizer
MIT License
498 stars 254 forks source link

NUnit 4.0 Breaks Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.NUnit #1127

Closed JasonBock closed 7 months ago

JasonBock commented 7 months ago

I updated one of my projects to use NUnit 4.0.0-beta.1, and when I ran my tests, I got an odd MissingMethodException, because somewhere within Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.NUnit, it's trying to call Assert.AreEqual(). This method no longer exists in NUnit 4 and was moved to a separate package in a new class, ClassicAssert. I can get around this by not upgrading to NUnit 4, but....I'm guessing the only real way to fix this is that someone will need to update the code in Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.NUnit to use a different assertion.

FWIW I'm referencing 1.1.1 of Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.NUnit

More info: https://docs.nunit.org/articles/nunit/Towards-NUnit4.html

CodingFlow commented 7 months ago

Now that the fix is merged, when will it be released? 🤔

sharwell commented 6 months ago

@CodingFlow it looks like there is an error in our publishing pipeline that needs to be resolved. The publish should be automatic once that is fixed.

Dot-H commented 6 months ago

Hi @sharwell, any news regarding your publishing pipeline ? 🙂

sharwell commented 6 months ago

Should be fixed now with build 1.1.2-beta1.23621.2

sangwei commented 5 months ago

I hit the same problem yesterday and had to downgrade nunit to 3 for it. Does anyone know when it will be released?

sharwell commented 5 months ago

@sangwei Are you able to test 1.1.2-beta1.23621.2 or newer?

Note that this package is published to the dotnet-tools feed: https://github.com/dotnet/roslyn-sdk/blob/ed53b613fa5ebf7c7f75041f1b9fa39b53b64a32/NuGet.config#L6

thomhurst commented 5 months ago

Is this going non-beta anytime soon?

juwens commented 4 months ago

@sharwell for some reasons, it still happens:

image

used versions:

    <PackageVersion Include="Microsoft.CodeAnalysis" Version="4.8.0" />
    <PackageVersion Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" />
    <PackageVersion Include="Microsoft.CodeAnalysis.Analyzer.Testing" Version="1.1.2-beta1.24074.2" />
    <PackageVersion Include="Microsoft.CodeAnalysis.CSharp" Version="4.8.0" />
    <PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing " Version="1.1.2-beta1.24074.2" />
    <PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing.NUnit " Version="1.1.2-beta1.24074.2" />
    <PackageVersion Include="Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.NUnit" Version="1.1.2-beta1.24074.2" />
    <PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.8.0" />
    <PackageVersion Include="Microsoft.CodeAnalysis.Common" Version="4.8.0" />
    <PackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="8.0.0" />
    <PackageVersion Include="NUnit" Version="4.0.1" />
    <PackageVersion Include="NUnit.Analyzers" Version="3.10.0" />
    <PackageVersion Include="NUnit3TestAdapter" Version="4.5.0" />

Implementation of NotEmpty looks like this:

        public virtual void NotEmpty<T>(string collectionName, IEnumerable<T> collection)
        {
            Assert.That(collection, Is.Not.Empty, CreateMessage($"expected '{collectionName}' to be non-empty, contains"));
        }
sharwell commented 4 months ago

@jasonmalinowski with two of three frameworks broken now, maybe it's time to take the approach of https://github.com/dotnet/roslyn-sdk/pull/1144 for all of them.

jasonmalinowski commented 4 months ago

@sharwell Fun!

lucahost commented 2 months ago

I'm having the same issue as @juwens described above - could we reopen this issue?

sharwell commented 2 months ago

@lucahost The plan I'm working for is removing the NuGet package altogether. Users can adopt this approach at any time by removing the .NUnit suffixes from the testing package references, and using DefaultVerifier instead of NUnitVerifier.

CodingFlow commented 3 weeks ago

@lucahost The plan I'm working for is removing the NuGet package altogether. Users can adopt this approach at any time by removing the .NUnit suffixes from the testing package references, and using DefaultVerifier instead of NUnitVerifier.

It's working for me by using DefaultVerifier instead of NUnitVerifier, but I can't remove the Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.NUnit package without getting compile errors:

image

sharwell commented 3 weeks ago

@CodingFlow When removing Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.NUnit, you would need to add Microsoft.CodeAnalysis.CSharp.SourceGenerators.Testing.