Skip to content

Add WinGet/Homebrew installer dogfood flows for PR builds#17115

Open
radical wants to merge 20 commits into
microsoft:mainfrom
radical:radical/installer-manifests
Open

Add WinGet/Homebrew installer dogfood flows for PR builds#17115
radical wants to merge 20 commits into
microsoft:mainfrom
radical:radical/installer-manifests

Conversation

@radical
Copy link
Copy Markdown
Member

@radical radical commented May 15, 2026 •
edited
Loading

Description

Adds GitHub Actions generation for Aspire CLI installer artifacts so PR/main/release CI publishes dogfoodable WinGet and Homebrew outputs alongside the existing Azure DevOps prepare stages. The dogfood scripts then know how to consume these artifacts, so anyone can now test the WinGet/Homebrew install path of any open PR locally, before merge — previously this only worked end-to-end against the post-merge AzDO build, so installer breakage was caught after the fact.

Try it out

From any open PR, install the CLI from the actual installer the release pipeline will use:

# Windows (WinGet)
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17115 -InstallMode WinGet"
# macOS (Homebrew)
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17115 --install-mode homebrew

Archive mode (default) and tool mode work everywhere; WinGet/Homebrew modes require the per-OS installers. Full reference: docs/dogfooding-pull-requests.md.

The implementation shares generation logic between AzDO and GitHub by moving WinGet and Homebrew artifact preparation into repo scripts. The AzDO templates now keep CI-specific setup/publishing while calling those shared scripts, and GitHub CI calls the same reusable installer-artifact workflow for each installer.

GitHub-produced installer artifacts are prerelease artifacts because the GitHub native CLI archives are PR/daily/staging builds, not stable release archives. The artifacts keep the normal ci.dot.net URL shape while computing hashes from local native archive artifacts.

This also fixes a WinGet-installed CLI first-run failure where bundle extraction wrote the package-owned layout under WinGet\Packages, but layout discovery validated relative to the raw WinGet\Links launcher path:

❌ An unexpected error occurred: Bundle extraction failed. Run 'aspire setup --force' to retry, or reinstall the Aspire CLI.

[DBUG] [LayoutDiscovery] TryDiscoverRelativeLayout: CLI at C:\Users\runneradmin\AppData\Local\Microsoft\WinGet\Links, checking for layout...
[DBUG] [LayoutDiscovery]   bundle/managed/: MISSING
[DBUG] [LayoutDiscovery]   bundle/dcp/: MISSING
[FAIL] [BundleService] Post-flip layout validation failed; attempting rollback.

Layout discovery now resolves launcher symlinks/reparse targets the same way bundle extraction does, then falls back to the raw launcher path for custom layouts.

Also adds an osx-x64 native CLI archive build so Homebrew cask generation can hash both macOS architectures.

Fixes #17305

Validation:

  • GitHub CI run 26145296164: WinGet installer job passed and uploaded winget-manifests-prerelease plus winget-aspire-cli-logs.
  • GitHub CI run 26145296164: Homebrew installer job passed and uploaded homebrew-cask-prerelease plus homebrew-aspire-cli-logs.
  • Parsed modified workflow YAML files with Ruby YAML.
  • Parsed eng/winget/prepare-manifest-artifact.ps1 with the PowerShell parser.
  • Checked eng/homebrew/prepare-cask-artifact.sh with bash -n.
  • Ran fake local-archive generation for both WinGet manifests and Homebrew cask.
  • Ran PRScriptInstallerModeTests.
  • Ran LayoutDiscoveryReparsePointTests, BundleServiceComputeDefaultExtractDirTests, and BundleServiceCrossRouteExtractionTests.

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
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026 •
edited
Loading

🚀 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 -- 17115

Or

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

