Skip to content

fix(aspnetcore): initialize web application factories through services - #6956

Merged
thomhurst merged 1 commit into
mainfrom
issue-6955-initialize-services
Oct 3, 2026
Merged

thomhurst merged 1 commit into
mainfrom
issue-6955-initialize-services

Conversation

@thomhurst

@thomhurst thomhurst commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

Description

WebApplicationTest.InitializeFactoryAsync eagerly initializes isolated applications through WebApplicationFactory.Server, which is specific to TestServer and is unsupported for Kestrel factories. Initialize through Services instead: this starts the application with either transport while preserving the existing semaphore, cancellation, and disposal behavior.

Related Issue

Related to #6955. This is a partial compatibility fix and should not close that issue: the separate WithWebHostBuilder Kestrel configuration/client delegation bug is tracked in dotnet/aspnetcore#69655.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Checklist

  • Reviewed the contributing guidelines and project code style.
  • Ran the existing ASP.NET Core regression suite, including isolated TestServer clients, cookies, redirects, logging, and hosted-service behavior.
  • No public API, source generator, reflection, or discovery changes; API snapshots, dual-mode discovery tests, and AOT publishing are not applicable.

Testing

dotnet test --project tests/TUnit.AspNetCore.Tests/TUnit.AspNetCore.Tests.csproj

All 324 tests passed: 108 on net8.0, 108 on net9.0, and 108 on net10.0. Existing regression coverage is reused; no new tests were added for this property substitution.

Also verified a standalone net10.0 reproduction using Microsoft.AspNetCore.Mvc.Testing 10.0.12: accessing Services starts a directly configured Kestrel factory and its client successfully requests /ping. The upstream report includes the reproduction and the remaining derived-factory failures.

Summary by CodeRabbit

  • Bug Fixes
    • Improved test application initialization for both TestServer- and Kestrel-based hosting, helping ensure services are ready before tests proceed. This provides more consistent startup behavior across supported hosting modes.

@thomhurst
thomhurst deployed to Pull Requests October 2, 2026 23:43 — with GitHub Actions Active
@thomhurst
thomhurst deployed to Pull Requests October 2, 2026 23:43 — with GitHub Actions Active
@thomhurst
thomhurst deployed to Pull Requests October 2, 2026 23:43 — with GitHub Actions Active
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-02T23:45:24.520050Z fc323fc PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dd140c55-45b7-4f6b-9cd1-68aff93cf30b
📥 Commits

Reviewing files that changed from the base of the PR and between ac4fc82 and fc323fc.

📒 Files selected for processing (1)
  • src/TUnit.AspNetCore.Core/WebApplicationTest.cs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The initialization semaphore now guards access to _factory.Services instead of _factory.Server. Updated comments describe the synchronous access and the host initialization supported by each property.

Changes

Host initialization

Layer / File(s) Summary
Guard Services initialization
src/TUnit.AspNetCore.Core/WebApplicationTest.cs
The semaphore now guards _factory.Services. Comments state that accessing Services initializes TestServer and Kestrel, while Server supports only TestServer.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to fc323

Host initialization remains serialized, and the change enables the inspected Kestrel factory path without changing TestServer behavior. No material merge risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to fc323

The change preserves per-test factory ownership, startup throttling, and disposal delegation. No security-control bypass was demonstrated, but this partial fix does not establish complete Kestrel compatibility or cleanup across derived factories.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The supported scope is application startup within the test process. Where Kestrel is successfully selected, reachable scope follows its configured listeners and application services. The supplied evidence does not establish tenant-wide, environment-wide, or privileged infrastructure exposure.

Trust Boundaries and Controls

  • observed — The wrapper does not add a second host owner: Services and DisposeAsync directly delegate to the same inner factory. The per-test creation lock and startup semaphore remain separate controls over shared factory mutation and concurrent host builds.

Resilience and Maintainability Implications

  • observed — Inspected upstream 10.0.12 disposal stops and disposes _host but contains no explicit cleanup of the legacy Kestrel _webHost field populated by CreateKestrelServer. This is pre-existing dependency behavior, not a demonstrated PR-introduced listener leak; it limits assurances about cleanup across every transport path.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: initializing ASP.NET Core web application factories through Services.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the host with care,
The Services property waits there.
TestServer and Kestrel start,
A guarded step now plays its part.
The rabbit hops away.

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Review

Small, focused change: InitializeFactoryAsync now touches _factory.Services instead of _factory.Server. Both getters go through WebApplicationFactory.EnsureServer(), so the eager host build still happens. It's now also valid for Kestrel-backed factories, where Server throws. The semaphore, Task.Run, cancellation and disposal paths are untouched, and the updated comments match the behaviour.

Observations (non-blocking):

  • No new test covers a Kestrel factory. I understand the upstream WithWebHostBuilder bug limits what can be tested, but a small test for a directly configured Kestrel factory would guard against regressions, if one is feasible.
  • The _ = _factory.Services discard relies on a property getter's side effect. A short comment already explains the intent, which is enough.

No issues found. LGTM.

@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Medium risk] Changes how test infrastructure initializes web application factories.

The PR appears safe to merge, with a non-blocking gap in Kestrel regression coverage.

Findings

  1. P2 Kestrel path lacks coverage ▶

Summary

The PR changes per-test factory initialization from Server to Services, allowing initialization without requiring a TestServer-specific property.

  • The existing semaphore, cancellation, and lifecycle structure remains unchanged.
  • The Kestrel-specific initialization path has no automated regression coverage.

Reviews (1) · Last reviewed commit: "fix(aspnetcore): initialize web applicat..."

Comment thread src/TUnit.AspNetCore.Core/WebApplicationTest.cs
@thomhurst
thomhurst enabled auto-merge (squash) October 3, 2026 00:22
@thomhurst
thomhurst merged commit 39013cc into main Oct 3, 2026
23 checks passed
@thomhurst
thomhurst deleted the issue-6955-initialize-services branch October 3, 2026 00:22
This was referenced Oct 9, 2026

This branch was successfully deployed

1 active deployment
Pull Requests — fc323fce Deployed Oct 2, 2026 by thomhurst via modularpipeline (macos-latest) #19656
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant