[net11.0] Use Helix job monitor for unit tests - #37852
Conversation
Move Helix completion and test-result publication into the standalone job monitor so transient Azure DevOps reporting failures do not deadletter otherwise passing work items. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37852Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37852" |
There was a problem hiding this comment.
Pull request overview
This PR updates the Azure Pipelines Helix unit-test stage to submit work to Helix in “job monitor” mode and adds the Helix Job Monitor tool to the repo-local .NET tool manifest, shifting result publication/waiting to a centralized monitor job.
Changes:
- Enable
EnableHelixJobMonitor=truewhen invokingeng/helix.projfrom the Helix unit-test stage. - Add a
helix-job-monitorjob to the Helix unit-test stage with an extended timeout. - Add
Microsoft.DotNet.Helix.JobMonitor(dotnet-helix-job-monitor) to.config/dotnet-tools.json.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| eng/pipelines/arcade/stage-helix-tests.yml | Enables Helix job-monitor submission mode and adds a monitor job to publish/wait centrally. |
| .config/dotnet-tools.json | Pins/restores the Helix Job Monitor dotnet tool used by the pipeline template. |
| - template: /eng/common/core-templates/job/helix-job-monitor.yml | ||
| parameters: | ||
| timeoutInMinutes: 360 | ||
| organization: dotnet | ||
| repository: maui | ||
| helixAccessToken: ${{ parameters.HelixAccessToken }} |
There was a problem hiding this comment.
The monitor intentionally starts alongside the submitter jobs. The pinned runner polls the same-stage Azure DevOps timeline and does not terminate until AreNonMonitorJobsComplete is true; while the submitters are running, a first poll with zero Helix jobs therefore continues polling and discovers their durable Helix records after submission. This is also the canonical Arcade template behavior (dependsOn defaults to an empty list), and the monitor design explicitly describes observing timeline jobs running alongside it. Adding dependsOn is not required here.
Install the monitor only in its Linux job after bootstrapping the pinned SDK, so repository-wide tool restores remain compatible and the non-executable Arcade bootstrap script is invoked through bash. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
Fresh exact-head review by 3 independent reviewers with adversarial consensus, plus the MAUI repository specialist.
No correctness, security, race, or result-publication defect was confirmed. One non-blocking configuration-maintenance finding is left inline.
The newest commit correctly removes the net11-only monitor from the root tool manifest, installs the branch-pinned monitor only in its Linux job, and invokes the generated bootstrap script through bash. The earlier dependsOn concern does not apply: the monitor intentionally runs beside the submitters, polls the same-stage timeline, and waits for non-monitor jobs before deciding that no Helix jobs were submitted.
Test coverage: No dedicated regression test is added in this one-file pipeline diff; the monitor/property/package contracts and public/internal template paths were checked against the pinned source and canonical Arcade template.
PR finalization: The title and description accurately match the exact implementation and explain the latest corrective commit.
| --helix-base-uri 'https://helix.dot.net/' \ | ||
| --polling-interval-seconds 30 \ | ||
| --fail-on-failed-tests true \ | ||
| --max-wait-minutes 355 \ |
There was a problem hiding this comment.
💡 Config Impact — Keep the monitor outer and inner timeouts coupled.
Flagged by: 2/3 reviewers after dispute
This inline copy hard-codes timeoutInMinutes: 360 separately from --max-wait-minutes 355, while the canonical Arcade template derives the latter as timeoutInMinutes - 5 specifically so the tool exits gracefully before Azure Pipelines terminates the job. If a later pipeline edit changes only one literal, the job can be killed before the monitor publishes its final summary. Please derive both from one value, or otherwise enforce the five-minute invariant at this call site.
There was a problem hiding this comment.
Addressed in b03c30e. helixJobMonitorTimeoutInMinutes now supplies the Azure job timeout, and --max-wait-minutes is derived from that parameter minus five minutes, preserving the graceful shutdown invariant from the canonical Arcade template.
Derive the monitor's graceful wait from the Azure job timeout so future pipeline changes preserve the five-minute shutdown window. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| --source-branch '$(Build.SourceBranch)' | ||
| displayName: Monitor Helix Jobs | ||
| env: | ||
| SYSTEM_ACCESSTOKEN: $(System.AccessToken) |
There was a problem hiding this comment.
The pinned Job Monitor reads HELIX_ACCESSTOKEN; this is also the environment variable set by Arcade’s canonical helix-job-monitor.yml template. The submission job uses a different client contract (HelixAccessToken), so aligning these names would break the monitor rather than improve compatibility.
| toolDll=$(find "$toolPath/.store" -path '*/tools/*/any/Microsoft.DotNet.Helix.JobMonitor.dll' -type f -print -quit) | ||
| if [ ! -f "$toolDll" ]; then | ||
| echo "Could not find the Helix Job Monitor DLL in '$toolPath/.store'." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "##vso[task.setvariable variable=HelixJobMonitorDll]$toolDll" | ||
| displayName: Install Helix Job Monitor | ||
|
|
||
| - bash: | | ||
| set -euo pipefail | ||
|
|
||
| bash ./eng/common/dotnet.sh exec "$(HelixJobMonitorDll)" \ |
There was a problem hiding this comment.
The DLL invocation is intentional and matches the canonical Arcade template’s isolated-package path: it runs the net11-targeted tool through MAUI’s pinned repo-local SDK. Invoking the shim directly would delegate runtime selection to the host environment, which is exactly what this job must avoid before/around SDK bootstrapping. The exact pinned package layout was also exercised successfully in builds 1568946 and 1569038.
| - name: helixJobMonitorTimeoutInMinutes | ||
| type: number | ||
| default: 360 |
There was a problem hiding this comment.
The parameter follows the canonical Arcade monitor template contract and defaults to 360 minutes; MAUI has no caller overriding it. Clamping a deliberately invalid job timeout would be misleading because a <=5-minute Azure job cannot preserve the required five-minute graceful-shutdown window. No change is needed for the configured path.
| bash ./eng/common/dotnet.sh exec "$(HelixJobMonitorDll)" \ | ||
| --helix-base-uri 'https://helix.dot.net/' \ | ||
| --polling-interval-seconds 30 \ | ||
| --fail-on-failed-tests true \ | ||
| --max-wait-minutes "$((${{ parameters.helixJobMonitorTimeoutInMinutes }} - 5))" \ |
There was a problem hiding this comment.
Duplicate of the timeout-contract comment above. The configured value is 360, there are no overrides, and silently clamping an invalid <=5-minute job timeout would not preserve the shutdown invariant.
|
@PureWeen stop reviewing until maui-pr is finished!!!!!!!! |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Root cause
maui-prbuilds 1568023 and 1568717 failed their Windows Helix lanes even though the affected xUnit commands completed with exit code 0. The latest run dead-lettered nine work items after the legacy per-work-item Azure Pipelines reporter hitTF10216service-unavailable responses and 100-second read timeouts.Fix
EnableHelixJobMonitor, which disables the legacy per-work-item reporter and hands completion/result publication to one monitor job.Microsoft.DotNet.Helix.JobMonitorversion only inside the dedicated Linux monitor job. This keeps the net11-targeted tool out of the repository-wide tool manifest, whose restore runs before the pinned SDK is available.bashbecause MAUI's generated copy is not executable.The monitor runs alongside the submitters in the same stage and gates on both the stage timeline and every discovered Helix job. Real test failures still fail the centralized monitor; only the unreliable legacy reporting path is removed.
No open
net11.0PR currently addresses the Helix reporter failure. This uses the job-monitor path already shipped by the branch-pinned Arcade SDK instead of suppressing test failures or dropping Azure DevOps test results.Validation
dotnet-helix-job-monitorversion11.0.0-beta.26379.102from an isolated empty cache.monitor=false,reporter=true,wait=true.monitor=true,reporter=false,wait=false.The first PR run, 1568855, exposed two integration issues in the initial implementation: adding the net11 tool to the root manifest broke pre-bootstrap restores, and the generated monitor template directly executed a non-executable script. Commit
d1ca1493b8corrects both by isolating installation and invoking the script throughbash.