Handle bugreport generation for different APIs (#685)

* Handle bugreport generation for different APIs * address review feedback * Apply suggestions from code review Co-authored-by: Přemek Vysoký <[email protected]> * return full file name for bug report * Update src/Microsoft.DotNet.XHarness.Android/Execution/IReportManager.cs Co-authored-by: Přemek Vysoký <[email protected]> Co-authored-by: Přemek Vysoký <[email protected]>
dotnet · Jul 28, 2021 · 016171d · 016171d
1 parent be39724
commit 016171d
Show file tree

Hide file tree

Showing 7 changed files with 134 additions and 21 deletions.
diff --git a/src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs b/src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs
@@ -23,6 +23,7 @@ public class AdbRunner
         private const string AdbInstallBrokenPipeError = "Failure calling service package: Broken pipe";
         private const string AdbInstallException = "Exception occurred while executing 'install':";
         private const string AdbShellPropertyForBootCompletion = "sys.boot_completed";
+        private int? _api;
         private readonly string _absoluteAdbExePath;
         private readonly ILogger _log;
         private readonly IAdbProcessManager _processManager;
@@ -32,7 +33,6 @@ public class AdbRunner
             { "app", "shell pm list packages -3"}
         };
 
-
         public AdbRunner(ILogger log, string adbExePath = "") : this(log, new AdbProcessManager(log), adbExePath) { }
 
         public AdbRunner(ILogger log, IAdbProcessManager processManager, string adbExePath = "")
@@ -71,9 +71,17 @@ public void SetActiveDevice(string? deviceSerialNumber)
         {
             _processManager.DeviceSerial = deviceSerialNumber ?? string.Empty;
 
+            _api = GetAPIVersion();
+
             _log.LogInformation($"Active Android device set to serial '{deviceSerialNumber}'");
         }
 
+        private int GetAPIVersion()
+        {
+            var output = RunAdbCommand("shell getprop ro.build.version.sdk");
+            return int.Parse(output.StandardOutput);
+        }
+
         private static string GetCliAdbExePath()
         {
             var currentAssemblyDirectory = Path.GetDirectoryName(typeof(AdbRunner).Assembly.Location);
@@ -127,21 +135,10 @@ public void DumpAdbLog(string outputFilePath, string filterSpec = "")
             }
         }
 
-        public void DumpBugReport(string outputFilePath)
+        public string DumpBugReport(string outputFilePathWithoutFormat)
         {
-            // give some time for bug report to be available
-            Thread.Sleep(3000);
-
-            var result = RunAdbCommand($"bugreport {outputFilePath}", TimeSpan.FromMinutes(5));
-            if (result.ExitCode != 0)
-            {
-                // Could throw here, but it would tear down a possibly otherwise acceptable execution.
-                _log.LogError($"Error getting ADB bugreport:{Environment.NewLine}{result}");
-            }
-            else
-            {
-                _log.LogInformation($"Wrote ADB bugreport to {outputFilePath}");
-            }
+            var reportManager = AdbReportFactory.CreateReportManager(_log, _api ?? GetAPIVersion());
+            return reportManager.DumpBugReport(this, outputFilePathWithoutFormat);
         }
 
         public void WaitForDevice()

