Skip to content

[ci] Fix internal builds - #20455

Merged
Ankit Jain (radical) merged 3 commits into
mainfrom
ankj/fix-template-test-failure
Sep 25, 2026
Merged

Ankit Jain (radical) merged 3 commits into
mainfrom
ankj/fix-template-test-failure

Conversation

@radical

Copy link
Copy Markdown
Member

Description

Official internal builds are broken because the Template Tests stage now needs about 25 minutes, but the test session is terminated after 20 minutes. The completed tests pass; the pipeline fails because Microsoft.Testing.Platform aborts the still-running session.

In build 3086035, 129 tests passed with no failures before the deadline:

Test run summary: Aborted
  total: 129
  failed: 0
  succeeded: 129
  duration: 20m 01s 713ms

The timeout exits with code 3. The existing --ignore-exit-code 8 behavior did not change and is unrelated: exit code 8 only represents a run in which no tests executed.

Why it started failing

#20046 added two 51-case starter-template classes, increasing the internal basic-build workload by 102 cases. GitHub CI shards these tests by class, but the internal pipeline runs every basic-build case in one process.

That PR also upgraded xUnit to v4. The internal run initially failed before executing tests because the runner still passed the removed --report-xunit options. #20314 corrected the reporting options, allowing the expanded suite to run and exposing the 20-minute timeout:

Root cause

The workload increase made the existing 20-minute session limit too short. A separate, previously latent evaluation-order bug prevented the template project from overriding that limit correctly.

MtpBaseArgs was composed in eng/Testing.props before the project file was evaluated. Although the template project declared a 15-minute hang timeout, the emitted command still used the repository defaults:

--hangdump-timeout 10m --timeout 20m

The Azure Pipelines job also had a 30-minute limit, leaving no setup or cleanup headroom around a 30-minute test session.

Fix

  • Compose shared MTP arguments in eng/Testing.targets, after project evaluation, so project timeout overrides take effect.
  • Give the combined internal basic-build invocation a 30-minute session timeout while retaining the 20-minute timeout for class-sharded runs.
  • Increase the Azure Pipelines job limit to 45 minutes.
  • Add regression coverage for the emitted class-sharded and internal argument sets.
  • Update documentation and workflow references to the new authoritative argument-composition location.

Validation

--hangdump-timeout 15m --timeout 30m

The template-test task succeeded. The validation used a template-only scratch pipeline and reused package artifacts from the earlier full build, so it does not represent a complete internal pipeline run at this PR's head.

Surprises and call-outs

  • Moving composition to Testing.targets makes existing project-level MTP timeout overrides effective across the repository, not only for template tests.
  • The current internal suite leaves about 4.5 minutes of session headroom. Further substantial matrix growth may require sharding the internal template run rather than increasing the timeout again.

Fixes # (issue)

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

Internal template builds abort after 20 minutes after the expanded
starter-template matrix, even though the template project declares custom
timeout values.

MtpBaseArgs was composed from defaults in Testing.props before project-level
properties were evaluated, so the declared 15-minute hang timeout was emitted
as 10 minutes and the basic-build session remained capped at 20 minutes.

Compose the MTP arguments from Testing.targets after project evaluation, give
the combined internal basic-build run a 30-minute session budget, and add
regression coverage for both the normal and basic-build argument sets.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The Template Tests job includes checkout, artifact staging, and SDK preparation
within its existing 30-minute limit. Raising the test session timeout to 30
minutes would still allow the enclosing Azure Pipelines job to terminate the
run before the test host reaches its own deadline.

Give the job a 45-minute budget so the combined basic-build suite receives its
full timeout plus setup and cleanup headroom.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
MTP argument composition now happens in Testing.targets so project timeout
overrides are evaluated before the shared runner arguments are built.

Update workflow comments, contributor guidance, test skills, and CI docs that
still named Testing.props as the composition source. Keep Testing.props
references only where the underlying filter properties remain defined there.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added the area-engineering-systems infrastructure helix infra engineering repo stuff label Sep 24, 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 -- 20455

Or

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

@github-actions

Copy link
Copy Markdown
Contributor

Tests selector

Selects the full PR test matrix + all PR-gated jobs (ALL) — a rule matching '.github/workflows/run-tests.yml' selects ALL


Selection computed for commit 2899487.

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

🔵 Needs a closer look

The official pipeline YAML changed, but validation covered only a reduced template-only pipeline rather than a complete internal run at the PR head.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes internal Template Tests timing out by honoring project-level MTP timeout overrides and increasing pipeline headroom.

Changes:

  • Moves MTP argument composition from Testing.props to Testing.targets.
  • Uses a 30-minute session timeout for combined internal template tests.
  • Adds regression coverage and updates related documentation.
