Skip to content

Unify GetLaunchSettings implementations between Aspire.Hosting and Aspire.Hosting.Testing#15828

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/unify-getlaunchsettings-implementations
Draft

Unify GetLaunchSettings implementations between Aspire.Hosting and Aspire.Hosting.Testing#15828
Copilot wants to merge 2 commits intomainfrom
copilot/unify-getlaunchsettings-implementations

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 2, 2026

The launch settings file-reading logic was duplicated between LaunchProfileExtensions (in Aspire.Hosting) and DistributedApplicationFactory (in Aspire.Hosting.Testing). This extracts the shared logic into a new LaunchSettingsReader class in the shared sources folder.

Changes

  • New src/Shared/LaunchProfiles/LaunchSettingsReader.cs — shared internal static class with:

    • GetLaunchSettingsFromDirectory(string? directoryPath, string resourceIdentifier) — builds the Properties/launchSettings.json path and reads it
    • ReadLaunchSettingsFile(string filePath, string resourceIdentifier) — deserializes JSON, throws DistributedApplicationException with a descriptive message on parse failure
  • LaunchProfileExtensions.cs — replaces inline stream-open + JsonSerializer.Deserialize + JsonException catch block with LaunchSettingsReader.ReadLaunchSettingsFile(...). All existing special-case logic (file-based apps, .run.json fallback) is preserved.

  • DistributedApplicationFactory.cs — replaces ~20 lines of duplicated path-building and file-reading in GetLaunchSettings(string?) with a single call to LaunchSettingsReader.GetLaunchSettingsFromDirectory(...).

  • Aspire.Hosting.Testing.csproj — adds explicit <Compile> include for the new shared file. Aspire.Hosting and Aspire.Hosting.Azure.Functions pick it up automatically via their existing $(SharedDir)LaunchProfiles\*.cs wildcard.

Error message strings are preserved exactly, maintaining backward compatibility with existing tests.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
    • No

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dnceng.pkgs.visualstudio.com
    • Triggering command: /usr/share/dotnet/dotnet dotnet build src/Aspire.Hosting/Aspire.Hosting.csproj -p:TreatWarningsAsErrors=false (dns block)
    • Triggering command: /usr/share/dotnet/dotnet dotnet build src/Aspire.Hosting.Testing/Aspire.Hosting.Testing.csproj -p:TreatWarningsAsErrors=false (dns block)
    • Triggering command: /usr/share/dotnet/dotnet dotnet build src/Aspire.Hosting.Azure.Functions/Aspire.Hosting.Azure.Functions.csproj -p:TreatWarningsAsErrors=false (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

…pire.Hosting.Testing

Agent-Logs-Url: https://github.com/microsoft/aspire/sessions/ff3fd7f9-715f-4cb8-af61-04cba3fc0081

Co-authored-by: radical <1472+radical@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor GetLaunchSettings implementations in Aspire.Hosting Unify GetLaunchSettings implementations between Aspire.Hosting and Aspire.Hosting.Testing Apr 3, 2026
Copilot AI requested a review from radical April 3, 2026 00:40
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15828

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15828"

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.

Unify implementations for GetLaunchSettings between Aspire.Hosting, and Aspire.Hosting.Testing

2 participants