Test Collection Cleanup Failure

[khteh]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

[Collection("Tests Collection")]
public class AuthorizeEndpointIntegrationTests
{
    private readonly HttpClient _client;
    public AuthorizeEndpointIntegrationTests(CustomWebApplicationFactory<Program> factory) => _client = factory.CreateClient();
}

public class CustomWebApplicationFactory<TStartup> : WebApplicationFactory<TStartup>, IDisposable where TStartup : class
{
    public void Dispose()
    {
        using (var scope = Services.CreateScope())
        {
            var scopedServices = scope.ServiceProvider;
            var myDb = scopedServices.GetRequiredService<MyDbContext>();
            SeedData.CleanUpTestData(myDb);
        }
        base.Dispose(); // XXX Throws
        GC.SuppressFinalize(this);
    }
}

Sometime it passes, sometime it fails.

Expected Behavior

No exception

Steps To Reproduce

No response

Exceptions (if any)

      Message: 
        [Test Collection Cleanup Failure (Tests Collection)]: System.AggregateException : One or more hosted services failed to stop. (Object reference not set to an instance of an object.) (Not started. Call Start first.)
        ---- System.NullReferenceException : Object reference not set to an instance of an object.
        ---- System.InvalidOperationException : Not started. Call Start first.

      Stack Trace: 
        Host.StopAsync(CancellationToken cancellationToken)
        WebApplicationFactory`1.DisposeAsync()
        WebApplicationFactory`1.Dispose(Boolean disposing)
        WebApplicationFactory`1.Dispose()
        CustomWebApplicationFactory`1.Dispose() line 77
        ----- Inner Stack Trace #1 (System.NullReferenceException) -----
        ServerSideSessionCleanupHost.StopAsync(CancellationToken cancellationToken) line 70
        Host.StopAsync(CancellationToken cancellationToken)
        ----- Inner Stack Trace #2 (System.InvalidOperationException) -----
        TokenCleanupHost.StopAsync(CancellationToken cancellationToken) line 67
        Host.StopAsync(CancellationToken cancellationToken)

.NET Version

7.0.304

Anything else?

.NET SDK:
Version: 7.0.304
Commit: 7e794e2806

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22621
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\7.0.304\

Host:
Version: 7.0.7
Architecture: x64
Commit: 5b20af47d9

.NET SDKs installed:
6.0.202 [C:\Program Files\dotnet\sdk]
6.0.401 [C:\Program Files\dotnet\sdk]
7.0.100 [C:\Program Files\dotnet\sdk]
7.0.304 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.18 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.18 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.9 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.18 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
x86 [C:\Program Files (x86)\dotnet]
registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
Not set

global.json file:
Not found

Learn more:
https://aka.ms/dotnet/info

Download .NET:
https://aka.ms/dotnet/download

[martincostello]

These exceptions seem to be coming from within IdentityServer thinking the server hasn't been started.

• TokenCleanupHost.StopAsync()
• ServerSideSessionCleanupHost.StopAsync()

Is the constructor working OK and the server is starting correctly? Is IdentityServer configured appropriately? I wonder if the sometimes-it-works/sometimes-it-doesn't-work is related to the background tasks in those hosted services.

Something you could try refactoring to see if it changes anything is to setup and tear down the fixture using IAsyncLifetime:

[Collection("Tests Collection")]
public class AuthorizeEndpointIntegrationTests : IAsyncLifetime
{
    private readonly CustomWebApplicationFactory<Program> _factory;
    public AuthorizeEndpointIntegrationTests(CustomWebApplicationFactory<Program> factory) => _factory = factory;

    public Task InitializeAsync()
    {
        using (_factory.CreateDefaultClient())
        {
            // Force the server to start
        }

        return Task.CompletedTask;
    }

    public async Task DisposeAsync()
    {
        await using (var scope = _factory.Services.CreateAsyncScope())
        {
            var scopedServices = scope.ServiceProvider;
            var myDb = scopedServices.GetRequiredService<MyDbContext>();
            SeedData.CleanUpTestData(myDb);
        }
    }
}

