[AndroidToolTask] Log tool output as error (#229)

Context: #208

Updates AndroidToolTask to capture all standard output from the tool and
log it as an error if the task fails.

Author: pjcollins

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,6 +14,8 @@ public abstract class AndroidToolTask : ToolTask
 
 		protected string WorkingDirectory { get; private set; }
 
+		StringBuilder toolOutput = new StringBuilder ();
+
 		public AndroidToolTask ()
 		{
 			WorkingDirectory = Directory.GetCurrentDirectory();
@@ -20,13 +24,24 @@ public AndroidToolTask ()
 		public override bool Execute ()
 		{
 			try {
-				return RunTask ();
+				bool taskResult = RunTask ();
+				if (!taskResult && !string.IsNullOrEmpty (toolOutput.ToString ())) {
+					Log.LogUnhandledToolError (TaskPrefix, 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 ();

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);
+		}
 	}
 }

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,15 +47,31 @@ 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)]
 		[TestCase (true, false, false)]
 		[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,30 @@ 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.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.");
+			}
+		}
 	}
 }

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" />