File Description
tests/​Infrastructure.Tests/​MicrosoftTestingPlatformTests.cs Tests emitted timeout and filter arguments.
tests/​Aspire.Templates.Tests/​README.md Documents timeout behavior.
tests/​Aspire.Templates.Tests/​Aspire.Templates.Tests.csproj Adds the internal 30-minute timeout.
eng/​Testing.targets Composes MTP arguments after project evaluation.
eng/​Testing.props Removes early argument composition.
eng/​pipelines/​azure-pipelines.yml Raises the Template Tests job timeout.
docs/​specs/​cli-identity-sidecar.md Updates the authoritative file reference.
docs/​ci/​mtp-args-pipeline.md Documents the revised argument pipeline.
AGENTS.md Updates testing guidance.
.github/​workflows/​run-tests.yml Updates the MTP source reference.
.github/​workflows/​reproduce-flaky-tests.yml Updates comments referencing MTP configuration.
.github/​workflows/​deployment-tests.yml Updates the MTP convention reference.
.agents/​skills/​fix-flaky-test/​SKILL.md Updates flaky-test guidance.
.agents/​skills/​fix-flaky-test/​run-test-repeatedly.sh Updates configuration comments.
.agents/​skills/​fix-flaky-test/​run-test-repeatedly.ps1 Updates configuration comments.
.agents/​skills/​cli-e2e-testing/​SKILL.md Updates CLI E2E guidance.

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

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@radical

Copy link
Copy Markdown
Member Author

[automated] Extended CI confidence requires action

Reviewer actions

  • Required before merge: Run the complete internal microsoft-aspire definition-1602 pipeline for the final PR head.
    Done when: a linked build maps to head 2899487646345d034ab8b59c60acbd82e2c1c1f6, the complete pipeline reaches the affected test stages without unexplained failed timeline tasks, and the Template Tests log shows --hangdump-timeout 15m --timeout 30m with the full basic-build set passing inside the final 45-minute job.
  • Required before merge: Dispatch the full Outerloop Tests workflow for the final PR head after accounting for the inherited zero-test failure.
    Done when: the non-PR dispatch emits the complete project/OS runsheet, exercises the relocated shared MTP arguments across that set, and every failure is either fixed or tied to an independently reproduced base-branch failure with an explicit maintainer disposition.
Ready-to-use pr-testing prompt

Use the pr-testing skill to test PR #20455 at head 2899487646345d034ab8b59c60acbd82e2c1c1f6. Focus on two gaps: run the complete internal definition-1602 pipeline from a mirror commit that maps exactly to the final GitHub head, inspect its timeline, Template Tests command line/results, and downstream artifact/stage wiring; then dispatch the full tests-outerloop.yml workflow, compare its emitted project/OS runsheet with a recent baseline, and reconcile every failure. Treat the Acquisition exit-code-8/zero-test symptom as inherited only if the same command and failure are demonstrated on the base branch. Do not run publishing or release side effects.

Change classification

  • Shared test-execution workload: MtpBaseArgs and TestRunnerAdditionalArguments move from eng/Testing.props to eng/Testing.targets, changing evaluation timing for every repository test project and specialized test runsheet.
  • Internal pipeline workload: the combined Aspire.Templates.Tests basic-build invocation gains a 30-minute session timeout and the definition-1602 Template Tests job increases from 30 to 45 minutes.
  • Validation and documentation: focused MSBuild-property regression coverage was added; workflow, skill, and documentation edits otherwise update references to the new authoritative targets file.

Confidence at a glance

🔴 Internal Azure DevOps definition 1602 — Complete final-head validation is missing
  • Why it applies: The PR directly changes eng/pipelines/azure-pipelines.yml, shared Arcade test arguments, and the internal Template Tests invocation.
  • Evidence: Build 3086819 ran a reduced Template Tests-only definition from scratch commit 4c7d63c6d80e4022cbf4aed72b94bad15e9c3fbe. Its load-bearing eng/Testing.props, eng/Testing.targets, and template-project changes match the final head. The downloaded test log records the exact command --filter-trait category=basic-build ... --hangdump-timeout 15m --timeout 30m and 175/175 tests passing in 25m 28s. The Template Tests task succeeded; the build was only partially successful because policy-injected supply-chain analysis reported warnings.
  • Gap or disposition: The scratch definition replaced the normal 716-line stage graph with one stage and reused packages/native artifacts from canceled build 3086728. It proves the template-test mechanism and 45-minute job allowance, but not the final complete pipeline, the shared target's effect on other internal test jobs, or normal producer/consumer integration. No definition-1602 build exists for the final head; branch history shows only canceled build 3086728 and reduced build 3086819.
  • Action: Run the complete final pipeline and inspect the timeline plus affected logs/artifacts, rather than relying on the headline result alone.
