pengweiqhca / Xunit.DependencyInjection

Use Microsoft.Extensions.DependencyInjection to resolve xUnit test cases.
MIT License
370 stars 49 forks source link

Startup process does not complete before first test is executed #122

Closed Will-at-FreedomDev closed 5 months ago

Will-at-FreedomDev commented 5 months ago

Describe the bug This might be the expected behavior, but I couldn't find any documentation leading me to believe that it was.

We had this setup, but when you put a delay in Configure(), it was apparent that Startup did not complete before the first test was executed. Actually, many of our tests started and completed before Configure() was completely finished.

public class Startup
{
    public void ConfigureHost(IHostBuilder hostBuilder)
    {
        _ = hostBuilder
            .ConfigureHostConfiguration(builder => { /* omitted */})
            .ConfigureServices((context, services) => { /* omitted */ });
    }

    public void ConfigureServices(IServiceCollection services)
    {
        // omitted
    }

    public void Configure(IServiceScopeFactory serviceScopeFactory)
    {
        // to make issue apparent, you can place a `Task.Delay()` here. Individual tests will start and complete while this is delayed
        Task.Delay(1000 * 60).Wait();

        // In our case, we used this area to create global data in a localdb and configure other services. 
        // It didn't make sense to do this in the initialization of a collection fixture because this startup process applied to more than one collection 
    }
}

To Reproduce Steps to reproduce the behavior:

  1. See above snippet

Expected behavior Nothing of XUnit should be created/initialized/started until Startup and every method, including .Configure() has completed. This includes:

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

Additional context This is our automated testing csproj contents

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

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>

    <IsPackable>false</IsPackable>

    <Configurations>Debug;Release</Configurations>
  </PropertyGroup>

  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
    <WarningsAsErrors>NU1605;1591;2021</WarningsAsErrors>
  </PropertyGroup>

  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'">
    <WarningsAsErrors>NU1605;1591;2021</WarningsAsErrors>
  </PropertyGroup>

  <ItemGroup>
    <None Remove="appsettings.AutomatedTesting.json" />
    <None Update="Resources\fake.png"><CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory></None>
  </ItemGroup>

  <ItemGroup>
    <Content Include="appsettings.AutomatedTesting.json">
      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
      <ExcludeFromSingleFile>true</ExcludeFromSingleFile>
      <CopyToPublishDirectory>PreserveNewest</CopyToPublishDirectory>
    </Content>
  </ItemGroup>

  <ItemGroup>
    <PackageReference Include="Azure.Storage.Blobs" Version="12.19.1" />
    <PackageReference Include="coverlet.collector" Version="3.2.0">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
    <PackageReference Include="Microsoft.Extensions.Configuration.Json" Version="8.0.0" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
    <PackageReference Include="System.Linq.Async" Version="6.0.1" />
    <PackageReference Include="TaskTupleAwaiter" Version="2.0.2" />
    <PackageReference Include="xunit" Version="2.7.0" />
    <PackageReference Include="Xunit.DependencyInjection" Version="9.0.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.7">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>

  <ItemGroup>
    <ProjectReference Include="..\Repository\Repository.csproj" /> <!-- .NET 8 library -->
    <ProjectReference Include="..\Web\Angular\Angular.csproj" /> <!-- ASP.NET Core 8 application -->
  </ItemGroup>

</Project>

This was an acceptable workaround that DID prevent any XUnit constructs from initializing/starting:

public class Startup
{
    public void ConfigureHost(IHostBuilder hostBuilder)
    {
        _ = hostBuilder
            .ConfigureHostConfiguration(builder => { /* omitted */})
            .ConfigureServices((context, services) => 
            { 
                ConfigureServices(services);
                Configure(services.BuildServiceProvider());
            });
    }

    private void ConfigureServices(IServiceCollection services)
    {
        // omitted
    }

    private void Configure(IServiceProvider serviceProvider)
    {
        // no XUnit construct initialization occurs now until this is completed.
        Task.Delay(1000 * 60).Wait();
    }
}

Edit: fixed a typo in the workaround code snippet.

pengweiqhca commented 5 months ago

You should use IHostedService to initialize data.

Will-at-FreedomDev commented 5 months ago

You should use IHostedService to initialize data.

Can you elaborate? Will fixtures/tests wait to initialize if we implement this?

Also, I feel like this is still a workaround to the bug report I submitted. Unless this is a known behavior/bug/limitation, that comment doesn't really address the root issue.

But as mentioned, we have an acceptable workaround as it is. I just wanted to submit this issue in case it was not known / other people will be able to find this issue for documentation purposes at least.

pengweiqhca commented 5 months ago
services.AddHostedService<SomeHostedService>();

class SomeHostedService : IHostedService
{
    public Task StartAsync(CancellationToken stoppingToken)
    {
        // Do any thing if you want.
    }

    public Task StopAsync(CancellationToken stoppingToken)
    {
        // Do any thing if you want.
    }
}
Will-at-FreedomDev commented 5 months ago

That works too as a workaround to the bug I reported. I would suggest updating some documentation somewhere to avoid using the host methods ConfigureServices and Configure as they have this race condition issue. I feel like the bug itself isn't worth addressing (assuming it's possible)

pengweiqhca commented 5 months ago

Thanks for your reminder. I have added a section titled Initialize some data on startup.