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

[zlepper]

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

• NUnit and NUnit3TestAdapter versions:

        <PackageReference Include="NUnit" Version="4.1.0" />
        <PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />

• Visual Studio edition and full version number (see Help About): Happens from the dotnet CLI.
• A short repro, preferably attached or pointing to a git repo or gist: (See later in the issue)
• What .net platform and version is being targeted: Dotnet 8 on linux

Currently NUnit has the following bug: nunit/nunit#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]

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 dotnet/sdk#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]

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]

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]

@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]

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]

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