From 55a23664e1a17703d0878131204fa91c15dc62ec Mon Sep 17 00:00:00 2001
From: Jonathan Peppers <jonathan.peppers@microsoft.com>
Date: Fri, 22 Dec 2017 10:04:04 -0600
Subject: [PATCH] [Xamarin.Android.Build.Tasks] <Aapt /> task showing warnings
 as errors

Fixes: #1134
Context: 2135856e

Dean's changes in 2135856e were more accurate at picking out warnings
& errors, but if the `level` value from the `Regex` was blank, it was
counting the message as an error. The `level` value is matching against
the words `warning` or `error`, ignoring case.

I think the fix here is to count the message as a warning if the
`level` is blank.

I added a Regex test case of what was on #1134. I added another test case
that verifies a message with a blank `level` comes through the build output
as a `warning`, not giving an `APT0000` error.
---
 src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs     |  2 +-
 .../AndroidRegExTests.cs                          |  8 ++++++++
 .../AndroidUpdateResourcesTest.cs                 | 15 +++++++++++++++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs
index 0fc1f9ce810..282537c7fac 100644
--- a/src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs
+++ b/src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs
@@ -371,7 +371,7 @@ protected void LogEventsFromTextOutput (string singleLine, MessageImportance mes
 					line = int.Parse (match.Groups["line"].Value) + 1;
 				var level = match.Groups["level"].Value.ToLowerInvariant ();
 				var message = match.Groups ["message"].Value;
-				if (message.Contains ("fakeLogOpen") || level.Contains ("warning")) {
+				if (message.Contains ("fakeLogOpen") || level.Contains ("warning") || string.IsNullOrEmpty (level)) {
 					LogWarning (singleLine);
 					return;
 				}
diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidRegExTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidRegExTests.cs
index d47a3e0ccc0..60f270368b2 100644
--- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidRegExTests.cs
+++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidRegExTests.cs
@@ -74,6 +74,14 @@ public IEnumerator GetEnumerator ()
 					/*expectedLevel*/	"",
 					/*expectedMessage*/	"max res 10, skipping values-sw600dp-land"
 				};
+				yield return new object [] {
+					/*message*/		"max res 10, skipping values-sw720dp-land-v13",
+					/*expectedToMatch*/	true,
+					/*expectedFile*/	"",
+					/*expectedLine*/	"",
+					/*expectedLevel*/	"",
+					/*expectedMessage*/	"max res 10, skipping values-sw720dp-land-v13"
+				};
 				yield return new object [] {
 					/*message*/		"Error: unable to generate entry for resource data",
 					/*expectedToMatch*/	true,
diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs
index 0af1190c1be..11bb458e228 100644
--- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs
+++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs
@@ -183,6 +183,21 @@ public void ReportAaptErrorsInOriginalFileName ()
 			}
 		}
 
+		[Test]
+		public void ReportAaptWarningsForBlankLevel ()
+		{
+			//This test should get the warning `Invalid file name: must contain only [a-z0-9_.]`
+			//    However, <Aapt /> still fails due to aapt failing, Resource.designer.cs is not generated
+			var proj = new XamarinAndroidApplicationProject ();
+			proj.AndroidResources.Add (new AndroidItem.AndroidResource ("Resources\\drawable\\Image (1).png") { BinaryContent = () => XamarinAndroidCommonProject.icon_binary_mdpi });
+			using (var b = CreateApkBuilder ("temp/ReportAaptWarningsForBlankLevel")) {
+				b.ThrowOnBuildFailure = false;
+				Assert.IsFalse (b.Build (proj), "Build should have failed.");
+				StringAssertEx.DoesNotContain ("APT0000", b.LastBuildOutput, "An error message with a blank \"level\", should only report a warning!");
+				Assert.IsTrue (b.Clean (proj), "Clean should have succeeded.");
+			}
+		}
+
 		[Test]
 		public void RepetiviteBuildUpdateSingleResource ()
 		{