Skip to content

Commit

Permalink
Fixes for Xharness issues 354 (again...), 385, and 389
Browse files Browse the repository at this point in the history 
- Make failure to pull files or enumerate devices fail instead of warning.
- Detect an alternative version of the broken pipe issue (same recourse)
  • Loading branch information
@MattGal
MattGal committed Dec 3, 2020
1 parent 322e702 commit 1730919
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 28 deletions.
24 changes: 13 additions & 11 deletions src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Runtime.InteropServices;
using System.Threading;
using Microsoft.DotNet.XHarness.Android.Execution;
using Microsoft.DotNet.XHarness.Common.Utilities;
using Microsoft.Extensions.Logging;

namespace Microsoft.DotNet.XHarness.Android
Expand All @@ -19,6 +20,7 @@ public class AdbRunner

private const string AdbEnvironmentVariableName = "ADB_EXE_PATH";
private const string AdbDeviceFullInstallFailureMessage = "INSTALL_FAILED_INSUFFICIENT_STORAGE";
private const string AdbInstallBrokenPipeError = "Failure calling service package: Broken pipe";
private const string AdbShellPropertyForBootCompletion = "sys.boot_completed";
private readonly string _absoluteAdbExePath;
private readonly ILogger _log;
Expand Down Expand Up @@ -193,26 +195,23 @@ public int InstallApk(string apkPath)

var result = RunAdbCommand($"install \"{apkPath}\"");

// If this keeps happening, we should look into exercising Helix infra retry logic when on Helix
// since users should be able assume tests themselves can't break the ADB server.
if (result.ExitCode == (int)AdbExitCodes.ADB_BROKEN_PIPE)
// Two possible retry scenarios, theoretically both can happen on the same run:

// 1. Pipe between ADB server and emulator device is broken; restarting the ADB server helps
if (result.ExitCode == (int)AdbExitCodes.ADB_BROKEN_PIPE || result.StandardError.Contains(AdbInstallBrokenPipeError))
{
_log.LogWarning($"Hit broken pipe error; Will make one attempt to restart ADB server, then retry the install");
KillAdbServer();
StartAdbServer();
needToRetry = true;
result = RunAdbCommand($"install \"{apkPath}\"");
}

// 2. Installation cache on device is messed up; restrting the device reliably seems to unblock this (unless the device is actually full, if so this will error the same)
if (result.ExitCode != (int)AdbExitCodes.SUCCESS && result.StandardError.Contains(AdbDeviceFullInstallFailureMessage))
{
_log.LogWarning($"It seems the package installation cache may be full on the device. We'll try to reboot it before trying one more time.{Environment.NewLine}Output:{result}");
RebootAndroidDevice();
WaitForDevice();
needToRetry = true;
}

if (needToRetry)
{
result = RunAdbCommand($"install \"{apkPath}\"");
}

Expand All @@ -224,6 +223,7 @@ public int InstallApk(string apkPath)
{
_log.LogInformation($"Successfully installed {apkPath}.");
}

return result.ExitCode;
}

Expand Down Expand Up @@ -318,7 +318,8 @@ public List<string> PullFiles(string devicePath, string localPath)

if (result.ExitCode != (int)AdbExitCodes.SUCCESS)
{
_log.LogError(message: $"ERROR: {result}");
throw new Exception($"Failed pulling files: {result}");

}
else
{
Expand Down Expand Up @@ -442,8 +443,9 @@ public void PushFiles(string localDirectory, string devicePath, bool removeIfPre
}
else
{
// May consider abandoning the run here instead of just printing errors.
// Abandon the run here, don't just guess.
_log.LogError($"Error: listing attached devices / emulators: {result.StandardError}. Check that any emulators have been started, and attached device(s) are connected via USB, powered-on, and unlocked.");
throw new Exception("One or more attached Android devices are offline");
}
return devicesAndArchitectures;
}
Expand Down
72 changes: 55 additions & 17 deletions src/Microsoft.DotNet.XHarness.CLI/Commands/Android/AndroidTestCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.DotNet.XHarness.Common.CLI;
using Microsoft.DotNet.XHarness.Common.CLI.CommandArguments;
using Microsoft.DotNet.XHarness.Common.CLI.Commands;
using Microsoft.DotNet.XHarness.Common.Utilities;
using Microsoft.Extensions.Logging;

namespace Microsoft.DotNet.XHarness.CLI.Commands.Android
Expand Down Expand Up @@ -95,7 +96,14 @@ protected override Task<ExitCode> InvokeInternal(ILogger logger)

// enumerate the devices attached and their architectures
// Tell ADB to only use that one (will always use the present one for systems w/ only 1 machine)
runner.SetActiveDevice(GetDeviceToUse(logger, runner, apkRequiredArchitecture));
var deviceToUse = GetDeviceToUse(logger, runner, apkRequiredArchitecture);

if (deviceToUse== null)
{
return Task.FromResult(ExitCode.ADB_DEVICE_ENUMERATION_FAILURE);
}

runner.SetActiveDevice(deviceToUse);

// Wait til at least device(s) are ready
runner.WaitForDevice();
Expand All @@ -120,6 +128,7 @@ protected override Task<ExitCode> InvokeInternal(ILogger logger)
// No class name = default Instrumentation
ProcessExecutionResults? result = runner.RunApkInstrumentation(apkPackageName, _arguments.InstrumentationName, _arguments.InstrumentationArguments, _arguments.Timeout);
bool processCrashed = false;
bool failurePullingFiles = false;