If you're still having issues, a self-contained reproducible example of the issue would help.

[ghost]

Hi @khteh. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[khteh]

Please provide more info / reference for // Force the server to start

[khteh]

Is public AuthorizeEndpointIntegrationTests(CustomWebApplicationFactory<Program> factory) => _factory; a mistake or this is a new C# syntax?

[martincostello]

Please provide more info / reference for // Force the server to start

Calling any of the methods that create an HttpClient will call the EnsureServer() method inside the factory, which will ensure that the HTTP server in the fixture is running before any tests methods are executed.

aspnetcore/src/Mvc/Mvc.Testing/src/WebApplicationFactory.cs

Line 139 in 1bda10b

private void EnsureServer()

Is public AuthorizeEndpointIntegrationTests(CustomWebApplicationFactory<Program> factory) => _factory; a mistake or this is a new C# syntax?

Which bit do you think is a mistake? It's the same syntax as your code, except I'm assigning the fixture into a field instead of the result of calling CreateClient().

[ghost]

Hi @khteh. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[khteh]

Isn't it done in my code? Please check the original post:

    public AuthorizeEndpointIntegrationTests(CustomWebApplicationFactory<Program> factory) => _client = factory.CreateClient();

There is no assignment in your constructor. You just referenced the field. I expected _factory = factory;. What do I miss?

[martincostello]

[Collection("Tests Collection")]
public class AuthorizeEndpointIntegrationTests
{
private readonly HttpClient _client;
public AuthorizeEndpointIntegrationTests(CustomWebApplicationFactory factory) => _client = factory.CreateClient();
}

Here's the code from your original post. I just amended it. It's using a C# expression-bodied constructor to set the value of the _client field with the value of result of calling factory.CreateClient(). My code just adapts this.

If you're not familiar with this syntax, it's the same as:

public class AuthorizeEndpointIntegrationTests : IAsyncLifetime
{
    private readonly CustomWebApplicationFactory<Program> _factory;

    public AuthorizeEndpointIntegrationTests(CustomWebApplicationFactory<Program> factory)
    {
        factory = _factory;
    }
}

Having written the above, I can now see it was just a minor typo in the GitHub written (not IDE written, so no compiler error checking) code example.

[ghost]

Hi @khteh. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[khteh]

I don't get the point of your answers / posts here. My original post already calls factory.CreateClient. Doesn't it "ensure that the HTTP server in the fixture is running before any tests methods are executed."?

[martincostello]

I'm suggesting moving it out of the constructor and keeping the fixture in the class, so that if it fails it's easier to separate out as it would happen in a dedicated method. The rationale being that sometimes constructors throw, and then the class is disposed, but a bug in the dispose code called via a finalizer might assume the constructor never fails, and then throw. Refactoring the fixture as I suggested would help with determining whether that is the case.

Whether you think that suggestion has merit or not, the exceptions appear to be coming from IdentityServer code, so if there's an issue in ASP.NET Core a fully-reproducible example is likely going to be needed to show that that is indeed the case.

[ghost]

Hi @khteh. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[ghost]

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimalistic repro project that illustrates the problem.

[khteh]

What's the diff betwee CreateClient and CreateDefaultClient? I tried both:

[Collection("Tests Collection")]
public class AuthorizeEndpointIntegrationTests : IAsyncLifetime
{
    private HttpClient _client;
    private readonly CustomWebApplicationFactory<Program> _factory;
    public AuthorizeEndpointIntegrationTests(CustomWebApplicationFactory<Program> factory) => _factory = factory;

    public Task DisposeAsync()
    {
        return Task.CompletedTask;
    }

    public Task InitializeAsync()
    {
        _client = _factory.CreateDefaultClient(); // Also tried CreateClient()
        return Task.CompletedTask;
    }
    [Fact]
    public async Task ValidParametersShouldPassTest()
    {
        RequestUrl ru = new RequestUrl("https://localhost:5001/connect/authorize");
        string url = ru.CreateAuthorizeUrl(
            clientId: "sellerportal",
            responseType: "code",
            redirectUri: "https://app.com/callback",
            scope: IdentityServerConstants.StandardScopes.Profile);
        HttpResponseMessage response = await _client.GetAsync(url);
        response.EnsureSuccessStatusCode();
    }
}

