From e713c33968b7eacf6323bfe1d504163e8216c9bb Mon Sep 17 00:00:00 2001
From: Peter Collins <pecolli@microsoft.com>
Date: Thu, 4 Apr 2024 11:14:14 -0700
Subject: [PATCH 1/6] [AndroidToolTask] Log tool output as error

Context: https://github.com/xamarin/xamarin-android-tools/issues/208

Updates AndroidToolTask to capture all standard output from the tool and
log it as an error if the task fails.
---
 .../AndroidToolTask.cs                        | 18 +++++-
 .../AndroidToolTaskTests.cs                   | 58 ++++++++++++++++---
 ...osoft.Android.Build.BaseTasks-Tests.csproj |  1 +
 3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
index cc01e8f5..c637c7cc 100644
--- a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
+++ b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
@@ -2,6 +2,8 @@
 
 using System;
 using System.IO;
+using System.Text;
+using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
 
 namespace Microsoft.Android.Build.Tasks
@@ -12,21 +14,35 @@ public abstract class AndroidToolTask : ToolTask
 
 		protected string WorkingDirectory { get; private set; }
 
+		StringBuilder toolOutput;
+
 		public AndroidToolTask ()
 		{
 			WorkingDirectory = Directory.GetCurrentDirectory();
+			toolOutput = new StringBuilder ();
 		}
 
 		public override bool Execute ()
 		{
 			try {
-				return RunTask ();
+				bool taskResult = RunTask ();
+				if (!taskResult && !string.IsNullOrEmpty (toolOutput?.ToString ())) {
+					Log.LogError (toolOutput.ToString ().Trim ());
+				}
+				toolOutput.Clear ();
+				return taskResult;
 			} catch (Exception ex) {
 				Log.LogUnhandledException (TaskPrefix, ex);
 				return false;
 			}
 		}
 
+		protected override void LogEventsFromTextOutput(string singleLine, MessageImportance messageImportance)
+		{
+			base.LogEventsFromTextOutput (singleLine, messageImportance);
+			toolOutput.AppendLine (singleLine);
+		}
+
 		// Most ToolTask's do not override Execute and
 		// just expect the base to be called
 		public virtual bool RunTask () => base.Execute ();
diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/AndroidToolTaskTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/AndroidToolTaskTests.cs
index 8b35ce5b..3d8762ba 100644
--- a/tests/Microsoft.Android.Build.BaseTasks-Tests/AndroidToolTaskTests.cs
+++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/AndroidToolTaskTests.cs
@@ -1,18 +1,24 @@
 using System.IO;
 using System.Collections.Generic;
-using System.Threading.Tasks;
 using Microsoft.Android.Build.BaseTasks.Tests.Utilities;
 using Microsoft.Android.Build.Tasks;
 using NUnit.Framework;
 using Microsoft.Build.Framework;
