Skip to content

Commit

Permalink
ILLink: Check for all expected warnings, then fail if any are unmatch…
Browse files Browse the repository at this point in the history 
…ed (dotnet#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.
  • Loading branch information
@jtschuster
jtschuster committed May 23, 2024
1 parent 7fdc74e commit 80414e9
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 31 deletions.
76 changes: 48 additions & 28 deletions src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
Expand Down Expand Up @@ -762,8 +763,11 @@ static IEnumerable<ICustomAttributeProvider> GetAttributeProviders (AssemblyDefi

void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logger, bool checkRemainingErrors)
{
List<MessageContainer> loggedMessages = logger.GetLoggedMessages ();
ImmutableArray<MessageContainer> allMessages = logger.GetLoggedMessages ();
List<MessageContainer> unmatchedMessages = [..allMessages];
List<(ICustomAttributeProvider, CustomAttribute)> expectedNoWarningsAttributes = new ();
List<string> missingMessageWarnings = [];
List<string> unexpectedMessageWarnings = [];
foreach (var attrProvider in GetAttributeProviders (original)) {
foreach (var attr in attrProvider.CustomAttributes) {
if (!IsProducedByLinker (attr))
Expand All @@ -776,27 +780,27 @@ void VerifyLoggedMessages (AssemblyDefinition original, TrimmingTestLogger logge

List<MessageContainer> 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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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;
}
}
Expand All @@ -917,22 +921,22 @@ 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;
}
}
continue;
} else {
if (LogMessageHasSameOriginMember (loggedMessage, attrProvider)) {
expectedWarningFound = true;
loggedMessages.Remove (loggedMessage);
unmatchedMessages.Remove (loggedMessage);
break;
}
continue;
}

expectedWarningFound = true;
loggedMessages.Remove (loggedMessage);
unmatchedMessages.Remove (loggedMessage);
break;
}

Expand All @@ -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;

Expand All @@ -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;

Expand All @@ -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)}");
}

Expand Down
8 changes: 5 additions & 3 deletions src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TrimmingTestLogger.cs
View file
Original file line number Diff line number Diff line change
@@ -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
{
Expand All @@ -14,9 +16,9 @@ public TrimmingTestLogger ()
MessageContainers = new List<MessageContainer> ();
}

public List<MessageContainer> GetLoggedMessages ()
public ImmutableArray<MessageContainer> GetLoggedMessages ()
{
return MessageContainers;
return MessageContainers.ToImmutableArray();
}

public void LogMessage (MessageContainer message)
Expand All @@ -29,4 +31,4 @@ public void LogMessage (MessageContainer message)
MessageContainers.Add (message);
}
}
}
}

0 comments on commit 80414e9

Please sign in to comment.