From fb4484e7857d7533ac6ea3aa9f0962f0e90bea24 Mon Sep 17 00:00:00 2001 From: Jackson Schuster <36744439+jtschuster@users.noreply.github.com> Date: Thu, 23 May 2024 13:51:08 -0700 Subject: [PATCH] ILLink: Check for all expected warnings, then fail if any are unmatched (#102286) In Linker tests, we fail at the first ExpectedWarning that isn't matched, and print all the messages that haven't been matched yet, even though they may match another ExpectedWarning that just hasn't been processed yet. Instead, we should keep a list of all expected warnings that were missing report them at once along with all unmatched warnings. This makes it much easier to debug when a test is failing. --- .../TestCasesRunner/ResultChecker.cs | 76 ++++++++++++------- .../TestCasesRunner/TrimmingTestLogger.cs | 8 +- 2 files changed, 53 insertions(+), 31 deletions(-) diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs index adee52bd4243d9..aff0d6ba1e6b64 100644 --- a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs +++ b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; @@ -762,8 +763,11 @@ static IEnumerable GetAttributeProviders (AssemblyDefi void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logger, bool checkRemainingErrors) { - List loggedMessages = logger.GetLoggedMessages (); + ImmutableArray allMessages = logger.GetLoggedMessages (); + List unmatchedMessages = [..allMessages]; List<(ICustomAttributeProvider, CustomAttribute)> expectedNoWarningsAttributes = new (); + List missingMessageWarnings = []; + List unexpectedMessageWarnings = []; foreach (var attrProvider in GetAttributeProviders (original)) { foreach (var attr in attrProvider.CustomAttributes) { if (!IsProducedByLinker (attr)) @@ -776,27 +780,27 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge List matchedMessages; if ((bool) attr.ConstructorArguments[1].Value) - matchedMessages = loggedMessages.Where (m => Regex.IsMatch (m.ToString (), expectedMessage)).ToList (); + matchedMessages = unmatchedMessages.Where (m => Regex.IsMatch (m.ToString (), expectedMessage)).ToList (); else - matchedMessages = loggedMessages.Where (m => m.ToString ().Contains (expectedMessage)).ToList (); ; - Assert.IsTrue ( - matchedMessages.Count > 0, - $"Expected to find logged message matching `{expectedMessage}`, but no such message was found.{Environment.NewLine}Logged messages:{Environment.NewLine}{string.Join (Environment.NewLine, loggedMessages)}"); + matchedMessages = unmatchedMessages.Where (m => m.ToString ().Contains (expectedMessage)).ToList (); ; + if (matchedMessages.Count == 0) + missingMessageWarnings.Add ($"Expected to find logged message matching `{expectedMessage}`, but no such message was found.{Environment.NewLine}"); foreach (var matchedMessage in matchedMessages) - loggedMessages.Remove (matchedMessage); + unmatchedMessages.Remove (matchedMessage); } break; case nameof (LogDoesNotContainAttribute): { var unexpectedMessage = (string) attr.ConstructorArguments[0].Value; - foreach (var loggedMessage in loggedMessages) { - Assert.That (() => { - if ((bool) attr.ConstructorArguments[1].Value) - return !Regex.IsMatch (loggedMessage.ToString (), unexpectedMessage); - return !loggedMessage.ToString ().Contains (unexpectedMessage); - }, - $"Expected to not find logged message matching `{unexpectedMessage}`, but found:{Environment.NewLine}{loggedMessage.ToString ()}{Environment.NewLine}Logged messages:{Environment.NewLine}{string.Join (Environment.NewLine, loggedMessages)}"); + foreach (var loggedMessage in unmatchedMessages) { + bool isRegex = (bool) attr.ConstructorArguments[1].Value; + bool foundMatch = isRegex + ? Regex.IsMatch (loggedMessage.ToString (), unexpectedMessage) + : loggedMessage.ToString ().Contains (unexpectedMessage); + + if (foundMatch) + unexpectedMessageWarnings.Add ($"Expected to not find logged message matching `{unexpectedMessage}`, but found:{Environment.NewLine}{loggedMessage.ToString ()}"); } } break; @@ -833,7 +837,7 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge string expectedOrigin = null; bool expectedWarningFound = false; - foreach (var loggedMessage in loggedMessages) { + foreach (var loggedMessage in unmatchedMessages) { if (loggedMessage.Category != MessageCategory.Warning || loggedMessage.Code != expectedWarningCodeNumber) continue; @@ -892,7 +896,7 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge actualName.EndsWith ("get_" + expectedMember.Name) || actualName.EndsWith ("set_" + expectedMember.Name))) { expectedWarningFound = true; - loggedMessages.Remove (loggedMessage); + unmatchedMessages.Remove (loggedMessage); break; } if (memberDefinition is not MethodDefinition) @@ -901,13 +905,13 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge if (memberDefinition.Name == ".cctor" && (expectedMember is FieldDefinition || expectedMember is PropertyDefinition)) { expectedWarningFound = true; - loggedMessages.Remove (loggedMessage); + unmatchedMessages.Remove (loggedMessage); break; } if (memberDefinition.Name == ".ctor" && (expectedMember is FieldDefinition || expectedMember is PropertyDefinition || memberDefinition.DeclaringType.FullName == expectedMember.FullName)) { expectedWarningFound = true; - loggedMessages.Remove (loggedMessage); + unmatchedMessages.Remove (loggedMessage); break; } } @@ -917,7 +921,7 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge memberDefinition.DeclaringType.FullName == "Program" && memberDefinition.DeclaringType.Module.Assembly.Name.Name == expectedAssembly.Name.Name) { expectedWarningFound = true; - loggedMessages.Remove (loggedMessage); + unmatchedMessages.Remove (loggedMessage); break; } } @@ -925,14 +929,14 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge } else { if (LogMessageHasSameOriginMember (loggedMessage, attrProvider)) { expectedWarningFound = true; - loggedMessages.Remove (loggedMessage); + unmatchedMessages.Remove (loggedMessage); break; } continue; } expectedWarningFound = true; - loggedMessages.Remove (loggedMessage); + unmatchedMessages.Remove (loggedMessage); break; } @@ -945,11 +949,11 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge } + ": " : ""; - Assert.IsTrue (expectedWarningFound, - $"Expected to find warning: {(fileName != null ? fileName + (sourceLine != null ? $"({sourceLine},{sourceColumn})" : "") + ": " : "")}" + + if (!expectedWarningFound) + missingMessageWarnings.Add ($"Expected to find warning: {(fileName != null ? fileName + (sourceLine != null ? $"({sourceLine},{sourceColumn})" : "") + ": " : "")}" + $"warning {expectedWarningCode}: {expectedOriginString}" + $"and message containing {string.Join (" ", expectedMessageContains.Select (m => "'" + m + "'"))}, " + - $"but no such message was found.{Environment.NewLine}Logged messages:{Environment.NewLine}{string.Join (Environment.NewLine, loggedMessages)}"); + $"but no such message was found."); } break; @@ -972,7 +976,7 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge int? unexpectedWarningCodeNumber = unexpectedWarningCode == null ? null : int.Parse (unexpectedWarningCode.Substring (2)); MessageContainer? unexpectedWarningMessage = null; - foreach (var mc in logger.GetLoggedMessages ()) { + foreach (var mc in unmatchedMessages) { if (mc.Category != MessageCategory.Warning) continue; @@ -987,12 +991,28 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge break; } - Assert.IsNull (unexpectedWarningMessage, - $"Unexpected warning found: {unexpectedWarningMessage}"); + if (unexpectedWarningMessage is not null) + { + unexpectedMessageWarnings.Add($"Unexpected warning found: {unexpectedWarningMessage}"); + } + } + + if (missingMessageWarnings.Any ()) + { + missingMessageWarnings.Add ("Unmatched Messages:" + Environment.NewLine); + missingMessageWarnings.AddRange (unmatchedMessages.Select (m => m.ToString ())); + missingMessageWarnings.Add (Environment.NewLine + "All Messages:" + Environment.NewLine); + missingMessageWarnings.AddRange (allMessages.Select (m => m.ToString ())); + Assert.Fail (string.Join (Environment.NewLine, missingMessageWarnings)); + } + + if (unexpectedMessageWarnings.Any()) + { + Assert.Fail (string.Join (Environment.NewLine, unexpectedMessageWarnings)); } if (checkRemainingErrors) { - var remainingErrors = loggedMessages.Where (m => Regex.IsMatch (m.ToString (), @".*(error | warning): \d{4}.*")); + var remainingErrors = unmatchedMessages.Where (m => Regex.IsMatch (m.ToString (), @".*(error | warning): \d{4}.*")); Assert.IsEmpty (remainingErrors, $"Found unexpected errors:{Environment.NewLine}{string.Join (Environment.NewLine, remainingErrors)}"); } diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TrimmingTestLogger.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TrimmingTestLogger.cs index f3b3213848e13b..fabf6ac59dca7b 100644 --- a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TrimmingTestLogger.cs +++ b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TrimmingTestLogger.cs @@ -1,7 +1,9 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.Collections.Generic; +using System.Collections.Immutable; namespace Mono.Linker.Tests.TestCasesRunner { @@ -14,9 +16,9 @@ public TrimmingTestLogger () MessageContainers = new List (); } - public List GetLoggedMessages () + public ImmutableArray GetLoggedMessages () { - return MessageContainers; + return MessageContainers.ToImmutableArray(); } public void LogMessage (MessageContainer message) @@ -29,4 +31,4 @@ public void LogMessage (MessageContainer message) MessageContainers.Add (message); } } -} \ No newline at end of file +}