-using Xamarin.Build;
+using NUnit.Framework.Internal;
+using System.Linq;
 
 namespace Microsoft.Android.Build.BaseTasks.Tests
 {
 	[TestFixture]
 	public class AndroidToolTaskTests
 	{
-		public class MyAndroidTask : AndroidTask {
+		List<BuildErrorEventArgs> errors;
+		List<BuildWarningEventArgs> warnings;
+		List<BuildMessageEventArgs> messages;
+		MockBuildEngine engine;
+
+		public class MyAndroidTask : AndroidTask
+		{
 			public override string TaskPrefix {get;} = "MAT";
 			public string Key { get; set; }
 			public string Value { get; set; }
@@ -25,7 +31,8 @@ public override bool RunTask ()
 			}
 		}
 
-		public class MyOtherAndroidTask : AndroidTask {
+		public class MyOtherAndroidTask : AndroidTask
+		{
 			public override string TaskPrefix {get;} = "MOAT";
 			public string Key { get; set; }
 			public bool ProjectSpecific { get; set; } = false;
@@ -40,6 +47,24 @@ public override bool RunTask ()
 			}
 		}
 
+		public class DotnetToolOutputTestTask : AndroidToolTask
+		{
+			public override string TaskPrefix {get;} = "DTOT";
+			protected override string ToolName => "dotnet";
+			protected override string GenerateFullPathToTool () => ToolExe;
+			public string CommandLineArgs { get; set; } = "--info";
+			protected override string GenerateCommandLineCommands () => CommandLineArgs;
+		}
+
+		[SetUp]
+		public void TestSetup()
+		{
+			errors = new List<BuildErrorEventArgs> ();
+			warnings = new List<BuildWarningEventArgs> ();
+			messages = new List<BuildMessageEventArgs> ();
+			engine = new MockBuildEngine (TestContext.Out, errors, warnings, messages);
+		}
+
 		[Test]
 		[TestCase (true, true, true)]
 		[TestCase (false, false, true)]
@@ -47,8 +72,6 @@ public override bool RunTask ()
 		[TestCase (false, true, false)]
 		public void TestRegisterTaskObjectCanRetrieveCorrectItem (bool projectSpecificA, bool projectSpecificB, bool expectedResult)
 		{
-			var engine = new MockBuildEngine (TestContext.Out) {
-			};
 			var task = new MyAndroidTask () {
 				BuildEngine = engine,
 				Key = "Foo",
@@ -72,8 +95,6 @@ public void TestRegisterTaskObjectCanRetrieveCorrectItem (bool projectSpecificA,
 		[TestCase (false, true, false)]
 		public void TestRegisterTaskObjectFailsWhenDirectoryChanges (bool projectSpecificA, bool projectSpecificB, bool expectedResult)
 		{
-			var engine = new MockBuildEngine (TestContext.Out) {
-			};
 			MyAndroidTask task;
 			var currentDir = Directory.GetCurrentDirectory ();
 			Directory.SetCurrentDirectory (Path.Combine (currentDir, ".."));
@@ -96,5 +117,26 @@ public void TestRegisterTaskObjectFailsWhenDirectoryChanges (bool projectSpecifi
 			task2.Execute ();
 			Assert.AreEqual (expectedResult, string.Compare (task2.Value, task.Value, ignoreCase: true) == 0);
 		}
+
+		[Test]
+		[TestCase ("invalidcommand", false, "You intended to execute a .NET program, but dotnet-invalidcommand does not exist.")]
+		[TestCase ("--info", true, "")]
+		public void FailedAndroidToolTaskShouldLogOutputAsError (string args, bool expectedResult, string expectedErrorText)
+		{
+			var task = new DotnetToolOutputTestTask () {
+				BuildEngine = engine,
+				CommandLineArgs = args,
+			};
+			var taskSucceeded = task.Execute ();
+			Assert.AreEqual (expectedResult, taskSucceeded, "Task execution did not return expected value.");
+
+			if (taskSucceeded) {
+				Assert.IsEmpty (errors, "Successful task should not have any errors.");
+			} else {
+				Assert.IsNotEmpty (errors, "Task expected to fail should have errors.");
+				Assert.IsTrue (errors.Any (e => e.Message.Contains (expectedErrorText)),
+					"Task expected to fail should contain expected error text.");
+			}
+		}
 	}
 }
diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/Microsoft.Android.Build.BaseTasks-Tests.csproj b/tests/Microsoft.Android.Build.BaseTasks-Tests/Microsoft.Android.Build.BaseTasks-Tests.csproj
index 52207653..7f0fb6d7 100644
--- a/tests/Microsoft.Android.Build.BaseTasks-Tests/Microsoft.Android.Build.BaseTasks-Tests.csproj
+++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/Microsoft.Android.Build.BaseTasks-Tests.csproj
@@ -9,6 +9,7 @@
     <IsPackable>false</IsPackable>
     <OutputPath>$(TestOutputFullPath)</OutputPath>
     <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
+    <RollForward>Major</RollForward>
   </PropertyGroup>
 
   <Import Project="..\..\src\Microsoft.Android.Build.BaseTasks\MSBuildReferences.projitems" />

From 6589d979dbfb95ea79c056a93d2eac689acd4df5 Mon Sep 17 00:00:00 2001
From: Peter Collins <pecolli@microsoft.com>
Date: Mon, 8 Apr 2024 13:43:41 -0700
Subject: [PATCH 2/6] Apply feedback

---
 src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs     | 2 +-
 .../UnhandledExceptionLogger.cs                              | 5 +++++
 .../AndroidToolTaskTests.cs                                  | 4 ++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
index c637c7cc..da167997 100644
--- a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
+++ b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
@@ -27,7 +27,7 @@ public override bool Execute ()
 			try {
 				bool taskResult = RunTask ();
 				if (!taskResult && !string.IsNullOrEmpty (toolOutput?.ToString ())) {
-					Log.LogError (toolOutput.ToString ().Trim ());
+					Log.LogUnhandledToolError (TaskPrefix, toolOutput.ToString ().Trim ());
 				}
 				toolOutput.Clear ();
 				return taskResult;
diff --git a/src/Microsoft.Android.Build.BaseTasks/UnhandledExceptionLogger.cs b/src/Microsoft.Android.Build.BaseTasks/UnhandledExceptionLogger.cs
index 07a3fc3d..67d20868 100644
--- a/src/Microsoft.Android.Build.BaseTasks/UnhandledExceptionLogger.cs
+++ b/src/Microsoft.Android.Build.BaseTasks/UnhandledExceptionLogger.cs
@@ -85,5 +85,10 @@ static void LogUnhandledException (Action<string,string> logCodedError, string p
 			else
 				logCodedError (prefix + "7000", ex.ToString ());
 		}
+
+		public static void LogUnhandledToolError (this TaskLoggingHelper log, string prefix, string toolOutput)
+		{
+			log.LogCodedError ($"XA{prefix}0000", toolOutput);
+		}
 	}
 }
diff --git a/tests/Microsoft.Android.Build.BaseTasks-Tests/AndroidToolTaskTests.cs b/tests/Microsoft.Android.Build.BaseTasks-Tests/AndroidToolTaskTests.cs
index 3d8762ba..834d3986 100644
--- a/tests/Microsoft.Android.Build.BaseTasks-Tests/AndroidToolTaskTests.cs
+++ b/tests/Microsoft.Android.Build.BaseTasks-Tests/AndroidToolTaskTests.cs
@@ -134,6 +134,10 @@ public void FailedAndroidToolTaskShouldLogOutputAsError (string args, bool expec
 				Assert.IsEmpty (errors, "Successful task should not have any errors.");
 			} else {
 				Assert.IsNotEmpty (errors, "Task expected to fail should have errors.");
+				Assert.AreEqual ("MSB6006", errors [0].Code,
+					$"Expected error code MSB6006 but got {errors [0].Code}");
+				Assert.AreEqual ("XADTOT0000", errors [1].Code,
+					$"Expected error code XADTOT0000 but got {errors [1].Code}");
 				Assert.IsTrue (errors.Any (e => e.Message.Contains (expectedErrorText)),
 					"Task expected to fail should contain expected error text.");
 			}

From 5a59b1bab7bcf54f7be5b53e3227b55e895c2646 Mon Sep 17 00:00:00 2001
From: Peter Collins <pecolli@microsoft.com>
Date: Mon, 8 Apr 2024 13:46:26 -0700
Subject: [PATCH 3/6] Formatting

---
 src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
index da167997..29281035 100644
--- a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
+++ b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
@@ -37,7 +37,7 @@ public override bool Execute ()
 			}
 		}
 
-		protected override void LogEventsFromTextOutput(string singleLine, MessageImportance messageImportance)
+		protected override void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance)
 		{
 			base.LogEventsFromTextOutput (singleLine, messageImportance);
 			toolOutput.AppendLine (singleLine);

From 3febc0b3a8cf33b93627d6aef6f1106c27a7d431 Mon Sep 17 00:00:00 2001
From: Peter Collins <pecolli@microsoft.com>
Date: Wed, 10 Apr 2024 14:11:40 -0700
Subject: [PATCH 4/6] Nullable fix

---
 src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
index 29281035..c7a21989 100644
--- a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
+++ b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
@@ -29,7 +29,7 @@ public override bool Execute ()
 				if (!taskResult && !string.IsNullOrEmpty (toolOutput?.ToString ())) {
 					Log.LogUnhandledToolError (TaskPrefix, toolOutput.ToString ().Trim ());
 				}
-				toolOutput.Clear ();
+				toolOutput?.Clear ();
 				return taskResult;
 			} catch (Exception ex) {
 				Log.LogUnhandledException (TaskPrefix, ex);

From 7388582f84826e08400e25f0a2aabffbf2f9db3f Mon Sep 17 00:00:00 2001
From: Peter Collins <pecolli@microsoft.com>
Date: Wed, 10 Apr 2024 15:09:14 -0700
Subject: [PATCH 5/6] Nullable cleanup

---
 src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
index c7a21989..008eeb0c 100644
--- a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
+++ b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
@@ -26,10 +26,10 @@ public override bool Execute ()
 		{
 			try {
 				bool taskResult = RunTask ();
-				if (!taskResult && !string.IsNullOrEmpty (toolOutput?.ToString ())) {
+				if (!taskResult && !string.IsNullOrEmpty (toolOutput.ToString ())) {
 					Log.LogUnhandledToolError (TaskPrefix, toolOutput.ToString ().Trim ());
 				}
-				toolOutput?.Clear ();
+				toolOutput.Clear ();
 				return taskResult;
 			} catch (Exception ex) {
 				Log.LogUnhandledException (TaskPrefix, ex);

From 3fb1fcdaf4dcb39d285869ee67de097277df18ca Mon Sep 17 00:00:00 2001
From: Peter Collins <pecolli@microsoft.com>
Date: Thu, 18 Apr 2024 13:17:10 -0700
Subject: [PATCH 6/6] Apply feedback

---
 src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
index 008eeb0c..5e9cfe83 100644
--- a/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
+++ b/src/Microsoft.Android.Build.BaseTasks/AndroidToolTask.cs
@@ -14,12 +14,11 @@ public abstract class AndroidToolTask : ToolTask
 
 		protected string WorkingDirectory { get; private set; }
 
-		StringBuilder toolOutput;
+		StringBuilder toolOutput = new StringBuilder ();
 
 		public AndroidToolTask ()
 		{
 			WorkingDirectory = Directory.GetCurrentDirectory();
-			toolOutput = new StringBuilder ();
 		}
 
 		public override bool Execute ()
