Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IHostApplicationLifetime StopApplication not respected in AspNetCore.Mvc.Testing #25857

Open
GeeWee opened this issue Sep 13, 2020 · 17 comments · Fixed by #29404
Open

IHostApplicationLifetime StopApplication not respected in AspNetCore.Mvc.Testing #25857

GeeWee opened this issue Sep 13, 2020 · 17 comments · Fixed by #29404
Labels
affected-few This issue impacts only small number of customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. investigate severity-minor This label is used by an internal tool
Milestone

Comments

@GeeWee
Copy link

GeeWee commented Sep 13, 2020

Describe the bug

I'm trying to run integration tests, with some things that call IHostApplicationLifetime.StopApplication from an IHostedService

In a "real project" this works, but it does not when using the TestServer.
Test case below

To Reproduce

namespace DefaultNamespace
{
    using System.Threading;
    using System.Threading.Tasks;
    using BetterHostedServices.Test;
    using Microsoft.AspNetCore.Builder;
    using Microsoft.AspNetCore.Hosting;
    using Microsoft.AspNetCore.Mvc.Testing;
    using Microsoft.AspNetCore.TestHost;
    using Microsoft.Extensions.DependencyInjection;
    using Microsoft.Extensions.Hosting;
    using Xunit;

    public class Test
    {
// Dummy startup that does nothing
        public class TestStartup
        {
            public void ConfigureServices(IServiceCollection services)
            {
            }

            public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
            {
            }
        }

// Hosted Service that stops the application
        public class HostedServiceThatShutsDown : IHostedService
        {
            private IHostApplicationLifetime lifetime;

            public HostedServiceThatShutsDown(IHostApplicationLifetime lifetime)
            {
                this.lifetime = lifetime;
            }

            public Task StartAsync(CancellationToken cancellationToken)
            {
                this.lifetime.StopApplication();
                return Task.CompletedTask;
            }

            public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
        }

// WebApplicationFactory with a few modifications so it can stand-alone
        public class TestWebApplicationFactory<TStartup> : WebApplicationFactory<TStartup> where TStartup : class
        {
            protected override void ConfigureWebHost(IWebHostBuilder builder)
            {
                builder.UseSolutionRelativeContentRoot(
                    "./tests/IntegrationUtils"); // Just because I have a custom dir in my project, can disregard
            }

            //from https://stackoverflow.com/questions/61159115/asp-net-core-testing-no-method-public-static-ihostbuilder-createhostbuilders
            protected override IHostBuilder CreateHostBuilder()
            {
                var builder = Host.CreateDefaultBuilder()
                    .ConfigureWebHostDefaults(x =>
                    {
                        x.UseStartup<DummyStartup>();
                    });
                return builder;
            }
        }

        [Fact]
        public void IssueRepro()
        {
            var factory = new TestWebApplicationFactory<TestStartup>()
                .WithWebHostBuilder(b =>
                    b.ConfigureTestServices(services =>
                    {
                        services.AddHostedService<HostedServiceThatShutsDown>();
                    }));

            var client = factory.CreateClient();

            // Works fine, but it shouldn't. The createClient above should error
            client.GetAsync("/");
        }
    }
}

The IHostedService calls StopApplication. I then don't expect the GetAsync call to work then.
The logs look like this:

info: Microsoft.Hosting.Lifetime[0]
      Application is shutting down...
info: Microsoft.Hosting.Lifetime[0]
      Application started. Press Ctrl+C to shut down.
info: Microsoft.Hosting.Lifetime[0]
      Hosting environment: Production
info: Microsoft.Hosting.Lifetime[0]
      Content root path: /home/geewee/programming/BetterHostedServices/tests/IntegrationUtils
info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost/  
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 18.5779ms 404 

So it starts off by writing "Application is shutting down" and then proceeds to start the application.

Further technical details

  • ASP.NET Core version: 3.1
  • Include the output of dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.1.401
 Commit:    39d17847db

Runtime Environment:
 OS Name:     fedora
 OS Version:  32
 OS Platform: Linux
 RID:         fedora.32-x64
 Base Path:   /usr/share/dotnet/sdk/3.1.401/

Host (useful for support):
  Version: 3.1.7
  Commit:  fcfdef8d6b

