Hosting.Testing: follow-ups from #21547 (OnBuilt timeout coverage and throwing OnBuilt test)

Open Beginner friendly
  • Is something wrong?
#21,748 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 2 days

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
75/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
csharp
Domain
testing

Research direction

Read src/Aspire.Hosting.Testing/DistributedApplicationEntryPointInvoker.cs and the DistributedApplicationFactory handling described in the issue. Check the existing tests for OnBuilt overrides, then add coverage for a throwing override using --crash-after-build and verify StartAsync throws that exception. Also change the task completion order so the build timeout excludes time spent in the callback.

Written by the indexing model from the issue text.

Description

needs-area-label

Follow-up to #21547 (Fix race publishing the built app in DistributedApplicationFactory). @sebastienros approved that PR and flagged two minor items as fine for a follow-up.

1. Build timeout still covers time spent in OnBuilt

In src/Aspire.Hosting.Testing/DistributedApplicationEntryPointInvoker.cs, the DistributedApplicationBuilt handler now calls owner._onBuilt?.Invoke(app) before owner._appTcs.TrySetResult(app). The factory awaits that task with WaitAsync(cancellationToken) under the build timeout (5 minutes by default, or DOTNET_HOST_FACTORY_RESOLVER_DEFAULT_TIMEOUT_IN_SECONDS). Previously OnBuiltCore ran after the factory call returned, so time spent in a user's OnBuilt override was not counted.

If an override blocks past the remaining budget, WaitAsync cancels, the catch in DistributedApplicationFactory.InvokeEntryPoint faults _exitTcs/_startedTcs, and StartAsync throws TaskCanceledException even though the build succeeded.

Fix: since nothing in the factory reads the task's result any more, complete the task first, then invoke the callback:

// Complete the task first so the factory's build timeout doesn't cover time spent in OnBuilt.
owner._appTcs.TrySetResult(app);
owner._onBuilt?.Invoke(app);
2. No test covers a throwing OnBuilt override

The catch in DistributedApplicationFactory.OnApplicationBuilt now runs inside the DistributedApplicationBuilt event, so it is what stops an exception from an OnBuilt override escaping into the AppHost's Build() call. The only OnBuilt override in the tests doesn't throw, so a regression would go unnoticed.

Fix: add a test with a factory whose OnBuilt throws, run with --crash-after-build so the app doesn't start, asserting that StartAsync throws the override's exception.

🤖 Posted by AspireAgent (AI Agent)

Dominant language
C#
Stars
6.4k
Forks
1k
Avg merge
3d 3h
Merged PRs (30d)
349

Getting set up

Open in Codespaces

Starts the project's dev container in your browser, under your own GitHub account.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from microsoft/aspire

All issues in microsoft/aspire

Similar issues

More C# issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.