search

nunit / nunit3-vs-adapter

NUnit 3.0 Visual Studio test adapter for use under VS 2012 or later
https://nunit.org
MIT License
202 stars 104 forks source link

Failure during unit test discovery doesn't cause entire test suite to fail #1186

Closed zlepper closed 5 days ago

zlepper commented 4 weeks ago

When reporting a bug, please provide the following information to speed up triage:

Currently NUnit has the following bug: https://github.com/nunit/nunit/issues/4589 where a little test like this:

public class Tests
{
    [Test]
    public void OneTest()
    {
        Console.WriteLine("All is good");
    }

    [Test]
    [Explicit("A reason goes here")]
    public void ExplicitTest()
    {

    }
}

That uses the [Explicit] is currently causing test discovery to fail when combined with filtering:

dotnet test --filter 'TestCategory!=Foo&FullyQualifiedName!~Bar.Baz'
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
Exception NUnit.Engine.NUnitEngineException,    Exception thrown executing tests in /home/rasmus/projects/DotnetBlameTimeoutTests/DotnetBlameTimeoutTests/bin/Debug/net8.0/DotnetBlameTimeoutTests.dll
An exception occurred in the driver while counting test cases.
   at NUnit.Engine.Runners.DirectTestRunner.CountTestCases(TestFilter filter)
   at NUnit.Engine.Runners.MasterTestRunner.CountTests(TestFilter filter)
   at NUnit.Engine.Runners.MasterTestRunner.RunTests(ITestEventListener listener, TestFilter filter)
   at NUnit.Engine.Runners.MasterTestRunner.Run(ITestEventListener listener, TestFilter filter)
   at NUnit.VisualStudio.TestAdapter.NUnitEngine.NUnitEngineAdapter.Run(ITestEventListener listener, TestFilter filter) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnitEngine\NUnitEngineAdapter.cs:line 108
   at NUnit.VisualStudio.TestAdapter.Execution.Run(TestFilter filter, DiscoveryConverter discovery, NUnit3TestExecutor nUnit3TestExecutor) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\Execution.cs:line 51
   at NUnit.VisualStudio.TestAdapter.VsTestExecution.Run(TestFilter filter, DiscoveryConverter discovery, NUnit3TestExecutor nUnit3TestExecutor) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\Execution.cs:line 154
   at NUnit.VisualStudio.TestAdapter.NUnit3TestExecutor.RunAssembly(String assemblyPath, IGrouping`2 testCases, TestFilter filter) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnit3TestExecutor.cs:line 295
InnerException: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.ArgumentOutOfRangeException: Index was out or range of valida values (Parameter 'index')
Actual value was 0.
   at NUnit.Framework.Interfaces.TNode.NodeList.ThrowArgumentOutOfRangeException(Int32 index)
   at NUnit.Framework.Interfaces.TNode.get_FirstChild()
   at NUnit.Framework.Internal.TestFilter.FromXml(TNode node)
   at NUnit.Framework.Internal.TestFilter.GetChildNodeFilters(TNode node)
   at NUnit.Framework.Internal.TestFilter.FromXml(TNode node)
   at NUnit.Framework.Internal.TestFilter.FromXml(String xmlText)
   at NUnit.Framework.Api.FrameworkController.CountTests(String filter)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
   --- End of inner exception stack trace ---
   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
   at System.Reflection.MethodBaseInvoker.InvokeWithOneArg(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at NUnit.Engine.Drivers.NUnitNetStandardDriver.ExecuteMethod(MethodInfo method, Object[] args)
   at NUnit.Engine.Drivers.NUnitNetStandardDriver.ExecuteMethod(String methodName, Object[] args)
   at NUnit.Engine.Drivers.NUnitNetStandardDriver.CountTestCases(String filter)
   at NUnit.Engine.Runners.DirectTestRunner.CountTestCases(TestFilter filter)
No test matches the given testcase filter `TestCategory!=Foo&FullyQualifiedName!~Bar.Baz` in /home/rasmus/projects/DotnetBlameTimeoutTests/DotnetBlameTimeoutTests/bin/Debug/net8.0/DotnetBlameTimeoutTests.dll

A bug like that itself is "fine". The main problem is that i just noticed we have not been running our tests properly in CI for a couple of months because of this. When this happens NUnit logs the warning and moves on. In my opinion it should really cause the entire test runner to fail so you know something is horribly wrong and should go investigate it

Csproj file:

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <TargetFramework>net8.0</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>

        <IsPackable>false</IsPackable>
        <IsTestProject>true</IsTestProject>
    </PropertyGroup>

    <ItemGroup>
        <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
        <PackageReference Include="NUnit" Version="4.1.0" />
        <PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
        <PackageReference Include="NUnit.Analyzers" Version="4.2.0">
          <PrivateAssets>all</PrivateAssets>
          <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
        </PackageReference>
    </ItemGroup>

</Project>
OsirisTerje commented 1 week ago

The framework crashes, then engine crashes, so do the adapter. However, the dotnet test runner (testhost) doesn't do that, it continues with the next one, or just terminates with no more.

There is very little we can do here, because we have lost control. All our components have crashed, and can't change the state of that test or test suite.

When using Azure Devops Pipelines, you can specify a minimum number of tests, that would report something like this.
But github actions is using dotnet test and there is no minimum there, although it has been requested, see https://github.com/dotnet/sdk/issues/37101.

You could easily build your own little tool to check the number of executed tests and kill the build if that goes too low.

I actually built such a tool some years ago, see https://www.nuget.org/packages/KDISim.CheckTest and source here: https://github.com/KDISim/CheckTest

zlepper commented 1 week ago

I apologise if I have misunderstood something here, however I did try to trace the code for this back when I reported this ticket. I don't believe the test host actually crashes here. Instead NUnit catches the exception, logs it and continues as if nothing happens, which means in this case no tests being reported available and the test assembly skipped.

I believe if the test runner actually crashes, dotnet test will report a failure.

OsirisTerje commented 1 week ago

It is I who should apologize. You are correct.

Instead NUnit catches the exception, logs it and continues as if nothing happens

Yes, indeed it does. But we have lost the context, so need to figure out how we can inform the test host.
Also need to take it account the exceptions that are not to stop the testing, there are a couple of those.

But, we can't stop the test host. It will continue with the next assembly (or has already started the next one, in parallel).

We could create a fake test case with the exception result, that should at least fail a CI run.

OsirisTerje commented 1 week ago

@zlepper Take a look here. https://github.com/OsirisTerje/AdapterIssue1186/actions/runs/9946048865/job/27475685880

It now fails, using a constructed test representing the assembly. Some more details needed, but this is a way it can work. You can also test it yourself, using the myget package (4.6.0-beta.4).

zlepper commented 1 week ago

That looks great! As long as it informs me that "shits on fire" then I'm happy and will hopefully not have to come back to tests that have been failing for months or weeks later :D :D

OsirisTerje commented 1 week ago

@zlepper Also seems the bug that crashed for you is in the process of getting fixed. https://github.com/nunit/nunit/pull/4760/files

It will be out in NUnit 4.2, and this fix in adapter will be out in NUnit3TestAdapter 4.6.