Skip to content

[ci] Fix Deployment E2E tests - #20308

Open
Ankit Jain (radical) with Copilot wants to merge 5 commits into
mainfrom
copilot/fix-nightly-run-failure
Open

Ankit Jain (radical) with Copilot wants to merge 5 commits into
mainfrom
copilot/fix-nightly-run-failure

Conversation

Copilot AI commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Description

What broke

Deployment tests can be dynamically skipped—for example, when Azure regional capacity is unavailable. MTP represents a run containing only skipped tests with exit code 8.

The workflow already passed --ignore-exit-code 8. That option normalizes the test module's result so an expected skip does not fail the job.

Why it started failing

Starting with .NET 11, dotnet test computes a separate result for the complete aggregated run. Even after the test module normalizes its result, dotnet test sees that no tests executed and returns exit code 8 again.

Test module → exit 8 → --ignore-exit-code 8 → success
                                              ↓
.NET 11 dotnet test aggregate → zero tests → exit 8 → workflow failure

The Deployment E2E history confirms the timing:

  • September 10, using .NET 10.0.401: AzureConnectorNamespaceDeploymentTests skipped its only test, reported Zero tests ran, and the job succeeded.
  • Update to .NET 11 RC1 SDK #19999 then updated the repository to .NET 11 RC1.
  • September 11, using .NET 11 RC1: the same test was skipped, but dotnet test returned exit code 8 and the job failed.

The fix

The affected workflows capture the aggregate exit code and pass it through checked-in Bash or PowerShell normalizers. Exit code 8 becomes success; actual test failures and host errors remain failures.

  • Deployment E2E and non-NuGet shared tests use dotnet test, so they normalize the aggregate result.
  • NuGet-dependent tests execute the test assembly directly, where the existing module-level option still returns success.

The regression tests execute the checked-in scripts directly for exit codes 0, 8, 2, and 7 instead of extracting and rewriting script fragments from YAML. Conditional test selection also routes changes to the shared runner and Deployment E2E consumers.

Surprises and call-outs

This fix also updates the non-NuGet paths in the shared run-tests.yml workflow. Those paths have the same aggregate exit-code behavior and are used by regular, quarantine, and outerloop test lanes.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

Copilot AI linked an issue Sep 22, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Fix nightly run failing for Deployment E2E Tests Treat MTP "zero tests ran" exit code as success in Deployment E2E workflow Sep 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 20308

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 20308"

@github-actions

This comment has been minimized.

@radical
Ankit Jain (radical) force-pushed the copilot/fix-nightly-run-failure branch from f36292a to 99c415e Compare September 24, 2026 22:40
@radical
Ankit Jain (radical) changed the base branch from main to ankj/fix-template-test-failure September 24, 2026 22:41
@radical
Ankit Jain (radical) requested a balanced review from Copilot September 24, 2026 22:43
@github-actions

This comment has been minimized.

@radical
Ankit Jain (radical) added this pull request to stack #20456 September 24, 2026 22:45
@radical
Ankit Jain (radical) marked this pull request as ready for review September 24, 2026 22:47
@radical Ankit Jain (radical) changed the title Treat MTP "zero tests ran" exit code as success in Deployment E2E workflow [ci] Fix Deployment E2E tests Sep 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The workflow logic is correct, focused regression coverage is included, and the documentation matches MTP behavior.

Review effort: Balanced
Findings: None

What changed in this PR

Updates the Deployment E2E workflow so MTP exit code 8 (“zero tests ran”) does not fail intentionally skipped scenarios.

Changes:

  • Explicitly classifies deployment-test exit codes.
  • Adds regression coverage for success, skip, failure, and crash codes.
  • Documents dotnet test aggregation behavior.
File Description
.github/​workflows/​deployment-tests.yml Treats exit codes 0 and 8 as successful.
tests/​Infrastructure.Tests/​WorkflowScripts/​DeploymentTestsWorkflowTests.cs Tests the workflow’s exit-code classification.
docs/​ci/​mtp-args-pipeline.md Documents the --ignore-exit-code limitation.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@github-actions