but to no avail.

[martincostello]

What's the diff betwee CreateClient and CreateDefaultClient?

They're just different overloads, but I suggested using that one because it's the default 😄

aspnetcore/src/Mvc/Mvc.Testing/src/WebApplicationFactory.cs

Lines 454 to 473 in 1bda10b

	public HttpClient CreateClient() =>
	CreateClient(ClientOptions);
	/// <summary>
	/// Creates an instance of <see cref="HttpClient"/> that automatically follows
	/// redirects and handles cookies.
	/// </summary>
	/// <returns>The <see cref="HttpClient"/>.</returns>
	public HttpClient CreateClient(WebApplicationFactoryClientOptions options) =>
	CreateDefaultClient(options.BaseAddress, options.CreateHandlers());
	/// <summary>
	/// Creates a new instance of an <see cref="HttpClient"/> that can be used to
	/// send <see cref="HttpRequestMessage"/> to the server. The base address of the <see cref="HttpClient"/>
	/// instance will be set to <c>http://localhost</c>.
	/// </summary>
	/// <param name="handlers">A list of <see cref="DelegatingHandler"/> instances to set up on the
	/// <see cref="HttpClient"/>.</param>
	/// <returns>The <see cref="HttpClient"/>.</returns>
	public HttpClient CreateDefaultClient(params DelegatingHandler[] handlers)

but to no avail.

So it doesn't fix it, but are the exceptions being thrown the same?

If it's still not working, please provide a self-contained repro including the IdentityServer parts where the exceptions are coming from.

[ghost]

Hi @khteh. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[khteh]

      Message: 
[Test Collection Cleanup Failure (Tests Collection)]: System.AggregateException : One or more hosted services failed to stop. (Object reference not set to an instance of an object.)
---- System.NullReferenceException : Object reference not set to an instance of an object.

      Stack Trace: 
Host.StopAsync(CancellationToken cancellationToken)
WebApplicationFactory`1.DisposeAsync()
WebApplicationFactory`1.Dispose(Boolean disposing)
WebApplicationFactory`1.Dispose()
CustomWebApplicationFactory`1.Dispose() line 77
----- Inner Stack Trace -----
TokenCleanupHost.StopAsync(CancellationToken cancellationToken) line 71
Host.StopAsync(CancellationToken cancellationToken)

[martincostello]

If it's still not working, please provide a self-contained repro including the IdentityServer parts where the exceptions are coming from.

[ghost]

Hi @khteh. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[khteh]

When is the DisposeAsync called vs the Dispose function defined in the CustomWebApplicationFactory?

public class CustomWebApplicationFactory<TStartup> : WebApplicationFactory<TStartup>, IDisposable where TStartup : class
{
    public void Dispose()
    {
        using (var scope = Services.CreateScope())
        {
            var scopedServices = scope.ServiceProvider;
            var appDb = scopedServices.GetRequiredService<MyDbContext>();
            SeedData.CleanUpTestData(appDb);
        }
        base.Dispose();
        GC.SuppressFinalize(this);
    }
}

I only want to clean up and dispose after all the tests in the "Test Collection" completes.

[martincostello]

Using DisposeAsync() would just be an improvement if you wanted to use async code in your clean up method, but that's a different topic. IAsyncLifetime was just a suggestion of a refactor to see if it made any difference. Apparently it doesn't, so we're back to the original issue of why are the exceptions being thrown. Let's set aside those parts of my suggestion.

I only want to clean up and dispose after all the tests in the "Test Collection" completes.

I understand what you're trying to do, but the call to base.Dispose() is throwing exceptions from IdentityServer related code. Without a repro, trying to find out why that is happening is going to be difficult. Finding the root cause will also clarify whether the issue is with your application code, ASP.NET Core or IdentityServer (or something different entirely).

[ghost]

Hi @khteh. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[ghost]

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.