Auto-enable staging channel when channel is set to staging#15422
Open
mitchdenny wants to merge 1 commit intomainfrom
Open
Auto-enable staging channel when channel is set to staging#15422mitchdenny wants to merge 1 commit intomainfrom
mitchdenny wants to merge 1 commit intomainfrom
Conversation
Add KnownFeatures.IsStagingChannelEnabled helper that checks both the feature flag and the configured channel. The staging channel is now considered enabled if either features.stagingChannelEnabled is true OR the channel setting is "staging" (case-insensitive). This removes the need to explicitly set features.stagingChannelEnabled when the user already has channel set to staging in their config. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15422Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15422" |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Aspire CLI staging-channel UX by making the staging channel automatically available whenever the configured channel is set to "staging", eliminating the need for users to also enable features.stagingChannelEnabled.
Changes:
- Added
KnownFeatures.IsStagingChannelEnabled(IFeatures, IConfiguration)to treat staging as enabled when either the feature flag is on orchannel == "staging"(case-insensitive). - Updated staging checks across CLI commands and packaging channel discovery to use the new helper.
- Added unit tests for the helper and updated E2E staging-channel coverage to validate behavior without the feature flag.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/KnownFeatures.cs | Adds helper method that centralizes “staging enabled” logic (flag OR channel==staging). |
| src/Aspire.Cli/Packaging/PackagingService.cs | Uses helper to decide whether to include the staging channel in discovered channels. |
| src/Aspire.Cli/Commands/InitCommand.cs | Switches staging availability checks to helper and injects IConfiguration for evaluation. |
| src/Aspire.Cli/Commands/NewCommand.cs | Switches staging availability checks to helper and injects IConfiguration for evaluation. |
| src/Aspire.Cli/Commands/UpdateCommand.cs | Switches both staging checks to helper and injects/stores IConfiguration. |
| tests/Aspire.Cli.Tests/Configuration/KnownFeaturesTests.cs | Adds unit tests for the helper method behavior. |
| tests/Aspire.Cli.EndToEnd.Tests/StagingChannelTests.cs | Updates E2E flow to verify staging works without explicitly enabling the staging feature flag. |
Comment on lines
+12
to
+19
| [Fact] | ||
| public void IsStagingChannelEnabled_ReturnsTrue_WhenChannelIsStaging() | ||
| { | ||
| var features = new TestFeatures(stagingChannelEnabled: false); | ||
| var configuration = BuildConfiguration(channel: PackageChannelNames.Staging); | ||
|
|
||
| Assert.True(KnownFeatures.IsStagingChannelEnabled(features, configuration)); | ||
| } |
There was a problem hiding this comment.
The test IsStagingChannelEnabled_ReturnsTrue_WhenChannelIsStaging duplicates IsStagingChannelEnabled_ReturnsTrue_WhenChannelIsStagingAndFlagExplicitlyFalse (same inputs/expectation). Consider removing one or consolidating into a single theory case to keep the suite focused on distinct behaviors.
Comment on lines
+57
to
+75
| [Fact] | ||
| public void IsStagingChannelEnabled_IsCaseInsensitive_ForChannelValue() | ||
| { | ||
| var features = new TestFeatures(stagingChannelEnabled: false); | ||
| var configuration = BuildConfiguration(channel: "Staging"); | ||
|
|
||
| Assert.True(KnownFeatures.IsStagingChannelEnabled(features, configuration)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void IsStagingChannelEnabled_IsCaseInsensitive_ForUppercaseChannelValue() | ||
| { | ||
| var features = new TestFeatures(stagingChannelEnabled: false); | ||
| var configuration = BuildConfiguration(channel: "STAGING"); | ||
|
|
||
| Assert.True(KnownFeatures.IsStagingChannelEnabled(features, configuration)); | ||
| } | ||
|
|
||
| [Fact] |
There was a problem hiding this comment.
The two case-insensitivity tests ("Staging" and "STAGING") cover the same behavior. Consider using a single [Theory] with multiple InlineData values (or keep one) to reduce duplication while still validating OrdinalIgnoreCase behavior.
Suggested change
| [Fact] | |
| public void IsStagingChannelEnabled_IsCaseInsensitive_ForChannelValue() | |
| { | |
| var features = new TestFeatures(stagingChannelEnabled: false); | |
| var configuration = BuildConfiguration(channel: "Staging"); | |
| Assert.True(KnownFeatures.IsStagingChannelEnabled(features, configuration)); | |
| } | |
| [Fact] | |
| public void IsStagingChannelEnabled_IsCaseInsensitive_ForUppercaseChannelValue() | |
| { | |
| var features = new TestFeatures(stagingChannelEnabled: false); | |
| var configuration = BuildConfiguration(channel: "STAGING"); | |
| Assert.True(KnownFeatures.IsStagingChannelEnabled(features, configuration)); | |
| } | |
| [Fact] | |
| [Theory] | |
| [InlineData("Staging")] | |
| [InlineData("STAGING")] | |
| public void IsStagingChannelEnabled_IsCaseInsensitive_ForChannelValue(string channel) | |
| { | |
| var features = new TestFeatures(stagingChannelEnabled: false); | |
| var configuration = BuildConfiguration(channel: channel); | |
| Assert.True(KnownFeatures.IsStagingChannelEnabled(features, configuration)); | |
| } | |
| [Fact] | |
| public void IsStagingChannelEnabled_ReturnsFalse_WhenChannelIsDailyAndFlagNotSet() | |
| { | |
| var features = new TestFeatures(stagingChannelEnabled: false); | |
| var configuration = BuildConfiguration(channel: PackageChannelNames.Daily); | |
| Assert.False(KnownFeatures.IsStagingChannelEnabled(features, configuration)); | |
| } |
Comment on lines
36
to
53
| @@ -49,7 +47,7 @@ public async Task StagingChannel_ConfigureAndVerifySettings_ThenSwitchChannels() | |||
| await auto.EnterAsync(); | |||
| await auto.WaitForSuccessPromptAsync(counter); | |||
|
|
|||
| // Set channel to staging | |||
| // Set channel to staging — this alone enables staging channel behavior | |||
| await auto.TypeAsync("aspire config set channel staging -g"); | |||
| await auto.EnterAsync(); | |||
| await auto.WaitForSuccessPromptAsync(counter); | |||
There was a problem hiding this comment.
This test is meant to validate staging works without features.stagingChannelEnabled, but it never explicitly ensures the flag is unset/false. To avoid accidental false positives if the default/fixture environment changes, consider deleting the flag (or setting it to false) at the start of the test before setting channel to staging.
Contributor
|
🎬 CLI E2E Test Recordings — 52 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23325335320 |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Auto-enable the staging channel when
channelis configured as"staging", removing the need to also explicitly setfeatures.stagingChannelEnabled: true.Previously, users had to set both
channel: stagingandfeatures.stagingChannelEnabled: truefor the staging channel to work. If they forgot the feature flag, the staging channel was silently unavailable. Now, settingchannel: stagingalone is sufficient — the feature flag still works as an independent toggle for cases where the channel isn't set to staging.Changes
KnownFeatures.IsStagingChannelEnabled(IFeatures, IConfiguration)static helper that returnstrueif the feature flag is on OR the configured channel is"staging"(case-insensitive)PackagingService,InitCommand,NewCommand,UpdateCommand(2 locations)IConfigurationto command constructors where needed (already in DI container)StagingChannelTeststo verify staging works without the feature flagValidation
KnownFeaturesTests: 8/8 passedPackagingServiceTests+UpdateCommandTests: 44/44 passedFixes # (issue)
Checklist