Fix ios run command for devices (dotnet#404)

premun · web-flow · commit 1d3ec3b30c26 · 2020-12-21T18:41:48.000+01:00
- `UninstallApp()` wasn't triggering for devices - mlaunch failures when running app didn't get detected Resolves dotnet#402
diff --git a/src/Microsoft.DotNet.XHarness.CLI/Commands/iOS/iOSAppCommand.cs b/src/Microsoft.DotNet.XHarness.CLI/Commands/iOS/iOSAppCommand.cs
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
+using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Text;
 using System.Threading;
@@ -159,24 +160,49 @@ private async Task<ExitCode> RunApp(ILogger logger, TestTargetOs target, Logs lo
                 return ExitCode.FAILED_TO_GET_BUNDLE_INFO;
             }
 
+            ExitCode exitCode = ExitCode.SUCCESS;
+
             if (!target.Platform.IsSimulator())
             {
-                var result = await InstallApp(
-                    appBundleInfo,
-                    deviceName,
-                    logger,
-                    target,
-                    logs,
-                    mainLog,
-                    cancellationToken);
+                Exception? installException = null;
 
-                if (result != ExitCode.SUCCESS)
+                try
                 {
-                    return result;
+                    (deviceName, exitCode) = await InstallApp(appBundleInfo, deviceName, logger, target, mainLog, cancellationToken);
                 }
-            }
+                catch (Exception e)
+                {
+                    installException = e;
+                }
+
+                if (exitCode != ExitCode.SUCCESS || installException != null)
+                {
+                    var message = new StringBuilder().AppendLine("Application installation failed:");
+
+                    if (ErrorKnowledgeBase.IsKnownInstallIssue(mainLog, out var failureMessage))
+                    {
+                        message.Append(failureMessage.Value.HumanMessage);
+                        if (failureMessage.Value.IssueLink != null)
+                        {
+                            message.AppendLine($" Find more information at {failureMessage.Value.IssueLink}");
+                        }
+                    }
+                    else if (installException != null)
+                    {
+                        message.AppendLine(installException.ToString());
+                    }
+
+                    logger.LogError(message.ToString());
 
-            ExitCode exitCode;
+                    return exitCode;
+                }
+
+                if (string.IsNullOrEmpty(deviceName))
+                {
+                    logger.LogError("Failed to get the name of the device where application was installed!");
+                    return ExitCode.PACKAGE_INSTALLATION_FAILURE;
+                }
+            }
 
             try
             {
@@ -201,22 +227,24 @@ private async Task<ExitCode> RunApp(ILogger logger, TestTargetOs target, Logs lo
             }
             catch (Exception e)
             {
+                var message = new StringBuilder().AppendLine("Application run failed:");
+
                 if (ErrorKnowledgeBase.IsKnownTestIssue(mainLog, out var failureMessage))
                 {
-                    var msg = $"Application run failed:{Environment.NewLine}{failureMessage.Value.HumanMessage}.";
+                    message.Append(failureMessage.Value.HumanMessage);
                     if (failureMessage.Value.IssueLink != null)
                     {
-                        msg += $" Find more information at {failureMessage.Value.IssueLink}";
+                        message.AppendLine($" Find more information at {failureMessage.Value.IssueLink}");
                     }
-
-                    logger.LogError(msg);
                 }
                 else
                 {
-                    logger.LogError($"Application run failed:{Environment.NewLine}{e}");
+                    message.AppendLine(e.ToString());
                 }
 
-                return ExitCode.APP_CRASH;
+                logger.LogError(message.ToString());
+
+                exitCode = ExitCode.APP_LAUNCH_FAILURE;
             }
             finally
             {
@@ -236,12 +264,11 @@ private async Task<ExitCode> RunApp(ILogger logger, TestTargetOs target, Logs lo
             return exitCode;
         }
 
-        private async Task<ExitCode> InstallApp(
+        private async Task<(string?, ExitCode)> InstallApp(
             AppBundleInformation appBundleInfo,
             string? deviceName,
             ILogger logger,
             TestTargetOs target,
-            Logs logs,
             IFileBackedLog mainLog,
             CancellationToken cancellationToken)
         {
@@ -259,12 +286,12 @@ private async Task<ExitCode> InstallApp(
             {
                 logger.LogError($"Failed to find suitable device for target {target.AsString()}" + Environment.NewLine +
                     "Please make sure the device is connected and unlocked.");
-                return ExitCode.DEVICE_NOT_FOUND;
+                return (null, ExitCode.DEVICE_NOT_FOUND);
             }
             catch (Exception e)
             {
                 logger.LogError($"Failed to install the app bundle:{Environment.NewLine}{e}");
-                return ExitCode.PACKAGE_INSTALLATION_FAILURE;
+                return (null, ExitCode.PACKAGE_INSTALLATION_FAILURE);
             }
 
             if (!result.Succeeded)
@@ -286,15 +313,20 @@ private async Task<ExitCode> InstallApp(
                     logger.LogError($"Failed to install the app bundle (exit code={result.ExitCode})");
                 }
 
-                return ExitCode.PACKAGE_INSTALLATION_FAILURE;
+                return (deviceName, ExitCode.PACKAGE_INSTALLATION_FAILURE);
             }
 
             logger.LogInformation($"Application '{appBundleInfo.AppName}' was installed successfully on device '{deviceName}'");
 
-            return ExitCode.SUCCESS;
+            return (deviceName, ExitCode.SUCCESS);
         }
 
-        private async Task UninstallApp(AppBundleInformation appBundleInfo, string deviceName, ILogger logger, IFileBackedLog mainLog, CancellationToken cancellationToken)
+        private async Task UninstallApp(
+            AppBundleInformation appBundleInfo,
+            string deviceName,
+            ILogger logger,
+            IFileBackedLog mainLog,
+            CancellationToken cancellationToken)
         {
             logger.LogInformation($"Uninstalling the application '{appBundleInfo.AppName}' from device '{deviceName}'");
 
diff --git a/src/Microsoft.DotNet.XHarness.iOS/AppRunner.cs b/src/Microsoft.DotNet.XHarness.iOS/AppRunner.cs
@@ -230,23 +230,22 @@ private async Task RunDeviceApp(
 
                 _mainLog.WriteLine("Starting test run");
 
-                await _processManager.ExecuteCommandAsync(
+                var result = await _processManager.ExecuteCommandAsync(
                     mlaunchArguments,
                     _mainLog,
                     timeout,
                     cancellationToken: cancellationToken);
+
+                if (!result.Succeeded)
+                {
+                    throw new Exception("Failed to run the application!");
+                }
             }
             finally
             {
                 deviceLogCapturer.StopCapture();
                 deviceSystemLog.Dispose();
             }
-
-            // Upload the system log
-            if (File.Exists(deviceSystemLog.FullPath))
-            {
-                _mainLog.WriteLine("A capture of the device log is: {0}", deviceSystemLog.FullPath);
-            }
         }
 
         private MlaunchArguments GetCommonArguments(int verbosity)
diff --git a/tests/Microsoft.DotNet.XHarness.iOS.Tests/AppRunnerTests.cs b/tests/Microsoft.DotNet.XHarness.iOS.Tests/AppRunnerTests.cs
@@ -404,39 +404,36 @@ public async Task RunOnDeviceSuccessfullyTest()
         }
 
         [Fact]
-        public async Task RunOnDeviceWithMissingSystemLogTest()
+        public async Task RunOnSimulatorWithMissingSystemLogTest()
         {
-            var testLog = new Mock<IFileBackedLog>();
-            testLog.SetupGet(x => x.FullPath).Returns(Path.GetTempFileName());
-            testLog.SetupGet(x => x.Description).Returns("test log");
-
-            SetupLogList(new[] { testLog.Object });
-            _logs
-                .Setup(x => x.Create("device-" + DeviceName + "-mocked_timestamp.log", LogType.SystemLog.ToString(), It.IsAny<bool?>()))
-                .Returns(testLog.Object);
+            _simulatorLoader
+                .Setup(x => x.FindSimulators(It.Is<TestTargetOs>(t => t.Platform == TestTarget.Simulator_tvOS), _mainLog.Object, It.IsAny<int>(), true, false))
+                .ReturnsAsync((_mockSimulator.Object, null));
 
-            var deviceLogCapturer = new Mock<IDeviceLogCapturer>();
+            var captureLog = new Mock<ICaptureLog>();
+            captureLog.SetupGet(x => x.FullPath).Returns(_simulatorLogPath);
+            captureLog.SetupGet(x => x.Description).Returns(LogType.TestLog.ToString());
 
-            var deviceLogCapturerFactory = new Mock<IDeviceLogCapturerFactory>();
-            deviceLogCapturerFactory
-                .Setup(x => x.Create(_mainLog.Object, testLog.Object, DeviceName))
-                .Returns(deviceLogCapturer.Object);
+            var captureLogFactory = new Mock<ICaptureLogFactory>();
+            captureLogFactory
+                .Setup(x => x.Create(
+                   Path.Combine(_logs.Object.Directory, _mockSimulator.Object.Name + ".log"),
+                   _mockSimulator.Object.SystemLog,
+                   false,
+                   It.IsAny<string>()))
+                .Returns(captureLog.Object);
 
-            var x = _logs.Object.First();
+            SetupLogList(new[] { captureLog.Object });
 
             var appInformation = GetMockedAppBundleInfo();
 
-            _exitCodeDetector
-                .Setup(x => x.DetectExitCode(appInformation, It.IsAny<IReadableLog>()))
-                .Returns(0);
-
             // Act
             var appRunner = new AppRunner(_processManager.Object,
                 _hardwareDeviceLoader.Object,
                 _simulatorLoader.Object,
                 _snapshotReporterFactory,
-                Mock.Of<ICaptureLogFactory>(),
-                deviceLogCapturerFactory.Object,
+                captureLogFactory.Object,
+                Mock.Of<IDeviceLogCapturerFactory>(),
                 _exitCodeDetector.Object,
                 _mainLog.Object,
                 _logs.Object,
@@ -445,31 +442,31 @@ public async Task RunOnDeviceWithMissingSystemLogTest()
 
             var (deviceName, exitCode) = await appRunner.RunApp(
                 appInformation,
-                new TestTargetOs(TestTarget.Device_iOS, null),
+                new TestTargetOs(TestTarget.Simulator_tvOS, null),
                 TimeSpan.FromSeconds(30));
 
             // Verify
-            Assert.Equal(DeviceName, deviceName);
+            Assert.Equal(SimulatorDeviceName, deviceName);
             Assert.Null(exitCode);
 
-            var expectedArgs = GetExpectedDeviceMlaunchArgs();
+            var expectedArgs = GetExpectedSimulatorMlaunchArgs();
 
             _processManager
                 .Verify(
                     x => x.ExecuteCommandAsync(
                        It.Is<MlaunchArguments>(args => args.AsCommandLine() == expectedArgs),
-                       It.IsAny<ILog>(),
+                       _mainLog.Object,
                        It.IsAny<TimeSpan>(),
                        null,
                        It.IsAny<CancellationToken>()),
                     Times.Once);
 
-            _hardwareDeviceLoader.VerifyAll();
+            _simulatorLoader.VerifyAll();
 
             _snapshotReporter.Verify(x => x.StartCaptureAsync(), Times.AtLeastOnce);
             _snapshotReporter.Verify(x => x.StartCaptureAsync(), Times.AtLeastOnce);
 
-            testLog.Verify(x => x.Dispose(), Times.AtLeastOnce);
+            captureLog.Verify(x => x.Dispose(), Times.AtLeastOnce);
         }
 
         private static AppBundleInformation GetMockedAppBundleInfo() =>
