Fail Android runs when pulled test results report failed tests

[Copilot]

The Android manual commands E2E leg reported success while the app's tests failed: System.Numerics.Vectors.Tests-x86 ran 1194 tests with 3 failures, yet the instrumentation, the XHarness command and the Helix work item all exited 0.

Root cause

InstrumentationRunner derives its exit code exclusively from the instrumentation's return-code bundle value. The testResults.xml that XHarness pulls off the device (via test-results-path) is only uploaded, never inspected. The prebuilt APK asset used by this scenario reports return-code=0 regardless of failures (MonoRunner.java forwards the managed entry point's return value), so nothing in the chain — XHarness, the result |= $? custom-commands script, the work item — ever saw a failure. The leg also does not publish testResults.xml to Azure DevOps, so there was no second safety net.

Not a regression: return-code-only detection dates to #121 (2020) and survived the InstrumentationRunner extraction in #874; the other Android legs use System.Buffers.Tests, which passes, so the gap was never visible.

Changes

• TestResultsAnalyzer (new) — returns the failed-test count from a results file for the formats the XHarness test runners emit: xUnit v2 (default), NUnit v2, NUnit v3. Missing, malformed or unrecognized files return null and are ignored, so no new failure modes are introduced.
• InstrumentationRunner — when a run would otherwise be SUCCESS and --expected-exit-code is 0, the pulled results files are checked and the run is reported as TESTS_FAILED if any of them report failures:

ExitCode exitCode = DetermineExitCode(...);

// Some applications report a zero exit code even when tests failed so we double check the test results
if (exitCode == ExitCode.SUCCESS && expectedExitCode == (int)ExitCode.SUCCESS && ContainsFailedTests(producedFiles))
{
    exitCode = ExitCode.TESTS_FAILED;
}

The reason is logged explicitly (Instrumentation reported a successful exit code but 'testResults.xml' contains 3 failed test(s)).

• Unit tests for the analyzer covering each supported format plus empty/unknown/malformed/missing files.

Why this approach

This mirrors what the Apple side already does (the orchestrator evaluates the parsed test report, not just the app's exit code), and fixes every consumer of android run/android test rather than just this repo's E2E leg. return-code stays authoritative for non-zero/expected-exit-code scenarios — the XML check can only turn a green run red. No change to the prebuilt APK assets (which this repo does not own) or shell-side XML grepping is needed.

Publishing testResults.xml to Azure DevOps from the E2E scenario is a worthwhile complement for reporting, but it would only fix this repo's leg and leave XHarness itself reporting the wrong exit code; it is left for a separate change.

Verification

The intended proof is the CI on this PR: the Android - Manual Commands leg should now go red with TESTS_FAILED because of exactly the three ConstructorWithUnsupportedTypes_* failures. Kept as a draft until that run confirms it.

• Fixes Android manual commands E2E reports success when tests fail #1664

[vitek-karas]

I checked a recent runtime-extra-platforms main build to estimate the impact of this change.

Across all seven Android test runs, two used android-headless and are unaffected. The five relevant library XHarness jobs covered 1,084 work items and 453,783 tests. This change would add one failed work item to the six already reported: System.Net.Sockets.Tests on arm64 NativeAOT returned XHarness exit code 0 and was marked passed, while its uploaded XML reported one failed test (SocketSendWouldBlock_ReturnsBytesSent) on both attempts.

All other XML failures in that build already produced TESTS_FAILED. So this sample would go from 6 to 7 failed work items and confirms the change exposes a real latent false-green.

[matouskozak]

Fyi. This will take the app from test-apk/x86_64 but the code before used test-apk/x86. It looks like the test is passing now so it appears that the other APK was probably stale and should be removed from the storage.

[vitek-karas]

I uploaded a new rebuilt version there - but yes, eventually we need to clean it up. Unfortunately I don't have access to list all the blobs which are there... :-)
I'll talk to the person who does.