refactor: move CorrelatedTUnitLogger to TUnit.Logging.Microsoft and auto-inject handlers

[thomhurst]

Summary

• Move CorrelatedTUnitLogger and CorrelatedTUnitLoggerProvider from TUnit.AspNetCore.Core to TUnit.Logging.Microsoft, removing the ASP.NET dependency so correlated logging works purely via Activity baggage and TestContext.ResolveFromActivityBaggage()
• Auto-inject ActivityPropagationHandler and TUnitTestIdHandler into all HttpClient instances created by TestWebApplicationFactory via new method shadows on CreateDefaultClient/CreateClient, obsoleting CreateClientWithTestContext
• Eagerly capture TestContext in TUnitTestIdHandler at construction time (with dynamic fallback), and deduplicate handler-prepend logic in TracedWebApplicationFactory
• Add 7 comprehensive tests covering Activity baggage resolution, correlated logger behavior, duplicate suppression, log level filtering, and error routing

Test plan

• All 13 ASP.NET Core tests pass (dotnet run --framework net10.0 in TUnit.AspNetCore.Tests)
• TUnit.AspNetCore.Core and TUnit.Logging.Microsoft build with 0 errors, 0 warnings
• CI passes across net8.0/net9.0/net10.0

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 19 complexity

Metric	Results
Complexity	19

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

[claude]

Code Review

This is a well-motivated refactor with a clear goal: remove the ASP.NET dependency from CorrelatedTUnitLogger* and make handler injection automatic. The Activity baggage fallback in TestContext.Current (line 129 of TestContext.cs) makes the approach sound. Good test coverage and the TUnitTestIdHandler eager-capture change are both solid improvements.

That said, there are a few issues that need attention before merge.

---

Critical: new vs override breaks polymorphism

TestWebApplicationFactory.cs lines 93–118

All three CreateClient/CreateDefaultClient methods use the new (hiding) keyword instead of override. But these are declared virtual in WebApplicationFactory<TEntryPoint> in the ASP.NET Core source. This means the auto-injection is silently bypassed whenever the factory is accessed through a WebApplicationFactory<TEntryPoint> reference — which happens commonly:

// WithWebHostBuilder returns WebApplicationFactory<T>, not TestWebApplicationFactory<T>
var isolatedFactory = factory.WithWebHostBuilder(b => { ... });
using var client = isolatedFactory.CreateClient(); // ❌ handlers NOT injected

The fix is to use override instead of new:

public override HttpClient CreateDefaultClient(params DelegatingHandler[] handlers)
public override HttpClient CreateClient()

The TracedWebApplicationFactory wrapper pattern avoids this problem entirely (because it composes rather than inherits), which is why it was written as a wrapper in the first place. The method-hiding approach in TestWebApplicationFactory has the same fragility that TracedWebApplicationFactory was designed to avoid.

---

CreateClient() silently ignores ClientOptions

TestWebApplicationFactory.cs line 114

The new CreateClient() calls CreateDefaultClient() + ConfigureClient() directly. But the base WebApplicationFactory.CreateClient() routes through CreateClient(ClientOptions), which respects the factory's configured options (AllowAutoRedirect, HandleCookies, MaxAutomaticRedirections, etc.).

By bypassing ClientOptions, any user who has overridden or configured these options will find them silently ignored when calling CreateClient(). The correct implementation should delegate to the base method after ensuring the handler override works:

public override HttpClient CreateClient(WebApplicationFactoryClientOptions options)
{
    // handlers are now prepended via the overridden CreateDefaultClient
    return base.CreateClient(options);
}

---

Missing CreateClient(WebApplicationFactoryClientOptions) shadow

TestWebApplicationFactory.cs

Since CreateClient() is shadowed/overridden, CreateClient(WebApplicationFactoryClientOptions) must also be shadowed/overridden. Otherwise:

var client = factory.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false });
// ❌ handlers NOT injected (calls base overload directly)

---

TracedWebApplicationFactory still duplicates handler injection logic

TracedWebApplicationFactory.cs lines 44, 53–54

The PR aimed to consolidate handler injection into TestWebApplicationFactory, but TracedWebApplicationFactory still manually prepends ActivityPropagationHandler and TUnitTestIdHandler. This is two separate sources of truth for the same logic.

