Create synthetic test failures for failed Helix work items - #17490
Create synthetic test failures for failed Helix work items#17490premun wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The pipeline still filters to ExitCode.HasValue, which can exclude terminal failed work items identified via state (preventing synthetic failures), and a new fake-service dictionary uses case-sensitive tuple keys that can make tests brittle.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the Helix Job Monitor’s Azure DevOps test upload flow to preserve Helix work-item failures in the Tests tab by uploading a synthetic failing result when a work item fails but no parseable test results can be produced.
Changes:
- Propagate per-work-item failure state through the upload pipeline and generate a synthetic failed
<work item>.WorkItemExecutionresult when a failed work item yields zero aggregated results. - Add a regression test asserting the synthetic failure is published for a failed work item with no results.
- Update the SDK README to document the new synthetic-result behavior.
File summaries
| File | Description |
|---|---|
| src/Microsoft.DotNet.Helix/Sdk/Readme.md | Documents the synthetic failure upload behavior for failed work items without parseable results. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/JobMonitorRunnerTests.cs | Adds regression coverage validating a synthetic failure is published. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/Fakes/FakeAzureDevOpsService.cs | Captures prepared results for assertions during test-result publishing. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResultUploadPipeline.cs | Threads work-item failure state into upload requests and creates synthetic failed results when aggregation yields none. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| IReadOnlyList<PendingWorkItem> newWorkItems = session.AddWorkItems( | ||
| workItems.Where(static workItem => workItem.ExitCode.HasValue), | ||
| isJobComplete); |
| public List<string> CreatedTestRuns { get; } = []; | ||
| public List<int> CompletedTestRunIds { get; } = []; | ||
| public Dictionary<int, List<WorkItemTestResults>> UploadedResultsByRunId { get; } = []; | ||
| public Dictionary<(string JobName, string WorkItemName), PreparedTestResults> PublishedPreparedResults { get; } = []; | ||
| public List<string> UploadedJobNames { get; } = []; |
There was a problem hiding this comment.
🟢 Approval recommended
The behavior change is well-scoped, covered by a regression test, and no correctness issues were found beyond minor maintainability nits.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/Microsoft.DotNet.Helix/JobMonitor/TestResultUploadPipeline.cs:259
- The synthetic failure result hard-codes a duration of 60 seconds with no explanation. Since this value is user-visible in Azure DevOps and effectively a sentinel, consider naming it (and documenting why it is 60) to avoid accidental changes and clarify intent.
src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/Fakes/FakeAzureDevOpsService.cs:46
- PublishedPreparedResults uses the default (case-sensitive) tuple comparer, but other work-item keyed collections in this fake (e.g., _recordedFailedTests/_uploadFailedTests) use FailedTestWorkItemComparer for OrdinalIgnoreCase semantics. Using the same comparer here avoids brittle tests if job/work item casing differs across inputs.
public List<string> CreatedTestRuns { get; } = [];
public List<int> CompletedTestRunIds { get; } = [];
public Dictionary<int, List<WorkItemTestResults>> UploadedResultsByRunId { get; } = [];
public Dictionary<(string JobName, string WorkItemName), PreparedTestResults> PublishedPreparedResults { get; } = [];
public List<string> UploadedJobNames { get; } = [];
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
<work item>.WorkItemExecutionresult when a failed work item has no parseable test resultsThis restores the Azure DevOps Tests-tab behavior of the legacy awaited SendToHelix flow. Context: dotnet/aspnetcore#69036