Flip meaning of AllowFailureWithoutError (#5743)

Fixes #5701. Changes its meaning without altering the default behavior. Also fixed tests and added one to verify that the default is as expected.
dotnet · Oct 2, 2020 · ca44138 · ca44138
1 parent 5b052e3
commit ca44138
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 12 deletions.
diff --git a/src/Build.UnitTests/BackEnd/FailingTask.cs b/src/Build.UnitTests/BackEnd/FailingTask.cs
@@ -10,11 +10,14 @@ public class FailingTask : Task
     {
         public override bool Execute()
         {
-            BuildEngine.GetType().GetProperty("AllowFailureWithoutError").SetValue(BuildEngine, EnableDefaultFailure);
+            if (!AllowFailureWithoutError.Equals("Default"))
+            {
+                BuildEngine.GetType().GetProperty("AllowFailureWithoutError").SetValue(BuildEngine, AllowFailureWithoutError.Equals("True"));
+            }
             return false;
         }
 
         [Required]
-        public bool EnableDefaultFailure { get; set; }
+        public string AllowFailureWithoutError { get; set; }
     }
 }
diff --git a/src/Build.UnitTests/BackEnd/OnError_Tests.cs b/src/Build.UnitTests/BackEnd/OnError_Tests.cs
@@ -563,18 +563,19 @@ public void OutOfOrderOnError()
         }
 
         [Theory]
-        [InlineData(false)]
-        [InlineData(true)]
-        public void ErrorWhenTaskFailsWithoutLoggingErrorEscapeHatch(bool emitError)
+        [InlineData("True")]
+        [InlineData("False")]
+        [InlineData("Default")]
+        public void ErrorWhenTaskFailsWithoutLoggingErrorEscapeHatch(string failureResponse)
         {
             MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure($@"
 <Project>
     <UsingTask TaskName=""FailingTask"" AssemblyFile=""{Assembly.GetExecutingAssembly().Location}"" />
     <Target Name=""MyTarget"">
-        <FailingTask EnableDefaultFailure=""{emitError}"" />
+        <FailingTask AllowFailureWithoutError=""{failureResponse}"" />
     </Target>
 </Project>");
-            if (emitError)
+            if (!string.Equals(failureResponse, "True"))
             {
                 logger.ErrorCount.ShouldBe(1);
                 logger.Errors.First().Code.ShouldBe("MSB4181");

diff --git a/src/Build/BackEnd/BuildManager/BuildParameters.cs b/src/Build/BackEnd/BuildManager/BuildParameters.cs
@@ -322,7 +322,7 @@ public bool UseSynchronousLogging
         /// <summary>
         /// Indicates whether to emit a default error if a task returns false without logging an error.
         /// </summary>
-        public bool AllowFailureWithoutError { get; set; } = true;
+        public bool AllowFailureWithoutError { get; set; } = false;
 
         /// <summary>
         /// Gets the environment variables which were set when this build was created.

diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs
@@ -938,7 +938,7 @@ private async Task<WorkUnitResult> ExecuteInstantiatedTask(ITaskExecutionHost ta
                 // that is logged as an error. MSBuild tasks are an exception because
                 // errors are not logged directly from them, but the tasks spawned by them.
                 IBuildEngine be = host.TaskInstance.BuildEngine;
-                if (taskReturned && !taskResult && !taskLoggingContext.HasLoggedErrors && (be is TaskHost th ? th.BuildRequestsSucceeded : false) && (be is IBuildEngine7 be7 ? be7.AllowFailureWithoutError : true))
+                if (taskReturned && !taskResult && !taskLoggingContext.HasLoggedErrors && (be is TaskHost th ? th.BuildRequestsSucceeded : false) && (be is IBuildEngine7 be7 ? !be7.AllowFailureWithoutError : true))
                 {
                     if (_continueOnError == ContinueOnError.WarnAndContinue)
                     {

diff --git a/src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs b/src/Build/BackEnd/Components/RequestBuilder/TaskHost.cs
@@ -676,7 +676,7 @@ public IReadOnlyDictionary<string, string> GetGlobalProperties()
         /// <summary>
         /// Enables or disables emitting a default error when a task fails without logging errors
         /// </summary>
-        public bool AllowFailureWithoutError { get; set; } = true;
+        public bool AllowFailureWithoutError { get; set; } = false;
         #endregion
 
         /// <summary>

diff --git a/src/MSBuild/OutOfProcTaskHostNode.cs b/src/MSBuild/OutOfProcTaskHostNode.cs
@@ -268,7 +268,7 @@ public bool IsRunningMultipleNodes
         /// <summary>
         /// Enables or disables emitting a default error when a task fails without logging errors
         /// </summary>
-        public bool AllowFailureWithoutError { get; set; } = true;
+        public bool AllowFailureWithoutError { get; set; } = false;
         #endregion
 
         #region IBuildEngine Implementation (Methods)

diff --git a/src/Shared/UnitTests/MockEngine.cs b/src/Shared/UnitTests/MockEngine.cs
@@ -51,7 +51,7 @@ internal MockEngine() : this(false)
 
         internal int Errors { get; set; }
 
-        public bool AllowFailureWithoutError { get; set; } = true;
+        public bool AllowFailureWithoutError { get; set; } = false;
 
         public BuildErrorEventArgs[] ErrorEvents => _errorEvents.ToArray();