feat: add ConfigureAppHost hook to AspireFixture#5202
Conversation
Add a protected virtual ConfigureAppHost method that allows users to configure DistributedApplicationOptions and HostApplicationBuilderSettings before the Aspire builder is created. Closes #5199
There was a problem hiding this comment.
Code Review: feat: add ConfigureAppHost hook to AspireFixture
Overall assessment: This is a clean, well-scoped addition that follows the established patterns in AspireFixture. The implementation is correct and the documentation is thorough. A few points are worth discussing before merging.
What the PR does
It exposes the configureBuilder callback parameter of DistributedApplicationTestingBuilder.CreateAsync as a protected virtual method on AspireFixture<TAppHost>. This gives subclasses a way to configure low-level DistributedApplicationOptions (e.g. disabling the dashboard) and HostApplicationBuilderSettings before the builder is created, filling the gap identified in issue #5199.
Design observations
1. Method-group vs. lambda — a subtle behavioral difference
The change passes ConfigureAppHost as a method group directly to CreateAsync:
var builder = await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>(Args, ConfigureAppHost);This is fine in practice, but worth noting for future maintainers: this correctly routes through virtual dispatch, so subclass overrides are called as expected. No issue, just worth documenting since delegate capture can trip people up if the method is ever moved to a static context.
2. Consistency with the proposed alternative in the issue
The issue author proposed an Action<...> property (ConfigureCallback). The chosen approach of a virtual method (ConfigureAppHost) is preferable — it is more discoverable (shows up in IDE override lists), harder to misuse (no accidental null delegate), and consistent with how ConfigureBuilder and WaitForResourcesAsync are structured. Good choice.
3. Empty body notation
The default empty implementation uses an inline empty body:
protected virtual void ConfigureAppHost(DistributedApplicationOptions options, HostApplicationBuilderSettings settings) { }ConfigureBuilder uses the same style, so this is consistent with the surrounding code.
4. CancellationToken passthrough
The DistributedApplicationTestingBuilder.CreateAsync overload being called accepts an optional CancellationToken that defaults to default here, matching the pre-existing call that was replaced. This is consistent with the rest of InitializeAsync. Worth noting as a potential future improvement: threading a real cancellation token through the builder creation step too.
Suggestions
Consider passing the CancellationToken explicitly (follow-up)
The InitializeAsync method already creates timeout-scoped CancellationTokenSource objects for later phases. For correctness under cancellation, consider threading a top-level token through the builder creation step as well:
var builder = await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>(Args, ConfigureAppHost, cancellationToken);This is a broader refactor and not strictly required for this PR — noting it as a potential follow-up.
XML doc: <see cref> resolution
The summary cross-references ConfigureBuilder, which is great for discoverability. One small nit: verify the <see cref> for DistributedApplicationTestingBuilder.CreateAsync{TEntryPoint}(...) resolves correctly in generated docs — cref signatures with fully-qualified type names can sometimes produce broken links in DocFX/Sandcastle depending on the using directives in scope.
Summary
The change is small, correct, well-documented, and follows the established patterns in AspireFixture. There are no architectural concerns or anti-patterns introduced. The main open question (CancellationToken threading) can reasonably be deferred as a follow-up.
Verdict: approve with minor optional nits.
Update lifecycle diagram, virtual methods table, and Args/ConfigureBuilder tip to include the new ConfigureAppHost hook.
There was a problem hiding this comment.
Follow-up review (new commit: 0e4ede5c)
The second commit adds documentation for ConfigureAppHost in docs/docs/examples/aspire.md. This directly addresses the discoverability concern from my previous review.
What changed
- Lifecycle diagram: Added
↳ ConfigureAppHost()as a sub-step underCreateAsync, clearly showing it runs beforeConfigureBuilder. This is well-placed and the visual hierarchy (↳ arrow) communicates the relationship intuitively. - Override reference table:
ConfigureAppHost(options, settings)is now listed in the correct order betweenArgsandConfigureBuilder, with a clear default value (No-op) and description. - Tip block: Updated from "Args vs ConfigureBuilder" to "Args vs ConfigureAppHost vs ConfigureBuilder" — the added bullet explains when to use each hook with a concrete example (
DisableDashboard), making the decision easy for users.
Assessment
The documentation is accurate, ordered correctly relative to the lifecycle, and provides actionable guidance. The concrete DisableDashboard example in the tip is exactly the right level of specificity.
Previous observations (CancellationToken threading as a follow-up, XML doc cref resolution) remain open but are not introduced or worsened by this commit.
This PR is ready to merge. Both the implementation and documentation are clean, consistent, and complete.
Summary
protected virtual void ConfigureAppHost(DistributedApplicationOptions, HostApplicationBuilderSettings)method toAspireFixture<TAppHost>DistributedApplicationTestingBuilder.CreateAsyncso users can configure low-level AppHost options before the builder is createdArgs,ConfigureBuilder,ResourceTimeout, etc.)Closes #5199
Usage
Test plan
dotnet build TUnit.Aspire/TUnit.Aspire.csprojpasses with 0 errors