Add comprehensive test coverage for CLI acquisition scripts#15995
Open
Add comprehensive test coverage for CLI acquisition scripts#15995
Conversation
Contributor
|
🚀 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" |
Contributor
|
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.
|
radical
force-pushed
the
copilot/implement-cli-testing-scripts
branch
from
c27293c to
8556d0d
Compare
Contributor
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. |
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>
radical
force-pushed
the
copilot/implement-cli-testing-scripts
branch
from
e982f47 to
0368dd0
Compare
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>
Contributor
|
🎬 CLI E2E Test Recordings — 58 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24277193743 |
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.
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