diff --git a/src/Microsoft.DotNet.XHarness.Android/Execution/AdbReportFactory.cs b/src/Microsoft.DotNet.XHarness.Android/Execution/AdbReportFactory.cs
@@ -0,0 +1,19 @@
+// 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 Microsoft.Extensions.Logging;
+
+namespace Microsoft.DotNet.XHarness.Android.Execution
+{
+    internal class AdbReportFactory
+    {
+        // This method return proper ReportManager based on API number of current device
+        // It allows to apply different logic for bugreport generation on API 21-23 and above
+        internal static IReportManager CreateReportManager(ILogger log, int api)
+        {
+            if (api > 23) return new NewReportManager(log);
+            else return new Api23AndOlderReportManager(log);
+        }
+    }
+}
diff --git a/src/Microsoft.DotNet.XHarness.Android/Execution/Api23AndOlderReportManager.cs b/src/Microsoft.DotNet.XHarness.Android/Execution/Api23AndOlderReportManager.cs
@@ -0,0 +1,42 @@
+// 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.Threading;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.DotNet.XHarness.Android.Execution
+{
+    internal class Api23AndOlderReportManager : IReportManager
+    {
+        private readonly ILogger _log;
+
+        public Api23AndOlderReportManager(ILogger log)
+        {
+            _log = log;
+        }
+
+        public string DumpBugReport(AdbRunner runner, string outputFilePathWithoutFormat)
+        {
+            // give some time for bug report to be available
+            Thread.Sleep(3000);
+
+            var result = runner.RunAdbCommand($"bugreport", TimeSpan.FromMinutes(5));
+
+            if (result.ExitCode != 0)
+            {
+                // Could throw here, but it would tear down a possibly otherwise acceptable execution.
+                _log.LogError($"Error getting ADB bugreport:{Environment.NewLine}{result}");
+                return string.Empty;
+            }
+            else
+            {
+                File.WriteAllText($"{outputFilePathWithoutFormat}.txt", result.StandardOutput);
+                _log.LogInformation($"Wrote ADB bugreport to {outputFilePathWithoutFormat}.txt");
+                return $"{outputFilePathWithoutFormat}.txt";
+            }
+        }
+    }
+}
diff --git a/src/Microsoft.DotNet.XHarness.Android/Execution/IReportManager.cs b/src/Microsoft.DotNet.XHarness.Android/Execution/IReportManager.cs
@@ -0,0 +1,11 @@
+// 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.
+
+namespace Microsoft.DotNet.XHarness.Android.Execution
+{
+    internal interface IReportManager
+    {
+        string DumpBugReport(AdbRunner runner, string outputFilePathWithoutFormat);
+    }
+}
diff --git a/src/Microsoft.DotNet.XHarness.Android/Execution/NewReportManager.cs b/src/Microsoft.DotNet.XHarness.Android/Execution/NewReportManager.cs
@@ -0,0 +1,39 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.DotNet.XHarness.Android.Execution
+{
+    class NewReportManager : IReportManager
+    {
+        private readonly ILogger _log;
+        public NewReportManager(ILogger log)
+        {
+            _log = log;
+        }
+
+        public string DumpBugReport(AdbRunner runner, string outputFilePathWithoutFormat)
+        {
+            // give some time for bug report to be available
+            Thread.Sleep(3000);
+
+            var result = runner.RunAdbCommand($"bugreport {outputFilePathWithoutFormat}.zip", TimeSpan.FromMinutes(5));
+
+            if (result.ExitCode != 0)
+            {
+                // Could throw here, but it would tear down a possibly otherwise acceptable execution.
+                _log.LogError($"Error getting ADB bugreport:{Environment.NewLine}{result}");
+                return string.Empty;
+            }
+            else
+            {
+                _log.LogInformation($"Wrote ADB bugreport to {outputFilePathWithoutFormat}.zip");
+                return $"{outputFilePathWithoutFormat}.zip";
+            }
+        }
+    }
+}
diff --git a/src/Microsoft.DotNet.XHarness.CLI/Commands/Android/AndroidRunCommand.cs b/src/Microsoft.DotNet.XHarness.CLI/Commands/Android/AndroidRunCommand.cs
@@ -194,7 +194,7 @@ public static ExitCode InvokeHelper(
 
                     if (processCrashed)
                     {
-                        runner.DumpBugReport(Path.Combine(outputDirectory, $"adb-bugreport-{apkPackageName}.zip"));
+                        runner.DumpBugReport(Path.Combine(outputDirectory, $"adb-bugreport-{apkPackageName}"));
                     }
                 }
 

diff --git a/tests/Microsoft.DotNet.XHarness.Android.Tests/AdbRunnerTests.cs b/tests/Microsoft.DotNet.XHarness.Android.Tests/AdbRunnerTests.cs
@@ -87,11 +87,12 @@ public void DumpAdbLog()
         public void DumpBugReport()
         {
             var runner = new AdbRunner(_mainLog.Object, _processManager.Object, s_adbPath);
-            string pathToDumpBugReport = Path.Join(s_scratchAndOutputPath, $"{Path.GetRandomFileName()}.zip");
+            runner.SetActiveDevice(string.Empty);
+            string pathToDumpBugReport = Path.Join(s_scratchAndOutputPath, Path.GetRandomFileName());
             runner.DumpBugReport(pathToDumpBugReport);
-            _processManager.Verify(pm => pm.Run(s_adbPath, $"bugreport {pathToDumpBugReport}", TimeSpan.FromMinutes(5)), Times.Once);
+            _processManager.Verify(pm => pm.Run(s_adbPath, $"bugreport {pathToDumpBugReport}.zip", TimeSpan.FromMinutes(5)), Times.Once);
 
-            Assert.Equal("Sample BugReport Output", File.ReadAllText(pathToDumpBugReport));
+            Assert.Equal("Sample BugReport Output", File.ReadAllText($"{pathToDumpBugReport}.zip"));
         }
 
         [Fact]
@@ -294,14 +295,18 @@ private ProcessExecutionResults CallFakeProcessManager(string process, string ar
                         if (s_bootCompleteCheckTimes > 0)
                         {
                             // Tell it we've booted.
-                            stdOut = "1{Environment.NewLine}";
+                            stdOut = $"1{Environment.NewLine}";
                         }
                         else
                         {
-                            stdOut = $"{Environment.NewLine}";
+                            stdOut = Environment.NewLine;
                         }
                         s_bootCompleteCheckTimes++;
                     }
+                    if ($"{allArgs[argStart + 1]} {allArgs[argStart + 2]}".Equals("getprop ro.build.version.sdk"))
+                    {
+                        stdOut = $"29{Environment.NewLine}";
+                    }
                     exitCode = 0;
                     break;
                 case "logcat":