@radical radical mentioned this pull request May 15, 2026
Open
@github-actions github-actions Bot mentioned this pull request May 15, 2026
Open
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 16, 2026

Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
GitHub was asked to rerun all failed jobs for that attempt, and the rerun is being tracked in the rerun attempt.
The job links below point to the failed attempt jobs that matched the retry-safe transient failure rules.

Matched test failure patterns (1 test)
  • Aspire.Cli.EndToEnd.Tests.KubernetesDeployWithGarnetTests.DeployK8sWithGarnet — Unable to access container registry during publish

@radical
Copy link
Copy Markdown
Member Author

radical commented May 19, 2026

TODO: Make sure to test on azdo

@radical radical force-pushed the radical/installer-manifests branch 2 times, most recently from 20d8ea4 to 6fec492 Compare May 20, 2026 22:19
@davidfowl davidfowl requested a review from danegsta May 21, 2026 05:10
radical and others added 15 commits May 21, 2026 02:26
@radical
fix(winget): harden local manifest dogfood installs
42db8e9 
Stage manifest YAML files before invoking winget, validate the pristine manifest, and serve local PR archives through a loopback HTTP listener so winget can install them with refreshed hashes.\n\nAlso make verification robust when winget reports success before the expected aspire.exe is on PATH, surface useful diagnostics on install failures, and warn when the installed winget version is older than the Mark-of-the-Web fix.

Co-authored-by: Copilot <[email protected]>
@radical
fix(homebrew): harden local cask dogfood installs
dac0025 
Teach the local Homebrew dogfood helper to rewrite casks to local PR archive files, verify aspire --version after install, and remove quarantine from local unsigned dogfood binaries before verification.\n\nThis keeps dogfood installs self-contained and fails loudly when the installed CLI cannot be executed.

Co-authored-by: Copilot <[email protected]>
@radical
ci(installer-artifacts): generate WinGet and Homebrew artifacts
56d6f21 
Add a reusable GitHub Actions workflow that prepares WinGet manifest and Homebrew cask artifacts from the native CLI archives, uploads those artifacts, and runs package-manager dogfood validation.\n\nMove the shared preparation logic into repo scripts so GitHub Actions and Azure Pipelines use the same generation and validation paths.

Co-authored-by: Copilot <[email protected]>
@radical
feat(acquisition): add PR installer dogfood modes
c9c2e4c 
Extend the PR acquisition scripts with WinGet and Homebrew install modes that download the generated installer artifacts plus their native archives, then invoke the bundled dogfood helpers.\n\nThe installer modes also populate the PR NuGet hive while avoiding persistent shell profile changes for temporary PR dogfood installs.

Co-authored-by: Copilot <[email protected]>
@radical
test(acquisition): cover installer artifact dogfood flows
45052ba 
Add acquisition coverage for installer-mode dispatch, local artifact dogfood helpers, generator output, hash handling, cleanup behavior, and WinGet local archive rewrite semantics.\n\nThe tests catch broken cask or manifest generation before relying on the full installer CI jobs.

Co-authored-by: Copilot <[email protected]>
@radical
ci(installer-artifacts): upload CLI logs from smoke tests
1912371 
Run the installer artifact smoke-test commands with trace logging and always collect the Aspire CLI log directory into a workflow artifact.

The console output usually shows the top-level failure, but the CLI writes the detailed exception and package/bundle diagnostics under .aspire/logs. Uploading those logs makes failures like bundle extraction issues debuggable without rerunning the job.

Co-authored-by: Copilot <[email protected]>
@radical
fix(cli): resolve launcher links during layout discovery
c90bb0e 
WinGet launches Aspire through a Links shim while the real package payload
and bundle sidecar live under the Packages directory. Bundle extraction
already resolves the launcher target before choosing the extraction root,
but layout discovery validated relative to the raw shim path and rejected
the freshly extracted bundle.

Resolve process-path links during relative layout discovery, then fall back
to the raw path if the resolved target does not contain a layout. This keeps
custom symlink layouts working while allowing WinGet and Homebrew-style
launchers to validate the package-owned bundle layout.

