Skip to content

Fix unit test failures not failing the build#33423

Merged
rmarinho merged 1 commit intomainfrom
fix-unit-test-exit-code
Jan 13, 2026
Merged

Fix unit test failures not failing the build#33423
rmarinho merged 1 commit intomainfrom
fix-unit-test-exit-code

Conversation

@jfversluis
Copy link
Member

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!

Description

This PR fixes a critical issue where unit test failures were not properly failing the build pipeline, allowing PRs to pass CI checks even when tests failed.

Problem

In /eng/pipelines/common/run-dotnet-preview.yml at line 58, when useExitCodeForErrors: true is set and tests fail, the script was:

  1. ✅ Logging an error message
  2. ✅ Marking the task as "Failed" using Azure DevOps logging commands
  3. Exiting with code 0 (success)

This caused jobs to show as succeededWithIssues instead of failed, and overall PR checks would pass even though unit tests failed.

Example

PR #33391 had failing unit tests:

  • Controls.Xaml.UnitTests failed with "Test suite had 1 failure(s)"
  • Build timeline showed "result": "succeededWithIssues"
  • But overall check status was SUCCESS ✅

Solution

Changed line 58 from:

exit 0

to:

exit $LASTEXITCODE

This ensures the PowerShell script exits with the actual failure code from the test run, properly failing the build pipeline.

Impact

After this change:

  • ✅ Unit test failures will properly fail the build
  • ✅ PR checks will show as failed when tests fail
  • ✅ Build status will accurately reflect test results
  • ✅ No more false positives where builds pass with failing tests

Testing

This fix will be validated by observing the build behavior on this PR itself. The change is minimal and surgical - only affecting the exit code propagation when tests fail.

The issue was in run-dotnet-preview.yml where when useExitCodeForErrors=true
and tests failed, the script would:
1. Log an error message
2. Mark the task as Failed
3. Exit with code 0 (success)

This caused builds to report 'succeededWithIssues' instead of 'failed',
allowing PRs to pass even with failing unit tests.

Changed exit 0 to exit $LASTEXITCODE to properly propagate the test
failure exit code, ensuring the build actually fails when tests fail.

Fixes #33391
Copilot AI review requested due to automatic review settings January 8, 2026 10:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a critical bug where unit test failures were not properly failing the Azure DevOps build pipeline. The issue was that when tests failed, the pipeline would exit with code 0 (success) despite logging errors, causing builds to pass with failing tests.

Key Changes:

  • Changed exit code from hardcoded 0 to $LASTEXITCODE to properly propagate test failure codes
  • Ensures pipeline jobs fail correctly when useExitCodeForErrors: true is set

@jfversluis jfversluis added area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions area-testing Unit tests, device tests testing-infrastructure Issue relating to testing infrastructure labels Jan 8, 2026
@jfversluis jfversluis added this to the .NET 10.0 SR3 milestone Jan 8, 2026
@rmarinho
Copy link
Member

rmarinho commented Jan 8, 2026

@jfversluis
Copy link
Member Author

@rmarinho I think we should have either one of these. Right now failing unit tests go undetected unless you go look at Azure DevOps so I don't think we want that. Do we know the reason why that was commented out?

@rmarinho rmarinho merged commit 58f88af into main Jan 13, 2026
29 checks passed
@rmarinho rmarinho deleted the fix-unit-test-exit-code branch January 13, 2026 10:39
kubaflo pushed a commit to kubaflo/maui that referenced this pull request Jan 16, 2026
> [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

## Description

This PR fixes a critical issue where unit test failures were not
properly failing the build pipeline, allowing PRs to pass CI checks even
when tests failed.

## Problem

In `/eng/pipelines/common/run-dotnet-preview.yml` at line 58, when
`useExitCodeForErrors: true` is set and tests fail, the script was:

1. ✅ Logging an error message
2. ✅ Marking the task as "Failed" using Azure DevOps logging commands
3. ❌ **Exiting with code 0 (success)**

This caused jobs to show as `succeededWithIssues` instead of `failed`,
and overall PR checks would pass even though unit tests failed.

### Example

PR dotnet#33391 had failing unit tests:
- Controls.Xaml.UnitTests failed with "Test suite had 1 failure(s)"
- Build timeline showed `"result": "succeededWithIssues"`
- But overall check status was SUCCESS ✅

## Solution

Changed line 58 from:
```powershell
exit 0
```

to:
```powershell
exit $LASTEXITCODE
```

This ensures the PowerShell script exits with the actual failure code
from the test run, properly failing the build pipeline.

## Impact

After this change:
- ✅ Unit test failures will properly fail the build
- ✅ PR checks will show as failed when tests fail  
- ✅ Build status will accurately reflect test results
- ✅ No more false positives where builds pass with failing tests

## Testing

This fix will be validated by observing the build behavior on this PR
itself. The change is minimal and surgical - only affecting the exit
code propagation when tests fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions area-testing Unit tests, device tests testing-infrastructure Issue relating to testing infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants