Add comprehensive test coverage for CLI acquisition scripts#15995
Add comprehensive test coverage for CLI acquisition scripts#15995
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15995Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15995" |
|
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.
|
c27293c to
8556d0d
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new Aspire.Acquisition.Tests test project to validate the CLI acquisition scripts in eng/scripts/, plus a small refactor to make the bash scripts safely “sourceable” for function-level testing. The PR also factors shared Process extension helpers into Aspire.TestUtilities and modernizes a couple of shared test helpers.
Changes:
- Introduce
tests/Aspire.Acquisition.Testswith unit-style function tests and tool-style script invocation tests for the acquisition scripts (bash + PowerShell). - Update
get-aspire-cli.sh/get-aspire-cli-pr.shto wrap execution in amain()guarded byBASH_SOURCEfor safe sourcing. - Move
Processlifecycle extension methods intotests/Aspire.TestUtilitiesand update existing test helpers to consume them.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Shared/TemplatesTesting/ToolCommandException.cs | Switch to primary-constructor style exception with inline property init. |
| tests/Shared/TemplatesTesting/CommandResult.cs | Switch CommandResult to primary-constructor style struct with inline property init. |
| tests/Infrastructure.Tests/WorkflowScripts/NodeCommand.cs | Import Aspire.TestUtilities for shared Process extensions. |
| tests/Infrastructure.Tests/PowerShellScripts/PowerShellCommand.cs | Remove local ProcessExtensions; now uses Aspire.TestUtilities. |
| tests/Aspire.TestUtilities/ProcessExtensions.cs | New shared Process extension helpers for safe termination/exit checks. |
| tests/Aspire.Acquisition.Tests/xunit.runner.json | Enables xUnit parallelization for the new test project. |
| tests/Aspire.Acquisition.Tests/Scripts/SourceabilityTests.cs | Verifies bash scripts can be sourced without running main. |
| tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptShellTests.cs | Bash release-script end-to-end-ish argument/dry-run tests. |
| tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptPSFunctionTests.cs | PowerShell release-script function-level tests (URLs, checksum, extraction, RID detection). |
| tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptPowerShellTests.cs | PowerShell release-script parameter/help/WhatIf behavior tests. |
| tests/Aspire.Acquisition.Tests/Scripts/ReleaseScriptFunctionTests.cs | Bash release-script function-level tests (URLs, checksum, extraction, profile edits, content-type). |
| tests/Aspire.Acquisition.Tests/Scripts/PSSourceabilityTests.cs | Verifies PS scripts can load functions without executing main (AST extraction). |
| tests/Aspire.Acquisition.Tests/Scripts/PRScriptShellTests.cs | Bash PR-script end-to-end-ish argument validation using a mock gh. |
| tests/Aspire.Acquisition.Tests/Scripts/PRScriptPSFunctionTests.cs | PowerShell PR-script function tests (RID, suffix extraction, temp cleanup, archive selection). |
| tests/Aspire.Acquisition.Tests/Scripts/PRScriptPowerShellTests.cs | PowerShell PR-script parameter/help/WhatIf behavior tests using a mock gh. |
| tests/Aspire.Acquisition.Tests/Scripts/PRScriptIntegrationTests.cs | Outerloop integration tests hitting real GitHub PR artifacts via gh. |
| tests/Aspire.Acquisition.Tests/Scripts/PRScriptFunctionTests.cs | Bash PR-script function tests (RID/suffix/temp cleanup/archive selection). |
| tests/Aspire.Acquisition.Tests/Scripts/Common/TestEnvironment.cs | Isolated temp HOME + mock tooling environment for script tests. |
| tests/Aspire.Acquisition.Tests/Scripts/Common/ScriptToolCommand.cs | Runs scripts via bash/pwsh under ToolCommand with repo-root resolution. |
| tests/Aspire.Acquisition.Tests/Scripts/Common/ScriptPaths.cs | Centralized relative paths to the acquisition scripts under eng/scripts. |
| tests/Aspire.Acquisition.Tests/Scripts/Common/ScriptFunctionCommand.cs | Wrapper for calling individual bash/PS functions in isolation (sourcing/AST extraction). |
| tests/Aspire.Acquisition.Tests/Scripts/Common/RealGitHubPRFixture.cs | Discovers a suitable merged PR/run with expected artifacts for integration tests. |
| tests/Aspire.Acquisition.Tests/Scripts/Common/FileHelper.cs | Unix executable-bit helper for mock tools and extracted binaries. |
| tests/Aspire.Acquisition.Tests/Scripts/Common/FakeArchiveHelper.cs | Generates fake archives + checksums and fake .nupkg files for tests. |
| tests/Aspire.Acquisition.Tests/README.md | Documents safety model and how to run unit/integration tests. |
| tests/Aspire.Acquisition.Tests/Aspire.Acquisition.Tests.csproj | New test project definition + links to shared ToolCommand infrastructure. |
| tags.lock | Adds a root-level lock-like file with a numeric value. |
| eng/scripts/get-aspire-cli.sh | Adds main() + BASH_SOURCE guard for sourceability; minor local var init. |
| eng/scripts/get-aspire-cli-pr.sh | Adds main() + BASH_SOURCE guard for sourceability; minor local var init. |
| Aspire.slnx | Adds the new test project to the solution. |
| .github/workflows/run-tests.yml | Exposes PR metadata + GH_TOKEN for the Acquisition test suite. |
e982f47 to
0368dd0
Compare
IEvangelist
left a comment
There was a problem hiding this comment.
2 issues found: 1 coverage gap and 1 test isolation bug.
IEvangelist
left a comment
There was a problem hiding this comment.
LGTM, that's a lot of CLI testing scripts.
Enable sourcing get-aspire-cli.sh and get-aspire-cli-pr.sh to load function definitions without executing the main flow. This is required for unit-testing individual script functions in isolation. Also initialize `local config_file=""` to prevent unbound-variable errors under `set -u` when no shell config file is found. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move ProcessExtensions from Infrastructure.Tests (internal) to Aspire.TestUtilities (public) so it can be shared by the new acquisition test project. Update Infrastructure.Tests to reference the shared version. Simplify CommandResult and ToolCommandException to use primary constructors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extend the existing requiresCliArchive token/metadata exports to also trigger when testShortName == 'Acquisition', so integration tests can access GitHub API when run as outerloop. Remove unrelated create-failing-test-issue.* entry from CI trigger skip-list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
New test project covering get-aspire-cli.sh, get-aspire-cli-pr.sh, and their PowerShell equivalents. Tests are organized in tiers: - Function-level tests (ScriptFunctionCommand): source a script and call individual functions in isolation — URL construction, platform detection, input validation, shell profile handling, archive ops. - Script-level tests (ScriptToolCommand): run full scripts with mock gh CLI and --dry-run to validate end-to-end parameter handling. - Piped install tests (ScriptHostFixture): serve scripts over HTTP and test curl|bash and irm|iex patterns against a real pipe. - Integration tests (RealGitHubPRFixture): query real GitHub PRs with --dry-run, gated behind OuterloopTest + Category=integration. Test infrastructure: - TestEnvironment: isolated temp dirs with mock HOME - FakeArchiveHelper: generates tar.gz/zip with .sha512 sidecars - Mock gh CLI: canned JSON for pr/run/api commands Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix Windows test failures where double quotes in ProcessStartInfo.Arguments were consumed as argument delimiters by MSVCRT command-line parser. Wrap the entire iex expression in outer quotes with escaped inner quotes so they survive as literal characters for pwsh -Command. Add 60-second timeouts to all piped install tests to prevent CI hang dumps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix macOS test failures where .NET's ProcessStartInfo.Arguments parser splits compound bash -c commands on spaces. Wrap the entire command string in quotes so it stays as a single argv entry. Add missing -Help switch parameter to get-aspire-cli-pr.ps1 (the release script has it but the PR script was missing it, causing iex test to fail with 'parameter not found'). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 'using' to all CreateCommandWithMockGhAsync call sites to ensure ToolCommand.Dispose() is called, which kills any running process. Without disposal, hung processes could accumulate under CI contention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lation
1. Mock gh api now returns endpoint-aware responses instead of always
returning {}. PR endpoint returns a realistic SHA, workflow endpoint
returns a realistic run ID. When --jq is present, returns the
pre-extracted value; otherwise returns the full JSON structure.
This ensures tests actually validate the PR lookup contract.
2. Override XDG_CONFIG_HOME in both ScriptFunctionCommand and
ScriptToolCommand to prevent bash scripts that consult
${XDG_CONFIG_HOME:-$HOME/.config} from reading real profile files
outside the test's temp home on developer machines with
XDG_CONFIG_HOME set.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
571b305 to
25f8a8e
Compare
joperezr
left a comment
There was a problem hiding this comment.
Overall this looks great — solid test infrastructure, comprehensive coverage across all 4 scripts, and good isolation via mock HOME/temp dirs. A few items to address before merging, particularly the first 6 below:
- GH_TOKEN unnecessarily exposed to non-integration Acquisition tests (security)
- Test class naming —
s_prefix is for static fields, not class names (convention) - TOCTOU port race in
ScriptHostFixture(flaky test risk) - Missing
[RequiresTools(["pwsh"])]on several PS test classes (test quality) - No default timeout on
ScriptToolCommand/ScriptFunctionCommand(test quality) - CTS not disposed in
PowerShellCommand/NodeCommand(resource leak, pre-existing) ToolCommandExceptionin global namespace (convention, pre-existing)- Bash
readonlyoverride is overly broad (test quality)
…pt v9 (#16174) - Restore pull-requests: write permission (required by the Issues API when updating PR resources, confirmed by 403 error in run #24421110155) - Update actions/github-script from v7.0.1 (Node.js 20) to v9.0.0 (Node.js 24) to avoid the June 2026 Node.js 20 deprecation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…16041) Bumps the npm_and_yarn group with 1 update in the /tests/Aspire.Cli.EndToEnd.Tests/Fixtures/JsPublish/nextjs directory: [next](https://github.com/vercel/next.js). Updates `next` from 15.5.14 to 15.5.15 - [Release notes](https://github.com/vercel/next.js/releases) - [Changelog](https://github.com/vercel/next.js/blob/canary/release.js) - [Commits](vercel/next.js@v15.5.14...v15.5.15) --- updated-dependencies: - dependency-name: next dependency-version: 15.5.15 dependency-type: direct:production dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Fix pr-docs-check docs PR flow Gate the workflow on significant user-facing changes and only draft aspire.dev documentation PRs when there is a clear docs gap. Recompile the workflow with gh aw v0.68.1, refresh the shared action lock, and restore the required safe_outputs checkout override for microsoft/aspire.dev under _repos/aspire.dev so cross-repo draft PR creation can switch repositories correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix pr-docs-check source-only checkouts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove Acquisition-specific GH_TOKEN injection from run-tests.yml (#14) - Rename s_-prefixed test classes to PascalCase (#15) - Add retry loop for TOCTOU port race in ScriptHostFixture (#16) - Add missing [RequiresTools(["pwsh"])] attributes (#17) - Add default 60s timeout to ScriptToolCommand/ScriptFunctionCommand (#18) - Fix CTS disposal in PowerShellCommand and NodeCommand (#19/#20) - Move ToolCommandException into Aspire.Templates.Tests namespace (#21) - Expand comment on broad readonly override in bash sourcing (#22) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
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.
|
|
🎬 CLI E2E Test Recordings — 69 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24438033409 |
|
No documentation PR is required for this change. Reason: This PR adds test infrastructure (
|
Summary
Adds a new test project (
Aspire.Acquisition.Tests) with ~160 tests covering the four CLI acquisition scripts (get-aspire-cli.sh,get-aspire-cli.ps1,get-aspire-cli-pr.sh,get-aspire-cli-pr.ps1). The goal is confidence to refactor these scripts — any incorrect change should break at least one test.Also includes minor script changes required for testability (wrapping in
main()withBASH_SOURCEguard, initializingconfig_filevariable underset -u).What's tested
Release script (
get-aspire-cli.sh)detect_os,detect_architecture, arch normalizationGITHUB_PATHintegrationadd_to_shell_profilefor bash/zsh, duplicate detection, dry-runPR script (
get-aspire-cli-pr.sh)detect_os,detect_architectureremove_temp_dirwith keep-archive and dry-run modesASPIRE_REPOoverride, artifact name prefixes, hive-only mode, unknown flagsPowerShell variants
Piped install patterns
curl | bashandcat | bashpatterns for bash scriptsirm | iexpattern for PowerShell scriptsTest infrastructure
ScriptFunctionCommand— sources a bash script and calls individual functions in isolation, enabling unit-level testing of internal helpersScriptToolCommand— runs the full script with arguments for end-to-end behavior testsScriptHostFixture— HTTP server for piped install tests (serves scripts overcurl/irm)TestEnvironment— creates per-test temp directories with mockHOME, mockcurl/ghscripts, andASPIRE_TEST_MODE=trueFakeArchiveHelper— generates fake.tar.gz/.ziparchives with.sha512sidecar filesghCLI handles--version,pr list,run list,run view,api, andrun downloadcommandsFiles changed
eng/scripts/get-aspire-cli.sh,get-aspire-cli-pr.sh—main()wrapper withBASH_SOURCEguard,config_fileinittests/Aspire.Acquisition.Tests/(~3,800 lines)ProcessExtensionsintotests/Aspire.TestUtilities/, simplifiedCommandResult/ToolCommandExceptionGH_TOKEN/PR metadata exports inrun-tests.ymlfor Acquisition testsAspire.slnx