Fail Android runs when pulled test results report failed tests

src/Microsoft.DotNet.XHarness.Android/InstrumentationRunner.cs

@@ -24,6 +24,7 @@ public class InstrumentationRunner
     private const string ShortMessageVariableName = "shortMsg";
     private const string ProcessCrashedShortMessage = "Process crashed";
     private const string InstrumentationResultPrefix = "INSTRUMENTATION_RESULT:";
+    private const string TestResultsFileType = "test-results";
 
     private readonly ILogger _logger;
     private readonly AdbRunner _runner;
@@ -105,6 +106,13 @@ public ExitCode RunApkInstrumentation(
 
         // Determine exit code and emit summary after all operations complete
         ExitCode exitCode = DetermineExitCode(result, logCatSucceeded, processCrashed, failurePullingFiles, instrumentationExitCode, expectedExitCode);
+
+        // 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;
+        }
+
         EmitRunSummary(exitCode, instrumentationExitCode, producedFiles, outputDirectory);
 
         return exitCode;
@@ -148,6 +156,30 @@ private ExitCode DetermineExitCode(ProcessExecutionResults result, bool logCatSu
         return ExitCode.SUCCESS;
     }
 
+    private bool ContainsFailedTests(List<DiagnosticsFile> producedFiles)
+    {
+        bool failedTestsFound = false;
+
+        foreach (var resultFile in producedFiles.Where(file => file.Type == TestResultsFileType && !string.IsNullOrEmpty(file.Path)))
+        {
+            int? failedTests = TestResultsAnalyzer.GetFailedTestCount(resultFile.Path!);
+
+            if (failedTests is null)
+            {
+                _logger.LogDebug($"Unable to determine the number of failed tests from '{resultFile.Path}'");
+                continue;
+            }
+
+            if (failedTests > 0)
+            {
+                _logger.LogError($"Instrumentation reported a successful exit code but '{resultFile.Name}' contains {failedTests} failed test(s)");
+                failedTestsFound = true;
+            }
+        }
+
+        return failedTestsFound;
+    }
+
     private (int? ExitCode, bool Crashed, bool FilePullFailed) ParseInstrumentationResult(string apkPackageName, string outputDirectory, string result, List<DiagnosticsFile> producedFiles)
     {
         // This is where test instrumentation can communicate outwardly that test execution failed
@@ -231,7 +263,7 @@ private bool PullResultXMLs(string apkPackageName, string outputDirectory, IRead
                 producedFiles.Add(new DiagnosticsFile
                 {
                     Name = Path.GetFileName(resultFile),
-                    Type = "test-results",
+                    Type = TestResultsFileType,
                     Path = Path.Combine(outputDirectory, Path.GetFileName(resultFile)),
                 });
             }

src/Microsoft.DotNet.XHarness.Android/TestResultsAnalyzer.cs

@@ -0,0 +1,72 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.IO;
+using System.Linq;
+using System.Xml.Linq;
+
+namespace Microsoft.DotNet.XHarness.Android;
+
+/// <summary>
+/// Reads test result files (XML) produced by test applications and tells whether they contain failed tests.
+/// This is needed because some applications report a zero instrumentation exit code even when tests failed.
+/// </summary>
+public static class TestResultsAnalyzer
+{
+    /// <summary>
+    /// Returns the number of failed tests found in given test results file.
+    /// Returns null when the file is missing or its format is not recognized (in which case we can't tell).
+    /// </summary>
+    public static int? GetFailedTestCount(string resultsFilePath)
+    {
+        if (string.IsNullOrEmpty(resultsFilePath) || !File.Exists(resultsFilePath))
+        {
+            return null;
+        }
+
+        XDocument document;
+        try
+        {
+            document = XDocument.Load(resultsFilePath);
+        }
+        catch (Exception)
+        {
+            return null;
+        }
+
+        XElement? root = document.Root;
+        if (root == null)
+        {
+            return null;
+        }
+
+        switch (root.Name.LocalName)
+        {
+            // xUnit v2 format (the default of the XHarness test runners)
+            case "assemblies":
+            case "assembly":
+                var assemblies = root.Name.LocalName == "assembly"
+                    ? new[] { root }
+                    : root.Elements().Where(e => e.Name.LocalName == "assembly").ToArray();
+
+                // An empty <assemblies /> element is a valid result file with no failures
+                return assemblies.Sum(assembly => GetIntAttribute(assembly, "failed") + GetIntAttribute(assembly, "errors"));
+
+            // NUnit v2 format
+            case "test-results":
+                return GetIntAttribute(root, "failures") + GetIntAttribute(root, "errors");
+
+            // NUnit v3 format
+            case "test-run":
+                return GetIntAttribute(root, "failed");
+
+            default:
+                return null;
+        }
+    }
+
+    private static int GetIntAttribute(XElement element, string attributeName)
+        => int.TryParse(element.Attribute(attributeName)?.Value, out int value) ? value : 0;
+}

tests/Microsoft.DotNet.XHarness.Android.Tests/TestResultsAnalyzerTests.cs

@@ -0,0 +1,114 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+#nullable enable
+
+using System;
+using System.IO;
+using Xunit;
+
+namespace Microsoft.DotNet.XHarness.Android.Tests;
+
+public class TestResultsAnalyzerTests : IDisposable
+{
+    private readonly string _tempDirectory;
+
+    public TestResultsAnalyzerTests()
+    {
+        _tempDirectory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+        Directory.CreateDirectory(_tempDirectory);
+    }
+
+    public void Dispose()
+    {
+        if (Directory.Exists(_tempDirectory))
+        {
+            Directory.Delete(_tempDirectory, true);
+        }
+    }
+
+    [Fact]
+    public void XUnitResultsWithFailuresAreDetected()
+    {
+        var path = WriteResultsFile(
+            @"<assemblies>
+                <assembly name=""System.Numerics.Vectors.Tests.dll"" total=""1194"" passed=""1191"" failed=""3"" skipped=""0"" errors=""0"" />
+              </assemblies>");
+
+        Assert.Equal(3, TestResultsAnalyzer.GetFailedTestCount(path));
+    }
+
+    [Fact]
+    public void XUnitResultsWithoutFailuresAreDetected()
+    {
+        var path = WriteResultsFile(
+            @"<assemblies>
+                <assembly name=""System.Buffers.Tests.dll"" total=""100"" passed=""100"" failed=""0"" skipped=""0"" errors=""0"" />
+              </assemblies>");
+
+        Assert.Equal(0, TestResultsAnalyzer.GetFailedTestCount(path));
+    }
+
+    [Fact]
+    public void XUnitErrorsAreCountedAsFailures()
+    {
+        var path = WriteResultsFile(
+            @"<assembly name=""Some.Tests.dll"" total=""10"" passed=""9"" failed=""0"" skipped=""0"" errors=""1"" />");
+
+        Assert.Equal(1, TestResultsAnalyzer.GetFailedTestCount(path));
+    }
+
+    [Fact]
+    public void NUnitV2ResultsWithFailuresAreDetected()
+    {
+        var path = WriteResultsFile(
+            @"<test-results name=""Some.Tests"" total=""10"" errors=""1"" failures=""2"" not-run=""0"" />");
+
+        Assert.Equal(3, TestResultsAnalyzer.GetFailedTestCount(path));
+    }
+
+    [Fact]
+    public void NUnitV3ResultsWithFailuresAreDetected()
+    {
+        var path = WriteResultsFile(
+            @"<test-run id=""2"" testcasecount=""10"" result=""Failed"" total=""10"" passed=""8"" failed=""2"" />");
+
+        Assert.Equal(2, TestResultsAnalyzer.GetFailedTestCount(path));
+    }
+
+    [Fact]
+    public void EmptyXUnitResultsMeanNoFailures()
+    {
+        var path = WriteResultsFile(@"<assemblies />");
+
+        Assert.Equal(0, TestResultsAnalyzer.GetFailedTestCount(path));
+    }
+
+    [Fact]
+    public void UnknownFormatIsNotEvaluated()
+    {
+        var path = WriteResultsFile(@"<some-other-format failed=""3"" />");
+
+        Assert.Null(TestResultsAnalyzer.GetFailedTestCount(path));
+    }
+
+    [Fact]
+    public void MalformedFileIsNotEvaluated()
+    {
+        var path = WriteResultsFile("this is not XML");
+
+        Assert.Null(TestResultsAnalyzer.GetFailedTestCount(path));
+    }
+
+    [Fact]
+    public void MissingFileIsNotEvaluated()
+        => Assert.Null(TestResultsAnalyzer.GetFailedTestCount(Path.Combine(_tempDirectory, "does-not-exist.xml")));
+
+    private string WriteResultsFile(string content)
+    {
+        var path = Path.Combine(_tempDirectory, "testResults.xml");
+        File.WriteAllText(path, content);
+        return path;
+    }
+}

tests/integration-tests/Android/Commands.Tests.proj

@@ -6,32 +6,31 @@
 
   <PropertyGroup>
     <TestPackageName>System.Numerics.Vectors.Tests</TestPackageName>
-    <XHarnessX86TestApkUrl>$(AssetsBaseUri)/android/test-apk/x86/$(TestPackageName)-x86.zip</XHarnessX86TestApkUrl>
-    <TestAppDestinationDir>$(ArtifactsTmpDir)test-app\android\x86</TestAppDestinationDir>
+    <TestArchitecture>x86_64</TestArchitecture>
+    <XHarnessTestApkUrl>$(AssetsBaseUri)/android/test-apk/$(TestArchitecture)/$(TestPackageName)-$(TestArchitecture).apk</XHarnessTestApkUrl>
+    <TestAppDestinationDir>$(ArtifactsTmpDir)test-app\android\$(TestArchitecture)</TestAppDestinationDir>
   </PropertyGroup>
 
   <Target Name="TestAndroid" BeforeTargets="CoreTest">
-    <DownloadFile SourceUrl="$(XHarnessX86TestApkUrl)" DestinationFolder="$(TestAppDestinationDir)" SkipUnchangedFiles="True" Retries="5">
+    <DownloadFile SourceUrl="$(XHarnessTestApkUrl)" DestinationFolder="$(TestAppDestinationDir)" SkipUnchangedFiles="True" Retries="5">
       <Output TaskParameter="DownloadedFile" ItemName="DownloadedApkFile" />
     </DownloadFile>
 
     <Message Text="Downloaded @(DownloadedApkFile) for XHarness Test purposes" Importance="High" />
-    <Unzip SourceFiles="@(DownloadedApkFile)" DestinationFolder="$(TestAppDestinationDir)" />
-    <Message Text="Extracted to $(TestAppDestinationDir)" Importance="High" />
 
     <ItemGroup>
-      <XHarnessApkToTest Include="$(TestAppDestinationDir)\$(TestPackageName)-x86.apk">
+      <XHarnessApkToTest Include="@(DownloadedApkFile)">
         <AndroidPackageName>net.dot.$(TestPackageName)</AndroidPackageName>
         <AndroidInstrumentationName>net.dot.MonoRunner</AndroidInstrumentationName>
         <WorkItemTimeout>00:30:00</WorkItemTimeout>
         <CustomCommands>
         <![CDATA[
           set -ex;
           deviceId=`xharness android device --app="$app"`;
-          xharness android install --device-id="$deviceId" --output-directory="$output_directory" --package-name="$package_name" --app="$app" --device-arch=x86 --verbosity=Debug;
+          xharness android install --device-id="$deviceId" --output-directory="$output_directory" --package-name="$package_name" --app="$app" --device-arch=$(TestArchitecture) --verbosity=Debug;
           set +e;
           result=0;
-          xharness android run --device-id="$deviceId" --output-directory="$output_directory" --package-name="$package_name" --verbosity=Debug;
+          xharness android run --device-id="$deviceId" --output-directory="$output_directory" --package-name="$package_name" --instrumentation=net.dot.MonoRunner --verbosity=Debug;
           ((result|=$?));
           xharness android uninstall --device-id="$deviceId" --package-name="$package_name";
           ((result|=$?));