Skip to content

Commit

Permalink
Address dotnet/runtime#45428 - there's a mystery state where the inst…
Browse files Browse the repository at this point in the history
…all cache is full or something, rebooting device helps. Also improve wait-for-device to actually check the boot_completed prop
  • Loading branch information
MattGal committed Dec 2, 2020
1 parent 9a028f5 commit 26032ab
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 5 deletions.
34 changes: 32 additions & 2 deletions src/Microsoft.DotNet.XHarness.Android/AdbRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public class AdbRunner
#region Constructor and state variables

private const string AdbEnvironmentVariableName = "ADB_EXE_PATH";
private const string AdbDeviceFullInstallFailureMessage = "INSTALL_FAILED_INSUFFICIENT_STORAGE";
private const string AdbShellPropertyForBootCompletion = "sys.boot_completed";
private readonly string _absoluteAdbExePath;
private readonly ILogger _log;
private readonly IAdbProcessManager _processManager;
Expand Down Expand Up @@ -90,6 +92,8 @@ private static string GetCliAdbExePath()

public string GetAdbState() => RunAdbCommand("get-state").StandardOutput;

public string RebootAndroidDevice() => RunAdbCommand("reboot").StandardOutput;

public void ClearAdbLog() => RunAdbCommand("logcat -c");

public void DumpAdbLog(string outputFilePath, string filterSpec = "")
Expand Down Expand Up @@ -141,6 +145,18 @@ public void WaitForDevice()
{
throw new Exception($"Error waiting for Android device/emulator. Std out:{result.StandardOutput} Std. Err: {result.StandardError}. Do you need to set the current device?");
}

// Once wait-for-device returns, we'll give it up to 30s for 'adb shell getprop sys.boot_completed' to be '1' (as opposed to empty) to make package managers happy
var bootCompleted = RunAdbCommand($"shell getprop {AdbShellPropertyForBootCompletion}");

int triesLeft = 3;
while (!bootCompleted.StandardOutput.Trim().StartsWith("1") && triesLeft > 1)
{
bootCompleted = RunAdbCommand($"shell getprop {AdbShellPropertyForBootCompletion}");
_log.LogDebug($"{AdbShellPropertyForBootCompletion} = '{bootCompleted.StandardOutput.Trim()}'");
Thread.Sleep((int)TimeSpan.FromSeconds(10).TotalMilliseconds);
triesLeft--;
}
}

public void StartAdbServer()
Expand All @@ -164,6 +180,7 @@ public void KillAdbServer()

public int InstallApk(string apkPath)
{
bool needToRetry = false;
_log.LogInformation($"Attempting to install {apkPath}: ");
if (string.IsNullOrEmpty(apkPath))
{
Expand All @@ -173,16 +190,29 @@ public int InstallApk(string apkPath)
{
throw new FileNotFoundException($"Could not find {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)
{
_log.LogWarning($"Hit broken pipe error; Will make one attempt to restart ADB server, and retry the install");

_log.LogWarning($"Hit broken pipe error; Will make one attempt to restart ADB server, then retry the install");
KillAdbServer();
StartAdbServer();
needToRetry = true;
}

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 Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public override string ToString()
output.AppendLine($"Standard Output:{Environment.NewLine}{StandardOutput}");
if (!string.IsNullOrEmpty(StandardError))
{
output.AppendLine($"Standard Error:{Environment.NewLine}{StandardOutput}");
output.AppendLine($"Standard Error:{Environment.NewLine}{StandardError}");
}
return output.ToString();
}
Expand Down
32 changes: 30 additions & 2 deletions tests/Microsoft.DotNet.XHarness.Android.Tests/AdbRunnerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class AdbRunnerTests : IDisposable
private static readonly string s_scratchAndOutputPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
private static readonly string s_adbPath = Path.Combine(s_scratchAndOutputPath, "adb");
private static string s_currentDeviceSerial = "";
private static int s_bootCompleteCheckTimes = 0;
private readonly Mock<ILogger> _mainLog;
private readonly Mock<IAdbProcessManager> _processManager;
private readonly Dictionary<Tuple<string, string>, int> _fakeDeviceList;
Expand Down Expand Up @@ -92,13 +93,17 @@ public void DumpBugReport()
[Fact]
public void WaitForDevice()
{
s_bootCompleteCheckTimes = 0; // Force simulating device is offline
var runner = new AdbRunner(_mainLog.Object, _processManager.Object, s_adbPath);
string fakeDeviceName = $"emulator-{new Random().Next(9999)}";
runner.SetActiveDevice(fakeDeviceName);
runner.WaitForDevice();

s_bootCompleteCheckTimes = 0; // Force simulating device is offline
runner.SetActiveDevice(null);
runner.WaitForDevice();
_processManager.Verify(pm => pm.Run(s_adbPath, $"wait-for-device", TimeSpan.FromMinutes(5)), Times.Exactly(2));
_processManager.Verify(pm => pm.Run(s_adbPath, "wait-for-device", TimeSpan.FromMinutes(5)), Times.Exactly(2));
_processManager.Verify(pm => pm.Run(s_adbPath, "shell getprop sys.boot_completed", TimeSpan.FromMinutes(5)), Times.Exactly(4));
}

[Fact]
Expand Down Expand Up @@ -149,6 +154,14 @@ public void KillApk()
Assert.Equal(0, exitCode);
}

[Fact]
public void RebootAndroidDevice()
{
var runner = new AdbRunner(_mainLog.Object, _processManager.Object, s_adbPath);
string result = runner.RebootAndroidDevice();
_processManager.Verify(pm => pm.Run(s_adbPath, "reboot", TimeSpan.FromMinutes(5)), Times.Once);
}

[Theory]
[InlineData(null)]
[InlineData("")]
Expand Down Expand Up @@ -187,7 +200,7 @@ public void RunInstrumentation(string instrumentationName)

#region Helper Functions
// Generates a list of fake devices, one per supported architecture so we can test AdbRunner's parsing of the output.
// As with most of these tests, if adb.exe changes (we are locked into specific version)
// As with most of these tests, if adb.exe changes, this will break (we are locked into specific version)
private Dictionary<Tuple<string, string>, int> InitializeFakeDeviceList()
{
var r = new Random();
Expand Down Expand Up @@ -244,6 +257,20 @@ private ProcessExecutionResults CallFakeProcessManager(string process, string ar
{
stdOut = _fakeDeviceList.Keys.Where(k => k.Item1 == s_currentDeviceSerial).Single().Item2;
}
if ($"{allArgs[argStart + 1]} {allArgs[argStart + 2]}".Equals("getprop sys.boot_completed"))
{
// Newline is strange, but this is actually what it looks like
if (s_bootCompleteCheckTimes > 0)
{
// Tell it we've booted.
stdOut = "1{Environment.NewLine}";
}
else
{
stdOut = $"{Environment.NewLine}";
}
s_bootCompleteCheckTimes++;
}
exitCode = 0;
break;
case "logcat":
Expand All @@ -258,6 +285,7 @@ private ProcessExecutionResults CallFakeProcessManager(string process, string ar
File.WriteAllText(outputPath, "Sample BugReport Output");
break;
case "install":
case "reboot":
case "uninstall":
case "wait-for-device":
// No output needed, but pretend to wait a little
Expand Down

0 comments on commit 26032ab

Please sign in to comment.