.NET Core SDKs installed:
  2.2.402 [/usr/share/dotnet/sdk]
  3.0.103 [/usr/share/dotnet/sdk]
  3.1.401 [/usr/share/dotnet/sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.2.8 [/usr/share/dotnet/shared/Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.2.8 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.3 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.7 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.2.8 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.3 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.7 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and it's version
    Rider, but I don't think that matters. All of this is run from the Console.
@BrennanConroy BrennanConroy added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed area-hosting labels Sep 14, 2020
@mkArtakMSFT mkArtakMSFT added this to the Next sprint planning milestone Sep 15, 2020
@ghost
Copy link

ghost commented Sep 15, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@SteveSandersonMS SteveSandersonMS added affected-few This issue impacts only small number of customers bug This issue describes a behavior which is not expected - a bug. severity-minor This label is used by an internal tool labels Oct 7, 2020 — with ASP.NET Core Issue Ranking
@ghost
Copy link

ghost commented Oct 9, 2020

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@quixoticaxis
Copy link

This, and the fact that MVC.Testing inner infrastructure only disposes the host without stopping it first, makes MVC.Testing really hard to use with any service that has lasting effects on infrastructure.

@javiercn
Copy link
Member

This is a dupe of #29348

@quixoticaxis
Copy link

quixoticaxis commented Jan 20, 2021

@javiercn hello. Sorry, but it does not look like the merged PR fixes this bug. And this bug is not a duplicate of my issue.

This bug describes the test host ignoring IHostApplicationLifetime.StopApplication.
My issue was the host not being stopped on dispose.

The PR only forces the host to stop when the factory is disposed.

@javiercn
Copy link
Member

@quixoticaxis My bad, I misunderstood it.

@javiercn javiercn reopened this Jan 20, 2021
@javiercn javiercn removed the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jan 20, 2021
@javiercn
Copy link
Member

@Tratcher I believe this is a potential issue/enhancement in TestServer, do you have any thoughts?

@Tratcher
Copy link
Member

@GeeWee While I appreciate the minimal repro, what are you really hoping to accomplish with StopApplication? It's not clear why StopApplication is a useful scenario for TestServer to emulate.

@quixoticaxis
Copy link

@Tratcher I don't know the reasons of this issue's author, but in our code we often encounter the following pattern:

public class ServiceA : IHostedService
{
    public Task StartAsync(CancellationToken token)
    {
        executing = ExecuteAsync();
    }

    private Task ExecuteAsync()
    {
        try
        {
            await serviceLogic.LoopAsync();
        }
        catch
        {
            Log("The service A cannot proceed, stopping the application");
            lifetime.StopApplication();
        }
    }

    private Task executing;
}

This code enables clean shutdown if any of the services fail, because in the real application StopAsync would be invoked on all of the services. It does not work in the tests though, which leads to inconveniencies (I described our scenario in #29348).

@plachor
Copy link

plachor commented Feb 15, 2021

Similar in case of OutOfMemory handling described in dotnet/runtime#48157 I would like to perform graceful shutdown once issue is detected and cover such negative path in an integration test.

@davidfowl
Copy link
Member

I guess the best it could do is stop future requests. That seems fine.

@jeremy001181
Copy link

jeremy001181 commented Feb 24, 2021

We also have a background service does following, would be good have a way to gracefully stop it after test completed (either pass or failed)

public class ServiceA : BackgroundService
    {
    protected override Task ExecuteAsync(CancellationToken stoppingToken){
                while (!stoppingToken.IsCancellationRequested)
                {
                             await pickupMessageFromQueueAndDoSomethingAboutIt();
                }
    }
    }

@Tratcher
Copy link
Member

@jeremy001181 that seems like a different ask than above. That would be handled by calling StopAsync on the host. (Not sure how accessible that is from WebApplicationFactory).

@ajeckmans
Copy link

ajeckmans commented Sep 23, 2021

Just want to add another use case. Our application does not migrate the database (this is deferred onto a different application that is guaranteed to run before any other in kubernetes). However in our tests we do want this to run and we want a separate database for all tests as they run in parallel. To that end we inject a startup filter that is ran before all others. This startup filter ties into IHostApplicationLifetime.ApplicationStopping and IHostApplicationLifetime.ApplicationStopped to delete the database it has just created and migrated.

Currently we're having to override WebApplicationFactory.Dispose and through reflection get the _host field and call StopAsync on that to trigger this path. An approach mentioned in #29348.

There is other reasons why we want these IHostApplicationLifetime callbacks to be triggered, but this one, which is solely for the tests itself, was not yet mentioned here.

I guess the only question left to ask is: Is this something that you guys would like to see implemented? If so, I would be willing to commit some time to make a PR.

@GeeWee
Copy link
Author

GeeWee commented Sep 23, 2021

Maybe also a little extra information for when this would be useful.

I've been developing BetterHostedServices which relies on being able to understand when the application is being shut down to do some of its magic. Currently I have to jump through a few hoops because it's not easily possible to integration-test anything that relies on shutting down the TestServer.

@tapizquent
Copy link

Are there any workarounds for this? It seems that WebApplicationFactory always leave the process running.
We are working with a .NET 5 app.

@quixoticaxis
Copy link

quixoticaxis commented Jan 22, 2022

Are there any workarounds for this? It seems that WebApplicationFactory always leave the process running.
We are working with a .NET 5 app.

@tapizquent , there was a PR that forces the host to stop on disposing the factory.
Disposing the factory should do the trick.

For other cases, you can use the hack I posted in #29348.

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-few This issue impacts only small number of customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug. investigate severity-minor This label is used by an internal tool
Projects
None yet
Development

Successfully merging a pull request may close this issue.