Merge pull request #9436 from xamarin/backport-test-importer-fixes

[Harness] Backport test importer fixes

tests/xharness/Microsoft.DotNet.XHarness.iOS.Shared/IResultParser.cs

@@ -11,6 +11,8 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 		// generates a xml result that will consider to be an error by the CI. Allows to catch errors in cases in which we are not talking about a test
 		// failure perse but the situation in which the app could not be built, timeout or crashed.
 		void GenerateFailure (ILogs logs, string source, string appName, string variation, string title, string message, string stderrPath, XmlResultJargon jargon);
+		
+		void GenerateFailure (ILogs logs, string source, string appName, string variation, string title, string message, StreamReader stderrReader, XmlResultJargon jargon);
 
 		// updates a given xml result to contain a list of attachments. This is useful for CI to be able to add logs as part of the attachments of a failing test.
 		void UpdateMissingData (string source, string destination, string applicationName, IEnumerable<string> attachments);

tests/xharness/Microsoft.DotNet.XHarness.iOS.Shared/TestReporter.cs

@@ -297,6 +297,8 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 			// from the TCP connection, we are going to fail when trying to read it and not parse it. Therefore, we are not only
 			// going to check if we are in CI, but also if the listener_log is valid.
 			var path = Path.ChangeExtension (test_log_path, "xml");
+			if (path == test_log_path)
+				path = Path.Combine (Path.GetDirectoryName (path), Path.GetFileNameWithoutExtension (path) + "-clean.xml");
 			resultParser.CleanXml (test_log_path, path);
 
 			if (ResultsUseXml && resultParser.IsValidXml (path, out var xmlType)) {
@@ -323,12 +325,9 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 					}
 					path = newFilename;
 
-					// write the human readable results in a tmp file, which we later use to step on the logs
-					var tmpFile = Path.Combine (Path.GetTempPath (), Guid.NewGuid ().ToString ());
-					(parseResult.resultLine, parseResult.failed) = resultParser.GenerateHumanReadableResults (path, tmpFile, xmlType);
-					File.Copy (tmpFile, test_log_path, true);
-					File.Delete (tmpFile);
-
+					var humanReadableLog = logs.CreateFile (Path.GetFileNameWithoutExtension (test_log_path) + ".log", LogType.NUnitResult.ToString ());
+					(parseResult.resultLine, parseResult.failed) = resultParser.GenerateHumanReadableResults (path, humanReadableLog, xmlType);
+					
 					// we do not longer need the tmp file
 					logs.AddFile (path, LogType.XmlLog.ToString ());
 					return parseResult;
@@ -396,6 +395,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 		// generate all the xml failures that will help the integration with the CI and return the failure reason
 		async Task GenerateXmlFailures (string failureMessage, bool crashed, string crashReason)
 		{
+			using var mainLogReader = mainLog.GetReader ();
 			if (!ResultsUseXml) // nothing to do
 				return;
 			if (!string.IsNullOrEmpty (crashReason)) {
@@ -406,7 +406,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 					appInfo.Variation,
 					$"App Crash {appInfo.AppName} {appInfo.Variation}",
 					$"App crashed: {failureMessage}",
-					mainLog.FullPath,
+					mainLogReader,
 					xmlJargon);
 			} else if (launchFailure) {
 				resultParser.GenerateFailure (
@@ -416,7 +416,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 					appInfo.Variation,
 					$"App Launch {appInfo.AppName} {appInfo.Variation} on {deviceName}",
 					$"{failureMessage} on {deviceName}",
-					mainLog.FullPath,
+					mainLogReader,
 					xmlJargon);
 			} else if (!isSimulatorTest && crashed && string.IsNullOrEmpty (crashReason)) {
 				// this happens more that what we would like on devices, the main reason most of the time is that we have had netwoking problems and the
@@ -430,7 +430,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 						appInfo.Variation,
 						$"TcpConnection on {deviceName}",
 						$"Device {deviceName} could not reach the host over tcp.",
-						mainLog.FullPath,
+						mainLogReader,
 						xmlJargon);
 				}
 			} else if (timedout) {
@@ -441,7 +441,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 					appInfo.Variation,
 					$"App Timeout {appInfo.AppName} {appInfo.Variation} on bot {deviceName}",
 					$"{appInfo.AppName} {appInfo.Variation} Test run timed out after {timeout.TotalMinutes} minute(s) on bot {deviceName}.",
-					mainLog.FullPath,
+					mainLogReader,
 					xmlJargon);
 			}
 		}

tests/xharness/Microsoft.DotNet.XHarness.iOS.Shared/XmlResultParser.cs

@@ -1,3 +1,4 @@
+#nullable enable
 using System;
 using System.Collections.Generic;
 using System.IO;
@@ -611,7 +612,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 			("skipped", "0"),
 			("asserts", "0"));
 
