From 44c88a84542108200dc532df16766f24876eac4a Mon Sep 17 00:00:00 2001
From: Jonathan Pryor <jonpryor@vt.edu>
Date: Tue, 24 Apr 2018 14:57:45 -0400
Subject: [PATCH] [tests] Report errors last in `RunApkTests` (Take 2!)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Context: https://jenkins.mono-project.com/job/xamarin-android-pr-builder/2963/testReport/

A build for PR #1489 ran 1110 tests; it *should* have run over 35,000.
Where are the missing tests?

What's missing are the `.apk` on-device tests; BCL tests make up
*most* of our expected test count.

Analysis of the build log shows that the on-device tests *are*
running, with test results being downloaded:

	…/android-toolchain/sdk/platform-tools/adb -s emulator-5570  pull "/data/data/Xamarin.Android.Bcl_Tests/files/.__override__/TestResults.xUnit.xml" "…/xamarin-android/tests/../bin/TestDebug/TestResult-Xamarin.Android.Bcl_Tests.xunit.xml"
	...
	/data/data/Xamarin.Android.Bcl_Tests/files/.__override__/TestResults.xUnit.xml: 1 file pulled. 53.5 MB/s (3273412 bytes in 0.058s)

but those results don't appear on the `testReport` URL.

The problem is that the `RenameApkTestCases` target is never being
executed, and the `RenameApkTestCases` is responsible for copying the
`TestResult*` files where Jenkins looks for them:

	Done building target "ReleaseAndroidTarget" in project "RunApkTests.targets" -- FAILED.:

The `RenameApkTestCases` target executes *after*
`ReleaseAndroidTarget`, but because `ReleaseAndroidTarget` failed, the
`RenameApkTestCases` target isn't executed.

Meanwhile, `ReleaseAndroidTarget` reports an error to ensure that
errors aren't overlooked; see commit 3b893cd4.

Allow the `RenameApkTestCases` target to execute by adding a new
`ReportComponentFailures` target. The `ReleaseAndroidTarget` will no
longer report errors, allowing the `RenameApkTestCases` target to
execute, while if any tests *do* fail, the `ReportComponentFailures`
target will generate an appropriate error.

Furthermore, update all the `<MSBuild/>` invocations within
`RunTests.targets` and add `ContinueOnError="ErrorAndContinue"` to all
of them, to help ensure that we run all the tests we have, even if
previous tests fail.
---
 build-tools/scripts/RunTests.targets | 15 +++++++++++++--
 build-tools/scripts/TestApks.targets |  2 ++
 tests/RunApkTests.targets            |  1 +
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/build-tools/scripts/RunTests.targets b/build-tools/scripts/RunTests.targets
index a19827b8585..cf03ca3dba2 100644
--- a/build-tools/scripts/RunTests.targets
+++ b/build-tools/scripts/RunTests.targets
@@ -38,19 +38,25 @@
       <_RenamedTestCases>@(_RenameNUnitTestCasesGlob)</_RenamedTestCases>
     </PropertyGroup>
     <MSBuild
+        ContinueOnError="ErrorAndContinue"
         Projects="$(MSBuildThisFileDirectory)TestApks.targets"
         Targets="RenameTestCases"
         Properties="Configuration=$(Configuration);RenameTestCasesGlob=$(_RenamedTestCases)"
     />
   </Target>
   <Target Name="RunJavaInteropTests">
-    <MSBuild Projects="$(JavaInteropSourceDirectory)\Java.Interop.sln" Condition=" '$(HostOS)' == 'Windows' " />
+    <MSBuild
+        Condition=" '$(HostOS)' == 'Windows' "
+        ContinueOnError="ErrorAndContinue"
+        Projects="$(JavaInteropSourceDirectory)\Java.Interop.sln"
+    />
     <Exec
         Command ="make -C &quot;$(JavaInteropSourceDirectory)&quot; CONFIGURATION=$(Configuration) all"
         Condition=" '$(HostOS)' != 'Windows' "
     />
     <SetEnvironmentVariable Name="ANDROID_SDK_PATH" Value="$(AndroidSdkFullPath)" />
     <MSBuild 
+        ContinueOnError="ErrorAndContinue"
         Projects="$(JavaInteropSourceDirectory)\build-tools\scripts\RunNUnitTests.targets"
         Properties="AndroidSdkDirectory=$(AndroidSdkDirectory)"
     />
@@ -61,6 +67,7 @@
       <_RenamedTestCases>@(_RenameJITestCasesGlob)</_RenamedTestCases>
     </PropertyGroup>
     <MSBuild
+        ContinueOnError="ErrorAndContinue"
         Projects="$(MSBuildThisFileDirectory)TestApks.targets"
         Targets="RenameTestCases"
         Properties="Configuration=$(Configuration);RenameTestCasesGlob=$(_RenamedTestCases)"
@@ -73,12 +80,16 @@
   </Target>
   <Target Name="RunApkTests">
     <Exec Command="$(_XABuild) %(_ApkTestProject.Identity) /t:SignAndroidPackage $(_XABuildProperties)" />
-    <MSBuild Projects="$(_TopDir)\tests\RunApkTests.targets" />
+    <MSBuild
+        ContinueOnError="ErrorAndContinue"
+        Projects="$(_TopDir)\tests\RunApkTests.targets"
+    />
     <Exec 
         Command="$(_XABuild) %(_ApkTestProjectAot.Identity) /t:SignAndroidPackage $(_XABuildProperties) /p:AotAssemblies=True"
         Condition=" '$(Configuration)' == 'Release' "
     />
     <MSBuild 
+        ContinueOnError="ErrorAndContinue"
         Projects="$(_TopDir)\tests\RunApkTests.targets"
         Condition=" '$(Configuration)' == 'Release' "
         Properties="AotAssemblies=True"
diff --git a/build-tools/scripts/TestApks.targets b/build-tools/scripts/TestApks.targets
index d098258adec..cbaf832b445 100644
--- a/build-tools/scripts/TestApks.targets
+++ b/build-tools/scripts/TestApks.targets
@@ -117,6 +117,8 @@
         ContinueOnError="True"
         Command="kill -KILL $(_EmuPid)"
     />
+  </Target>
+  <Target Name="ReportComponentFailures">
     <Error
         Condition="'@(_FailedComponent)' != ''"
         Text="Execution of the following components did not complete successfully: @(_FailedComponent->'%(Identity)', ', ')"
diff --git a/tests/RunApkTests.targets b/tests/RunApkTests.targets
index 63f9be5a3f8..0ff9fc37eb4 100644
--- a/tests/RunApkTests.targets
+++ b/tests/RunApkTests.targets
@@ -30,6 +30,7 @@
       RunTestApks;
       ReleaseAndroidTarget;
       RenameApkTestCases;
+      ReportComponentFailures;
     </RunApkTestsDependsOn>
   </PropertyGroup>
   <Target Name="RunApkTests"