using (logger.BeginScope("Post-test copy and cleanup"))
{
Expand All @@ -136,7 +145,15 @@ protected override Task<ExitCode> InvokeInternal(ILogger logger)
if (resultValues.ContainsKey(possibleResultKey))
{
logger.LogInformation($"Found XML result file: '{resultValues[possibleResultKey]}'(key: {possibleResultKey})");
runner.PullFiles(resultValues[possibleResultKey], _arguments.OutputDirectory);
try
{
runner.PullFiles(resultValues[possibleResultKey], _arguments.OutputDirectory);
}
catch (Exception toLog)
{
logger.LogError(toLog, "Hit error (typically permissions) trying to pull {filePathOnDevice}", resultValues[possibleResultKey]);
failurePullingFiles = true;
}
}
}
if (resultValues.ContainsKey(TestRunSummaryVariableName))
Expand Down Expand Up @@ -174,10 +191,18 @@ protected override Task<ExitCode> InvokeInternal(ILogger logger)
// Optionally copy off an entire folder
if (!string.IsNullOrEmpty(_arguments.DeviceOutputFolder))
{
var logs = runner.PullFiles(_arguments.DeviceOutputFolder, _arguments.OutputDirectory);
foreach (string log in logs)
try
{
logger.LogDebug($"Found output file: {log}");
var logs = runner.PullFiles(_arguments.DeviceOutputFolder, _arguments.OutputDirectory);
foreach (string log in logs)
{
logger.LogDebug($"Found output file: {log}");
}
}
catch (Exception toLog)
{
logger.LogError(toLog, "Hit error (typically permissions) trying to pull {filePathOnDevice}", _arguments.DeviceOutputFolder);
failurePullingFiles = true;
}
}

Expand All @@ -195,6 +220,11 @@ protected override Task<ExitCode> InvokeInternal(ILogger logger)
{
logger.LogError($"Non-success instrumentation exit code: {instrumentationExitCode}, expected: {_arguments.ExpectedExitCode}");
}
else if (failurePullingFiles)
{
logger.LogError($"Received expected instrumentation exit code ({instrumentationExitCode}), but we hit errors pulling files from the device (see log for details.)");
return Task.FromResult(ExitCode.DEVICE_FILE_COPY_FAILURE);
}
else
{
return Task.FromResult(ExitCode.SUCCESS);
Expand All @@ -210,22 +240,30 @@ protected override Task<ExitCode> InvokeInternal(ILogger logger)

private string? GetDeviceToUse(ILogger logger, AdbRunner runner, string apkRequiredArchitecture)
{
var allDevicesAndTheirArchitectures = runner.GetAttachedDevicesAndArchitectures();
if (allDevicesAndTheirArchitectures.Count > 0)
try
{
if (allDevicesAndTheirArchitectures.Any(kvp => kvp.Value != null && kvp.Value.Equals(apkRequiredArchitecture, StringComparison.OrdinalIgnoreCase)))
var allDevicesAndTheirArchitectures = runner.GetAttachedDevicesAndArchitectures();
if (allDevicesAndTheirArchitectures.Count > 0)
{
var firstAvailableCompatible = allDevicesAndTheirArchitectures.Where(kvp => kvp.Value != null && kvp.Value.Equals(apkRequiredArchitecture, StringComparison.OrdinalIgnoreCase)).FirstOrDefault();
logger.LogInformation($"Using first-found compatible device of {allDevicesAndTheirArchitectures.Count} total- serial: '{firstAvailableCompatible.Key}' - Arch: {firstAvailableCompatible.Value}");
return firstAvailableCompatible.Key;
}
else
{
logger.LogWarning($"No devices found with architecture '{apkRequiredArchitecture}'. Just returning first available device; installation will likely fail, but we'll try anyways.");
return allDevicesAndTheirArchitectures.Keys.First();
if (allDevicesAndTheirArchitectures.Any(kvp => kvp.Value != null && kvp.Value.Equals(apkRequiredArchitecture, StringComparison.OrdinalIgnoreCase)))
{
var firstAvailableCompatible = allDevicesAndTheirArchitectures.Where(kvp => kvp.Value != null && kvp.Value.Equals(apkRequiredArchitecture, StringComparison.OrdinalIgnoreCase)).FirstOrDefault();
logger.LogInformation($"Using first-found compatible device of {allDevicesAndTheirArchitectures.Count} total- serial: '{firstAvailableCompatible.Key}' - Arch: {firstAvailableCompatible.Value}");
return firstAvailableCompatible.Key;
}
else
{
// In this case, the enumaration worked but nothing matched the APK; fail out.
logger.LogError($"No devices found with architecture '{apkRequiredArchitecture}'.");
return null;
}
}
logger.LogError("No attached device detected");
}
catch (Exception toLog)
{
logger.LogError(toLog, "Exception thrown while trying to find compatible device with architecture [architecture]", apkRequiredArchitecture);
}
logger.LogError("No attached device detected");
return null;
}

Expand Down
2 changes: 2 additions & 0 deletions src/Microsoft.DotNet.XHarness.Common/CLI/ExitCode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public enum ExitCode
DEVICE_NOT_FOUND = 81,
RETURN_CODE_NOT_SET = 82,
APP_LAUNCH_FAILURE = 83,
DEVICE_FILE_COPY_FAILURE = 84,
ADB_DEVICE_ENUMERATION_FAILURE = 85,

#endregion
}
Expand Down

0 comments on commit 1730919

Please sign in to comment.