Directly throw exceptions from the build task when unit testing

Previously we would let the build task catch exceptions, and would let
the ResolvePackagesWithJsonFileContents re-throw a test exception if the
build task returned false. This caused us to round-trip the rich
exception information to a string, which is irritating. It's easier to
not do that.

5 files changed, 17 insertions, 56 deletions

diff --git a/src/Microsoft.NuGet.Build.Tasks.Tests/Microsoft.NuGet.Build.Tasks.Tests.csproj b/src/Microsoft.NuGet.Build.Tasks.Tests/Microsoft.NuGet.Build.Tasks.Tests.csproj
index 9937dc7..2792822 100644
--- a/src/Microsoft.NuGet.Build.Tasks.Tests/Microsoft.NuGet.Build.Tasks.Tests.csproj
+++ b/src/Microsoft.NuGet.Build.Tasks.Tests/Microsoft.NuGet.Build.Tasks.Tests.csproj
@@ -55,7 +55,6 @@
       <DependentUpon>Json.resx</DependentUpon>
     </Compile>
     <Compile Include="NugetTestHelpers.cs" />
-    <Compile Include="PackageResolutionTestException.cs" />
     <Compile Include="PreprocessorTests.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="ReferenceResolutionTests.cs" />
diff --git a/src/Microsoft.NuGet.Build.Tasks.Tests/NugetTestHelpers.cs b/src/Microsoft.NuGet.Build.Tasks.Tests/NugetTestHelpers.cs
index b5428ac..44d848b 100644
--- a/src/Microsoft.NuGet.Build.Tasks.Tests/NugetTestHelpers.cs
+++ b/src/Microsoft.NuGet.Build.Tasks.Tests/NugetTestHelpers.cs
@@ -58,10 +58,9 @@ namespace Microsoft.NuGet.Build.Tasks.Tests
                 task.ProjectLanguage = projectLanguage;
                 task.TargetMonikers = new ITaskItem[] { new TaskItem(targetMoniker) };
 
-                if (!task.Execute())
-                {
-                    throw new PackageResolutionTestException(sw.ToString());
-                }
+                // When we create the task for unit-testing purposes, the constructor sets an internal bit which should always
+                // cause task.Execute to throw.
+                Assert.True(task.Execute());
 
                 var analyzers = task.ResolvedAnalyzers;
                 var copyLocalItems = task.ResolvedCopyLocalItems;
diff --git a/src/Microsoft.NuGet.Build.Tasks.Tests/PackageResolutionTestException.cs b/src/Microsoft.NuGet.Build.Tasks.Tests/PackageResolutionTestException.cs
deleted file mode 100644
index f3bb441..0000000
--- a/src/Microsoft.NuGet.Build.Tasks.Tests/PackageResolutionTestException.cs
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright (c) .NET Foundation. All rights reserved. 
-// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. 
-
-using System;
-using System.Runtime.Serialization;
-
-namespace Microsoft.NuGet.Build.Tasks.Tests
-{
-    /// <summary>
-    /// An exception thrown if a helper <see cref="NuGetTestHelpers"/> says package resolution failed.
-    /// </summary>
-    [Serializable]
-    internal class PackageResolutionTestException : Exception
-    {
-        public PackageResolutionTestException()
-        {
-        }
-
-        public PackageResolutionTestException(string message) : base(message)
-        {
-        }
-
-        public PackageResolutionTestException(string message, Exception innerException) : base(message, innerException)
-        {
-        }
-
-        protected PackageResolutionTestException(SerializationInfo info, StreamingContext context) : base(info, context)
-        {
-        }
-    }
-}
\ No newline at end of file
diff --git a/src/Microsoft.NuGet.Build.Tasks.Tests/ReferenceResolutionTests.cs b/src/Microsoft.NuGet.Build.Tasks.Tests/ReferenceResolutionTests.cs
index 86dcefe..86f6988 100644
--- a/src/Microsoft.NuGet.Build.Tasks.Tests/ReferenceResolutionTests.cs
+++ b/src/Microsoft.NuGet.Build.Tasks.Tests/ReferenceResolutionTests.cs
@@ -82,24 +82,21 @@ namespace Microsoft.NuGet.Build.Tasks.Tests
         [Fact]
         public static void TestReferenceResolutionWithMissingRuntimeIDAndNoFallback()
         {
-            var exception = Assert.Throws<PackageResolutionTestException>(() =>
+            var exception = Assert.Throws<ExceptionFromResource>(() =>
                 NuGetTestHelpers.ResolvePackagesWithJsonFileContents(
                     Default.GetString(Json.Json.Win10),
                     targetMoniker: ".NETCore,Version=v5.0",
                     runtimeIdentifier: "missing-runtime-identifier",
                     allowFallbackOnTargetSelection: false));
 
-            var expected =
-                string.Format(Strings.MissingRuntimeInRuntimesSection,
-                    "missing-runtime-identifier", "\"missing-runtime-identifier\": { }");
-
-            Assert.Contains(expected, exception.Message);
+            Assert.Equal(nameof(Strings.MissingRuntimeInRuntimesSection), exception.ResourceName);
+            Assert.Equal(new[] { "missing-runtime-identifier", "\"missing-runtime-identifier\": { }" }, exception.MessageArgs);
         }
 
         [Fact]
         public static void TestReferenceResolutionWithMissingRuntimeIDAndNoFallbackAndNoRuntimesSection()
         {
-            var exception = Assert.Throws<PackageResolutionTestException>(() =>
+            var exception = Assert.Throws<ExceptionFromResource>(() =>
                 NuGetTestHelpers.ResolvePackagesWithJsonFileContents(
                     Default.GetString(Json.Json.Win10),
                     targetMoniker: ".NETCore,Version=v5.0",
@@ -107,28 +104,22 @@ namespace Microsoft.NuGet.Build.Tasks.Tests
                     allowFallbackOnTargetSelection: false,
                     projectJsonFileContents: "{ }"));
 
-            var expected =
-                string.Format(Strings.MissingRuntimesSection,
-                    "\"runtimes\": { \"missing-runtime-identifier\": { } }");
-
-            Assert.Contains(expected, exception.Message);
+            Assert.Equal(nameof(Strings.MissingRuntimesSection), exception.ResourceName);
+            Assert.Equal(new[] { "\"runtimes\": { \"missing-runtime-identifier\": { } }" }, exception.MessageArgs);
         }
 
         [Fact]
         public static void TestReferenceResolutionWithMissingTargetMonikerAndNoFallback()
         {
-            var exception = Assert.Throws<PackageResolutionTestException>(() =>
+            var exception = Assert.Throws<ExceptionFromResource>(() =>
                 NuGetTestHelpers.ResolvePackagesWithJsonFileContents(
                     Default.GetString(Json.Json.Win10),
                     targetMoniker: "Missing,Version=1.0",
                     runtimeIdentifier: "missing-runtime-identifier",
                     allowFallbackOnTargetSelection: false));
 
-            var expected =
-                string.Format(Strings.MissingFramework,
-                    "Missing,Version=1.0");
-
-            Assert.Contains(expected, exception.Message);
+            Assert.Equal(nameof(Strings.MissingFramework), exception.ResourceName);
+            Assert.Equal(new[] { "Missing,Version=1.0" }, exception.MessageArgs);
         }
 
         [Fact]
diff --git a/src/Microsoft.NuGet.Build.Tasks/ResolveNuGetPackageAssets.cs b/src/Microsoft.NuGet.Build.Tasks/ResolveNuGetPackageAssets.cs
index f8e04a7..cc2979f 100644
--- a/src/Microsoft.NuGet.Build.Tasks/ResolveNuGetPackageAssets.cs
+++ b/src/Microsoft.NuGet.Build.Tasks/ResolveNuGetPackageAssets.cs
@@ -43,6 +43,7 @@ namespace Microsoft.NuGet.Build.Tasks
         private readonly DirectoryExists _directoryExists = new DirectoryExists(Directory.Exists);
         private readonly FileExists _fileExists = new FileExists(File.Exists);
         private readonly TryGetRuntimeVersion _tryGetRuntimeVersion = new TryGetRuntimeVersion(TryGetRuntimeVersion);
+        private readonly bool _reportExceptionsToMSBuildLogger = true;
 
         internal ResolveNuGetPackageAssets(DirectoryExists directoryExists, FileExists fileExists, TryGetRuntimeVersion tryGetRuntimeVersion)
             : this()
@@ -61,6 +62,8 @@ namespace Microsoft.NuGet.Build.Tasks
             {
                 _tryGetRuntimeVersion = tryGetRuntimeVersion;
             }
+
+            _reportExceptionsToMSBuildLogger = false;
         }
         #endregion
 
@@ -187,12 +190,12 @@ namespace Microsoft.NuGet.Build.Tasks
                 ExecuteCore();
                 return true;
             }
-            catch (ExceptionFromResource e)
+            catch (ExceptionFromResource e) when (_reportExceptionsToMSBuildLogger)
             {
                 Log.LogErrorFromResources(e.ResourceName, e.MessageArgs);
                 return false;
             }
-            catch (Exception e)
+            catch (Exception e) when (_reportExceptionsToMSBuildLogger)
             {
                 // Any user-visible exceptions we throw should be ExceptionFromResource, so here we should dump stacks because
                 // something went very wrong.