Improve test failure messages (#89967)

mrvoorhe · web-flow · commit 4415472b81ad · 2023-08-07T09:42:42.000+02:00
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Assertions/ExpectNonZeroExitCodeAttribute.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases.Expectations/Assertions/ExpectNonZeroExitCodeAttribute.cs
@@ -0,0 +1,14 @@
+﻿// 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;
+
+namespace Mono.Linker.Tests.Cases.Expectations.Assertions;
+
+[AttributeUsage (AttributeTargets.Class)]
+public class ExpectNonZeroExitCodeAttribute : BaseExpectedLinkedBehaviorAttribute
+{
+	public ExpectNonZeroExitCodeAttribute (int value)
+	{
+	}
+}
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/InvalidArguments.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/CommandLine/InvalidArguments.cs
@@ -3,6 +3,7 @@
 
 namespace Mono.Linker.Tests.Cases.CommandLine
 {
+	[ExpectNonZeroExitCode (1)]
 	[SetupLinkerArgument ("--verbose", "--invalidArgument")]
 	[LogContains ("Unrecognized command-line option")]
 	[NoLinkedOutput]
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/FeatureSettings/FeatureSubstitutionsInvalid.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/FeatureSettings/FeatureSubstitutionsInvalid.cs
@@ -3,6 +3,7 @@
 
 namespace Mono.Linker.Tests.Cases.FeatureSettings
 {
+	[ExpectNonZeroExitCode (1)]
 	[SetupLinkerSubstitutionFile ("FeatureSubstitutionsInvalid.xml")]
 	[SetupLinkerArgument ("--feature", "NoValueFeature", "true")]
 	[LogContains ("FeatureSubstitutionsInvalid.xml'. Feature 'NoValueFeature' does not specify a 'featurevalue' attribute")]
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkAttributes/TypedArgumentsErrors.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/LinkAttributes/TypedArgumentsErrors.cs
@@ -4,6 +4,7 @@
 
 namespace Mono.Linker.Tests.Cases.LinkAttributes
 {
+	[ExpectNonZeroExitCode (1)]
 	[SetupLinkAttributesFile ("TypedArgumentsErrors.xml")]
 	[IgnoreLinkAttributes (false)]
 	[NoLinkedOutput]
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Logging/CommonLogs.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Logging/CommonLogs.cs
@@ -3,6 +3,7 @@
 
 namespace Mono.Linker.Tests.Cases.Logging
 {
+	[ExpectNonZeroExitCode (1)]
 	[SetupCompileBefore ("LogStep.dll", new[] { "Dependencies/LogStep.cs" }, new[] { "illink.dll", "Mono.Cecil.dll" })]
 	[SetupLinkerArgument ("--custom-step", "Log.LogStep,LogStep.dll")]
 	[SetupLinkerArgument ("--verbose")]
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Warnings/CanDisableWarnAsError.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Warnings/CanDisableWarnAsError.cs
@@ -3,6 +3,7 @@
 
 namespace Mono.Linker.Tests.Cases.Warnings
 {
+	[ExpectNonZeroExitCode (1)]
 	[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]
 	[SkipKeptItemsValidation]
 	[SkipRemainingErrorsValidation]
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Warnings/CanSingleWarnWithWarnAsError.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Warnings/CanSingleWarnWithWarnAsError.cs
@@ -5,6 +5,7 @@
 
 namespace Mono.Linker.Tests.Cases.Warnings
 {
+	[ExpectNonZeroExitCode (1)]
 	[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]
 	[SkipKeptItemsValidation]
 	[SetupCompileBefore ("library.dll", new[] { typeof (TriggerWarnings_Lib) })]
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Warnings/CanWarnAsError.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Warnings/CanWarnAsError.cs
@@ -3,6 +3,7 @@
 
 namespace Mono.Linker.Tests.Cases.Warnings
 {
+	[ExpectNonZeroExitCode (1)]
 	[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]
 	[SkipKeptItemsValidation]
 	[SetupLinkerSubstitutionFile ("CanWarnAsErrorSubstitutions.xml")]
diff --git a/src/tools/illink/test/Mono.Linker.Tests.Cases/Warnings/InvalidWarningVersion.cs b/src/tools/illink/test/Mono.Linker.Tests.Cases/Warnings/InvalidWarningVersion.cs
@@ -3,6 +3,7 @@
 
 namespace Mono.Linker.Tests.Cases.Warnings
 {
+	[ExpectNonZeroExitCode (1)]
 	[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]
 	[SetupLinkerArgument ("--verbose")]
 	[SetupLinkerArgument ("--warn", "invalid")]
diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/LinkedTestCaseResult.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/LinkedTestCaseResult.cs
@@ -17,8 +17,9 @@ public class LinkedTestCaseResult
 		public readonly ManagedCompilationResult CompilationResult;
 		public readonly LinkerTestLogger Logger;
 		public readonly LinkerCustomizations Customizations;
+		public readonly int ExitCode;
 
-		public LinkedTestCaseResult (TestCase testCase, NPath inputAssemblyPath, NPath outputAssemblyPath, NPath expectationsAssemblyPath, TestCaseSandbox sandbox, TestCaseMetadataProvider metadataProvider, ManagedCompilationResult compilationResult, LinkerTestLogger logger, LinkerCustomizations customizations)
+		public LinkedTestCaseResult (TestCase testCase, NPath inputAssemblyPath, NPath outputAssemblyPath, NPath expectationsAssemblyPath, TestCaseSandbox sandbox, TestCaseMetadataProvider metadataProvider, ManagedCompilationResult compilationResult, LinkerTestLogger logger, LinkerCustomizations customizations, int exitCode)
 		{
 			TestCase = testCase;
 			InputAssemblyPath = inputAssemblyPath;
@@ -29,6 +30,7 @@ public LinkedTestCaseResult (TestCase testCase, NPath inputAssemblyPath, NPath o
 			CompilationResult = compilationResult;
 			Logger = logger;
 			Customizations = customizations;
+			ExitCode = exitCode;
 		}
 	}
 }
diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/ResultChecker.cs
@@ -7,6 +7,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Linq;
+using System.Text;
 using System.Text.RegularExpressions;
 using Mono.Cecil;
 using Mono.Cecil.Cil;
@@ -110,8 +111,12 @@ public virtual void Check (LinkedTestCaseResult linkResult)
 
 			try {
 				var original = ResolveOriginalsAssembly (linkResult.ExpectationsAssemblyPath.FileNameWithoutExtension);
+
+				VerifyExitCode (linkResult, original);
+
 				if (!HasAttribute (original, nameof (NoLinkedOutputAttribute))) {
 					Assert.IsTrue (linkResult.OutputAssemblyPath.FileExists (), $"The linked output assembly was not found.  Expected at {linkResult.OutputAssemblyPath}");
+
 					var linked = ResolveLinkedAssembly (linkResult.OutputAssemblyPath.FileNameWithoutExtension);
 
 					if (ShouldValidateIL (original)) {
@@ -239,6 +244,26 @@ void PerformOutputSymbolChecks (AssemblyDefinition original, NPath outputDirecto
 			}
 		}
 
+		void VerifyExitCode (LinkedTestCaseResult linkResult, AssemblyDefinition original)
+		{
+			if (TryGetCustomAttribute (original, nameof(ExpectNonZeroExitCodeAttribute), out var attr)) {
+				var expectedExitCode = (int) attr.ConstructorArguments[0].Value;
+				Assert.AreEqual (expectedExitCode, linkResult.ExitCode, $"Expected exit code {expectedExitCode} but got {linkResult.ExitCode}.  Output was:\n{FormatLinkerOutput()}");
+			} else {
+				if (linkResult.ExitCode != 0) {
+					Assert.Fail($"Linker exited with an unexpected non-zero exit code of {linkResult.ExitCode} and output:\n{FormatLinkerOutput()}");
+				}
+			}
+
+			string FormatLinkerOutput ()
+			{
+				var sb = new StringBuilder ();
+				foreach (var message in linkResult.Logger.GetLoggedMessages ())
+					sb.AppendLine (message.ToString ());
+				return sb.ToString ();
+			}
+		}
+
 		void VerifyKeptSymbols (CustomAttribute symbolsAttribute)
 		{
 			var assemblyName = (string) symbolsAttribute.ConstructorArguments[0].Value;
diff --git a/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TestRunner.cs b/src/tools/illink/test/Mono.Linker.Tests/TestCasesRunner/TestRunner.cs
@@ -109,9 +109,9 @@ private LinkedTestCaseResult Link (TestCase testCase, TestCaseSandbox sandbox, M
 			AddLinkOptions (sandbox, compilationResult, builder, metadataProvider);
 
 			LinkerTestLogger logger = new LinkerTestLogger ();
-			linker.Link (builder.ToArgs (), linkerCustomizations, logger);
+			var exitCode = linker.Link (builder.ToArgs (), linkerCustomizations, logger);
 
-			return new LinkedTestCaseResult (testCase, compilationResult.InputAssemblyPath, sandbox.OutputDirectory.Combine (compilationResult.InputAssemblyPath.FileName), compilationResult.ExpectationsAssemblyPath, sandbox, metadataProvider, compilationResult, logger, linkerCustomizations);
+			return new LinkedTestCaseResult (testCase, compilationResult.InputAssemblyPath, sandbox.OutputDirectory.Combine (compilationResult.InputAssemblyPath.FileName), compilationResult.ExpectationsAssemblyPath, sandbox, metadataProvider, compilationResult, logger, linkerCustomizations, exitCode);
 		}
 
 		protected virtual void AddLinkOptions (TestCaseSandbox sandbox, ManagedCompilationResult compilationResult, LinkerArgumentBuilder builder, TestCaseMetadataProvider metadataProvider)

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`
`4`	`4`	`namespace Mono.Linker.Tests.Cases.CommandLine`
`5`	`5`	`{`
	`6`	`+ [ExpectNonZeroExitCode (1)]`
`6`	`7`	`[SetupLinkerArgument ("--verbose", "--invalidArgument")]`
`7`	`8`	`[LogContains ("Unrecognized command-line option")]`
`8`	`9`	`[NoLinkedOutput]`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`
`4`	`4`	`namespace Mono.Linker.Tests.Cases.FeatureSettings`
`5`	`5`	`{`
	`6`	`+ [ExpectNonZeroExitCode (1)]`
`6`	`7`	`[SetupLinkerSubstitutionFile ("FeatureSubstitutionsInvalid.xml")]`
`7`	`8`	`[SetupLinkerArgument ("--feature", "NoValueFeature", "true")]`
`8`	`9`	`[LogContains ("FeatureSubstitutionsInvalid.xml'. Feature 'NoValueFeature' does not specify a 'featurevalue' attribute")]`
Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@`
`4`	`4`
`5`	`5`	`namespace Mono.Linker.Tests.Cases.LinkAttributes`
`6`	`6`	`{`
	`7`	`+ [ExpectNonZeroExitCode (1)]`
`7`	`8`	`[SetupLinkAttributesFile ("TypedArgumentsErrors.xml")]`
`8`	`9`	`[IgnoreLinkAttributes (false)]`
`9`	`10`	`[NoLinkedOutput]`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`
`4`	`4`	`namespace Mono.Linker.Tests.Cases.Logging`
`5`	`5`	`{`
	`6`	`+ [ExpectNonZeroExitCode (1)]`
`6`	`7`	`[SetupCompileBefore ("LogStep.dll", new[] { "Dependencies/LogStep.cs" }, new[] { "illink.dll", "Mono.Cecil.dll" })]`
`7`	`8`	`[SetupLinkerArgument ("--custom-step", "Log.LogStep,LogStep.dll")]`
`8`	`9`	`[SetupLinkerArgument ("--verbose")]`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`
`4`	`4`	`namespace Mono.Linker.Tests.Cases.Warnings`
`5`	`5`	`{`
	`6`	`+ [ExpectNonZeroExitCode (1)]`
`6`	`7`	`[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]`
`7`	`8`	`[SkipKeptItemsValidation]`
`8`	`9`	`[SkipRemainingErrorsValidation]`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@`
`5`	`5`
`6`	`6`	`namespace Mono.Linker.Tests.Cases.Warnings`
`7`	`7`	`{`
	`8`	`+ [ExpectNonZeroExitCode (1)]`
`8`	`9`	`[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]`
`9`	`10`	`[SkipKeptItemsValidation]`
`10`	`11`	`[SetupCompileBefore ("library.dll", new[] { typeof (TriggerWarnings_Lib) })]`
Original file line number	Diff line number	Diff line change
`@@ -17,8 +17,9 @@ public class LinkedTestCaseResult`
`17`	`17`	`public readonly ManagedCompilationResult CompilationResult;`
`18`	`18`	`public readonly LinkerTestLogger Logger;`
`19`	`19`	`public readonly LinkerCustomizations Customizations;`
	`20`	`+ public readonly int ExitCode;`
`20`	`21`
`21`		`- public LinkedTestCaseResult (TestCase testCase, NPath inputAssemblyPath, NPath outputAssemblyPath, NPath expectationsAssemblyPath, TestCaseSandbox sandbox, TestCaseMetadataProvider metadataProvider, ManagedCompilationResult compilationResult, LinkerTestLogger logger, LinkerCustomizations customizations)`
	`22`	`+ public LinkedTestCaseResult (TestCase testCase, NPath inputAssemblyPath, NPath outputAssemblyPath, NPath expectationsAssemblyPath, TestCaseSandbox sandbox, TestCaseMetadataProvider metadataProvider, ManagedCompilationResult compilationResult, LinkerTestLogger logger, LinkerCustomizations customizations, int exitCode)`
`22`	`23`	`{`
`23`	`24`	`TestCase = testCase;`
`24`	`25`	`InputAssemblyPath = inputAssemblyPath;`
`@@ -29,6 +30,7 @@ public LinkedTestCaseResult (TestCase testCase, NPath inputAssemblyPath, NPath o`
`29`	`30`	`CompilationResult = compilationResult;`
`30`	`31`	`Logger = logger;`
`31`	`32`	`Customizations = customizations;`
	`33`	`+ ExitCode = exitCode;`
`32`	`34`	`}`
`33`	`35`	`}`
`34`	`36`	`}`