This comment has been minimized.

@radical

Copy link
Copy Markdown
Member

[automated] Extended CI confidence is partially established — final-head PR CI and the affected shared-runner smoke are green, but the Deployment E2E workflow’s final exit-code path has not run from head 9ae5cc2f4e168f41c38659062ecc872bd86947ee.

Reviewer actions

  • Recommended: dispatch only AzureConnectorNamespaceDeploymentTests from the final PR branch.
    Done when: the linked run uses head 9ae5cc2…, logs MTP exit code 8 for the intentionally skipped test, does not set test_failed=true, and the deployment job finishes green.
Ready-to-use pr-testing prompt

Use the pr-testing skill to test PR #20308 at head 9ae5cc2f4e168f41c38659062ecc872bd86947ee, using exact base 2899487646345d034ab8b59c60acbd82e2c1c1f6. Treat it as CI-infrastructure validation, not CLI dogfooding. Dispatch .github/workflows/deployment-tests.yml from the PR branch with test_filter=AzureConnectorNamespaceDeploymentTests and no pr_number, so no PR comment is posted. Verify the test is intentionally skipped, dotnet test returns 8, the workflow logs Test run exit code: 8, test_failed is not emitted, the “Check for test failures” step is skipped, and the job is green. Do not broaden the run to unrelated deployment scenarios.

Change classification

  • Deployment workload change: .github/workflows/deployment-tests.yml explicitly classifies MTP exit codes so an intentionally skipped matrix scenario does not fail the nightly job.
  • Shared GitHub test-runner change: .github/workflows/run-tests.yml maps aggregated exit code 8 to success for non-NuGet dotnet test paths on Bash and PowerShell; its consumers include regular PR tests plus quarantine and outerloop through specialized-test-runner.yml.
  • Focused contract coverage: two new Infrastructure.Tests classes execute the workflow-derived Bash/PowerShell classification for exit codes 0, 8, 2, and 7.

Confidence at a glance

🟡 Deployment E2E workflow — Final-head runtime path is not yet exercised
  • Why it applies: The PR changes the deployment job’s real result-classification path and whether the downstream failure step runs.
  • Evidence: The latest nightly run before this PR, run 35950420904, skipped AzureConnectorNamespaceDeploymentTests, reported Test run summary: Zero tests ran and exit code 8, then failed because the old || echo "test_failed=true" path marked the job red. The final diff replaces that path with explicit 0/8 classification.
  • Gap or disposition: DeploymentTestsWorkflowTests executes the extracted final run: script under bash -e with a real GITHUB_OUTPUT file and verifies 0/8 stay green while 2/7 set the failure output. That is strong hermetic evidence, but no final-head deployment-tests.yml run has confirmed GitHub step/output behavior and the downstream if: in the generated job.
  • Action: Run only the Connector Namespace class from the final branch; the scenario is already opt-out and should exercise the skip path without deploying its resource.
🟢 Shared runner and specialized workflows — The original cross-platform failure is fixed at final head
  • Why it applies: run-tests.yml is called by normal tests.yml jobs and by quarantine/outerloop through specialized-test-runner.yml; changing its non-NuGet test command affects all three lanes.
  • Evidence: At intermediate head 99c415e…, Outerloop run 36068798123 failed Acquisition on Ubuntu, Windows, and macOS with zero tests, plus its aggregate result. At final head, Outerloop run 36071674595 exercised the changed branch on all three operating systems: each log reported Zero tests ran, exit code 8, and All selected tests were skipped; treating MTP exit code 8 as success. All three jobs and Final Results passed.
  • Gap or disposition: Quarantined Tests run 36071674434 also passed its Acquisition jobs on all three operating systems. The PR-triggered specialized runs cover one project by design, but that project is the exact previously failing consumer and directly exercises the changed branch.
