Testing: set launch profile name based on target AppHost#4287
Merged
ReubenBond merged 1 commit intomainfrom Jun 4, 2024
Merged
Testing: set launch profile name based on target AppHost#4287ReubenBond merged 1 commit intomainfrom
ReubenBond merged 1 commit intomainfrom
Conversation
ghost
added
the
area-app-model
ReubenBond
commented
May 24, 2024
| private static bool TrySelectLaunchProfileFromEnvironment(ProjectResource projectResource, [NotNullWhen(true)] out string? launchProfileName) | ||
| private static bool TrySelectLaunchProfileFromDefaultAnnotation(ProjectResource projectResource, [NotNullWhen(true)] out string? launchProfileName) | ||
| { | ||
| var launchProfileEnvironmentVariable = Environment.GetEnvironmentVariable("DOTNET_LAUNCH_PROFILE"); |
Member
Author
There was a problem hiding this comment.
Moved to WithProjectDefaults via DefaultLaunchProfileAnnotation. See https://github.com/dotnet/aspire/pull/4287/files#diff-9e18fbbbb697e4ee2da32654325b13f2d6b8b8659e5a34747a63c8a81781aaedR227
mitchdenny
reviewed
May 26, 2024
| var appHostLaunchSettings = GetLaunchSettings(appHostProjectPath); | ||
| if (appHostLaunchSettings?.Profiles.FirstOrDefault().Key is { } profileName) | ||
| { | ||
| additionalConfig["AppHost:DefaultLaunchProfileName"] = profileName; |
Member
There was a problem hiding this comment.
Wonder whether we should start putting these keys in constants somewhere.
Member
Author
There was a problem hiding this comment.
We probably should do a sweep of them at some point
mitchdenny
reviewed
May 26, 2024
mitchdenny
reviewed
May 26, 2024
| [LocalOnlyTheory] | ||
| [InlineData(false)] | ||
| [InlineData(true)] | ||
| public async Task SelectsFirstLaunchProfile(bool genericEntryPoint) |
Member
There was a problem hiding this comment.
Should we be adding an API for selecting the launch profile when using the test builder?
Member
Author
There was a problem hiding this comment.
Perhaps in a different PR?
mitchdenny
reviewed
May 26, 2024
ReubenBond
force-pushed
the
rebond/testing-launch-profile-and-content-root
branch
2 times, most recently
from
0bd82f2 to
1a86ece
Compare
mitchdenny
approved these changes
Jun 4, 2024
ReubenBond
force-pushed
the
rebond/testing-launch-profile-and-content-root
branch
from
1a86ece to
40554cd
Compare
…h profiles. Propagate ContentRoot.
ReubenBond
force-pushed
the
rebond/testing-launch-profile-and-content-root
branch
from
40554cd to
c4a92fe
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Also: set AppHost
ContentRootPath.This adds a
DefaultLaunchProfileAnnotationto all projects where the launch profile is not explicitly set, which lets us override it via config (not just process-wide env var). This lets us set this value on a per-app-host basis in tests, which we cannot do using env vars (they are global state).Fixes #4286
Fixes #4282
Possibly fixes #4274
cc @DamianEdwards
Microsoft Reviewers: Open in CodeFlow