Bug (xUnit v2.x): Collection fixture is being disposed even if all its tests are skipped.

[voroninp]

I get the following error when my tests are run in GitLab:

Error Message:
   [Test Collection Cleanup Failure (PostgresSqlContainerCollection)]: System.NullReferenceException : Object reference not set to an instance of an object.
  Stack Trace:
     at Common.TestUtils.PostgresSqlContainerFixture.DisposeAsync() in Common.TestUtils/PostgresSqlContainerFixture.cs:line 17

Our current GitLab setting causes tests which use Docker to fail, so I temporarily skip all the tests which use Test Containers.

Here's the code of the fixture:

public sealed class PostgresSqlContainerFixture : IAsyncLifetime
{
    private static readonly PostgreSqlBuilder Builder = new();

    public PostgreSqlContainer Container { get; private set; } = null!;

    public async Task DisposeAsync()
    {
        await Container.StopAsync();
    }

    public async Task InitializeAsync()
    {
        Container = Builder.Build();
        await Container.StartAsync();
    }
}

I cannot find an exception coming from InitializeAsync in GitLab log, but I assume it is called, and exception is swallowed/ignored.
Maybe it's not called at all. I do not know for sure.

I changed InitializeAsync to

public async Task InitializeAsync()
{
    throw new Exception();
}

And I started seeing these errors when run tests locally:

  Failed Given user is NOT a member of current organization when checking this fact then returns false [1 ms]
  Error Message:
   System.Exception : Exception of type 'System.Exception' was thrown.
  Stack Trace:
     at Common.TestUtils.PostgresSqlContainerFixture.InitializeAsync() in Common.TestUtils\PostgresSqlContainerFixture.cs:line 18
  Failed Given user is NOT a member of current organization when checking this fact then returns false [1 ms]
  Error Message:
   [Test Collection Cleanup Failure (PostgresSqlContainerCollection)]: System.NullReferenceException : Object reference not set to an instance of an object.
  Stack Trace:
     at Common.TestUtils.PostgresSqlContainerFixture.DisposeAsync() in Common.TestUtils\PostgresSqlContainerFixture.cs:line 13

However, my project to reproduce the issue does not indicate InitializeAsync gets called at all:

PS FailingCollectionFixture> dotnet test
  Determining projects to restore...
  All projects are up-to-date for restore.
  FailingCollectionFixture -> FailingCollectionFixture\bin\Debug\net8.0\FailingCollectionFixture.dll
Test run for FailingCollectionFixture\bin\Debug\net8.0\FailingCollectionFixture.dll (.NETCoreApp,Version=v8.0)
VSTest version 17.11.1 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.12]     FailingCollectionFixture.Tests.Test [SKIP]
[xUnit.net 00:00:00.13]     [Test Collection Cleanup Failure (Collection)] System.Exception
  Skipped FailingCollectionFixture.Tests.Test [1 ms]
  Failed FailingCollectionFixture.Tests.Test [1 ms]
  Error Message:
   [Test Collection Cleanup Failure (Collection)]: System.Exception : Failed when disposing.
  Stack Trace:
     at FailingCollectionFixture.CollectionFixture.DisposeAsync() in FailingCollectionFixture\Tests.cs:line 22

Failed!  - Failed:     1, Passed:     0, Skipped:     1, Total:     2, Duration: 9 ms - FailingCollectionFixture.dll (net8.0)

So what we have:

It's either only DisposeAsync being always called, or both InitializeAsync and DisposeAsync being called and both failing.

[voroninp]

As an update: my GitLab pipeline succeeded after this change:

public async Task DisposeAsync()
{
    if (Container is null)
    {
        return;
    }

    await Container.StopAsync();
}

It means InitializeAsync is probably not called at all.

I suspect having method Task DisposeAsync() makes it async disposable even without the implementation of IAsyncDisposable interface.

[0xced]

Hey @voroninp, you might be interested in testing the new (unreleased yet) Testcontainers.Xunit package. It should handle container not starting properly. See testcontainers/testcontainers-dotnet#1165, you'll need to checkout my feature/Testcontainers.Xunit branch and add Testcontainers.Xunit as a project reference if you want to experiment with it.

[0xced]

A preview version of Testcontainers.Xunit has been released on NuGet. Testing with xUnit.net should get you started.

Please try it and report any issue on the testcontainers-dotnet repository.

[voroninp]

@0xced , I'll take a look, thanks.

Yet it does not cancel the fact that current xUnit's behaviour is at least strange.

[0xced]

Yet it does not cancel the fact that current xUnit's behaviour is at least strange.

Indeed! I looked at your FailingCollectionFixture project and I think the actual issue is that when the test is skipped the exception in the InitializeAsync method is somehow swallowed. Running under a debugger with a breakpoint confirms that the InitializeAsync method is called.

[bradwilson]

Here is my simple repro:

using System.Threading.Tasks;
using Xunit;
using Xunit.Abstractions;
using Xunit.Sdk;

namespace xunit3066;

public class Fixture(IMessageSink messageSink) : IAsyncLifetime
{
    public Task DisposeAsync()
    {
        messageSink.OnMessage(new DiagnosticMessage("DisposeAsync"));

        return Task.CompletedTask;
    }

    public Task InitializeAsync()
    {
        messageSink.OnMessage(new DiagnosticMessage("InitializeAsync"));

        return Task.CompletedTask;
    }
}

[CollectionDefinition("UnitTest1Collection")]
public class UnitTest1Collection : ICollectionFixture<Fixture>
{ }

[Collection("UnitTest1Collection")]
public class UnitTest1
{
    [Fact(Skip = "Skipped Test")]
    public void Test1()
    { }
}

When running the test, you can see that both InitializeAsync and DisposeAsync are called:

This is what I would expect.

It does appear that the exception in InitializeAsync in your example is being swallowed, and that's because those exceptions would end up being reported as test failures... but in this case, your test is skipped, so the reporting is not happening. If you change your test to not skip, you'll see the failure:

You're in a strange edge case where all of your tests are skipped, and we prioritize the skipping of the test over failing due to startup exceptions, so you're never seeing your exception happen. It's still happening, you're just not seeing it get reported, because we have no vehicle to report it with. It is extremely unusual to have a fixture created and every test it's assigned to is skipped.

There is not much we can do here, and I consider this to be (strangely) working as designed.

---

As a helpful tip to avoid situations like this in the future, IMO you should find any usage of the ! operator (as in your property initializer) to be a warning that you're at risk of throwing a NullReferenceException. Myself, I prefer to write such classes like this, because they have better traceability (and your Dispose is at no risk of throwing NRE now):

public sealed class PostgresSqlContainerFixture : IAsyncLifetime
{
    private static readonly PostgreSqlBuilder Builder = new();
    private PostgreSqlContainer? container;

    public PostgreSqlContainer Container =>
        container ?? throw new InvalidOperationException("Container was not initialized");

    public async Task DisposeAsync()
    {
        if (container is not null)
            await container.StopAsync();
    }

    public async Task InitializeAsync()
    {
        container = Builder.Build();
        await container.StartAsync();
    }
}

[bradwilson]

Closing as "by design".