-		static void WriteFailure (XmlWriter writer, string message, StreamReader stderr = null)
+		static void WriteFailure (XmlWriter writer, string message, StreamReader? stderr = null)
 		{
 			writer.WriteStartElement ("failure");
 			writer.WriteStartElement ("message");
@@ -714,11 +715,10 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 			writer.WriteEndElement (); // assemblies
 		}
 
-		static void GenerateFailureXml (string destination, string title, string message, string stderrPath, XmlResultJargon jargon)
+		static void GenerateFailureXml (string destination, string title, string message, StreamReader stderrReader, XmlResultJargon jargon)
 		{
 			XmlWriterSettings settings = new XmlWriterSettings { Indent = true };
 			using (var stream = File.CreateText (destination))
-			using (var stderrReader = new StreamReader (stderrPath))
 			using (var xmlWriter = XmlWriter.Create (stream, settings)) {
 				xmlWriter.WriteStartDocument ();
 				switch (jargon) {
@@ -736,21 +736,27 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared {
 			}
 		}
 
-		public void GenerateFailure (ILogs logs, string source, string appName, string variation, string title, string message, string stderrPath, XmlResultJargon jargon)
+		public void GenerateFailure (ILogs logs, string source, string appName, string variation, string title,
+			string message, StreamReader stderr, XmlResultJargon jargon)
 		{
 			// VSTS does not provide a nice way to report build errors, create a fake
 			// test result with a failure in the case the build did not work
 			var failureLogXml = logs.Create ($"vsts-nunit-{source}-{Helpers.Timestamp}.xml", LogType.XmlLog.ToString ());
 			if (jargon == XmlResultJargon.NUnitV3) {
 				var failureXmlTmp = logs.Create ($"nunit-{source}-{Helpers.Timestamp}.tmp", "Failure Log tmp");
-				GenerateFailureXml (failureXmlTmp.FullPath, title, message, stderrPath, jargon);
+				GenerateFailureXml (failureXmlTmp.FullPath, title, message, stderr, jargon);
 				// add the required attachments and the info of the application that failed to install
 				var failure_logs = Directory.GetFiles (logs.Directory).Where (p => !p.Contains ("nunit")); // all logs but ourself
 				UpdateMissingData (failureXmlTmp.FullPath, failureLogXml.FullPath, $"{appName} {variation}", failure_logs);
 			} else {
-				GenerateFailureXml (failureLogXml.FullPath, title, message, stderrPath, jargon);
+				GenerateFailureXml (failureLogXml.FullPath, title, message, stderr, jargon);
 			}
 		}
+		public void GenerateFailure (ILogs logs, string source, string appName, string variation, string title, string message, string stderrPath, XmlResultJargon jargon)
+		{
+			using var stderrReader = new StreamReader (stderrPath);
+			GenerateFailure (logs, source, appName, variation, title, message, stderrReader, jargon);
+		}
 
 		public static string GetVSTSFilename (string filename)
 			=> Path.Combine (Path.GetDirectoryName (filename), $"vsts-{Path.GetFileName (filename)}");

tests/xharness/TestTasks/RunDevice.cs

@@ -130,6 +130,7 @@ namespace Xharness.TestTasks {
 						// Install the app
 						InstallLog = new AppInstallMonitorLog (testTask.Logs.Create ($"install-{Helpers.Timestamp}.log", "Install log"));
 						try {
+							using var logReader = InstallLog.GetReader ();
 							testTask.Runner.MainLog = this.InstallLog;
 							var install_result = await testTask.Runner.InstallAsync (InstallLog.CancellationToken);
 							if (!install_result.Succeeded) {
@@ -143,7 +144,8 @@ namespace Xharness.TestTasks {
 										testTask.Variation,
 										$"AppInstallation on {testTask.Device.Name}",
 										$"Install failed on {testTask.Device.Name}, exit code: {install_result.ExitCode}",
-										InstallLog.FullPath, xmlResultJargon);
+										logReader,
+										xmlResultJargon);
 							}
 						} finally {
 							InstallLog.Dispose ();

tests/xharness/TestTasks/RunTest.cs

@@ -67,16 +67,18 @@ namespace Xharness.TestTasks {
 				testTask.FailureMessage = BuildTask.FailureMessage;
 				if (!string.IsNullOrEmpty (BuildTask.KnownFailure))
 					testTask.KnownFailure = BuildTask.KnownFailure;
-				if (generateXmlFailures)
+				if (generateXmlFailures) {
+					var logReader = BuildTask.BuildLog.GetReader ();
 					ResultParser.GenerateFailure (
-						logs: testTask.Logs, 
+						logs: testTask.Logs,
 						source: "build",
 						appName: testTask.TestName,
 						variation: testTask.Variation,
 						title: $"App Build {testTask.TestName} {testTask.Variation}",
 						message: $"App could not be built {testTask.FailureMessage}.",
-						stderrPath: BuildTask.BuildLog.FullPath,
+						stderrReader: logReader,
 						jargon: xmlResultJargon);
+				}
 			} else {
 				testTask.ExecutionResult = TestExecutingResult.Built;
 			}