Repository navigation
Fix remaining outerloop test failures - #20457
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 20457Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 20457" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The targeted fixes are consistent with the surrounding infrastructure and no unresolved correctness issues were found.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes scheduled outerloop failures in CLI and Dashboard tests.
Changes:
- Provisions JDK 21 for CLI outerloop tests with focused metadata coverage.
- Provides an empty terminal stream in the Dashboard Playwright fixture.
- Restricts known Fluent UI accessibility exemptions to affected rules and nodes.
| File | Description |
|---|---|
tests/Infrastructure.Tests/WorkflowScripts/CiWorkflowTests.cs |
Verifies CLI outerloop Java metadata. |
tests/Aspire.Dashboard.Tests/Integration/Playwright/Infrastructure/MockDashboardClient.cs |
Returns an empty terminal update stream. |
tests/Aspire.Dashboard.Tests/Integration/Playwright/AccessibilityTests.cs |
Adds scoped Fluent UI accessibility allowances. |
tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj |
Requires Java for outerloop execution. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
deb354c to
85c2f06
Compare
This comment has been minimized.
This comment has been minimized.
85c2f06 to
e07e61d
Compare
This comment has been minimized.
This comment has been minimized.
|
[automated] Extended CI confidence requires action Reviewer actions
Change classification
🔴 Dashboard terminal outerloop stability — Final-head validation exposed five new failures
🟡 Final-head outerloop result — Red jobs are inherited outside the incremental diff
🟢 CLI Java outerloop coverage — Covered on all configured operating systems
🟢 Dashboard accessibility exemptions — Covered by non-zero final-head outerloop execution
🟢 Regular PR CI and selector contract — Covered
🟢 Quarantine dispatch — Covered
⚪ Internal Azure DevOps and deployment E2E — Not applicable
|
e07e61d to
9e3c1c3
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
[automated] # PR Testing Report PR Information
Change Validated
Local ValidationResult: 5 passed, 0 failed, 0 skipped. Outerloop ValidationThe manually dispatched workflow ran against the expected head commit.
Aggregate Workflow ResultThe full workflow concluded with failure because other jobs were red:
The CLI Java validation targeted by this change passed completely. The aggregate failures require separate attribution because the PR also contains Dashboard changes and the failed jobs are outside the two-file revision made here. ResultCLI Java outerloop behavior verified; full outerloop workflow remains red from other jobs. |
f0af7ba to
5fb82b9
Compare
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
[automated] Final-head review and outerloop validation report. PR #20457 Review and Outerloop Test ReportPR information
Final diff reviewed
Review resultNo blocking correctness findings. Both remaining changes are relevant and appropriately scoped. Conditional CLI Java provisioningThe outerloop runsheet emitted
The previous failure signature was absent: The condition avoids provisioning Java in regular CLI jobs, whose filters exclude the Dashboard mock terminal subscriptionScheduled The generic Playwright mock does not provide terminal descriptors. Returning a Across all four outerloop workflow attempts, the original Focused and final-head validation:
The first final-head macOS CLI attempt reached the global 10-minute hang-dump timeout Remaining outerloop failuresCLI end-to-endAll four attempts failed: The same test failed in recent scheduled Dashboard PlaywrightThe fourth attempt executed 85 tests on each operating system:
The repeated reported test failures were: Both fail on recent scheduled ConclusionThe PR's two fixes are validated and the remaining outerloop failures should not block this PR. The Dashboard terminal failures and Radius deployment failure reproduce outside this |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The outerloop-only Maven reactor tests compile Java 21 fixtures, but the CLI test project did not advertise a JDK requirement. Specialized CI jobs could therefore select an older Java version and fail with: error: release version 21 not supported Set RequiresJava only for outerloop runs so regular CLI jobs do not provision Java unnecessarily. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Dashboard Playwright tests initialize the shared layout, which now subscribes to terminal updates even in scenarios that do not configure terminals. The mock threw during that initialization, and terminal-dock tests still targeted the header button removed by #20528. Return a completed empty terminal stream from the shared mock. Open the empty dock through AppHost activation and use the settings button as the surviving focus target after collapse. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
2f43c4c to
dc6669c
Compare
Tests selector2 / 97 PR test projects · 0 PR jobs, from 3 changed files. Selected PR test projects (2 / 97)
Selected PR jobs (0)none How these were chosen — grouped by what changed🧪 🧪 🧪 Job reasonsnone Selection computed for commit |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
James Newton-King (JamesNK)
left a comment
There was a problem hiding this comment.
Reviewed commit dc6669c. No new high-confidence issues found in the changed code.
|
✅ No documentation update needed. Step 5 branch taken: Triggered signals: none ( Allowlist justification: all 3 changed files match
This matches |
|
LGTM. Thanks Ankit Jain (@radical) ! |
Description
The scheduled outerloop run exposed two independent setup and test-contract regressions:
The CLI Maven reactor tests are outerloop-only, but
Aspire.Cli.Testsdid not advertise its Java requirement, so specialized jobs could run with an older JDK. Dashboard shared-layout initialization also subscribes to terminal updates in Playwright tests that do not configure terminals, where the mock previously threw. Separately, two terminal-dock scenarios still targeted the navigation button intentionally removed by #20528.Mark
Aspire.Cli.Testsas requiring Java only whenRunOuterloopTests=true, return a completed empty terminal stream from the shared Dashboard mock, and update the terminal-dock scenarios to use the supported AppHost activation path and a surviving settings-button focus target.Validation:
CiWorkflowTests: 7 passed.TerminalDockTests: all 7 outerloop cases passed locally on macOS arm64.Checklist