Co-authored-by: Copilot <[email protected]>
@radical
fix(installer): validate Homebrew casks in PR artifact prep
e49b2f0 
Move Homebrew cask validation behind a shared script so GitHub Actions
and Azure DevOps run the same syntax, style, and audit checks during
artifact preparation. PR artifacts are not notarized, so Offline
validation rewrites the cask URLs to loopback archive URLs and runs the
Homebrew audit with signing disabled while still exercising download and
extraction. Full release validation keeps the normal signing audit and
writes the summary consumed by Homebrew submission.

Also keep the cask itself aligned with Homebrew style by declaring the
minimum supported macOS version and using an explicit %Q string for the
install-route sidecar. Do not add a zap stanza because the cask should
not remove shared ~/.aspire state.

Co-authored-by: Copilot <[email protected]>
@radical
fix(installer): keep Homebrew dogfood compatible with hosted macOS
3280694 
Remove the Homebrew cask macOS dependency from the generated Aspire cask.
The dependency is optional metadata, and GitHub's macOS 15 runner rejected
local dogfood installs with "This software does not run on macOS versions
other than Monterey." Keeping the cask dependency-free preserves the
existing dogfood behavior while the shared cask validation still catches
syntax, style, audit, download, and extraction issues.

Also suppress benign connection reset traces from the loopback archive
server used during offline Homebrew audit validation.

Co-authored-by: Copilot <[email protected]>
@radical
ci(release): guard WinGet/Homebrew submit on production branches
e8fce4f 
The `🟣Submit to WinGet` and `🟣Submit PR to Homebrew/homebrew-cask`
steps in `release-publish-nuget.yml` previously ran on any branch when
`dryRun=false`. 1ES PT's branch validation logs a non-blocking warning
when the source branch isn't a production branch, but the submission
still goes through — so a misqueued run on a feature, fork, or dogfood
branch could open a PR upstream on Homebrew/homebrew-cask or
microsoft/winget-pkgs.

Add a hard condition on each submit step that only allows execution
when `Build.SourceBranch` is `refs/heads/main`, `refs/heads/release/*`
or `refs/heads/internal/release/*` — matching the existing
"production branch" predicate already used elsewhere in the pipeline
YAML (`azure-pipelines.yml` `_RunAsInternalBuild` / `$isReleaseBranch`).

On a non-production branch the steps now skip cleanly, with the
displayed step name in the run summary making the gating visible.

Co-authored-by: Copilot <[email protected]>
@radical
Add empty zap stanza to Homebrew cask template
95b8713 
Upstream homebrew/cask labels generated PRs with `missing-zap` when no zap
stanza is present. Add an explicit `zap trash: []` to silence the label
without removing any user files. `~/.aspire` is shared with non-Homebrew
Aspire state (PR hives, dev certs, telemetry, runtime sockets, manually
installed CLIs) that must outlive `brew uninstall --zap aspire`.

