From a8346dcc301875ef9cede0500f3736a73dbbc3cf Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Thu, 7 May 2020 20:19:41 -0400 Subject: [PATCH] [iOS] Ensure that when we use other loggers, we can generate xml failures. (#159) Not all the loggers support the FullPath property and that will result in runtime exceptions. On the other hand, they all support the GetReader methods, so switch to that. We keep the path one since is nice for tests. Related iOS PR: https://github.com/xamarin/xamarin-macios/pull/8534 --- .../IResultParser.cs | 8 +- .../TestReporter.cs | 58 ++++++------ .../XmlResultParser.cs | 93 +++++++++++-------- 3 files changed, 90 insertions(+), 69 deletions(-) diff --git a/src/Microsoft.DotNet.XHarness.iOS.Shared/IResultParser.cs b/src/Microsoft.DotNet.XHarness.iOS.Shared/IResultParser.cs index ee0770d..9dd9e3c 100644 --- a/src/Microsoft.DotNet.XHarness.iOS.Shared/IResultParser.cs +++ b/src/Microsoft.DotNet.XHarness.iOS.Shared/IResultParser.cs @@ -16,10 +16,16 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared { /// /// Generates an 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. + /// failure per se 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); + /// + /// Generates an 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 per se 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, StreamReader stderrReader, XmlResultJargon jargon); + /// /// Updates 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. /// diff --git a/src/Microsoft.DotNet.XHarness.iOS.Shared/TestReporter.cs b/src/Microsoft.DotNet.XHarness.iOS.Shared/TestReporter.cs index 6df57ea..3099de6 100644 --- a/src/Microsoft.DotNet.XHarness.iOS.Shared/TestReporter.cs +++ b/src/Microsoft.DotNet.XHarness.iOS.Shared/TestReporter.cs @@ -1,4 +1,5 @@ -// Licensed to the .NET Foundation under one or more agreements. +#nullable enable +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. @@ -50,12 +51,12 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared /// Additional logs that will be sent with the report in case of a failure. /// Used by the Xamarin.Xharness project to add BuildTask logs. /// - readonly string additionalLogsDirectory; + readonly string? additionalLogsDirectory; /// /// Callback needed for the Xamarin.Xharness project that does extra logging in case of a crash. /// - readonly ExceptionLogger exceptionLogger; + readonly ExceptionLogger? exceptionLogger; bool waitedForExit = true; bool launchFailure; @@ -84,12 +85,12 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared XmlResultJargon xmlJargon, string device, TimeSpan timeout, - string additionalLogsDirectory = null, - ExceptionLogger exceptionLogger = null, + string? additionalLogsDirectory = null, + ExceptionLogger? exceptionLogger = null, bool generateHtml = false) { this.processManager = processManager ?? throw new ArgumentNullException(nameof(processManager)); - this.deviceName = device; // can be null on simulators + this.deviceName = device; // can be null on simulators this.listener = simpleListener ?? throw new ArgumentNullException(nameof(simpleListener)); this.mainLog = mainLog ?? throw new ArgumentNullException(nameof(mainLog)); this.runLog = runLog ?? throw new ArgumentNullException(nameof(runLog)); @@ -130,6 +131,9 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared while (!reader.EndOfStream) { var line = await reader.ReadLineAsync(); + + if (line == null) continue; + if (line.StartsWith("Application launched. PID = ", StringComparison.Ordinal)) { var pidstr = line.Substring("Application launched. PID = ".Length); @@ -151,12 +155,12 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared return pidData; } - // parse the main log to get the pid + // parse the main log to get the pid async Task GetPidFromMainLog() { int pid = -1; using var log_reader = mainLog.GetReader(); // dispose when we leave the method, which is what we want - string line; + string? line; while ((line = await log_reader.ReadLineAsync()) != null) { const string str = "was launched with pid '"; @@ -175,7 +179,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared } // return the reason for a crash found in a log - void GetCrashReason(int pid, ILog crashLog, out string crashReason) + void GetCrashReason(int pid, ILog crashLog, out string? crashReason) { crashReason = null; using var crashReader = crashLog.GetReader(); // dispose when we leave the method @@ -184,7 +188,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared var reader = JsonReaderWriterFactory.CreateJsonReader(Encoding.UTF8.GetBytes(text), new XmlDictionaryReaderQuotas()); var doc = new XmlDocument(); doc.Load(reader); - foreach (XmlNode node in doc.SelectNodes($"/root/processes/item[pid = '" + pid + "']")) + foreach (XmlNode? node in doc.SelectNodes($"/root/processes/item[pid = '" + pid + "']")) { Console.WriteLine(node?.InnerXml); Console.WriteLine(node?.SelectSingleNode("reason")?.InnerText); @@ -196,7 +200,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared async Task TcpConnectionFailed() { using var reader = new StreamReader(mainLog.FullPath); - string line; + string? line; while ((line = await reader.ReadLineAsync()) != null) { if (line.Contains("Couldn't establish a TCP connection with any of the hostnames")) @@ -207,7 +211,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared return false; } - // kill any process + // kill any process Task KillAppProcess(int pid, CancellationTokenSource cancellationSource) { var launchTimedout = cancellationSource.IsCancellationRequested; @@ -276,7 +280,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared isSimulatorTest = true; await CollectResult(processExecution); - if (!Success.Value) + if (Success != null && !Success.Value) { var (pid, launchFailure) = await GetPidFromRunLog(); this.launchFailure = launchFailure; @@ -297,12 +301,12 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared await CollectResult(processExecution); } - async Task<(string ResultLine, bool Failed)> GetResultLine(string logPath) + async Task<(string? ResultLine, bool Failed)> GetResultLine(string logPath) { - string resultLine = null; + string? resultLine = null; bool failed = false; using var reader = new StreamReader(logPath); - string line = null; + string? line = null; while ((line = await reader.ReadLineAsync()) != null) { if (line.Contains("Tests run:")) @@ -320,9 +324,9 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared return (ResultLine: resultLine, Failed: failed); } - async Task<(string resultLine, bool failed, bool crashed)> ParseResultFile(AppBundleInformation appInfo, string test_log_path, bool timed_out) + async Task<(string? resultLine, bool failed, bool crashed)> ParseResultFile(AppBundleInformation appInfo, string test_log_path, bool timed_out) { - (string resultLine, bool failed, bool crashed) parseResult = (null, false, false); + (string? resultLine, bool failed, bool crashed) parseResult = (null, false, false); if (!File.Exists(test_log_path)) { parseResult.crashed = true; // if we do not have a log file, the test crashes @@ -333,9 +337,9 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared // However, for our own reporting, we still want the console-based log. This log is embedded inside the xml produced // by Touch.Unit, so we need to extract it and write it to disk. We also need to re-save the xml output, since Touch.Unit // wraps the NUnit xml output with additional information, which we need to unwrap so that Jenkins understands it. - // + // // On the other hand, the nunit and xunit do not have that data and have to be parsed. - // + // // This if statement has a small trick, we found out that internet sharing in some of the bots (VSTS) does not work, in // that case, we cannot do a TCP connection to xharness to get the log, this is a problem since if we did not get the xml // from the TCP connection, we are going to fail when trying to read it and not parse it. Therefore, we are not only @@ -398,7 +402,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared mainLog.WriteLine(new string('#', 10)); using (var stream = new StreamReader(path)) { - string line; + string? line; while ((line = await stream.ReadLineAsync()) != null) { mainLog.WriteLine(line); @@ -466,7 +470,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) + async Task GenerateXmlFailures(string failure, bool crashed, string? crashReason) { if (!ResultsUseXml) // nothing to do return; @@ -478,7 +482,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared appInfo.AppName, appInfo.Variation, $"App Crash {appInfo.AppName} {appInfo.Variation}", - $"App crashed: {failureMessage}", + $"App crashed: {failure}", mainLog.FullPath, xmlJargon); } @@ -490,7 +494,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared appInfo.AppName, appInfo.Variation, $"App Launch {appInfo.AppName} {appInfo.Variation} on {deviceName}", - $"{failureMessage} on {deviceName}", + $"{failure} on {deviceName}", mainLog.FullPath, xmlJargon); } @@ -526,9 +530,9 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared } } - public async Task<(TestExecutingResult ExecutingResult, string ResultMessage)> ParseResult() + public async Task<(TestExecutingResult ExecutingResult, string? ResultMessage)> ParseResult() { - var result = (ExecutingResult: TestExecutingResult.Finished, ResultMessage: (string)null); + (TestExecutingResult ExecutingResult, string? ResultMessage)result = (ExecutingResult: TestExecutingResult.Finished, ResultMessage: null); var crashed = false; if (File.Exists(listener.TestLog.FullPath)) { @@ -597,7 +601,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared if (!Success.Value) { int pid = -1; - string crashReason = null; + string? crashReason = null; foreach (var crashLog in crashLogs) { try diff --git a/src/Microsoft.DotNet.XHarness.iOS.Shared/XmlResultParser.cs b/src/Microsoft.DotNet.XHarness.iOS.Shared/XmlResultParser.cs index 0e00ebd..3d6d2cf 100644 --- a/src/Microsoft.DotNet.XHarness.iOS.Shared/XmlResultParser.cs +++ b/src/Microsoft.DotNet.XHarness.iOS.Shared/XmlResultParser.cs @@ -1,3 +1,4 @@ +#nullable enable // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. @@ -34,39 +35,38 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared if (!File.Exists(path)) return false; - using (var stream = File.OpenText(path)) - { - string line; - while ((line = stream.ReadLine()) != null) - { // special case when get got the tcp connection - if (line.Contains("ping")) - continue; - if (line.Contains("test-run")) - { // first element of the NUnitV3 test collection - type = XmlResultJargon.NUnitV3; - return true; - } - if (line.Contains("TouchUnitTestRun")) - { - type = XmlResultJargon.TouchUnit; - return true; - } - if (line.Contains("test-results")) - { // first element of the NUnitV3 test collection - type = XmlResultJargon.NUnitV2; - return true; - } - if (line.Contains("")) - { // first element of the xUnit test collection - type = XmlResultJargon.xUnit; - return true; - } + using var stream = File.OpenText(path); + string? line; + while ((line = stream.ReadLine()) != null) + { // special case when get got the tcp connection + if (line.Contains("ping")) + continue; + if (line.Contains("test-run")) + { // first element of the NUnitV3 test collection + type = XmlResultJargon.NUnitV3; + return true; + } + if (line.Contains("TouchUnitTestRun")) + { + type = XmlResultJargon.TouchUnit; + return true; + } + if (line.Contains("test-results")) + { // first element of the NUnitV3 test collection + type = XmlResultJargon.NUnitV2; + return true; + } + if (line.Contains("")) + { // first element of the xUnit test collection + type = XmlResultJargon.xUnit; + return true; } } + return false; } - static (string resultLine, bool failed) ParseNUnitV3Xml(StreamReader stream, StreamWriter writer) + static (string resultLine, bool failed) ParseNUnitV3Xml(StreamReader stream, StreamWriter? writer) { long testcasecount, passed, failed, inconclusive, skipped; bool failedTestRun = false; // result = "Failed" @@ -144,7 +144,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared return (resultLine, failedTestRun); } - static (string resultLine, bool failed) ParseTouchUnitXml(StreamReader stream, StreamWriter writer) + static (string resultLine, bool failed) ParseTouchUnitXml(StreamReader stream, StreamWriter? writer) { long total, errors, failed, notRun, inconclusive, ignored, skipped, invalid; total = errors = failed = notRun = inconclusive = ignored = skipped = invalid = 0L; @@ -178,7 +178,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared return (resultLine, total == 0 || errors != 0 || failed != 0); } - static (string resultLine, bool failed) ParseNUnitXml(StreamReader stream, StreamWriter writer) + static (string resultLine, bool failed) ParseNUnitXml(StreamReader stream, StreamWriter? writer) { long total, errors, failed, notRun, inconclusive, ignored, skipped, invalid; total = errors = failed = notRun = inconclusive = ignored = skipped = invalid = 0L; @@ -253,7 +253,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared return (resultLine, total == 0 | errors != 0 || failed != 0); } - static (string resultLine, bool failed) ParsexUnitXml(StreamReader stream, StreamWriter writer) + static (string resultLine, bool failed) ParsexUnitXml(StreamReader stream, StreamWriter? writer) { long total, errors, failed, notRun, inconclusive, ignored, skipped, invalid; total = errors = failed = notRun = inconclusive = ignored = skipped = invalid = 0L; @@ -343,7 +343,7 @@ namespace Microsoft.DotNet.XHarness.iOS.Shared using (var reader = new StreamReader(source)) using (var writer = new StreamWriter(destination)) { - string line; + string? line; while ((line = reader.ReadLine()) != null) { if (line.StartsWith("ping", StringComparison.Ordinal) || line.Contains("TouchUnitTestRun") || line.Contains("NUnitOutput") || line.Contains("