Log an error during traversal project loading

src/CBT.NuGet.UnitTests/AggregatePackageTests.cs

@@ -10,24 +10,13 @@
 
 namespace CBT.NuGet.UnitTests
 {
-    public class AggregatePackageTests : IDisposable
+    public class AggregatePackageTests : TestBase
     {
-        private string _basePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
-        public AggregatePackageTests()
-        {
-            Directory.CreateDirectory(_basePath);
-        }
-
-        public void Dispose()
-        {
-            Directory.Delete(_basePath, recursive: true);
-        }
-
-        private void CreateDummyPackage(string _basePath, ICollection<string> filePaths)
+        private void CreateDummyPackage(string basePath, ICollection<string> filePaths)
         {
             foreach (var file in filePaths)
             {
-                var fullFilePath = Path.Combine(_basePath, file);
+                var fullFilePath = Path.Combine(basePath, file);
                 var parent = Path.GetDirectoryName(fullFilePath);
                 if (!Directory.Exists(parent))
                 {
@@ -43,9 +32,9 @@ private void CreateDummyPackage(string _basePath, ICollection<string> filePaths)
         [Fact]
         public void ParseAggregatePackageTest()
         {
-            string pkg = Path.Combine(_basePath, "pkg");
-            string pkg2 = Path.Combine(_basePath, "pkg2");
-            string pkg3 = Path.Combine(_basePath, "pkg3");
+            string pkg = Path.Combine(TestRootPath, "pkg");
+            string pkg2 = Path.Combine(TestRootPath, "pkg2");
+            string pkg3 = Path.Combine(TestRootPath, "pkg3");
 
             CreateDummyPackage(pkg, new[] { "fool.txt", "friend\\bat.txt", "cow.txt" });
             CreateDummyPackage(pkg2, new[] { "cammel.txt", "sour\\bat.txt", "cow.txt" });
@@ -54,7 +43,7 @@ public void ParseAggregatePackageTest()
             var aggPkgs = new AggregatePackages();
             aggPkgs.BuildEngine = new CBTBuildEngine();
             aggPkgs.PackagesToAggregate = $"foo={pkg}|{pkg2}|!{pkg3};foo2= \t {pkg}   |     {pkg2} | !  {pkg3}  \t";
-            aggPkgs.AggregateDestRoot = Path.Combine(_basePath, ".agg");
+            aggPkgs.AggregateDestRoot = Path.Combine(TestRootPath, ".agg");
 
             var parsedPackagesEnumerator = aggPkgs.ParsePackagesToAggregate().GetEnumerator();
             parsedPackagesEnumerator.MoveNext();
@@ -76,9 +65,9 @@ public void ParseAggregatePackageTest()
         public void CreateAggregatePackageTest()
         {
 
-            string pkg = Path.Combine(_basePath, "pkg");
-            string pkg2 = Path.Combine(_basePath, "pkg2");
-            string pkg3 = Path.Combine(_basePath, "pkg3");
+            string pkg = Path.Combine(TestRootPath, "pkg");
+            string pkg2 = Path.Combine(TestRootPath, "pkg2");
+            string pkg3 = Path.Combine(TestRootPath, "pkg3");
 
             CreateDummyPackage(pkg, new[] { "fool.txt", "friend\\bat.txt", "cow.txt" });
             CreateDummyPackage(pkg2, new[] { "cammel.txt", "sour\\bat.txt", "cow.txt" });
@@ -87,7 +76,7 @@ public void CreateAggregatePackageTest()
             var aggPkgs = new AggregatePackages();
             aggPkgs.BuildEngine = new CBTBuildEngine();
             aggPkgs.PackagesToAggregate = $"foo={pkg}|{pkg2}|!{pkg3};foo2={pkg}|!{pkg2}";
-            aggPkgs.AggregateDestRoot = Path.Combine(_basePath, ".agg");
+            aggPkgs.AggregateDestRoot = Path.Combine(TestRootPath, ".agg");
 
             var parsedPackagesEnumerator = aggPkgs.ParsePackagesToAggregate().GetEnumerator();
             parsedPackagesEnumerator.MoveNext();
@@ -127,8 +116,8 @@ public void CreateAggregatePackageTest()
         [Fact]
         public void WritePropsAggregatePackageTest()
         {
-            string propsFile = Path.Combine(_basePath, "props", "foo.props");
-            string propsFileExpect = Path.Combine(_basePath, "props", "fooExpected.props");
+            string propsFile = Path.Combine(TestRootPath, "props", "foo.props");
+            string propsFileExpect = Path.Combine(TestRootPath, "props", "fooExpected.props");
             Dictionary<string, string> props = new Dictionary<string, string>();
             props.Add("foo", "Myvalue");
             props.Add("foo2", "MyValue");

src/CBT.NuGet.UnitTests/CBT.NuGet.UnitTests.csproj

@@ -24,6 +24,8 @@
     <Compile Include="ExtensionMethods.cs" />
     <Compile Include="MSBuildProjectLoaderTests.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
+    <Compile Include="TestBase.cs" />
+    <Compile Include="TestBuildEngine.cs" />
   </ItemGroup>
   <ItemGroup>
     <None Include="project.json" />

src/CBT.NuGet.UnitTests/MSBuildProjectLoaderTests.cs

@@ -1,48 +1,61 @@
 using CBT.NuGet.Internal;
 using Microsoft.Build.Evaluation;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
 using Microsoft.MSBuildProjectBuilder;
 using Shouldly;
 using System;
 using System.Collections.Generic;
-using System.IO;
 using System.Linq;
 using Xunit;
 
 namespace CBT.NuGet.UnitTests
 {
-    public class MSBuildProjectLoaderTests : IDisposable
+    public class MSBuildProjectLoaderTests : TestBase
     {
         private const string MSBuildToolsVersion = "4.0";
-        private readonly string _basePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
 
-        private readonly string _dirsAPath;
-        private readonly string _projectAPath;
-        private readonly string _projectBPath;
+        private readonly TestBuildEngine _buildEngine = new TestBuildEngine();
+        private readonly Lazy<TaskLoggingHelper> _logLazy;
 
         public MSBuildProjectLoaderTests()
         {
-            _dirsAPath = Path.Combine(_basePath, "dirsA.proj");
-            _projectAPath = Path.Combine(_basePath, "ProjectA.proj");
-            _projectBPath = Path.Combine(_basePath, "ProjectB.proj");
-
-            var projB = ProjectBuilder.Create()
-                .Save(_projectBPath);
-
-            ProjectBuilder
-                .Create()
-                .AddItem($"ProjectReference={projB.ProjectRoot.FullPath}")
-                .Save(_projectAPath);
+            _logLazy = new Lazy<TaskLoggingHelper>(() => new TaskLoggingHelper(_buildEngine, "TaskName"), isThreadSafe: true);
+        }
 
-            ProjectBuilder
-                .Create()
-                .AddProperty("IsTraversal=true")
-                .AddItem($"ProjectFile={_projectAPath}")
-                .Save(_dirsAPath);
+        public TaskLoggingHelper Log => _logLazy.Value;
+
+        [Fact]
+        public void ArgumentNullException_Log()
+        {
+            ArgumentNullException exception = Should.Throw<ArgumentNullException>(() =>
+            {
+                MSBuildProjectLoader unused = new MSBuildProjectLoader(globalProperties: null, toolsVersion: null, log: null);
+            });
+
+            exception.ParamName.ShouldBe("log");
         }
 
-        public void Dispose()
+        [Fact]
+        public void BuildFailsIfError()
         {
-            Directory.Delete(_basePath, recursive: true);
+            var dirsProj = ProjectBuilder
+                .Create()
+                .AddProperty("IsTraversal=true")
+                .AddItem("ProjectFile=does not exist")
+                .Save(GetTempFileName());
+
+            MSBuildProjectLoader loader = new MSBuildProjectLoader(null, MSBuildToolsVersion, Log);
+            loader.LoadProjectsAndReferences(new[] {dirsProj.FullPath});
+
+            Log.HasLoggedErrors.ShouldBe(true);
+
+            _buildEngine.LoggedEvents.Count.ShouldBe(1);
+            BuildErrorEventArgs errorEvent = _buildEngine.LoggedEvents.FirstOrDefault() as BuildErrorEventArgs;
+
+            errorEvent.ShouldNotBeNull();
+
+            errorEvent?.Message.ShouldStartWith("The project file could not be loaded. Could not find file ");
         }
 
         [Fact]
@@ -54,31 +67,57 @@ public void GlobalPropertiesSetCorrectly()
                 {"Property2", "CEEC5C9FF0F344DAA32A0F545460EB2C"}
             };
 
-            MSBuildProjectLoader loader = new MSBuildProjectLoader(expectedGlobalProperties, MSBuildToolsVersion, ProjectLoadSettings.Default);
+            var projectA = ProjectBuilder
+                .Create()
+                .Save(GetTempFileName());
 
-            ProjectCollection projectCollection = loader.LoadProjectsAndReferences(new[] {_projectAPath});
+            MSBuildProjectLoader loader = new MSBuildProjectLoader(expectedGlobalProperties, MSBuildToolsVersion, Log);
+
+            ProjectCollection projectCollection = loader.LoadProjectsAndReferences(new[] {projectA.FullPath});
 
             projectCollection.GlobalProperties.ShouldBe(expectedGlobalProperties);
         }
 
         [Fact]
         public void ProjectReferencesWork()
         {
-            MSBuildProjectLoader loader = new MSBuildProjectLoader(null, MSBuildToolsVersion);
+            var projectB = ProjectBuilder.Create()
+                .Save(GetTempFileName());
+
+            var projectA = ProjectBuilder
+                .Create()
+                .AddProjectReference(projectB)
+                .Save(GetTempFileName());
 
-            ProjectCollection projectCollection = loader.LoadProjectsAndReferences(new[] {_projectAPath});
+            MSBuildProjectLoader loader = new MSBuildProjectLoader(null, MSBuildToolsVersion, Log);
 
-            projectCollection.LoadedProjects.Select(i => i.FullPath).ShouldBe(new[] {_projectAPath, _projectBPath});
+            ProjectCollection projectCollection = loader.LoadProjectsAndReferences(new[] {projectA.FullPath});
+
+            projectCollection.LoadedProjects.Select(i => i.FullPath).ShouldBe(new[] {projectA.FullPath, projectB.FullPath});
         }
 
         [Fact]
         public void TraversalReferencesWork()
         {
-            MSBuildProjectLoader loader = new MSBuildProjectLoader(null, MSBuildToolsVersion);
+            var projectB = ProjectBuilder.Create()
+                .Save(GetTempFileName());
+
+            var projectA = ProjectBuilder
+                .Create()
+                .AddProjectReference(projectB)
+                .Save(GetTempFileName());
+
+            var dirsProj = ProjectBuilder
+                .Create()
+                .AddProperty("IsTraversal=true")
+                .AddItem($"ProjectFile={projectA.FullPath}")
+                .Save(GetTempFileName());
+
+            MSBuildProjectLoader loader = new MSBuildProjectLoader(null, MSBuildToolsVersion, Log);
 
-            ProjectCollection projectCollection = loader.LoadProjectsAndReferences(new[] {_dirsAPath});
+            ProjectCollection projectCollection = loader.LoadProjectsAndReferences(new[] {dirsProj.FullPath});
 
-            projectCollection.LoadedProjects.Select(i => i.FullPath).ShouldBe(new[] {_dirsAPath, _projectAPath, _projectBPath});
+            projectCollection.LoadedProjects.Select(i => i.FullPath).ShouldBe(new[] {dirsProj.FullPath, projectA.FullPath, projectB.FullPath});
         }
     }
 }

src/CBT.NuGet.UnitTests/TestBase.cs

@@ -0,0 +1,38 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace CBT.NuGet.UnitTests
+{
+    public abstract class TestBase : IDisposable
+    {
+        public string TestRootPath { get; } = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
+
+        protected string GetTempFileName()
+        {
+            Directory.CreateDirectory(TestRootPath);
+
+            return Path.Combine(TestRootPath, Path.GetRandomFileName());
+        }
+
+        protected virtual void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                if (Directory.Exists(TestRootPath))
+                {
+                    Directory.Delete(TestRootPath, recursive: true);
+                }
+            }
+        }
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+    }
+}

src/CBT.NuGet.UnitTests/TestBuildEngine.cs

@@ -0,0 +1,37 @@
+using Microsoft.Build.Framework;
+using System;
+using System.Collections;
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Linq;
+
+namespace CBT.NuGet.UnitTests
+{
+    public class TestBuildEngine : IBuildEngine
+    {
+        private readonly ConcurrentBag<BuildEventArgs> _events = new ConcurrentBag<BuildEventArgs>();
+
+        public int ColumnNumberOfTaskNode => 0;
+
+        public bool ContinueOnError => false;
+
+        public int LineNumberOfTaskNode => 0;
+
+        public IReadOnlyCollection<BuildEventArgs> LoggedEvents => _events.ToList().AsReadOnly();
+
+        public string ProjectFileOfTaskNode => null;
+
+        public bool BuildProjectFile(string projectFileName, string[] targetNames, IDictionary globalProperties, IDictionary targetOutputs)
+        {
+            throw new NotSupportedException();
+        }
+
+        public void LogCustomEvent(CustomBuildEventArgs e) => _events.Add(e);
+
+        public void LogErrorEvent(BuildErrorEventArgs e) => _events.Add(e);
+
+        public void LogMessageEvent(BuildMessageEventArgs e) => _events.Add(e);
+
+        public void LogWarningEvent(BuildWarningEventArgs e) => _events.Add(e);
+    }
+}

src/CBT.NuGet/Internal/MSBuildProjectLoader.cs

@@ -5,6 +5,8 @@
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
+using Microsoft.Build.Exceptions;
+using Microsoft.Build.Utilities;
 
 namespace CBT.NuGet.Internal
 {
@@ -38,17 +40,21 @@ internal sealed class MSBuildProjectLoader
         /// </summary>
         private readonly string _toolsVersion;
 
+        private readonly TaskLoggingHelper _log;
+
         /// <summary>
         /// Initializes a new instance of the MSBuildProjectLoader class.
         /// </summary>
         /// <param name="globalProperties">Specifies the global properties to use when loading projects.</param>
         /// <param name="toolsVersion">Specifies the ToolsVersion to use when loading projects.</param>
         /// <param name="projectLoadSettings">Specifies the <see cref="ProjectLoadSettings"/> to use when loading projects.</param>
-        public MSBuildProjectLoader(IDictionary<string, string> globalProperties, string toolsVersion, ProjectLoadSettings projectLoadSettings = ProjectLoadSettings.Default)
+        /// <param name="log"></param>
+        public MSBuildProjectLoader(IDictionary<string, string> globalProperties, string toolsVersion, TaskLoggingHelper log, ProjectLoadSettings projectLoadSettings = ProjectLoadSettings.Default)
         {
             _globalProperties = globalProperties;
             _toolsVersion = toolsVersion;
             _projectLoadSettings = projectLoadSettings;
+            _log = log ?? throw new ArgumentNullException(nameof(log));
         }
 
         /// <summary>
@@ -153,7 +159,17 @@ private bool TryLoadProject(string path, string toolsVersion, ProjectCollection
 
             long now = DateTime.Now.Ticks;
 
-            project = new Project(path, null, toolsVersion, projectCollection, projectLoadSettings);
+            try
+            {
+                project = new Project(path, null, toolsVersion, projectCollection, projectLoadSettings);
+            }
+            catch (Exception e)
+            {
+                _log.LogErrorFromException(e);
+
+                return false;
+            }
+
 
             if (CollectStats)
             {