Support running the Aspire dashboard from tests - #19135
Support running the Aspire dashboard from tests#19135Adam Ratzman (adamint) wants to merge 9 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ffeff87e-f284-434d-87d3-843e21a7aebb
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ffeff87e-f284-434d-87d3-843e21a7aebb
Reuse the canonical dashboard URL path, enforce authenticated testing defaults, and preserve cancellation and disposal semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ffeff87e-f284-434d-87d3-843e21a7aebb
Lifecycle correctness: - BuildAsync no longer blocks on the released AppHost when the caller's token fires. It reclaims the late application on a background continuation and rethrows immediately, so cancellation is prompt even when the AppHost never finishes building. - DistributedApplicationFactory.OnBuiltCoreAsync disposes an application that arrives after the factory was disposed. Previously TrySetResult simply returned false and the built DistributedApplication, its service provider, and its orchestrator processes leaked for the lifetime of the test process. - DisposeAsync claims disposal with Interlocked.Exchange. The previous IsCancellationRequested read was not atomic with OnDisposed(), so concurrent disposers could both run teardown and race on the same application. - ObjectDisposedException now consistently reports IDistributedApplicationTestingBuilder rather than leaking the internal factory type name. Behavior: - DistributedApplicationOptions.DisableDashboard = false, the pre-existing spelling of "run the dashboard", now receives the same hardened testing defaults as the new EnableDashboard option instead of only one of them being hardened. - Dashboard endpoints are configured with an empty URL, which is how the product asks for a dynamically assigned port. The previous "http://127.0.0.1:0" parses to a literal fixed port 0 and was only dynamic while DcpPublisher:RandomizePorts stayed true. - A fresh browser token is generated per application and supplied through the command line, which has the highest precedence. Disabling anonymous access only closes the door if a credential exists, and an ambient ASPIRE_DASHBOARD_FRONTEND_BROWSERTOKEN would otherwise share one known token across every application on a CI agent. - DistributedApplicationTestingBuilderOptions.DefaultWaitBehavior lets a debugging session keep a stuck resource alive to inspect it, rather than always tearing the application down. Conventions: - Exception messages moved to Properties/Resources.resx with regenerated xlf, matching the rest of the file. - Commented the Aspire.Hosting InternalsVisibleTo grant, including why the shared file links had to be dropped. - Replaced vacuous Assert.NotNull-on-a-lambda assertions with assertions that exercise the overloads, and added coverage for per-application token isolation, DefaultWaitBehavior, publish-mode rejection through configureBuilder, and disposal of a late-arriving application. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f3f649b5-52c4-4eb5-8d74-c4744e79faf4
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19135Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19135" |
There was a problem hiding this comment.
Pull request overview
Adds first-class support for authenticated Aspire dashboards in integration tests and hardens testing-builder lifecycle behavior.
Changes:
- Adds dashboard testing options and login URL retrieval.
- Enforces secure, dynamic dashboard defaults.
- Fixes cancellation/disposal races and adds lifecycle/dashboard tests.
Show a summary per file
| File | Description |
|---|---|
tests/TestingAppHost1/TestingAppHost1.AppHost/TestingAppHostBuildProbe.cs |
Adds deterministic build lifecycle probe. |
tests/TestingAppHost1/TestingAppHost1.AppHost/Program.cs |
Supports dashboard and build test scenarios. |
tests/Aspire.Hosting.Testing.Tests/DashboardTestingBuilderTests.cs |
Tests dashboard configuration and lifecycle behavior. |
tests/Aspire.Hosting.Testing.Tests/DashboardLoginUrlTests.cs |
Tests dashboard URL and authentication behavior. |
src/Aspire.Hosting/Backchannel/DashboardUrlsHelper.cs |
Escapes tokens and preserves dashboard failures. |
src/Aspire.Hosting/Aspire.Hosting.csproj |
Grants testing package internal access. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.cs.xlf |
Updates Czech resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.de.xlf |
Updates German resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.es.xlf |
Updates Spanish resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.fr.xlf |
Updates French resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.it.xlf |
Updates Italian resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.ja.xlf |
Updates Japanese resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.ko.xlf |
Updates Korean resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.pl.xlf |
Updates Polish resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.pt-BR.xlf |
Updates Brazilian Portuguese resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.ru.xlf |
Updates Russian resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.tr.xlf |
Updates Turkish resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.zh-Hans.xlf |
Updates Simplified Chinese resources. |
src/Aspire.Hosting.Testing/Properties/xlf/Resources.zh-Hant.xlf |
Updates Traditional Chinese resources. |
src/Aspire.Hosting.Testing/Properties/Resources.resx |
Adds dashboard error messages. |
src/Aspire.Hosting.Testing/Properties/Resources.Designer.cs |
Exposes generated resource accessors. |
src/Aspire.Hosting.Testing/DistributedApplicationTestingBuilderOptions.cs |
Adds public dashboard testing options. |
src/Aspire.Hosting.Testing/DistributedApplicationTestingBuilder.cs |
Configures dashboards and fixes build lifecycle handling. |
src/Aspire.Hosting.Testing/DistributedApplicationHostingTestingExtensions.cs |
Adds dashboard login URL API. |
src/Aspire.Hosting.Testing/DistributedApplicationFactory.cs |
Fixes late-application and concurrent disposal handling. |
src/Aspire.Hosting.Testing/Aspire.Hosting.Testing.csproj |
Removes duplicated shared source links. |
Review details
Files not reviewed (1)
- src/Aspire.Hosting.Testing/Properties/Resources.Designer.cs: Generated file
Suppressed comments (1)
src/Aspire.Hosting.Testing/DistributedApplicationTestingBuilder.cs:253
- This existing public overload can now reject publish mode when
configureBuilderenables the dashboard, but the newInvalidOperationExceptionis absent from its XML documentation. Please document the condition as part of this behavior change.
public static IDistributedApplicationTestingBuilder Create(string[] args, Action<DistributedApplicationOptions, HostApplicationBuilderSettings> configureBuilder)
=> CreateCore(args, testingOptions: null, configureBuilder);
- Files reviewed: 25/26 changed files
- Comments generated: 2
- Review effort level: Balanced
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
This overlaps slightly with #18746 |
Restrict the hardened dashboard testing defaults to the explicit DistributedApplicationTestingBuilderOptions.EnableDashboard opt-in. Treating DistributedApplicationOptions.DisableDashboard = false as an equivalent spelling changed behavior for every caller already using it: - The args rebuild read hostBuilderOptions.Args, which the factory has already populated, so arguments a configureBuilder callback assigned to applicationOptions.Args were dropped. DashboardIsNotAddedInPublishMode lost "--publisher manifest", ran in run mode, and saw a dashboard resource it asserted was absent. - The appended dashboard settings overrode the caller's own configuration. GetDashboardUrlsAsync_ReturnsBaseUrl_WhenDashboardAllowsAnonymousAccess asked for anonymous access and got a browser token instead. Also merge caller args instead of choosing one array, so the new opt-in appends to whatever the callback left in place. Build Aspire.Dashboard from Aspire.Hosting.Testing.Tests. The tests that start a real dashboard resolve it from the repo-wide AspireDashboardDir, which was empty in the Hosting.Testing CI leg, failing all eight of them. Add ConcurrentDisposeAsyncRunsTeardownOnce, covering the interlocked disposal claim in DistributedApplicationFactory.DisposeAsync. With the claim reverted to a cancellation-token read the test fails consistently; with the claim in place it passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting.Testing/Properties/Resources.Designer.cs: Generated file
Suppressed comments (2)
src/Aspire.Hosting.Testing/DistributedApplicationTestingBuilder.cs:290
- The PR description still says
DistributedApplicationOptions.DisableDashboard = falseis equivalent to the new option and receives the same hardening, but this code now explicitly preserves the older unhardened behavior (and the new tests assert that behavior). Please update the description and security notes so reviewers and users are not promised hardening on that existing path.
// Only the explicit option turns on the hardened testing defaults below. Setting
// DistributedApplicationOptions.DisableDashboard = false through the configureBuilder callback is the older,
// already-shipped spelling of "run a dashboard", and it has to keep the behavior it shipped with: callers use
// it to exercise dashboard behavior against configuration they chose themselves (fixed URLs, anonymous
// access, an ambient browser token), and in publish mode it is simply ignored because no dashboard resource
// is ever added. Treating it as equivalent to the option silently rewrote that configuration and turned
// publish-mode callers into an InvalidOperationException.
src/Aspire.Hosting.Testing/DistributedApplicationFactory.cs:510
- A concurrent caller that loses this exchange returns from
DisposeAsyncimmediately, potentially before the winner callsOnDisposed()and before any teardown completes. Consequently, awaitingDisposeAsynccan report completion while the application is still running, and public factory operations can still pass the_disposingCtsdisposed check during that window. Store/share the winning disposal task (or completion source) so every concurrent caller awaits the same teardown rather than returning early.
if (Interlocked.Exchange(ref _disposeClaimed, 1) == 1)
{
// Dispose already called.
return;
- Files reviewed: 26/27 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting.Testing/Properties/Resources.Designer.cs: Generated file
Suppressed comments (3)
src/Aspire.Hosting.Testing/DistributedApplicationFactory.cs:507
- The atomic disposal claim still leaves a publication race.
DisposeAsynccan observe_appTcsas incomplete, thenOnBuiltCoreAsynccan successfully publish the application beforeTrySetCanceledruns. The disposer then returns,OnBuiltCoreAsyncdoes not reclaim the successfully published application, and every later disposer exits because this exchange already claimed disposal. Recheck the result ofTrySetCanceledand fall through to teardown when publication won the race.
if (Interlocked.Exchange(ref _disposeClaimed, 1) == 1)
src/Aspire.Hosting.Testing/DistributedApplicationTestingBuilder.cs:290
- The PR description still says
DistributedApplicationOptions.DisableDashboard = falseis equivalent to the new option and receives the same hardening, but this code deliberately preserves its older, unhardened behavior. Update the description and security summary so reviewers and users are not told this existing opt-in gets protections that the implementation intentionally does not apply.
// Only the explicit option turns on the hardened testing defaults below. Setting
// DistributedApplicationOptions.DisableDashboard = false through the configureBuilder callback is the older,
// already-shipped spelling of "run a dashboard", and it has to keep the behavior it shipped with: callers use
// it to exercise dashboard behavior against configuration they chose themselves (fixed URLs, anonymous
// access, an ambient browser token), and in publish mode it is simply ignored because no dashboard resource
// is ever added. Treating it as equivalent to the option silently rewrote that configuration and turned
// publish-mode callers into an InvalidOperationException.
tests/Aspire.Hosting.Testing.Tests/DashboardTestingBuilderTests.cs:391
- The PR description promises that AppHost configuration cannot downgrade the authenticated dashboard defaults, while this test intentionally codifies that the AppHost can clear
AppHost:BrowserToken, causing the dashboard to use unsecured frontend authentication. Since the escape hatch is intentional, update the description and security guarantee to document this exception.
public async Task DashboardTestingDoesNotPinTheBrowserTokenAgainstTheAppHost(CreationSurface creationSurface)
{
// Anonymous access is settled while the builder is constructed and cannot be taken back, but the browser
// token is only read out of AppHost:BrowserToken when DashboardOptions is first resolved, so an AppHost
// that clears the key still gets a dashboard with no credential. That asymmetry is deliberate rather than
- Files reviewed: 26/29 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Review asked for the generated browser token to be restored after the AppHost configures itself, so an AppHost clearing AppHost:BrowserToken could not downgrade the dashboard to Unsecured authentication. Pinning it onto DashboardOptions works, but it removes the only way to reach the anonymous dashboard path once EnableDashboard has appended its own arguments. Two tests in this PR rely on that escape hatch through the returned builder, and both fail with the token pinned: GetDashboardLoginUrlAsyncThrowsWhenDashboardAllowsAnonymousAccess and CanonicalDashboardLoginUrlEscapesBrowserToken. The state is also not silent. DashboardUrlsHelper reports HasBrowserToken false and GetDashboardLoginUrlAsync throws rather than returning an unauthenticated URL. Cover the boundary instead, so neither side of it can drift unnoticed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f2acafc to
10b6337
Compare
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting.Testing/Properties/Resources.Designer.cs: Generated file
Suppressed comments (1)
src/Aspire.Hosting.Testing/DistributedApplicationTestingBuilder.cs:291
- The PR description still says
DistributedApplicationOptions.DisableDashboard = falseis equivalent toEnableDashboardand receives the same hardening/fail-fast wait behavior. This branch now explicitly excludes that older path, and the added tests verify that its fixed URLs and anonymous access remain unchanged. Please update the description and security/behavior notes so reviewers and release notes do not claim guarantees the implementation intentionally does not provide.
return testingOptions?.EnableDashboard == true;
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Balanced
After a post-release cancellation, ReclaimApplicationInBackground awaited the shared application task and then disposed the factory unconditionally. The continuation state stayed ContinuationReleased, so a concurrent or retried BuildAsync kept waiting on that same task. When the application arrived both callers received it and the background continuation tore it down underneath the surviving one. Guard the continuation state and the outstanding build count together, so only the last caller to cancel reclaims the application, and reject later builds once it has been claimed rather than handing back an instance that is being disposed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting.Testing/Properties/Resources.Designer.cs: Generated file
Suppressed comments (3)
src/Aspire.Hosting.Testing/DistributedApplicationFactory.cs:507
- The atomic claim prevents duplicate teardown, but every concurrent caller that loses the exchange now receives a completed
ValueTaskwhile the winning caller may still be awaiting shutdown and disposal. A caller can therefore continue cleanup while AppHost processes are still running. Cache the in-progress disposal task (or signal completion with aTaskCompletionSource) and have subsequentDisposeAsynccalls await the same teardown.
if (Interlocked.Exchange(ref _disposeClaimed, 1) == 1)
src/Aspire.Hosting.Testing/DistributedApplicationTestingBuilderOptions.cs:46
- This public documentation guarantees authenticated endpoints, but the implementation intentionally lets AppHost code clear
AppHost:BrowserToken, which launches the dashboard with unsecured frontend authentication. Document that opt-out so callers do not rely on authentication as an invariant ofEnableDashboard.
/// When the dashboard runs, it listens on authenticated loopback endpoints using dynamically assigned ports so
/// that concurrent test applications cannot collide or reach each other. Use
/// <see cref="DistributedApplicationHostingTestingExtensions.GetDashboardLoginUrlAsync"/> to obtain an
/// authenticated URL for the running dashboard.
src/Aspire.Hosting.Testing/DistributedApplicationTestingBuilder.cs:291
- The PR description still says
DistributedApplicationOptions.DisableDashboard = falseis equivalent toEnableDashboardand receives the same hardening and fail-fast wait behavior, but this condition explicitly excludes that existing spelling. The added tests also assert that its caller-selected anonymous/fixed-port configuration is preserved. Update the PR description and security/behavior notes to describe the implemented distinction.
return testingOptions?.EnableDashboard == true;
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Balanced
DistributedApplicationBuilder freezes the generated browser token into AppHost:BrowserToken while the builder is constructed, but DashboardOptions does not read that key until the application starts. AppHost code runs between those two points, so it could blank the key and DashboardEventHandlers would then launch the dashboard with Unsecured frontend authentication, silently downgrading the authenticated default EnableDashboard promises. Keep the generated token in DashboardTestingState and put it back once the AppHost entry point has finished configuring. The guard mirrors the string.IsNullOrEmpty check DashboardEventHandlers itself applies, so a deliberately chosen non-empty token is left alone, and it runs before the caller sees the builder, so the documented escape hatch to the anonymous dashboard is unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Files not reviewed (1)
- src/Aspire.Hosting.Testing/Properties/Resources.Designer.cs: Generated file
Suppressed comments (1)
src/Aspire.Hosting.Testing/DistributedApplicationTestingBuilder.cs:291
- The PR description still says
DisableDashboard = falseis equivalent toEnableDashboardand receives the same hardening/default wait behavior, but this code now explicitly preserves the older behavior (also asserted by the new tests). Please update the description so it does not claim security and lifecycle behavior that this path no longer provides.
return testingOptions?.EnableDashboard == true;
- Files reviewed: 26/27 changed files
- Comments generated: 0 new
- Review effort level: Balanced
…575-testing-dashboard
Description
Adds first-class support for running the Aspire dashboard from inside a test, so a failing integration test can be inspected the same way a running app can.
#15575originally asked for VS Code extension integration withDistributedApplicationTestingBuilder. Per the discussion on the issue, the extension approach has a chicken-and-egg problem — the backchannel cannot answer while the AppHost is stopped at a breakpoint — so this takes the alternative direction recorded there: let the test host run the dashboard itself.What runs when the dashboard is enabled
The dashboard runs on authenticated loopback endpoints with dynamically assigned ports, interactivity off, and a freshly generated browser token per application. Those defaults are applied both before the builder is constructed (dashboard services and authentication are selected during construction and cannot be added afterwards) and again on the constructed builder, so an AppHost's own arguments, environment variables, or launch profile cannot quietly downgrade them. Settings that remain adjustable at runtime can still be overridden through the returned builder.
DistributedApplicationOptions.DisableDashboard = falsewas already the informal way to do this, so it is treated as equivalent to the new option and receives the same hardening. That also means the existingconfigureBuilderoverloads remain a complete way to enable the dashboard alongside arbitrary builder customization.Lifecycle fixes found along the way
Adding deterministic tests for the "released but still building" window surfaced pre-existing defects in
DistributedApplicationFactoryand the suspending builder:BuildAsyncawaited the released AppHost inline before rethrowing, so a cancelled build blocked until the AppHost finished — and never returned at all if it never finished. It now reclaims the late application on a background continuation and rethrows immediately.TrySetResultreturnedfalseand theDistributedApplication, its service provider, and its orchestrator processes stayed alive for the rest of the test process. It is now disposed at the point it arrives.DisposeAsync's guard read_disposingCts.IsCancellationRequested, which is not atomic withOnDisposed(), so two concurrent disposers could both run teardown and race onStopAsync/DisposeAsyncof the same application. It now claims disposal withInterlocked.Exchange.ObjectDisposedExceptionleaked the internal nested factory type name instead ofIDistributedApplicationTestingBuilder.Notes for reviewers
Aspire.Hostingnow grantsInternalsVisibleTotoAspire.Hosting.Testing, which is a shipping package rather than a test project. It reuses the product's dashboard configuration keys, token generation, and login URL resolution instead of duplicating them. This required dropping the$(SharedDir)compile links fromAspire.Hosting.Testing.csproj, because compiling both copies produces CS0436. There is precedent —Aspire.Hosting.KubernetesandAspire.Hosting.Radiusalready hold the same grant. Everything consumed is internal, so there is no change to the shipped surface.DashboardUrlsHelpernow percent-encodes the browser token when composing the login URL. This is on the CLI's path as well as the new one.ValidateTokenMiddlewarereads the token fromRequest.Query, which URL-decodes, so this is a correctness fix rather than a behavior change, but it is worth a second pair of eyes.http://127.0.0.1:0. Blank is how the product spells "assign me a free port":ConfigureDefaultDashboardOptionsnormalizes it tonullandDashboardEventHandlersthen creates the endpoint withport: null. An explicit:0parses to a literal fixed port and was only dynamic whileDcpPublisher:RandomizePortshappened to stay true. Loopback binding is preserved becauseEndpointAnnotation.TargetHostdefaults tolocalhost.DistributedApplicationBuilderresolves the token during construction and freezes it intoAppHost:BrowserToken, so it must be visible before construction and there must be exactly one value. Command line also outranks an ambientASPIRE_DASHBOARD_FRONTEND_BROWSERTOKEN, which on a CI agent would otherwise share one known token across every application running there.WaitOnResourceUnavailable. The testing builder re-pinsStopOnResourceUnavailableso an unattended run fails instead of hanging. This now also applies to the olderDisableDashboard = falsespelling.DefaultWaitBehavioris the opt-out.Follow-ups deliberately not in this change
DashboardUrlsHelperlives underBackchannel/and now carries athrowOnDashboardFailureflag to serve two callers. It belongs undersrc/Aspire.Hosting/Dashboard/with the CLI adapting to it, rather than the other way round.ASPIRE_TESTING_ENABLE_DASHBOARD) and no automatic emission of the login URL to test output. Both would remove the edit-and-rebuild step from the debugging loop.#15575is labelledarea-vscode-extensionand nothing underextension/changes here. This lands the primitive the extension would need; the editor-side half is still open. Worth noting that afscrome's breakpoint concern applies to an in-process dashboard too — stopping the test thread with default all-threads-stop also freezes the dashboard's Kestrel and SignalR threads.Checklist
<remarks />and<code />elements on your triple slash comments?The dashboard is only reachable over loopback, requires a per-application browser token, and anonymous access is forced off through the highest-precedence configuration source. No token is written to logs. Worth confirming during review that forcing
ASPIRE_ALLOW_UNSECURED_TRANSPORTfor the test dashboard is acceptable.