🔴 Outerloop shared invocation — Full-project execution is missing
  • Why it applies: Moving shared TestRunnerAdditionalArguments and MtpBaseArgs is an outerloop execution-infrastructure change. A pull-request run intentionally selects only one project, so it cannot establish behavior across the complete outerloop set.
  • Evidence: Final-head runs 36064410832 and 36069130854 both selected Acquisition on macOS, Ubuntu, and Windows. Their logs show the expected relocated arguments, including --ignore-exit-code 8 --hangdump-timeout 10m --timeout 20m --filter-trait outerloop=true, but every test was skipped and MTP returned exit code 8; the three OS jobs and final-results aggregator failed.
  • Gap or disposition: The failure is inherited rather than introduced here: scheduled main run 35946233512 shows the same Acquisition zero-test/exit-code-8 failure with the same effective MTP arguments, alongside other pre-existing outerloop failures. The PR smoke therefore exposes no unique PR-caused test failure, but it also cannot validate the shared relocation across projects that contain runnable outerloop tests.
  • Action: Run the full dispatch and compare its emitted project/OS set with baseline; do not treat the one-project PR smoke as full outerloop coverage.
🟢 Final-head GitHub CI and red-check reconciliation — No unresolved PR-caused failure found
  • Why it applies: The PR selects ALL, changes shared test plumbing, and GitHub currently displays failure records from multiple runs at the same final SHA.
  • Evidence: The selector comment confirms the full PR matrix for 2899487. CI run 36069130952 attempt 2 completed successfully with 413 successful jobs and no failed jobs. Its Infrastructure job passed 1,343 tests; the previously red Infrastructure, Hosting.Azure, and Linux deno-debugger jobs all passed in this later final-head run. Quarantined Tests run 36069130884 also passed its Acquisition smoke on all three operating systems.
  • Gap or disposition: All 13 displayed final-head red records are accounted for. Eight are the two duplicated Outerloop runs: three Acquisition OS failures plus one aggregate in each run, classified above as inherited/pre-existing. Five belong to earlier CI run 36064410757: Infrastructure, Hosting.Azure, Linux deno-debugger, and two aggregate checks; the later complete CI run passed those same leaf jobs, so these are superseded/transient rather than unresolved. The separate first attempt of run 36069130952 also had transient JavaScript, RabbitMQ v6, and two Windows extension-E2E failures, all replaced by the successful second attempt.
  • Action: None for ordinary GitHub PR CI; the remaining required work is the specialized internal and full-outerloop validation above.
🟢 Focused timeout regression coverage — Final-head assertions passed
  • Why it applies: The root cause was MSBuild evaluation order: project timeout properties were not reflected when MtpBaseArgs was composed in a props file.
  • Evidence: MicrosoftTestingPlatformTests.ProjectTimeoutOverridesFlowToMtpArguments evaluates the template project with and without RunOnlyBasicBuildTemplateTests, asserting 20-minute class-sharded and 30-minute internal session timeouts, the 15-minute hang timeout, and the conditional basic-build filter. The final-head Infrastructure job passed all 1,343 tests, and reduced internal build 3086819 independently observed the 30-minute command line in the real test task.
  • Gap or disposition: The focused test would fail if argument composition moved back before project evaluation or the conditional override stopped flowing. It does not replace the complete definition-1602 or full outerloop runs.
⚪ Deployment end-to-end execution — Not applicable
  • Why it applies: deployment-tests.yml was reviewed because it appears in the diff and invokes MTP.
  • Evidence: Its only change is a comment updating eng/Testing.props to eng/Testing.targets. The deployment command continues to pass --ignore-exit-code 8 explicitly and does not consume MtpBaseArgs; no deployment scenario, Azure authorization, selector, generated application, package, or result artifact changed.
  • Gap or disposition: A live Azure deployment would not validate a distinct changed deployment behavior.

@radical
Ankit Jain (radical) merged commit ecb0fc1 into main Sep 25, 2026
1286 of 1305 checks passed
@microsoft-github-policy-service microsoft-github-policy-service Bot added this to the 14.0 milestone Sep 25, 2026
@radical

Copy link
Copy Markdown
Member Author

/backport to release/13.6

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/13.6 (link to workflow run)

@aspire-repo-bot

Copy link
Copy Markdown
Contributor

Ankit Jain (@radical) backporting to release/13.6 failed, the patch most likely resulted in conflicts. Please backport manually!

git am output
$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Fix template test timeout overrides in CI
Using index info to reconstruct a base tree...
M	tests/Aspire.Templates.Tests/README.md
Falling back to patching base and 3-way merge...
Auto-merging tests/Aspire.Templates.Tests/README.md
Applying: Allow template tests to use the extended timeout
Using index info to reconstruct a base tree...
M	eng/pipelines/azure-pipelines.yml
Falling back to patching base and 3-way merge...
Auto-merging eng/pipelines/azure-pipelines.yml
CONFLICT (content): Merge conflict in eng/pipelines/azure-pipelines.yml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0002 Allow template tests to use the extended timeout
Error: The process '/usr/bin/git' failed with exit code 128

Link to workflow output

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-engineering-systems infrastructure helix infra engineering repo stuff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants