Skip to content

Commit 0a30217

Browse files
Copilotmatouskozak
andcommitted
Hide ADB output during retries when installation succeeds
Co-authored-by: matouskozak <55735845+matouskozak@users.noreply.github.com>
1 parent e9ce69f commit 0a30217

File tree

2 files changed

+164
-2
lines changed

2 files changed

+164
-2
lines changed

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,10 +332,17 @@ public int CopyHeadlessFolder(string testPath, bool sharedRuntime = false)
332332
// 2. Installation cache on device is messed up; restarting the device reliably seems to unblock this (unless the device is actually full, if so this will error the same)
333333
if (result.ExitCode != (int)AdbExitCodes.SUCCESS && result.StandardError.Contains(AdbDeviceFullInstallFailureMessage))
334334
{
335-
_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}");
335+
var firstAttemptResult = result;
336+
_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.");
336337
RebootAndroidDevice();
337338
WaitForDevice();
338339
result = RunAdbCommand(new[] { "push", testPath, targetDirectory });
340+
341+
// Only log the initial failure output if the retry also failed
342+
if (result.ExitCode != (int)AdbExitCodes.SUCCESS)
343+
{
344+
_log.LogWarning($"Initial failure output:{Environment.NewLine}{firstAttemptResult}");
345+
}
339346
}
340347

341348
// 3. Installation timed out or failed with exception; restarting the ADB server, reboot the device and give more time for installation
@@ -392,10 +399,17 @@ public int InstallApk(string apkPath)
392399
// 2. Installation cache on device is messed up; restarting the device reliably seems to unblock this (unless the device is actually full, if so this will error the same)
393400
if (result.ExitCode != (int)AdbExitCodes.SUCCESS && result.StandardError.Contains(AdbDeviceFullInstallFailureMessage))
394401
{
395-
_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}");
402+
var firstAttemptResult = result;
403+
_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.");
396404
RebootAndroidDevice();
397405
WaitForDevice();
398406
result = RunAdbCommand(new[] { "install", apkPath });
407+
408+
// Only log the initial failure output if the retry also failed
409+
if (result.ExitCode != (int)AdbExitCodes.SUCCESS)
410+
{
411+
_log.LogWarning($"Initial failure output:{Environment.NewLine}{firstAttemptResult}");
412+
}
399413
}
400414

401415
// 3. Installation timed out or failed with exception; restarting the ADB server, reboot the device and give more time for installation

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

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,154 @@ public void UninstallApk()
171171
Assert.Equal(0, exitCode);
172172
}
173173

174+
[Fact]
175+
public void InstallApk_StorageFailure_SuccessfulRetry_HidesInitialOutput()
176+
{
177+
// Test that when installation fails due to storage but succeeds on retry,
178+
// the initial failure output is NOT logged prominently
179+
var logMessages = new List<string>();
180+
var mockLog = new Mock<ILogger>();
181+
mockLog.Setup(l => l.Log(
182+
It.IsAny<LogLevel>(),
183+
It.IsAny<EventId>(),
184+
It.IsAny<It.IsAnyType>(),
185+
It.IsAny<Exception>(),
186+
It.IsAny<Func<It.IsAnyType, Exception, string>>()))
187+
.Callback((LogLevel level, EventId eventId, object state, Exception exception, Delegate formatter) =>
188+
{
189+
logMessages.Add($"{level}: {state}");
190+
});
191+
192+
int installAttempts = 0;
193+
var mockProcessManager = new Mock<IAdbProcessManager>();
194+
mockProcessManager.Setup(pm => pm.Run(
195+
It.IsAny<string>(),
196+
It.IsAny<IEnumerable<string>>(),
197+
It.IsAny<TimeSpan>()))
198+
.Returns((string p, IEnumerable<string> args, TimeSpan t) =>
199+
{
200+
var argsArray = args.ToArray();
201+
if (argsArray.Contains("install"))
202+
{
203+
installAttempts++;
204+
if (installAttempts == 1)
205+
{
206+
// First attempt: fail with storage error
207+
return new ProcessExecutionResults
208+
{
209+
ExitCode = 1,
210+
StandardError = "Failure [INSTALL_FAILED_INSUFFICIENT_STORAGE]",
211+
StandardOutput = "Failed to install APK"
212+
};
213+
}
214+
else
215+
{
216+
// Second attempt: succeed
217+
return new ProcessExecutionResults
218+
{
219+
ExitCode = 0,
220+
StandardOutput = "Success",
221+
StandardError = ""
222+
};
223+
}
224+
}
225+
// Handle boot completion check for WaitForDevice
226+
if (argsArray.Length >= 3 && argsArray[0] == "shell" && argsArray[1] == "getprop" && argsArray[2] == "sys.boot_completed")
227+
{
228+
return new ProcessExecutionResults
229+
{
230+
ExitCode = 0,
231+
StandardOutput = $"1{Environment.NewLine}",
232+
StandardError = ""
233+
};
234+
}
235+
// For reboot, wait-for-device, etc.
236+
return new ProcessExecutionResults { ExitCode = 0, StandardOutput = "", StandardError = "" };
237+
});
238+
239+
var runner = new AdbRunner(mockLog.Object, mockProcessManager.Object, s_adbPath);
240+
string fakeApkPath = Path.Join(s_scratchAndOutputPath, $"{Path.GetRandomFileName()}.apk");
241+
File.Create(fakeApkPath).Close();
242+
243+
int exitCode = runner.InstallApk(fakeApkPath);
244+
245+
Assert.Equal(0, exitCode);
246+
Assert.Equal(2, installAttempts); // Should have retried once
247+
248+
// Verify that the initial failure output is NOT in the log messages
249+
// The log should contain the warning about storage, but NOT "Output:" with the full failure
250+
var combinedLog = string.Join(Environment.NewLine, logMessages);
251+
Assert.Contains("package installation cache may be full", combinedLog);
252+
Assert.DoesNotContain("Initial failure output:", combinedLog);
253+
}
254+
255+
[Fact]
256+
public void InstallApk_StorageFailure_FailedRetry_ShowsInitialOutput()
257+
{
258+
// Test that when installation fails due to storage and ALSO fails on retry,
259+
// the initial failure output IS logged
260+
var logMessages = new List<string>();
261+
var mockLog = new Mock<ILogger>();
262+
mockLog.Setup(l => l.Log(
263+
It.IsAny<LogLevel>(),
264+
It.IsAny<EventId>(),
265+
It.IsAny<It.IsAnyType>(),
266+
It.IsAny<Exception>(),
267+
It.IsAny<Func<It.IsAnyType, Exception, string>>()))
268+
.Callback((LogLevel level, EventId eventId, object state, Exception exception, Delegate formatter) =>
269+
{
270+
logMessages.Add($"{level}: {state}");
271+
});
272+
273+
int installAttempts = 0;
274+
var mockProcessManager = new Mock<IAdbProcessManager>();
275+
mockProcessManager.Setup(pm => pm.Run(
276+
It.IsAny<string>(),
277+
It.IsAny<IEnumerable<string>>(),
278+
It.IsAny<TimeSpan>()))
279+
.Returns((string p, IEnumerable<string> args, TimeSpan t) =>
280+
{
281+
var argsArray = args.ToArray();
282+
if (argsArray.Contains("install"))
283+
{
284+
installAttempts++;
285+
// Both attempts fail with storage error
286+
return new ProcessExecutionResults
287+
{
288+
ExitCode = 1,
289+
StandardError = "Failure [INSTALL_FAILED_INSUFFICIENT_STORAGE]",
290+
StandardOutput = "Failed to install APK"
291+
};
292+
}
293+
// Handle boot completion check for WaitForDevice
294+
if (argsArray.Length >= 3 && argsArray[0] == "shell" && argsArray[1] == "getprop" && argsArray[2] == "sys.boot_completed")
295+
{
296+
return new ProcessExecutionResults
297+
{
298+
ExitCode = 0,
299+
StandardOutput = $"1{Environment.NewLine}",
300+
StandardError = ""
301+
};
302+
}
303+
// For reboot, wait-for-device, etc.
304+
return new ProcessExecutionResults { ExitCode = 0, StandardOutput = "", StandardError = "" };
305+
});
306+
307+
var runner = new AdbRunner(mockLog.Object, mockProcessManager.Object, s_adbPath);
308+
string fakeApkPath = Path.Join(s_scratchAndOutputPath, $"{Path.GetRandomFileName()}.apk");
309+
File.Create(fakeApkPath).Close();
310+
311+
int exitCode = runner.InstallApk(fakeApkPath);
312+
313+
Assert.NotEqual(0, exitCode);
314+
Assert.Equal(2, installAttempts); // Should have retried once
315+
316+
// Verify that the initial failure output IS in the log messages
317+
var combinedLog = string.Join(Environment.NewLine, logMessages);
318+
Assert.Contains("Initial failure output:", combinedLog);
319+
Assert.Contains("INSTALL_FAILED_INSUFFICIENT_STORAGE", combinedLog);
320+
}
321+
174322
[Fact]
175323
public void KillApk()
176324
{

0 commit comments

Comments
 (0)