[xharness] Do not mark DeviceLoaders as loaded when loading fails (#9367)

tests/xharness/Microsoft.DotNet.XHarness.iOS.Shared.Tests/Hardware/DevicesTest.cs

@@ -113,10 +113,53 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared.Tests.Hardware {
 			Assert.AreEqual (0, devices.ConnectedTV.Count ());
 		}
 
-		private void AssertArgumentValue (MlaunchArgument arg, string expected, string message = null)
+		[Test]
+		public async Task FindAndCacheDevicesWithFailingMlaunchTest ()
 		{
-			var value = arg.AsCommandLineArgument ().Split (new char [] { '=' }, 2).LastOrDefault ();
-			Assert.AreEqual (expected, value, message);
+			string processPath = null;
+			MlaunchArguments passedArguments = null;
+
+			// Moq.SetupSequence doesn't allow custom callbacks so we need to count ourselves
+			var calls = 0;
+
+			// moq It.Is is not working as nicelly as we would like it, we capture data and use asserts
+			processManager.Setup (p => p.RunAsync (It.IsAny<Process> (), It.IsAny<MlaunchArguments> (), It.IsAny<ILog> (), It.IsAny<TimeSpan?> (), It.IsAny<Dictionary<string, string>> (), It.IsAny<CancellationToken?> (), It.IsAny<bool?> ()))
+				.Returns<Process, MlaunchArguments, ILog, TimeSpan?, Dictionary<string, string>, CancellationToken?, bool?> ((p, args, log, t, env, token, d) => {
+					calls++;
+
+					if (calls == 1) {
+						// Mlaunch can sometimes time out and we are testing that a subsequent Load will trigger it again
+						return Task.FromResult (new ProcessExecutionResult { ExitCode = 137, TimedOut = true });
+					}
+
+					processPath = p.StartInfo.FileName;
+					passedArguments = args;
+
+					// we get the temp file that was passed as the args, and write our sample xml, which will be parsed to get the devices :)
+					var tempPath = args.Where (a => a is ListDevicesArgument).First ().AsCommandLineArgument ();
+					tempPath = tempPath.Substring (tempPath.IndexOf ('=') + 1).Replace ("\"", string.Empty);
+
+					var name = GetType ().Assembly.GetManifestResourceNames ().Where (a => a.EndsWith ("devices.xml", StringComparison.Ordinal)).FirstOrDefault ();
+					using (var outputStream = new StreamWriter (tempPath))
+					using (var sampleStream = new StreamReader (GetType ().Assembly.GetManifestResourceStream (name))) {
+						string line;
+						while ((line = sampleStream.ReadLine ()) != null)
+							outputStream.WriteLine (line);
+					}
+					return Task.FromResult (new ProcessExecutionResult { ExitCode = 0, TimedOut = false });
+				});
+
+			Assert.ThrowsAsync<Exception> (async () => await devices.LoadDevices (executionLog.Object));
+
+			Assert.IsEmpty (devices.ConnectedDevices);
+			Assert.AreEqual (1, calls);
+			await devices.LoadDevices (executionLog.Object);
+			Assert.AreEqual (2, calls);
+			Assert.IsNotEmpty (devices.ConnectedDevices);
+			await devices.LoadDevices (executionLog.Object);
+			Assert.AreEqual (2, calls);
+			await devices.LoadDevices (executionLog.Object);
+			Assert.AreEqual (2, calls);
 		}
 	}
 }

tests/xharness/Microsoft.DotNet.XHarness.iOS.Shared.Tests/Hardware/SimulatorsTest.cs

@@ -149,10 +149,41 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared.Tests.Hardware {
 			Assert.AreEqual (expected, sims.Count (), $"{target} simulators count");
 		}
 