TracedWebApplicationFactory wraps a generic WebApplicationFactory<TEntryPoint> (not necessarily a TestWebApplicationFactory<TEntryPoint>), so deduplication is non-trivial. But at minimum the duplication is worth calling out. Consider extracting the handler-prepend logic to a shared helper:

internal static DelegatingHandler[] PrependTUnitHandlers(DelegatingHandler[] handlers)
{
    var all = new DelegatingHandler[handlers.Length + 2];
    all[0] = new ActivityPropagationHandler();
    all[1] = new TUnitTestIdHandler();
    Array.Copy(handlers, 0, all, 2, handlers.Length);
    return all;
}

---

Minor: CorrelatedLogger_SkipsOutput_WhenPerTestLoggerActive test correctness

ActivityBaggageCorrelationTests.cs line 119

The test creates TUnitLoggerProvider(testContext) which registers in TUnitLoggingRegistry.PerTestLoggingActive, then checks that CorrelatedTUnitLogger skips output. This looks correct — the PerTestLoggingActive check in CorrelatedTUnitLogger.Log uses TestContext.Current at call time, which is still in scope here (no flow suppression).

However, the test doesn't verify the CorrelatedTUnitLogger would have written output absent the suppression — it's testing the negative case only. A more rigorous test would also assert the positive case (logger writes when TUnitLoggerProvider is absent) in the same test.

---

What's good

• Moving CorrelatedTUnitLogger* to TUnit.Logging.Microsoft is a good decoupling decision — it removes an ASP.NET Core dependency from the shared path.
• The _testContext = TestContext.Current eager capture in TUnitTestIdHandler correctly handles the async/await case where AsyncLocal values don't flow across thread transitions.
• The Activity baggage fallback in TestContext.Current (line 129, TestContext.cs) is properly plumbed, making the Activity-based tests in ActivityBaggageCorrelationTests valid.
• The 7 new tests are well-structured with good use of RunWithSuppressedFlow to isolate the baggage path from the AsyncLocal path.
• Deduplicating the TracedWebApplicationFactory CreateDefaultClient(Uri, ...) body (now just two lines instead of the array-copy block) is a clean improvement.

---

Summary

The new vs override issue is the most important: it means users accessing the factory via WebApplicationFactory<T> (e.g., after WithWebHostBuilder) will silently not get the auto-injected handlers, which defeats the feature's purpose. The ClientOptions bypass is a secondary correctness issue. Addressing both with override + adding a CreateClient(WebApplicationFactoryClientOptions) override should resolve the polymorphism problems cleanly.

[thomhurst]

Thanks for the detailed review. A few responses:

new vs override — not applicable

CreateClient(), CreateClient(WebApplicationFactoryClientOptions), CreateDefaultClient(params DelegatingHandler[]), and CreateDefaultClient(Uri, params DelegatingHandler[]) are all non-virtual on WebApplicationFactory<TEntryPoint> (verified against Microsoft.AspNetCore.Mvc.Testing v10.0.5 in the NuGet cache). Using override produces CS0506. Method hiding with new is the only option.

This is also why TracedWebApplicationFactory was designed as a composition wrapper rather than inheriting from WebApplicationFactory — it avoids the non-virtual method limitation entirely.

In TUnit's test patterns, factories are always typed as the concrete TestWebApplicationFactory<T> subclass (via [ClassDataSource]), so the new methods are always resolved correctly.

CreateClient() and ClientOptions

Since CreateClient(WebApplicationFactoryClientOptions) is also non-virtual, there's no way to intercept it. Our CreateClient() calls CreateDefaultClient() + ConfigureClient(client) which handles BaseAddress. The default ClientOptions behavior (auto-redirect, cookies) is handled by the base HttpClientHandler defaults, so the common case works. Users needing custom options can call CreateDefaultClient(customHandlers) directly.

Handler deduplication

TracedWebApplicationFactory wraps a generic WebApplicationFactory<T> (not a TestWebApplicationFactory<T>), so it can't delegate to the base class's shadow methods. The 4-line array-prepend pattern is simple enough that a shared helper would add indirection without meaningful benefit.

Test positive case

The positive case (correlated logger writes output when no per-test logger is active) is already covered by CorrelatedLogger_WritesToCorrectTest_ViaActivityBaggage and CorrelatedLogger_RoutesErrorToStdErr in the same test class, plus the HTTP integration tests in CorrelatedLoggingResolverTests.