Add TerminalRun IAsyncDisposable for consistent CLI E2E diagnostics capture#17576
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17576Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17576" |
There was a problem hiding this comment.
Pull request overview
Introduces a TerminalRun : IAsyncDisposable helper that wraps the CLI E2E terminal lifecycle so CaptureAspireDiagnosticsAsync, exit/Enter, and await pendingRun are always executed at the end of a test — including when the test body throws. Migrates 10 existing CLI E2E tests to the new pattern and documents the convention in the cli-e2e-testing skill. Remaining tests will be migrated in follow-ups.
Changes:
- Add
TerminalRuntype andCliE2ETestHelpers.StartRun(...)factory. - Replace manual
pendingRun/exit/await pendingRunin 10 tests withawait using var terminalRun = CliE2ETestHelpers.StartRun(...). - Document the
TerminalRunpattern (DO/DON'T) in.agents/skills/cli-e2e-testing/SKILL.md.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.EndToEnd.Tests/Helpers/TerminalRun.cs | New IAsyncDisposable ensuring diagnostics capture, exit, and run-await on dispose, each best-effort. |
| tests/Aspire.Cli.EndToEnd.Tests/Helpers/CliE2ETestHelpers.cs | Adds StartRun factory returning a TerminalRun. |
| tests/Aspire.Cli.EndToEnd.Tests/WaitCommandTests.cs | Migrate CreateStartWaitAndStopAspireProject to StartRun. |
| tests/Aspire.Cli.EndToEnd.Tests/StopNonInteractiveTests.cs | Migrate StopNonInteractiveSingleAppHost to StartRun. |
| tests/Aspire.Cli.EndToEnd.Tests/StartStopTests.cs | Migrate StopWithNoRunningAppHostExitsSuccessfully to StartRun. |
| tests/Aspire.Cli.EndToEnd.Tests/SmokeTests.cs | Migrate three smoke tests to StartRun. |
| tests/Aspire.Cli.EndToEnd.Tests/DocsCommandE2ETests.cs | Migrate docs E2E test to StartRun. |
| tests/Aspire.Cli.EndToEnd.Tests/ConfigHealingTests.cs | Migrate InvalidAppHostPathWithComments_IsHealedOnRun to StartRun. |
| tests/Aspire.Cli.EndToEnd.Tests/CertificatesCommandTests.cs | Migrate two certificate tests to StartRun. |
| tests/Aspire.Cli.EndToEnd.Tests/BundleSmokeTests.cs | Migrate bundle smoke test to StartRun. |
| .agents/skills/cli-e2e-testing/SKILL.md | Documents the TerminalRun pattern with DO/DON'T examples. |
…apture Introduce a TerminalRun type that wraps terminal.RunAsync and ensures CaptureAspireDiagnosticsAsync, exit, and await pendingRun always run at the end of CLI E2E tests via IAsyncDisposable. - Add TerminalRun.cs with IAsyncDisposable lifecycle management - Add CliE2ETestHelpers.StartRun factory method - Apply to 10 tests as initial rollout - Update cli-e2e-testing skill with new pattern
1b99429 to
0e854c9
Compare
|
deployment tests? |
Working up to them. |
Replace manual pendingRun/exit/await pattern with the TerminalRun IAsyncDisposable wrapper across all 67 test files. TerminalRun handles diagnostics capture, exit command, and workspace artifact collection automatically on dispose. Remove redundant testBodyFailed try/catch/finally blocks and manual CaptureAspireDiagnosticsAsync calls that are now handled by TerminalRun disposal.
|
❓ CLI E2E Tests unknown — 107 passed, 0 failed, 2 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26572248035 |
|
✅ No documentation update needed. docs_optional → No signals triggered (signal_count = 0). The advisory This PR adds a |
* Add TerminalRun IAsyncDisposable for consistent CLI E2E diagnostics capture (#17576) * Fix CLI Ctrl+C/SIGTERM shutdown: responsive cancellation and double-signal bug (#17588) * Fix CLI Ctrl+C shutdown taking too long during AppHost startup - Make ConsoleCancellationManager.Cancel() non-blocking so Ctrl+C handler returns immediately - Pass cancellation token through to WaitAsync in CancelAppHostStartupAsync so Ctrl+C exits promptly instead of waiting for the full 5s timeout - Support second Ctrl+C for immediate force exit (Environment.Exit) - Add logging support to ConsoleCancellationManager via SetLogger() - Add comprehensive unit tests for ConsoleCancellationManager - Add integration test for RunCommand cancellation during startup timeout Fixes #17569 * Fix review comments: volatile logger, accurate comments, clearer warning * Clean up * Use WaitForSuccessPromptFailFastAsync in CLI E2E tests Replace WaitForSuccessPromptAsync with WaitForSuccessPromptFailFastAsync across all CLI E2E tests. The fail-fast variant detects error prompts immediately and throws instead of hanging for up to 500s waiting for a success prompt that will never arrive. This prevents 10+ minute CI timeouts when a command fails with a non-zero exit code. * Fix double-signal bug and consolidate test helpers - Fix ConsoleCancellationManager double-signal bug: move Console.CancelKeyPress to else branch so it only registers on platforms without PosixSignalRegistration. Previously both handlers fired for the same SIGINT, causing immediate force-kill. - Add SIGQUIT/Ctrl+Break registration for Windows parity. - Remove old WaitForSuccessPromptAsync (no fail-fast) and rename WaitForSuccessPromptFailFastAsync to WaitForSuccessPromptAsync. - Remove duplicate RunCommandFailFastAsync (identical to RunCommandAsync). * Fix stale comment and restrict SIGQUIT to Windows only * Remove unused legacy builder methods and update E2E skill docs * Don't force when debugging * More logging
Brings 43 release-branch commits forward onto main now that 13.4.0 has shipped. This PR replaces the original automated merge (microsoft#17804) which had to be closed so that conflict resolution and post-merge cleanups could be made on a non-protected branch. Conflict resolution summary (33 files): * Equivalent backports (took main's commit identity): ChannelUpdateWorkflowTests, LoggingHelpersTests, the four extension test files, AspireEditorCommandProvider, appHostDiscovery. * Release-only forwards (preserved): microsoft#17732 / microsoft#17756 Foundry hosted-agent protocol selection and cross-compute-environment endpoint references, microsoft#17573 stabilize PrebuiltAppHostServer staging globalPackagesFolder path, microsoft#17743 staging-identity CLI darc feed routing. * Main-only forwards (preserved): microsoft#17506 Show discovered AppHosts in Aspire pane, microsoft#17547 Localize Aspire skills metadata errors, microsoft#17801 VS Code v1.12.0, microsoft#17297 Aspire CLI npm package release integration, microsoft#17576 TerminalRun IAsyncDisposable, microsoft#17721 / microsoft#17723 VS Code telemetry, microsoft#17671 ATS baseline fix (re-applied manually on top of Foundry source taken from release). * Hybrid (manually spliced): docs/contributing.md - kept main's restructured layout and inserted release's staging-validation paragraph; HostedAgentBuilder- Extension - took release base then re-applied microsoft#17671 asHostedAgent rename; UpdateCommandTests - took main and injected microsoft#17743's OverrideCliInformationalVersionConfigKey block. Post-merge cleanups included in this PR: * eng/Versions.props: revert StabilizePackageVersion to false (was flipped to true on release/13.4 by microsoft#17520 for shipping 13.4.0; main must stay in preview mode). * .github/workflows/generate-api-diffs.yml: retarget back to main (was pointed at release/13.4 by microsoft#17696 release prep). * .github/workflows/backmerge-release.yml: update from release/13.3 to release/13.4 (was stale - missed the 13.4 release-time bump). * .github/workflows/milestone-assignment.yml: audited - already correct (main -> 13.5, release/13.4 -> 13.4.x); no change. This merge commit must be preserved - do not squash on merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Add a
TerminalRuntype (IAsyncDisposable) that wraps the CLI E2E terminal lifecycle, ensuringCaptureAspireDiagnosticsAsyncalways runs at the end of a test — even when the test fails. This replaces the manualexit/await pendingRunboilerplate and guarantees diagnostics are consistently captured across all tests.Before: Each test manually called
exit,Enter, andawait pendingRunat the end. Tests that failed mid-execution would skip diagnostics capture entirely.After: Tests use
await using var terminalRun = CliE2ETestHelpers.StartRun(...)which automatically handles diagnostics capture, terminal exit, and run completion on disposal.Applied to 10 tests as an initial rollout. The remaining tests will be migrated once this pattern is validated.
Checklist