From 377ea472ab2ae72c0c5e9070f9c15b020576fecd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= <premek.vysoky@microsoft.com>
Date: Wed, 18 May 2022 16:37:48 +0200
Subject: [PATCH] Categorize Android instrumentation timeouts (#889)

---
 .../AdbRunner.cs                                  |  8 +++++---
 .../InstrumentationRunner.cs                      | 15 ++++++++++++++-
 .../AndroidHeadless/AndroidHeadlessRunCommand.cs  |  2 +-
 .../AdbRunnerTests.cs                             |  2 +-
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs b/src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs
index bf3472a0d1efa..5b67da0c50e18 100644
--- a/src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs
+++ b/src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs
@@ -131,7 +131,7 @@ public string RebootAndroidDevice()
     public void EnableWifi(bool enable) => RunAdbCommand("shell", "svc", "wifi", enable ? "enable" : "disable")
         .ThrowIfFailed($"Failed to {(enable ? "enable" : "disable")} WiFi on the device");
 
-    public void DumpAdbLog(string outputFilePath, string filterSpec = "")
+    public bool TryDumpAdbLog(string outputFilePath, string filterSpec = "")
     {
         // Workaround: Doesn't seem to have a flush() function and sometimes it doesn't have the full log on emulators.
         Thread.Sleep(3000);
@@ -141,12 +141,14 @@ public void DumpAdbLog(string outputFilePath, string filterSpec = "")
         {
             // Could throw here, but it would tear down a possibly otherwise acceptable execution.
             _log.LogError($"Error getting ADB log:{Environment.NewLine}{result}");
+            return false;
         }
         else
         {
             Directory.CreateDirectory(Path.GetDirectoryName(outputFilePath) ?? throw new ArgumentNullException(nameof(outputFilePath)));
             File.WriteAllText(outputFilePath, result.StandardOutput);
             _log.LogInformation($"Wrote current ADB log to {outputFilePath}");
+            return true;
         }
     }
 
@@ -1049,11 +1051,11 @@ public ProcessExecutionResults RunApkInstrumentation(string apkName, string? ins
 
         if (result.ExitCode == (int)AdbExitCodes.INSTRUMENTATION_TIMEOUT)
         {
-            _log.LogInformation($"Running instrumentation class {displayName} timed out after waiting {stopWatch.Elapsed.TotalSeconds} seconds");
+            _log.LogWarning("Running instrumentation class {name} timed out after waiting {seconds} seconds", displayName, stopWatch.Elapsed.TotalSeconds);
         }
         else
         {
-            _log.LogInformation($"Running instrumentation class {displayName} took {stopWatch.Elapsed.TotalSeconds} seconds");
+            _log.LogInformation("Running instrumentation class {name} took {seconds} seconds", displayName, stopWatch.Elapsed.TotalSeconds);
         }
 
         _log.LogDebug(result.ToString());
diff --git a/src/Microsoft.DotNet.XHarness.Android/InstrumentationRunner.cs b/src/Microsoft.DotNet.XHarness.Android/InstrumentationRunner.cs
index 274c6aab82db4..9eed88401cf30 100644
--- a/src/Microsoft.DotNet.XHarness.Android/InstrumentationRunner.cs
+++ b/src/Microsoft.DotNet.XHarness.Android/InstrumentationRunner.cs
@@ -48,6 +48,7 @@ public ExitCode RunApkInstrumentation(
 
         bool processCrashed = false;
         bool failurePullingFiles = false;
+        bool logCatSucceeded;
 
         using (_logger.BeginScope("Post-test copy and cleanup"))
         {
@@ -74,7 +75,7 @@ public ExitCode RunApkInstrumentation(
                 }
             }
 
-            _runner.DumpAdbLog(Path.Combine(outputDirectory, $"adb-logcat-{apkPackageName}-{(instrumentationName ?? "default")}.log"));
+            logCatSucceeded = _runner.TryDumpAdbLog(Path.Combine(outputDirectory, $"adb-logcat-{apkPackageName}-{(instrumentationName ?? "default")}.log"));
 
             if (processCrashed)
             {
@@ -82,6 +83,18 @@ public ExitCode RunApkInstrumentation(
             }
         }
 
+        if (result.ExitCode == (int)AdbExitCodes.INSTRUMENTATION_TIMEOUT)
+        {
+            // In case emulator crashes halfway through, it sometimes manifests as a timeout too
+            // However, in this case, we usually fail to pull the log which means the emulator did indeed crash
+            if (!logCatSucceeded)
+            {
+                return ExitCode.SIMULATOR_FAILURE;
+            }
+
+            return ExitCode.TIMED_OUT;
+        }
+
         if (processCrashed)
         {
             return ExitCode.APP_CRASH;
diff --git a/src/Microsoft.DotNet.XHarness.CLI/Commands/AndroidHeadless/AndroidHeadlessRunCommand.cs b/src/Microsoft.DotNet.XHarness.CLI/Commands/AndroidHeadless/AndroidHeadlessRunCommand.cs
index e43554c302e33..1227b663b0b73 100644
--- a/src/Microsoft.DotNet.XHarness.CLI/Commands/AndroidHeadless/AndroidHeadlessRunCommand.cs
+++ b/src/Microsoft.DotNet.XHarness.CLI/Commands/AndroidHeadless/AndroidHeadlessRunCommand.cs
@@ -118,7 +118,7 @@ public static ExitCode InvokeHelper(
                 }
             }
 
-            runner.DumpAdbLog(Path.Combine(outputDirectory, $"adb-logcat-{testAssembly}-default.log"));
+            runner.TryDumpAdbLog(Path.Combine(outputDirectory, $"adb-logcat-{testAssembly}-default.log"));
         }
 
         if (failurePullingFiles)
diff --git a/tests/Microsoft.DotNet.XHarness.Android.Tests/AdbRunnerTests.cs b/tests/Microsoft.DotNet.XHarness.Android.Tests/AdbRunnerTests.cs
index 19a9d17153453..07614e153b236 100644
--- a/tests/Microsoft.DotNet.XHarness.Android.Tests/AdbRunnerTests.cs
+++ b/tests/Microsoft.DotNet.XHarness.Android.Tests/AdbRunnerTests.cs
@@ -76,7 +76,7 @@ public void DumpAdbLog()
     {
         var runner = new AdbRunner(_mainLog.Object, _processManager.Object, s_adbPath);
         string pathToDumpLogTo = Path.Join(s_scratchAndOutputPath, $"{Path.GetRandomFileName()}.log");
-        runner.DumpAdbLog(pathToDumpLogTo);
+        runner.TryDumpAdbLog(pathToDumpLogTo);
         VerifyAdbCall("logcat", "-d", "");
 
         Assert.Equal("Sample LogCat Output", File.ReadAllText(pathToDumpLogTo));