Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions tests/Aspire.Cli.EndToEnd.Tests/Helpers/CliE2EAutomatorHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,23 +147,33 @@ internal static async Task SourceAspireCliEnvironmentAsync(
}

/// <summary>
/// Verifies the installed Aspire CLI version matches the expected version.
/// Verifies the installed Aspire CLI version matches the expected build.
/// Always checks the dynamic version prefix from eng/Versions.props.
/// For non-stabilized builds (all normal PR builds), also verifies the commit SHA suffix.
/// </summary>
#pragma warning disable IDE0060 // commitSha is unused during stabilized builds — restore when merging back to main
internal static async Task VerifyAspireCliVersionAsync(
this Hex1bTerminalAutomator auto,
string commitSha,
SequenceCounter counter)
#pragma warning restore IDE0060
{
var versionPrefix = CliE2ETestHelpers.GetVersionPrefix();
var isStabilized = CliE2ETestHelpers.IsStabilizedBuild();

await auto.TypeAsync("aspire --version");
await auto.EnterAsync();

// When the build is stabilized (StabilizePackageVersion=true), the CLI version
// is just "13.2.0" with no commit SHA suffix. When not stabilized, it includes
// the SHA (e.g., "13.2.0-preview.1.g<sha>"). In both cases, "13.2.0" is present.
// TODO: This change should be reverted on the integration to the main branch.
await auto.WaitUntilTextAsync("13.2.0", timeout: TimeSpan.FromSeconds(10));
// Always verify the version prefix matches the branch's version (e.g., "13.3.0").
await auto.WaitUntilTextAsync(versionPrefix, timeout: TimeSpan.FromSeconds(10));

// For non-stabilized builds (all PR CI builds), also verify the commit SHA suffix
// to uniquely identify the exact build. Stabilized builds (official releases only)
// produce versions without SHA suffixes, so we skip this check.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the release/13.2 branch, eng/Versions.props has StabilizePackageVersion=true. This method reads that file value, so IsStabilizedBuild() always returns true on this branch — making the SHA verification block below dead code.

The PR description states "StabilizePackageVersion=false in all PR builds", but that's a CI pipeline override (/p:StabilizePackageVersion=false), not the file on disk. Since this reads the source file rather than querying the actual build configuration, the if (!isStabilized && ...) check never executes on release/13.2.

The test still passes (version prefix alone is sufficient), but the commit SHA check described in the PR's scenario table for release/13.2 PR builds won't actually run. Consider reading the stabilization state from the CLI's own version output (e.g., presence of + in the version string) instead of the source file.

if (!isStabilized && commitSha.Length == 40)
{
var shortCommitSha = commitSha[..8];
var expectedVersionSuffix = $"g{shortCommitSha}";
await auto.WaitUntilTextAsync(expectedVersionSuffix, timeout: TimeSpan.FromSeconds(10));
}
Comment on lines +159 to +176
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VerifyAspireCliVersionAsync’s SHA-suffix validation depends on IsStabilizedBuild(), but IsStabilizedBuild currently parses Versions.props without evaluating MSBuild conditions/overrides. This can make the SHA check get skipped even in PR CI runs, reducing the test’s ability to prove the CLI came from the current PR build. Suggest basing the decision on the observed aspire --version output (e.g., wait until the screen contains the prefix and then, if it contains +g, require the matching g{shortSha}; otherwise require an exact stabilized version).

Copilot uses AI. Check for mistakes.

await auto.WaitForSuccessPromptAsync(counter);
}
Expand Down
48 changes: 48 additions & 0 deletions tests/Aspire.Cli.EndToEnd.Tests/Helpers/CliE2ETestHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.CompilerServices;
using System.Xml.Linq;
using Aspire.Cli.Tests.Utils;
using Hex1b;
using Xunit;
Expand Down Expand Up @@ -309,4 +310,51 @@ internal static string ToContainerPath(string hostPath, TemporaryWorkspace works
var relativePath = Path.GetRelativePath(workspace.WorkspaceRoot.FullName, hostPath);
return $"/workspace/{workspace.WorkspaceRoot.Name}/" + relativePath.Replace('\\', '/');
}

/// <summary>
/// Reads the VersionPrefix (e.g., "13.3.0") from eng/Versions.props by parsing
/// the MajorVersion, MinorVersion, and PatchVersion MSBuild properties.
/// </summary>
internal static string GetVersionPrefix()
{
var repoRoot = GetRepoRoot();
var versionsPropsPath = Path.Combine(repoRoot, "eng", "Versions.props");

var doc = XDocument.Load(versionsPropsPath);
var ns = doc.Root?.Name.Namespace ?? XNamespace.None;

string? GetProperty(string name) =>
doc.Descendants(ns + name).FirstOrDefault()?.Value;

var major = GetProperty("MajorVersion")
?? throw new InvalidOperationException("MajorVersion not found in eng/Versions.props");
var minor = GetProperty("MinorVersion")
?? throw new InvalidOperationException("MinorVersion not found in eng/Versions.props");
var patch = GetProperty("PatchVersion")
?? throw new InvalidOperationException("PatchVersion not found in eng/Versions.props");

return $"{major}.{minor}.{patch}";
}

/// <summary>
/// Checks whether the build is stabilized (StabilizePackageVersion=true in eng/Versions.props).
/// Stabilized builds produce version strings without commit SHA suffixes (e.g., "13.2.0" instead
/// of "13.2.0-preview.1.25175.1+g{sha}"). This is only true for official release builds,
/// never for normal PR CI builds.
/// </summary>
internal static bool IsStabilizedBuild()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Both GetVersionPrefix() and IsStabilizedBuild() independently call GetRepoRoot(), build the same eng/Versions.props path, and parse it with XDocument.Load(). Since they're called back-to-back from VerifyAspireCliVersionAsync, this parses the same XML file twice. Consider a single helper that loads the document once and returns both values.

{
var repoRoot = GetRepoRoot();
var versionsPropsPath = Path.Combine(repoRoot, "eng", "Versions.props");

var doc = XDocument.Load(versionsPropsPath);
var ns = doc.Root?.Name.Namespace ?? XNamespace.None;

// The default value in Versions.props uses a Condition to default to "false",
// so we read the element's text directly.
var stabilize = doc.Descendants(ns + "StabilizePackageVersion")
.FirstOrDefault()?.Value;

return string.Equals(stabilize, "true", StringComparison.OrdinalIgnoreCase);
Comment on lines +353 to +358
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsStabilizedBuild reads the raw element value from eng/Versions.props, but that property is conditionally assigned (and can be overridden via /p:StabilizePackageVersion=...). With the current Versions.props shape, this method can return a value that doesn’t match the effective build setting (e.g., PR builds overriding it to false), which will cause downstream version validation to skip/enable SHA checks incorrectly. Consider evaluating the property via MSBuild (same global properties as the build) or inferring stabilization from the actual aspire --version output (presence/absence of +g{sha} / prerelease label).

Copilot uses AI. Check for mistakes.
}
}
Loading