Co-authored-by: Copilot <[email protected]>
@radical
ci(installers): share production-branch guard via _IsProductionBranch
2d90625 
The set of branches that publish — main, release/*, internal/release/* —
was hard-coded in three places:

  * publish-homebrew.yml condition guard
  * publish-winget.yml condition guard
  * _PackagesPublished in azure-pipelines.yml

Adding or removing a publishing branch required editing all three. The
two publish-template guards in particular drift independently because
they're far from the build-pipeline definition.

Hoist the branch set into _IsProductionBranch in common-variables.yml.
_PackagesPublished now composes it with the existing non-PR / internal
checks, and the publish-template guards reduce to one line:

    eq(variables['_IsProductionBranch'], 'true')

release-publish-nuget.yml already imports common-variables.yml, so the
publish templates see the new variable. Cross-file template-variable
references inside ${{ }} expressions already work here — the previous
_PackagesPublished definition read _RunAsPublic and Build.Reason the
same way.

Co-authored-by: Copilot <[email protected]>
@radical
ci(installers): extract installed-CLI smoke test to shared scripts
3763a14 
prepare-installer-artifacts.yml carried two near-identical inline smoke
tests — one pwsh, one bash — that scaffold an aspire-starter project and
run restore against the just-installed CLI. The two blocks drift
independently and there is no equivalent in AzDO: the only post-install
check inside validate-cask-artifact.sh and prepare-manifest-artifact.ps1
is `aspire --version`. If AzDO ever wants the same coverage there will
be a third copy.

Extract to eng/scripts/smoke-installed-cli.{sh,ps1}. The workflow now
calls them after the installer-specific PATH refresh, which stays inline
because it describes the GitHub Actions session state, not the smoke
logic. AzDO is not adopted in this change.

Co-authored-by: Copilot <[email protected]>
@radical
ci(homebrew): authenticate the upstream-cask existence probe
cdab0f8 
The "Prepare Homebrew cask artifact" job in prepare-installer-artifacts.yml
failed on PRs with:

    Determining whether aspire is a new upstream cask...
    Error: could not determine whether Casks/a/aspire.rb exists upstream (HTTP 403)
    ##[error]Process completed with exit code 1.

The probe in validate-cask-artifact.sh::detect_upstream_cask_is_new makes an
unauthenticated GET to api.github.com to classify the cask as new vs. update.
Unauthenticated requests share a 60/hour rate-limit pool across the runner
IP — easy to exhaust on GH-hosted macOS — and return 403 when throttled. The
script treats any non-200/404 as a fatal misclassification (the audit mode
and the eventual bot PR body both key off this signal), so the 403 fails
the whole job.

Send an `Authorization: Bearer <token>` header when GH_TOKEN or GITHUB_TOKEN
is present (auth raises the limit to 1000/h+). When neither is set, fall
back to unauthenticated — preserving prior behavior for callers without a
token in scope.

Pass `secrets.GITHUB_TOKEN` into the Homebrew step via GH_TOKEN env so the
script picks it up. AzDO callers of the same script have no GH token in
scope today and continue running unauthenticated; the limit is per-IP and
the AzDO Microsoft-hosted pool generally has plenty of headroom there.

Failing run for reference:
https://github.com/microsoft/aspire/actions/runs/26195583728/job/77075657321

Co-authored-by: Copilot <[email protected]>
@radical
ci(installers): gate URL/summary validation on production branches
eb507fd 
The "Validate installer URLs from manifest", "Validate ReleaseNotesUrl",
"Validate Homebrew validation summary", and "Validate download URLs from
cask" steps in the publish-winget/publish-homebrew templates run inside
the publish job to surface broken artifact URLs or missing prepare-stage
checks before submitting an upstream PR.

These checks only make sense when we're actually about to submit:

- The URL probes hit canonical ci.dot.net paths that exist only after
  packages have been published on a production release build. On a
  non-production branch the prerelease artifacts go to different paths
  (or aren't published to ci.dot.net at all), so the probes fail with
  404s before the publish-template's submit-step guard is reached.

- The Homebrew validation-summary check requires brewInstall/brewUninstall
  to be marked 'passed', which only happens when the prepare stage runs
  with runInstallTest=true -- true only for production-publishing builds.
  On a non-prod prepare the required checks are absent and the validator
  fails before our Submit guard runs.

Same effect as the existing dryRun=false clause that already gated the
Homebrew summary validator: in dry-run we're not submitting, so URL
reachability and summary completeness aren't useful signals.

Reuse the existing Submit-step condition verbatim
(dryRun=false AND _IsProductionBranch=true) so these checks run if and
only if we'd actually submit a PR upstream.

Observed in dogfood release-publish-nuget run on ankj/homebrew-release:
both validators failed -> Submit step never reached -> no upstream PRs
opened (which the existing Submit-step guard would have produced anyway),
but the spurious red made the dogfood run unreadable.

Co-authored-by: Copilot <[email protected]>
@radical radical force-pushed the radical/installer-manifests branch from e298e86 to eb507fd Compare May 21, 2026 06:36
@radical
docs(dogfood): drop unsupported x86 arch from PR-dogfood reference
8eb08aa 
The PR dogfood scripts only support x64 and arm64:

* eng/scripts/get-aspire-cli-pr.ps1 uses
  ValidateSet("", "x64", "arm64") for -Architecture.
* eng/scripts/get-aspire-cli-pr.sh's
  get_cli_architecture_from_architecture accepts only amd64/x64
  and arm64; anything else hits the "Architecture ... not supported"
  error path.

The reference table in docs/dogfooding-pull-requests.md previously
listed `x86` as an allowed --arch / -Architecture value, which
would mislead anyone trying to cross-target it: the scripts would
hard-fail rather than fall back.

Co-authored-by: Copilot <[email protected]>
@radical radical changed the title Add GitHub installer artifact generation Add WinGet/Homebrew installer dogfood flows for PR builds May 21, 2026
@radical radical marked this pull request as ready for review May 21, 2026 06:54
Copilot AI review requested due to automatic review settings May 21, 2026 06:54
Copilot AI reviewed May 21, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds end-to-end “dogfood” generation and installation flows for Aspire CLI WinGet manifests and Homebrew casks produced from GitHub Actions PR builds, reusing shared repo scripts across GitHub Actions and Azure DevOps. It also fixes a WinGet first-run failure by updating CLI layout discovery to resolve launcher symlinks/reparse targets consistently.

Changes:

  • Add reusable GitHub Actions workflow + jobs to prepare/upload WinGet/Homebrew installer artifacts (and smoke-test the installed CLI).
  • Move WinGet/Homebrew artifact preparation and validation into shared eng/* scripts, and update AzDO templates to call them.
  • Fix CLI layout discovery for WinGet “Links” shims by resolving process-path symlinks before relative layout probing; add tests and dogfood-mode tests for PR installer scripts.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Aspire.Cli.Tests/Layout/LayoutDiscoveryReparsePointTests.cs Adds coverage for process-path symlink resolution and fallback behavior in layout discovery.
tests/Aspire.Acquisition.Tests/Scripts/PRScriptInstallerModeTests.cs Adds end-to-end tests for PR installer modes (WinGet/Homebrew) and dogfood behaviors.
src/Aspire.Cli/Layout/LayoutDiscovery.cs Updates relative layout discovery to resolve symlink targets first (with fallback) and adds a test hook.
eng/winget/README.md Documents new artifact-preparation and dogfood flows for WinGet.
eng/winget/prepare-manifest-artifact.ps1 New shared script to generate/validate/test WinGet manifests and stage dogfood helper.
eng/winget/dogfood.ps1 Enhances dogfood install to support local archive serving + SHA refresh + improved verification/diagnostics.
eng/scripts/smoke-installed-cli.sh New bash smoke test to validate an already-installed aspire CLI via new + restore.
eng/scripts/smoke-installed-cli.ps1 New PowerShell smoke test to validate an already-installed aspire CLI via new + restore.
eng/scripts/README.md Updates PR dogfood docs to include WinGet/Homebrew install modes.
eng/scripts/get-aspire-cli-pr.sh Adds WinGet/Homebrew installer modes (download + install) for PR artifacts.
eng/scripts/get-aspire-cli-pr.ps1 Adds WinGet/Homebrew installer modes (download + install) for PR artifacts.
eng/pipelines/templates/publish-winget.yml Tightens submission gating to production branches; adds draft conversion and reduces unnecessary steps in non-submit scenarios.
eng/pipelines/templates/publish-homebrew.yml Tightens submission gating to production branches; reuses draft-conversion mutation variable and avoids non-submit failures.
eng/pipelines/templates/prepare-winget-manifest.yml Switches AzDO prepare to call shared WinGet artifact-prep script.
eng/pipelines/templates/prepare-homebrew-cask.yml Switches AzDO prepare to call shared Homebrew artifact-prep script.
eng/pipelines/common-variables.yml Introduces _IsProductionBranch as a single source of truth for “publishing branch” checks.
eng/pipelines/azure-pipelines.yml Updates _PackagesPublished to depend on _IsProductionBranch.
eng/homebrew/validate-cask-artifact.sh New shared validator for cask syntax/style/audit/install (supports offline/local-archive validation).
eng/homebrew/README.md Documents new Homebrew preparation/validation scripts and GitHub Actions prerelease artifact flows.
eng/homebrew/prepare-cask-artifact.sh New shared script to generate + validate cask artifacts and include dogfood helper.
eng/homebrew/dogfood.sh Extends dogfood install to optionally use locally downloaded archives, plus stronger verification/shadow checks.
eng/homebrew/aspire.rb.template Adjusts sidecar write string literal form and adds explicit empty zap to satisfy audit expectations.
docs/dogfooding-pull-requests.md Updates PR dogfooding documentation to include WinGet/Homebrew modes and prerequisites.
.github/workflows/tests.yml Adds macOS x64 archive build and installer-artifact preparation jobs; wires them into overall CI dependencies.
.github/workflows/prepare-installer-artifacts.yml New reusable workflow to prepare, dogfood-install, smoke-test, and upload WinGet/Homebrew installer artifacts.

Comment thread eng/scripts/smoke-installed-cli.sh Outdated
Comment thread eng/scripts/smoke-installed-cli.ps1 Outdated
Comment thread eng/homebrew/README.md Outdated
@radical
fix(installer): drop destructive rm -rf from smoke helpers; clarify c…
548668b 
…ask audit doc

`smoke-installed-cli.sh` and `smoke-installed-cli.ps1` previously wiped a
configurable `--work-dir` with `rm -rf` / `Remove-Item -Recurse -Force` to
make reruns idempotent. The script is only ever invoked from
`prepare-installer-artifacts.yml` with no arguments, so the destructive
path only mattered as defense against caller misuse — but a passed-in
sensitive path (e.g. `/`, `$HOME`, a drive root) would still be wiped
without warning.

Restructure both scripts so `--work-dir` / `-WorkDir` is documented as
a *parent* directory under which a fresh `aspire-cli-smoke.XXXXXX`
subdirectory is created via `mktemp` (or `[guid]::NewGuid()` on
PowerShell). The destructive operation is removed entirely; nothing
that the caller passes in is ever deleted. CI tears down
`RUNNER_TEMP` between jobs, so the lack of an explicit cleanup is
fine in CI and friendlier for local interactive use (the scaffolded
project survives until the user removes it).

Also corrects `eng/homebrew/README.md` to match the actual
`validate-cask-artifact.sh` behavior: `brew audit` only passes
`--new` when the cask is absent upstream (existing casks fail the
additional `--new` checks), and `--no-signing` is added in offline
mode (used for PR-artifact validation against unsigned local
archives), not unconditionally. The previous wording would have
mis-guided anyone reading the doc.

Co-authored-by: Copilot <[email protected]>
@radical radical requested review from Copilot and joperezr May 21, 2026 07:18
Copilot AI reviewed May 21, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Comment thread eng/winget/dogfood.ps1 Outdated
radical and others added 3 commits May 21, 2026 03:39
@radical
docs(installer): fix dogfood.ps1 diag-log stream comment
a08bf7e 
`Show-LatestWinGetDiagLog` uses `Write-Host` (matching the rest of
`eng/winget/dogfood.ps1`, where Write-Host is the prevailing
convention), not stderr. The earlier comment claimed the newest diag
log was printed to stderr, which was inaccurate and could mislead a
future reader trying to capture / filter diagnostic output.

Update the comment to match the code: the log is written to the host
console via Write-Host, which GitHub Actions and the host capture
into the same job log as the surrounding install output.

Co-authored-by: Copilot <[email protected]>
@radical
Merge remote-tracking branch 'origin/main' into radical/installer-man…
496fc65 
…ifests
@radical
test(cli/acquisition): widen PeerInstallProbeTests timeouts and renam…
c621e1d 
…e misleading test

Two distinct flakes were observed in `Tests / Cli / Cli (windows-latest)`
on PR microsoft#17115 (CI run 26212960557, attempt 1):

1. `ProbeAsync_PeerOmitsPathStatus_DefaultsToNotOnPath` failed with
   `Expected PeerProbeResult.Ok, got PeerProbeResult.Failed.
    Reason: Peer produced no usable output (and --version fallback).`
   Under heavy CI load (the failing run logged 87.5% CPU and saturated
   disk via the HEARTBEAT line) the cmd.exe-based fake peer script
   takes longer than the production 5s probe timeout just to spawn,
   so the probe synthesizes `Failed: timed out after 5.0s` before the
   peer prints stdout. This is the same class of CI flake that commit
   858825b fixed for the negative-path stderr tests via
   `ProbeFakeFailureAsync`; that fix did not cover the positive-path
   tests, which still used the default 5s ctor.

2. `ProbeAsync_PeerHangs_TimesOutAndKills` failed with
   `Expected probe to return within a few seconds after timing out;
    took 00:00:05.0820126.`
   The probe configured a 300ms internal timeout and the test asserted
   the wallclock came back in under 5s. On Windows under load the
   spawn -> ping -> terminate round trip alone ate 5.08s, exceeding the
   budget by 80ms. The 5s outer budget is too tight for what the
   test claims to catch: a probe that ignores its configured timeout
   would still trip a 15s bound, while the real production timeout
   path (which is what the `AndKills` suffix was meant to cover) is
   bounded by the 300ms ctor argument and synthesizes the `Failed`
   result regardless of the outer wallclock.

   The `AndKills` part of the name also overstates what the test
   asserts. The fake peer process cleanup is via `using var fakePeer`
   disposal, not a direct assertion against the production probe.
   The process-termination semantic is rigorously covered by
   `ProbeAsync_CallerCancels_KillsSpawnedProcess` in the same file,
   which uses a PID file + `Process.HasExited`. Rename to
   `ProbeAsync_PeerHangs_TimesOutAndReturnsFailed` to match what is
   actually checked.

Changes:

- Introduce `CreateProbeWithGenerousTimeout()` helper mirroring the
  pattern from `ProbeFakeFailureAsync` (30s internal timeout, well
  above the production 5s default).
- Replace all 10 positive-path uses of the default
  `new PeerInstallProbe(ProbeLogger)` ctor with the helper. The
  timeout-path tests (`ProbeAsync_PeerHangs_TimesOutAndReturnsFailed`
  with the 300ms ctor, and `ProbeAsync_CallerCancels_KillsSpawnedProcess`
  with the 30s ctor) keep their dedicated constructors.
- Widen the outer-wallclock budget in
  `ProbeAsync_PeerHangs_TimesOutAndReturnsFailed` from 5s to 15s, and
  rename it from the `...AndKills` form that overstated the assertion.
- Update the cross-reference comment in `ProbeFakeFailureAsync` to
  the new test name.

Co-authored-by: Copilot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@danegsta danegsta Awaiting requested review from danegsta

@mitchdenny mitchdenny Awaiting requested review from mitchdenny mitchdenny is a code owner

@JamesNK JamesNK Awaiting requested review from JamesNK JamesNK is a code owner

@davidfowl davidfowl Awaiting requested review from davidfowl davidfowl is a code owner

@joperezr joperezr Awaiting requested review from joperezr

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

WinGet-installed Aspire CLI fails bundle discovery when launched through Links shim

2 participants

@radical