Conversation
…219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15455Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15455" |
There was a problem hiding this comment.
Pull request overview
Adds a new Windows-only PR validation job to dogfood the Aspire CLI built in the same workflow run by creating starter templates, running aspire start, and verifying resources become ready via aspire wait.
Changes:
- Introduces
cli_starter_validation_winjob to install the PR-built Aspire CLI, trust dev certs, and smoke test starter templates on Windows. - Captures and uploads diagnostics/logs as workflow artifacts for debugging failures.
- Wires the new job into the workflow’s aggregate/required job dependencies and skip-logic for PRs.
| $startOutput = @( | ||
| if (Test-Path $startStdOutPath) { Get-Content $startStdOutPath } | ||
| if (Test-Path $startStdErrPath) { Get-Content $startStdErrPath } | ||
| ) -join [Environment]::NewLine |
There was a problem hiding this comment.
Get-Content without -Raw returns an array of lines; when combined via the outer @(...) and -join, PowerShell may stringify arrays using its default separator (spaces), which can drop original newlines and make logs/regex checks less reliable. Prefer reading stdout/stderr with Get-Content -Raw (and then concatenating with a single newline between them) so the combined log preserves exact output formatting.
| $startOutput = @( | |
| if (Test-Path $startStdOutPath) { Get-Content $startStdOutPath } | |
| if (Test-Path $startStdErrPath) { Get-Content $startStdErrPath } | |
| ) -join [Environment]::NewLine | |
| $startStdOut = if (Test-Path $startStdOutPath) { Get-Content -Raw $startStdOutPath } else { '' } | |
| $startStdErr = if (Test-Path $startStdErrPath) { Get-Content -Raw $startStdErrPath } else { '' } | |
| $startOutput = ($startStdOut, $startStdErr) -join [Environment]::NewLine |
| $process | Wait-Process -Timeout ([int]$env:MAX_STARTUP_SECONDS) -ErrorAction Stop | ||
| } | ||
| catch { | ||
| if (-not $process.HasExited) { | ||
| $process | Stop-Process -Force -ErrorAction SilentlyContinue | ||
| } | ||
|
|
||
| throw "${templateId}: aspire start did not exit within $($env:MAX_STARTUP_SECONDS) seconds." | ||
| } | ||
|
|
||
| $elapsed = (Get-Date) - $startAt |
There was a problem hiding this comment.
This can be flaky at the boundary: Wait-Process -Timeout may return successfully when the process exits near the timeout, but the separately computed $elapsed can still be >= MAX_STARTUP_SECONDS due to scheduling/measurement overhead, causing an unexpected failure. To avoid false negatives, either remove the elapsed check (since timeout already enforces it) or change it to a strictly-greater check with a small buffer (e.g., allow a couple seconds over) to account for timing granularity.
| throw "${templateId}: aspire start failed with exit code $($process.ExitCode)." | ||
| } | ||
|
|
||
| if ($elapsed.TotalSeconds -ge [int]$env:MAX_STARTUP_SECONDS) { |
There was a problem hiding this comment.
This can be flaky at the boundary: Wait-Process -Timeout may return successfully when the process exits near the timeout, but the separately computed $elapsed can still be >= MAX_STARTUP_SECONDS due to scheduling/measurement overhead, causing an unexpected failure. To avoid false negatives, either remove the elapsed check (since timeout already enforces it) or change it to a strictly-greater check with a small buffer (e.g., allow a couple seconds over) to account for timing granularity.
| if ($elapsed.TotalSeconds -ge [int]$env:MAX_STARTUP_SECONDS) { | |
| $maxStartupSeconds = [int]$env:MAX_STARTUP_SECONDS | |
| $timingBufferSeconds = 2 | |
| if ($elapsed.TotalSeconds -gt ($maxStartupSeconds + $timingBufferSeconds)) { |
| - name: Create starter app and validate startup | ||
| timeout-minutes: 10 | ||
| shell: pwsh | ||
| run: | | ||
| $ErrorActionPreference = 'Stop' | ||
| $PSNativeCommandUseErrorActionPreference = $true |
There was a problem hiding this comment.
The workflow embeds a large multi-template PowerShell harness inline, which makes the YAML hard to review/maintain and harder to run locally for debugging. Consider moving the validation logic into a tracked script (e.g., eng/scripts/cli-starter-validation.ps1) and have the workflow call it with parameters (timeouts, templates, expected resources). This keeps the workflow concise, enables reuse, and centralizes changes for future template/resource updates.
…Copilot@users.noreply.github.com>
…19+Copilot@users.noreply.github.com>
sebastienros
requested review from
JamesNK,
davidfowl and
mitchdenny
as code owners
…6219+Copilot@users.noreply.github.com>
…6219+Copilot@users.noreply.github.com>
…3556219+Copilot@users.noreply.github.com>
|
🎬 CLI E2E Test Recordings — 52 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23370216804 |
Description
Adds a new Windows PR CI validation for the Aspire CLI in
tests.ymlonmain. The workflow dogfoods the CLI built by the current PR run, creates the starter templates, runsaspire start, trusts HTTPS development certificates up front, and verifies the expected resources come up withaspire wait.This mirrors the release/13.2 change, but it is adapted to the current
mainworkflow shape and keeps the PR-only job filtered to themicrosoftorg.Validation:
.github/workflows/tests.ymlsuccessfully with Ruby YAML parsing.mainbranch layout.Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: