Skip to content

Comments

feat: add Args property to AspireFixture for AppHost arguments#4982

Merged
thomhurst merged 2 commits intomainfrom
feat/aspire-fixture-args
Feb 22, 2026
Merged

feat: add Args property to AspireFixture for AppHost arguments#4982
thomhurst merged 2 commits intomainfrom
feat/aspire-fixture-args

Conversation

@thomhurst
Copy link
Owner

Summary

  • Adds a protected virtual string[] Args property to AspireFixture<TAppHost> that forwards command-line arguments to DistributedApplicationTestingBuilder.CreateAsync<TAppHost>(args)
  • Fixes the gap where ConfigureBuilder is called after CreateAsync, making it impossible to set configuration values consumed during AppHost builder creation
  • Updates docs with the new property, usage example, and guidance on Args vs ConfigureBuilder

Closes #4973

Test plan

  • Verify TUnit.Aspire builds successfully
  • Verify that overriding Args in a subclass passes values through to CreateAsync
  • Verify that the default (empty args) preserves existing behavior (no-args CreateAsync call)

…s to AppHost

Expose a virtual `Args` property on `AspireFixture<TAppHost>` so users can
pass command-line arguments to `DistributedApplicationTestingBuilder.CreateAsync`.
This allows setting configuration values that the AppHost consumes during
builder creation, before `ConfigureBuilder` is called.

Closes #4973
Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Code Review

This PR addresses a real gap — runs after , so configuration consumed during AppHost builder construction was unreachable. The fix is correct and the docs are clear.

One Architectural Concern: Unnecessary Conditional Branch

The change introduces two code paths where one would do:

// Current (added by this PR):
var args = Args;
var builder = args.Length > 0
    ? await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>(args)
    : await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>();

The string[] args overload of CreateAsync almost certainly handles an empty array identically to the no-arg overload (it maps to CreateBuilder(args) inside Aspire, and an empty args array is the same as no args for IConfiguration). The conditional therefore creates two divergent code paths for no practical reason.

Suggested simplification:

var builder = await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>(Args);

This removes the branch and the local variable entirely, making the intent clearer: "always pass args, even if empty." If there is a subtle difference between the two overloads that justifies the conditional, it should be called out in a comment — otherwise future readers will wonder if it was intentional.

Minor: Documentation arg format

The docs example uses:

protected override string[] Args =>
[
    "UseVolumes=false",
    "UsePostgresWithPersistentLifetime=false"
];

The Key=value format (without --) works with .NET's command-line configuration provider, but --Key=value is the idiomatic format. Consider showing the more standard form to avoid confusion for users who are used to .NET CLI conventions.

What's Good

  • The design is correct — it's opt-in and doesn't break existing subclasses.
  • Caching in a local before the conditional avoids calling a virtual property twice, which is good practice.
  • Docs clearly explain the "Args vs ConfigureBuilder" distinction, which is exactly the right thing to surface.
  • The XML doc comments are thorough and accurate.

Overall this is a well-scoped, targeted fix. The main ask is to simplify the conditional away, or document why it's needed.

Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Code Review

This PR addresses a real gap — ConfigureBuilder runs after CreateAsync, so configuration consumed during AppHost builder construction was unreachable. The fix is correct and the docs are clear.

One Architectural Concern: Unnecessary Conditional Branch

The InitializeAsync change introduces two code paths where one would do:

// Current (added by this PR):
var args = Args;
var builder = args.Length > 0
    ? await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>(args)
    : await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>();

The string[] args overload of CreateAsync almost certainly handles an empty array identically to the no-arg overload (it maps to CreateBuilder(args) inside Aspire, and an empty args array is the same as no args for IConfiguration). The conditional therefore creates two divergent code paths for no practical reason.

Suggested simplification:

var builder = await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>(Args);

This removes the branch and the local variable entirely, making the intent clearer: "always pass args, even if empty." If there is a subtle difference between the two overloads that justifies the conditional, it should be called out in a comment — otherwise future readers will wonder if it was intentional.

Minor: Documentation arg format

The docs example uses:

protected override string[] Args =>
[
    "UseVolumes=false",
    "UsePostgresWithPersistentLifetime=false"
];

The Key=value format (without --) works with .NET's command-line configuration provider, but --Key=value is the idiomatic format. Consider showing the more standard form to avoid confusion for users who are used to .NET CLI conventions.

What's Good

  • The protected virtual design is correct — it's opt-in and doesn't break existing subclasses.
  • Caching Args in a local before the conditional avoids calling a virtual property twice, which is good defensive practice.
  • The docs clearly explain the "Args vs ConfigureBuilder" distinction, which is exactly the right context to surface.
  • The XML doc comments are thorough and accurate.

Overall this is a well-scoped, targeted fix. The main ask is to simplify the conditional away, or document why it's needed.

… format

Address review feedback:
- Always pass Args to CreateAsync instead of branching on empty
- Use --Key=value format in docs/XML examples for .NET CLI consistency
Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Follow-up Review

Both concerns from the previous review have been addressed in commit ba7971fd:

Conditional branch removed ✅

The InitializeAsync call is now simply:

var builder = await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>(Args);

Clean and direct — no unnecessary branching, no redundant local variable.

Documentation arg format fixed ✅

The docs now use the idiomatic --Key=value form (e.g., "--UseVolumes=false"), consistent with .NET CLI conventions and what users will expect.

Overall

This is a well-scoped, minimal fix for a real limitation (#4973). The protected virtual string[] Args => []; design is correct: opt-in, backward-compatible, easy to discover, and clearly documented with an "Args vs ConfigureBuilder" guide. Nothing further to address.

Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Follow-up Review

Both concerns from the previous review have been addressed in commit ba7971fd:

Conditional branch removed

The InitializeAsync call is now simply:

var builder = await DistributedApplicationTestingBuilder.CreateAsync<TAppHost>(Args);

Clean and direct — no unnecessary branching, no redundant local variable.

Documentation arg format fixed

The docs now use the idiomatic --Key=value form (e.g., "--UseVolumes=false"), consistent with .NET CLI conventions and what users will expect.

Overall

This is a well-scoped, minimal fix for a real limitation (#4973). The protected virtual string[] Args => []; design is correct: opt-in, backward-compatible, easy to discover, and clearly documented with an "Args vs ConfigureBuilder" guide. Nothing further to address.

@thomhurst thomhurst merged commit 1d69f20 into main Feb 22, 2026
15 of 16 checks passed
@thomhurst thomhurst deleted the feat/aspire-fixture-args branch February 22, 2026 21:20
@claude claude bot mentioned this pull request Feb 22, 2026
1 task
This was referenced Feb 23, 2026
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