Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions TUnit.AspNetCore/TestWebApplicationFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Microsoft.AspNetCore.TestHost;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using TUnit.AspNetCore.Extensions;
using TUnit.AspNetCore.Interception;
using TUnit.Core;
Expand All @@ -14,31 +15,53 @@ namespace TUnit.AspNetCore;
/// </summary>
public abstract class TestWebApplicationFactory<TEntryPoint> : WebApplicationFactory<TEntryPoint> where TEntryPoint : class
{

public WebApplicationFactory<TEntryPoint> GetIsolatedFactory(
TestContext testContext,
WebApplicationTestOptions options,
Action<IServiceCollection> configureServices,
Action<WebHostBuilderContext, IConfigurationBuilder> configureConfiguration,
Action<IServiceCollection> configureIsolatedServices,
Action<IConfigurationBuilder> configureIsolatedStartupConfiguration,
Action<WebHostBuilderContext, IConfigurationBuilder> configureIsolatedAppConfiguration,
Action<IWebHostBuilder>? configureWebHostBuilder = null)
{
return WithWebHostBuilder(builder =>
{
// Apply user's escape hatch configuration first
configureWebHostBuilder?.Invoke(builder);
var configurationBuilder = new ConfigurationManager();
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disposable 'ConfigurationManager' is created but not disposed.

Suggested change
var configurationBuilder = new ConfigurationManager();
using var configurationBuilder = new ConfigurationManager();

Copilot uses AI. Check for mistakes.
ConfigureStartupConfiguration(configurationBuilder);
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ConfigureStartupConfiguration method is invoked in two different code paths:

  1. In CreateHostBuilder (line 63) for the global factory instance
  2. In GetIsolatedFactory (line 30) for isolated per-test factories

This means that for isolated factories created via GetIsolatedFactory, the startup configuration from ConfigureStartupConfiguration will be applied twice - once when the base factory's host is created, and once when building the isolated factory. This could lead to configuration being duplicated or overridden unexpectedly.

Consider whether ConfigureStartupConfiguration should only be applied in CreateHostBuilder (for the global factory), or if the method in GetIsolatedFactory should skip calling the base ConfigureStartupConfiguration when it's already been applied by the base factory.

Suggested change
ConfigureStartupConfiguration(configurationBuilder);

Copilot uses AI. Check for mistakes.
configureIsolatedStartupConfiguration(configurationBuilder);

foreach (var keyValuePair in configurationBuilder.AsEnumerable())
{
builder.UseSetting(keyValuePair.Key, keyValuePair.Value);
}

// Then apply standard configuration
builder
.ConfigureAppConfiguration(configureConfiguration)
.ConfigureAppConfiguration(configureIsolatedAppConfiguration)
.ConfigureTestServices(services =>
{
configureServices(services);
configureIsolatedServices(services);
services.AddSingleton(testContext);
});

if (options.EnableHttpExchangeCapture)
{
builder.ConfigureTestServices(services => services.AddHttpExchangeCapture());
}

configureWebHostBuilder?.Invoke(builder);
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The configureWebHostBuilder escape hatch is now called AFTER the standard configuration (services, configuration, HTTP exchange capture). The PR description mentions "enhance test configuration isolation" but this changes the order compared to the comment on the removed line 29 which said "Apply user's escape hatch configuration first".

This is a subtle behavioral change: previously the escape hatch ran first and could be overridden by standard configuration; now it runs last and can override standard configuration. While the new order may be more useful (allowing tests to override framework defaults), this is a breaking change that should be:

  1. Documented in the method's XML comments
  2. Called out in the PR description or release notes
  3. Verified against existing tests that may rely on the old order

Consider whether this ordering change is intentional and desired.

Copilot uses AI. Check for mistakes.
});
}

Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new ConfigureStartupConfiguration method should have XML documentation comments to explain:

  • Its purpose and when it's called
  • How it differs from ConfigureTestConfiguration (in WebApplicationTest)
  • That it's called during host builder creation, before the application starts
  • An example of when to use it versus app configuration

This is especially important since this is part of the public API surface that test authors will extend.

Suggested change
/// <summary>
/// Configures host-level startup configuration for tests that use this factory.
/// </summary>
/// <remarks>
/// <para>
/// This method is invoked during creation of the generic host builder, before the
/// application startup code runs. Values added to <paramref name="configurationBuilder"/>
/// become part of the host configuration and can be overridden by later configuration
/// sources in the application configuration pipeline.
/// </para>
/// <para>
/// Unlike <c>ConfigureTestConfiguration</c> on <c>WebApplicationTest</c>, which configures
/// the application configuration (for example via
/// <see cref="IWebHostBuilder.ConfigureAppConfiguration(System.Action{WebHostBuilderContext, IConfigurationBuilder})"/>) for a
/// specific test run, this method is intended for host-level or global defaults that
/// apply to all tests using this <see cref="TestWebApplicationFactory{TEntryPoint}"/>.
/// It is called as part of <see cref="IHostBuilder.ConfigureHostConfiguration(System.Action{IConfigurationBuilder})"/>.
/// </para>
/// <para>
/// Use this method when you need to establish baseline configuration for the test host,
/// such as default environment variables, in-memory key/value pairs, or connection strings
/// that should be present for every test. For per-test or scenario-specific configuration,
/// prefer using the <c>configureIsolatedStartupConfiguration</c> or
/// <c>configureIsolatedAppConfiguration</c> delegates passed to
/// <see cref="GetIsolatedFactory(TestContext, WebApplicationTestOptions, System.Action{IServiceCollection}, System.Action{IConfigurationBuilder}, System.Action{WebHostBuilderContext, IConfigurationBuilder}, System.Action{IWebHostBuilder}?)"/>
/// or application-level hooks such as <c>ConfigureTestConfiguration</c>.
/// </para>
/// </remarks>
/// <param name="configurationBuilder">
/// The <see cref="IConfigurationBuilder"/> used to construct the host configuration
/// before the application starts.
/// </param>

Copilot uses AI. Check for mistakes.
protected virtual void ConfigureStartupConfiguration(IConfigurationBuilder configurationBuilder)
{
}

protected override IHostBuilder? CreateHostBuilder()
{
var hostBuilder = base.CreateHostBuilder();

hostBuilder?.ConfigureHostConfiguration(ConfigureStartupConfiguration);

return hostBuilder;
}
}
1 change: 1 addition & 0 deletions TUnit.AspNetCore/WebApplicationTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public async Task InitializeFactoryAsync(TestContext testContext)
testContext,
_options,
ConfigureTestServices,
ConfigureTestConfiguration,
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing ConfigureTestConfiguration here causes it to be applied twice - once as startup configuration (this line) and once as app configuration (line 119). This was introduced when the new configureIsolatedStartupConfiguration parameter was added to GetIsolatedFactory.

The startup configuration parameter should either be an empty lambda or a separate dedicated method. Consider changing line 118 to _ => { } unless there's a specific need for different startup vs app configuration behavior in tests.

Suggested change
ConfigureTestConfiguration,
_ => { },

Copilot uses AI. Check for mistakes.
(_, config) => ConfigureTestConfiguration(config),
ConfigureWebHostBuilder);

Expand Down
10 changes: 9 additions & 1 deletion TUnit.Example.Asp.Net.TestProject/WebApplicationFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,16 @@ protected override void ConfigureWebHost(IWebHostBuilder builder)
{
{ "Database:ConnectionString", PostgreSql.Container.GetConnectionString() },
{ "Redis:ConnectionString", Redis.Container.GetConnectionString() },
{ "Kafka:ConnectionString", Kafka.Container.GetBootstrapAddress() }
{ "Kafka:ConnectionString", Kafka.Container.GetBootstrapAddress() },
});
});
}

protected override void ConfigureStartupConfiguration(IConfigurationBuilder configurationBuilder)
{
configurationBuilder.AddInMemoryCollection(new Dictionary<string, string?>
{
{ "SomeKey", "SomeValue" }
});
}
}
5 changes: 5 additions & 0 deletions TUnit.Example.Asp.Net/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@

var builder = WebApplication.CreateBuilder(args);

if (builder.Configuration["SomeKey"] != "SomeValue")
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This validation check will throw an exception during application startup if the configuration value is not set correctly. While this demonstrates that the startup configuration feature works, throwing an exception in production code for test-specific configuration is problematic.

Consider one of these alternatives:

  1. Remove this validation after verifying the feature works, as it's test infrastructure code in production code
  2. Guard it with an environment check: if (builder.Environment.IsDevelopment() && builder.Configuration["SomeKey"] != "SomeValue")
  3. Move this validation to a dedicated test that verifies the configuration was set correctly, rather than embedding it in Program.cs

The current approach will cause the application to fail to start in any non-test environment where "SomeKey" isn't set to "SomeValue".

Suggested change
if (builder.Configuration["SomeKey"] != "SomeValue")
if (builder.Environment.IsDevelopment() &&
builder.Configuration["SomeKey"] != "SomeValue")

Copilot uses AI. Check for mistakes.
{
throw new InvalidOperationException("SomeKey is not SomeValue - But we set it in WebApplicationFactory");
}

builder.Services.AddOpenApi();
builder.Services.Configure<DatabaseOptions>(builder.Configuration.GetSection(DatabaseOptions.SectionName));
builder.Services.Configure<RedisOptions>(builder.Configuration.GetSection(RedisOptions.SectionName));
Expand Down
Loading