-		private void AssertArgumentValue (MlaunchArgument arg, string expected, string message = null)
+		[Test]
+		public async Task FindAndCacheSimulatorsWithFailingMlaunchTest ()
 		{
-			var value = arg.AsCommandLineArgument ().Split (new char [] { '=' }, 2).LastOrDefault ();
-			Assert.AreEqual (expected, value, message);
+			// Moq.SetupSequence doesn't allow custom callbacks so we need to count ourselves
+			var calls = 0;
+
+			processManager
+				.Setup (p => p.RunAsync (It.IsAny<Process>(), It.Is<MlaunchArguments> (args => args.Any (a => a is ListSimulatorsArgument)), It.IsAny<ILog> (), It.IsAny<TimeSpan> (), It.IsAny<Dictionary<string, string>> (), It.IsAny<CancellationToken?> (), It.IsAny<bool?> ()))
+				.Returns<Process, MlaunchArguments, ILog, TimeSpan, Dictionary<string, string>, CancellationToken?, bool?> ((process, args, log, t, env, token, diagnostics) => {
+					calls++;
+
+					if (calls == 1) {
+						// Mlaunch can sometimes time out and we are testing that a subsequent Load will trigger it again
+						return Task.FromResult (new ProcessExecutionResult { ExitCode = 137, TimedOut = true });
+					}
+
+					// We get the temp file that was passed as the args, and write our sample xml, which will be parsed to get the devices
+					var tempPath = args.Where (a => a is ListSimulatorsArgument).First ().AsCommandLineArgument ();
+					tempPath = tempPath.Substring (tempPath.IndexOf ('=') + 1).Replace ("\"", string.Empty);
+
+					CopySampleData (tempPath);
+					return Task.FromResult (new ProcessExecutionResult { ExitCode = 0, TimedOut = false });
+				});
+
+			Assert.ThrowsAsync<Exception> (async () => await simulators.LoadDevices (executionLog.Object));
+
+			Assert.IsEmpty (simulators.AvailableDevices);
+			Assert.AreEqual (1, calls);
+			await simulators.LoadDevices (executionLog.Object);
+			Assert.AreEqual (2, calls);
+			Assert.IsNotEmpty (simulators.AvailableDevices);
+			await simulators.LoadDevices (executionLog.Object);
+			Assert.AreEqual (2, calls);
+			await simulators.LoadDevices (executionLog.Object);
+			Assert.AreEqual (2, calls);
 		}
 	}
 }

tests/xharness/Microsoft.DotNet.XHarness.iOS.Shared/Hardware/HardwareDeviceLoader.cs

@@ -3,6 +3,7 @@ using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using System.Xml;
 using Microsoft.DotNet.XHarness.iOS.Shared.Collections;
@@ -27,6 +28,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared.Hardware {
 	}
 
 	public class HardwareDeviceLoader : IHardwareDeviceLoader {
+		readonly SemaphoreSlim semaphore = new SemaphoreSlim (1);
 		readonly IProcessManager processManager;
 		bool loaded;
 
@@ -46,14 +48,16 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared.Hardware {
 
 		public async Task LoadDevices (ILog log, bool includeLocked = false, bool forceRefresh = false, bool listExtraData = false)
 		{
+			await semaphore.WaitAsync ();
+
 			if (loaded) {
-				if (!forceRefresh)
+				if (!forceRefresh) {
+					semaphore.Release ();
 					return;
+				}
 				connectedDevices.Reset ();
 			}
 
-			loaded = true;
-
 			var tmpfile = Path.GetTempFileName ();
 			try {
 				using (var process = new Process ()) {
@@ -92,11 +96,14 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared.Hardware {
 						}
 						connectedDevices.Add (d);
 					}
+
+					loaded = true;
 				}
 			} finally {
 				connectedDevices.SetCompleted ();
 				File.Delete (tmpfile);
 				log.Flush ();
+				semaphore.Release ();
 			}
 		}
 

tests/xharness/Microsoft.DotNet.XHarness.iOS.Shared/Hardware/SimulatorLoader.cs

@@ -49,7 +49,6 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared.Hardware {
 				available_devices.Reset ();
 				available_device_pairs.Reset ();
 			}
-			loaded = true;
 
 			await Task.Run (async () => {
 				var tmpfile = Path.GetTempFileName ();
@@ -115,6 +114,8 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared.Hardware {
 							});
 						}
 					}
+
+					loaded = true;
 				} finally {
 					supported_runtimes.SetCompleted ();
 					supported_device_types.SetCompleted ();