Switch to fail-fast prompt waits after WaitUntilAsync in CLI E2E tests#15934
Switch to fail-fast prompt waits after WaitUntilAsync in CLI E2E tests#15934
Conversation
…E2E tests Agent-Logs-Url: https://github.com/microsoft/aspire/sessions/b30cde2e-8639-49e0-9687-9db0f1466b2a Co-authored-by: radical <1472+radical@users.noreply.github.com>
… throw in WaitUntilAsync lambdas Agent-Logs-Url: https://github.com/microsoft/aspire/sessions/454b53f5-fae3-4211-84ae-fa23f729622a Co-authored-by: radical <1472+radical@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 -- 15934Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15934" |
There was a problem hiding this comment.
Pull request overview
This PR updates Aspire CLI end-to-end tests to use a fail-fast prompt wait helper after certain WaitUntilAsync output checks, aiming to reduce long timeouts when a CLI command exits non-zero and returns an error prompt.
Changes:
- Replaced
WaitForSuccessPromptAsync(counter)withWaitForSuccessPromptFailFastAsync(counter)in several non-interactive command test flows. - Applied the change across banner, doctor, agent help, and CPM “aspire add” scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| tests/Aspire.Cli.EndToEnd.Tests/DoctorCommandTests.cs | Uses fail-fast prompt waiting after aspire doctor output checks. |
| tests/Aspire.Cli.EndToEnd.Tests/CentralPackageManagementTests.cs | Uses fail-fast prompt waiting after aspire add flow completion. |
| tests/Aspire.Cli.EndToEnd.Tests/BannerTests.cs | Uses fail-fast prompt waiting after banner/help output checks. |
| tests/Aspire.Cli.EndToEnd.Tests/AgentCommandTests.cs | Uses fail-fast prompt waiting after some help-output WaitUntilAsync checks. |
| s => s.ContainsText(RootCommandStrings.BannerWelcomeText) && s.ContainsText("Telemetry"), | ||
| timeout: TimeSpan.FromSeconds(30), description: "waiting for banner and telemetry notice on first run"); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
| await auto.WaitForSuccessPromptFailFastAsync(counter); |
There was a problem hiding this comment.
Switching to WaitForSuccessPromptFailFastAsync only checks for an ERR prompt after this WaitUntilAsync has already succeeded. If the command exits with an error before printing the expected banner text, WaitUntilAsync will still spin until its timeout. To truly fail fast on non-zero exits, consider adding an ERR-prompt check inside the WaitUntilAsync predicate (or wrap WaitUntilAsync in a helper that returns/throws when [N ERR: is observed).
| s => s.ContainsText(RootCommandStrings.BannerWelcomeText) && s.ContainsText("CLI"), | ||
| timeout: TimeSpan.FromSeconds(30), description: "waiting for banner with version info"); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
| await auto.WaitForSuccessPromptFailFastAsync(counter); |
There was a problem hiding this comment.
Same concern as above: this fail-fast prompt check runs only after WaitUntilAsync completes, so it won’t shorten timeouts when the command fails before producing the expected output. Consider integrating ERR prompt detection into the WaitUntilAsync predicate (or using a helper that does both).
| return s.ContainsText(HelpGroupStrings.HelpHint); | ||
| }, timeout: TimeSpan.FromSeconds(30), description: "waiting for help output to complete"); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
| await auto.WaitForSuccessPromptFailFastAsync(counter); |
There was a problem hiding this comment.
This fail-fast prompt wait happens after the help-output WaitUntilAsync, so it won’t prevent a full WaitUntilAsync timeout if the command errors before emitting HelpHint. If the goal is to fail immediately on non-zero exits, consider checking for an ERR prompt in the WaitUntilAsync loop as well.
| s => s.ContainsText("dev-certs") && s.ContainsText("partially trusted"), | ||
| timeout: TimeSpan.FromSeconds(60), description: "doctor to complete with partial trust warning"); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
| await auto.WaitForSuccessPromptFailFastAsync(counter); |
There was a problem hiding this comment.
WaitForSuccessPromptFailFastAsync runs after the output-text WaitUntilAsync. If aspire doctor fails before emitting the expected text, the test will still wait the full WaitUntilAsync timeout. If you’re aiming to fail fast on [N ERR: prompts, consider adding ERR-prompt detection to the WaitUntilAsync predicate (or using a helper that combines both checks).
| return s.ContainsText("certificate is trusted"); | ||
| }, timeout: TimeSpan.FromSeconds(60), description: "doctor to complete with trusted certificate"); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
| await auto.WaitForSuccessPromptFailFastAsync(counter); |
There was a problem hiding this comment.
Same pattern here: switching the prompt wait to fail-fast doesn’t help if WaitUntilAsync never becomes true due to an early command failure; it will still spin until the timeout. Consider detecting [N ERR: inside the WaitUntilAsync predicate (or using a wrapper helper) so failures surface immediately.
| s => s.ContainsText("mcp") && s.ContainsText("init"), | ||
| timeout: TimeSpan.FromSeconds(30), description: "agent help showing mcp and init subcommands"); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
| await auto.WaitForSuccessPromptFailFastAsync(counter); |
There was a problem hiding this comment.
Fail-fast prompt detection occurs only after WaitUntilAsync succeeds. If the help command fails before printing the expected text, WaitUntilAsync will still time out. If the intent is to avoid long timeouts on non-zero exits, consider adding an ERR-prompt check inside WaitUntilAsync (or use a helper that fails fast on ERR while waiting for output).
| s => s.ContainsText("tools") && s.ContainsText("call"), | ||
| timeout: TimeSpan.FromSeconds(30), description: "mcp help showing tools and call subcommands"); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
| await auto.WaitForSuccessPromptFailFastAsync(counter); |
There was a problem hiding this comment.
Same concern: this only fails fast after WaitUntilAsync succeeds. If aspire mcp --help exits with an error before producing the expected tokens, the test will still wait the full WaitUntilAsync timeout. Consider integrating ERR prompt detection into the WaitUntilAsync predicate (or a wrapper helper).
| s => s.ContainsText("dev-certs") && s.ContainsText("deprecated") && s.ContainsText("aspire agent init"), | ||
| timeout: TimeSpan.FromSeconds(60), description: "doctor output with deprecated warning and fix suggestion"); | ||
| await auto.WaitForSuccessPromptAsync(counter); | ||
| await auto.WaitForSuccessPromptFailFastAsync(counter); |
There was a problem hiding this comment.
This fail-fast prompt wait comes after the output-text WaitUntilAsync. If aspire doctor fails before emitting the expected warning text, WaitUntilAsync will still run until timeout. To match the stated goal of fail-fast on ERR prompts, consider checking for [N ERR: within the WaitUntilAsync predicate as well.
| } | ||
|
|
||
| await auto.WaitForSuccessPromptAsync(counter); | ||
| await auto.WaitForSuccessPromptFailFastAsync(counter); |
There was a problem hiding this comment.
In this test, WaitUntilAsync above waits for either the version-selection prompt or a success prompt. If the command exits with an error prompt, WaitUntilAsync will still time out before reaching this fail-fast check. Consider updating the preceding WaitUntilAsync predicate to also return on an ERR prompt (and throw), or using a helper that waits for OK/ERR and fails fast on ERR while still allowing the optional version-selection interaction.
|
@mitchdenny The automated reviewer flagged the same concern on all 9 changed sites: This is technically correct — the current change only helps in the narrower case where the expected output text does appear but the command then exits with a non-zero code. The more common failure scenario (command errors immediately, never produces the expected text) isn't covered. To truly fail fast, ERR-prompt detection would need to happen inside the await auto.WaitUntilAsync(s =>
{
if (s.ContainsErrPrompt(counter)) throw new Exception("Command failed unexpectedly");
return s.ContainsText("expected output");
}, timeout: TimeSpan.FromSeconds(30));Or better yet, a helper like That said, this PR is still a strict improvement over the baseline — do you want to treat the reviewer's suggestion as a follow-up enhancement, or address it in this PR? -- copilot |
Description
Reviewed all
WaitUntilAsynccalls inAspire.Cli.EndToEnd.Testsand added fail-fast error detection to those that wait for output from completing (non-interactive) commands but had no guard against the command producing an error prompt.Motivation: When a CLI command exits with a non-zero code and the test is using
WaitUntilAsyncto look for specific success text, the wait spins until the full timeout (30–180 s) before the test fails. The fix usesWaitForSuccessPromptFailFastAsyncafter each text-checkWaitUntilAsync, which fails immediately upon seeing an error prompt and gives faster feedback in CI.Cases updated — all are completing (non-interactive) commands:
BannerTests.cs(3 sites)aspire cache clear,aspire --banner,aspire --nologo --helpWaitForSuccessPromptAsync→WaitForSuccessPromptFailFastAsyncDoctorCommandTests.cs(2 sites)aspire doctor(both tests)WaitForSuccessPromptAsync→WaitForSuccessPromptFailFastAsyncAgentCommandTests.cs(3 sites)aspire agent --help,aspire mcp --help, deprecated-configaspire doctorWaitForSuccessPromptAsync→WaitForSuccessPromptFailFastAsyncCentralPackageManagementTests.cs(1 site)aspire addWaitForSuccessPromptAsync→WaitForSuccessPromptFailFastAsyncThe
WaitUntilAsynclambdas are kept focused on checking for expected output text; error-prompt detection is handled by the establishedWaitForSuccessPromptFailFastAsynchelper.Cases kept as-is (justified):
SmokeTests.cs,JsReactTemplateTests.cs—aspire runis long-running; no completion prompt until Ctrl+C; already throws on regression case.PlaywrightCliInstallTests.cs(4×),AgentCommandTests.cs(4×) — interactive prompts mid-command; command is still running, no prompt yet.ConfigDiscoveryTests.cs— already checks forERR:text in the lambda.StartStopTests.cs—WaitForAnyPromptAsyncfollows; both valid text outcomes checked.CliE2EAutomatorHelpers.cs:112— initial bash prompt before prompt-counting is configured; no sequence number available.CliE2EAutomatorHelpers.cs:285,ProjectReferenceTests.cs— already handle both success and error paths.TypeScriptCodegenValidationTests.cs— expects failure;WaitForAnyPromptAsyncfollows.DescribeCommandTests.cs— simpleecho/catshell commands with 10 s timeout.Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: