Always send project evaluation finished Fixes #10113 (#10365)

Fixes #10113 Context Evaluating a project is supposed to send a ProjectEvaluationFinished event. If it fails, however, it doesn't. Changes Made Wrap Evaluate in a try/finally block to ensure ProjectEvaluationFinished is always sent Testing Created a unit test
dotnet · Jul 18, 2024 · 7b1e619 · 7b1e619
1 parent 47f364c
commit 7b1e619
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 14 deletions.
diff --git a/src/Build.UnitTests/Evaluation/Evaluator_Tests.cs b/src/Build.UnitTests/Evaluation/Evaluator_Tests.cs
@@ -50,6 +50,23 @@ public void Dispose()
             GC.Collect();
         }
 
+        [Fact]
+        public void EnsureProjectEvaluationFinishedIsLogged()
+        {
+            using TestEnvironment env = TestEnvironment.Create();
+            TransientTestFile projectFile = env.CreateFile("project.proj", $@"
+<Project Sdk=""Microsoft.NETT.Sdk"">
+  <Target Name=""DefaultTarget"">
+  </Target>
+</Project>
+");
+
+            MockLogger logger = new();
+            using ProjectCollection collection = new(new Dictionary<string, string>(), [logger], ToolsetDefinitionLocations.Default);
+            Assert.Throws<InvalidProjectFileException>(() => collection.LoadProject(projectFile.Path));
+            logger.EvaluationFinishedEvents.ShouldNotBeEmpty();
+        }
+
         [Theory]
         [MemberData(nameof(ImportLoadingScenarioTestData))]
         public void VerifyLoadingImportScenarios(string importParameter, bool shouldSucceed)

diff --git a/src/Build/Evaluation/Evaluator.cs b/src/Build/Evaluation/Evaluator.cs
@@ -333,7 +333,26 @@ internal static void Evaluate(
                 loggingService,
                 buildEventContext);
 
-            evaluator.Evaluate();
+            try
+            {
+                evaluator.Evaluate();
+            }
+            finally
+            {
+                IEnumerable globalProperties = null;
+                IEnumerable properties = null;
+                IEnumerable items = null;
+
+                if (evaluator._evaluationLoggingContext.LoggingService.IncludeEvaluationPropertiesAndItems)
+                {
+                    globalProperties = evaluator._data.GlobalPropertiesDictionary;
+                    properties = Traits.LogAllEnvironmentVariables ? evaluator._data.Properties : evaluator.FilterOutEnvironmentDerivedProperties(evaluator._data.Properties);
+                    items = evaluator._data.Items;
+                }
+
+                evaluator._evaluationLoggingContext.LogProjectEvaluationFinished(globalProperties, properties, items, evaluator._evaluationProfiler.ProfiledResult);
+            }
+
             MSBuildEventSource.Log.EvaluateStop(root.ProjectFileLocation.File);
         }
 
@@ -798,19 +817,6 @@ private void Evaluate()
             }
 
             ErrorUtilities.VerifyThrow(_evaluationProfiler.IsEmpty(), "Evaluation profiler stack is not empty.");
-
-            IEnumerable globalProperties = null;
-            IEnumerable properties = null;
-            IEnumerable items = null;
-
-            if (this._evaluationLoggingContext.LoggingService.IncludeEvaluationPropertiesAndItems)
-            {
-                globalProperties = _data.GlobalPropertiesDictionary;
-                properties = Traits.LogAllEnvironmentVariables ? _data.Properties : FilterOutEnvironmentDerivedProperties(_data.Properties);
-                items = _data.Items;
-            }
-
-            _evaluationLoggingContext.LogProjectEvaluationFinished(globalProperties, properties, items, _evaluationProfiler.ProfiledResult);
         }
 
         private IEnumerable FilterOutEnvironmentDerivedProperties(PropertyDictionary<P> dictionary)