🟢 Final-head PR checks and contract tests — All current checks are accounted for
  • Why it applies: The shared reusable workflow change selects the full PR matrix, and the workflow-script tests are the focused regression guard.
  • Evidence: CI run 36071674390 completed successfully at 9ae5cc2…; its Infrastructure.Tests job passed all 1,355 tests. The final-head check rollup contains 439 checks: 429 succeeded, 10 were skipped, and zero are red. Outerloop and Quarantined Tests are also green at the same SHA.
  • Gap or disposition: There are no PR-caused, inherited, or unresolved red checks on the final head. Skipped checks are expected matrix/gate outcomes, not failures.
🟢 Historical deployment failures — Target failure is isolated from unrelated nightly failures
  • Why it applies: The latest scheduled deployment run is red for five matrix jobs, so its failures must not be attributed wholesale to this incremental PR.
  • Evidence: In run 35950420904, the Connector Namespace failure is the pre-fix exit-code-8 symptom addressed here. The other four red jobs are pre-existing and outside the exact 2899487…9ae5cc2 diff: FoundryDotnetProjectDeploymentTests lacked Microsoft.CognitiveServices/accounts/AIServices/agents/write; FoundryHostedAgentDeploymentTests hit ResourcesForSkuUnavailable in westus3; AzureSandboxesDotnetProjectDeploymentTests received HTTP 401 from its deployed endpoint; and TypeScriptAzureProvisioningDeploymentTests lacked an Azure principal parameter.
  • Gap or disposition: Those four failures remain unresolved in the latest nightly evidence, but they are inherited Azure/scenario failures rather than final-head PR checks or regressions caused by this change. A filtered Connector Namespace dispatch avoids conflating them with validation of this PR.
⚪ Internal Azure DevOps definition 1602 — Not applicable
  • Why it is not applicable: The exact incremental diff changes GitHub workflows, their focused contract tests, and documentation only. It does not touch or feed eng/pipelines/**, eng/common/**, Arcade integration, internal template-test plumbing, signing, packaging, publishing, installers, or official-build artifact layout.

@github-actions

This comment has been minimized.

Base automatically changed from ankj/fix-template-test-failure to main September 25, 2026 18:12
Copilot AI and others added 2 commits September 25, 2026 14:12
…kflow

Co-authored-by: radical <1472+radical@users.noreply.github.com>
The Acquisition outerloop jobs fail on every platform when all selected
tests are skipped. Microsoft Testing Platform returns exit code 8 for the
test module, but dotnet test recomputes the aggregate result and still
exits 8 despite --ignore-exit-code 8.

Map only the aggregated zero-tests code to success in the shared Bash and
PowerShell dotnet test paths. Preserve every other nonzero exit code and
leave direct assembly execution unchanged, where the MTP option already
works.

Add focused tests that execute the classification logic extracted from the
real workflow for success, zero tests, test failure, and host crash exit
codes on both shell variants.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment on lines +144 to +153
Starting with the .NET 11 SDK, `dotnet test` decides the zero-tests result for
the whole run from its
[aggregated results](https://learn.microsoft.com/dotnet/core/tools/dotnet-test-mtp#whole-run-and-per-module-minimums).
`--ignore-exit-code 8` still makes direct test-module execution return 0, but an
all-empty or all-skipped `dotnet test` run returns 8 from the orchestrator. The
non-NuGet paths in `run-tests.yml` therefore normalize the result through
`normalize-mtp-exit-code.sh` or `normalize-mtp-exit-code.ps1`. The Deployment
E2E workflow runs its complete test command through `run-deployment-test.sh`,
which uses the same Bash normalizer before deciding whether to set its failure
output. Other nonzero exit codes remain failures.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a bug here in the .NET SDK? Should this be consistent coming from them?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would mean dotnet test should have special behavior for MTP.

This branch was successfully deployed

1 active deployment
deployment-testing — 3c2b2e3c Deployed Sep 25, 2026 by radical via Deploy (Deployment.EndToEnd-AcaExistingRegistryDeploymentTests) #809
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nightly run failing: Deployment E2E Tests

4 participants