Merge pull request #1005 from github/jcansdale/dont-spam-log

Stop TryOpenRepository tests spamming the log

Author: jcansdale

src/GitHub.Exports/Services/IVSServices.cs

@@ -6,10 +6,6 @@ namespace GitHub.Services
     public interface IVSServices
     {
         string VSVersion { get; }
-
-        void ActivityLogMessage(string message);
-        void ActivityLogWarning(string message);
-        void ActivityLogError(string message);
         bool TryOpenRepository(string directory);
     }
 }

src/GitHub.Exports/Services/VSServices.cs

@@ -13,25 +13,30 @@
 using Microsoft.VisualStudio.Shell.Interop;
 using Rothko;
 using Serilog;
-using DTE = EnvDTE.DTE;
+using EnvDTE;
 
 namespace GitHub.Services
 {
     [Export(typeof(IVSServices))]
     [PartCreationPolicy(CreationPolicy.Shared)]
     public class VSServices : IVSServices
     {
-        static readonly ILogger log = LogManager.ForContext<VSServices>();
+        readonly ILogger log;
         readonly IGitHubServiceProvider serviceProvider;
 
         // Use a prefix (~$) that is defined in the default VS gitignore.
         public const string TempSolutionName = "~$GitHubVSTemp$~";
 
-
         [ImportingConstructor]
-        public VSServices(IGitHubServiceProvider serviceProvider)
+        public VSServices(IGitHubServiceProvider serviceProvider) :
+            this(serviceProvider, LogManager.ForContext<VSServices>())
+        {
+        }
+
+        public VSServices(IGitHubServiceProvider serviceProvider, ILogger log)
         {
             this.serviceProvider = serviceProvider;
+            this.log = log;
         }
 
         string vsVersion;
@@ -45,41 +50,6 @@ public string VSVersion
             }
         }
 
-
-        public void ActivityLogMessage(string message)
-        {
-            var log = serviceProvider.GetActivityLog();
-            if (log != null)
-            {
-                if (!ErrorHandler.Succeeded(log.LogEntry((UInt32)__ACTIVITYLOG_ENTRYTYPE.ALE_INFORMATION,
-                            Info.ApplicationInfo.ApplicationSafeName, message)))
-                    Console.WriteLine(string.Format(CultureInfo.CurrentCulture, "Could not log message to activity log: {0}", message));
-            }
-        }
-
-        public void ActivityLogError(string message)
-        {
-            var log = serviceProvider.GetActivityLog();
-            if (log != null)
-            {
-
-                if (!ErrorHandler.Succeeded(log.LogEntry((UInt32)__ACTIVITYLOG_ENTRYTYPE.ALE_ERROR,
-                            Info.ApplicationInfo.ApplicationSafeName, message)))
-                    Console.WriteLine(string.Format(CultureInfo.CurrentCulture, "Could not log error to activity log: {0}", message));
-            }
-        }
-
-        public void ActivityLogWarning(string message)
-        {
-            var log = serviceProvider.GetActivityLog();
-            if (log != null)
-            {
-                if (!ErrorHandler.Succeeded(log.LogEntry((UInt32)__ACTIVITYLOG_ENTRYTYPE.ALE_WARNING,
-                            Info.ApplicationInfo.ApplicationSafeName, message)))
-                    Console.WriteLine(string.Format(CultureInfo.CurrentCulture, "Could not log warning to activity log: {0}", message));
-            }
-        }
-
         /// <summary>Open a repository in Team Explorer</summary>
         /// <remarks>
         /// There doesn't appear to be a command that directly opens a target repo.
@@ -105,7 +75,7 @@ public bool TryOpenRepository(string repoPath)
             }
 
             var repoDir = os.Directory.GetDirectory(repoPath);
-            if(!repoDir.Exists)
+            if (!repoDir.Exists)
             {
                 return false;
             }
@@ -172,7 +142,7 @@ string GetVSVersion()
                     return setupInstance.GetInstallationName().TrimPrefix(InstallationNamePrefix);
                 }
             }
-            catch(Exception ex)
+            catch (Exception ex)
             {
                 log.Error(ex, "Error getting the Visual Studio version");
             }

test/UnitTests/GitHub.Exports/VSServicesTests.cs

@@ -6,6 +6,7 @@
 using NUnit.Framework;
 using DTE = EnvDTE.DTE;
 using Rothko;
+using Serilog;
 
 public class VSServicesTests
 {
@@ -27,24 +28,22 @@ public void SolutionCreateThrows_ReturnsFalse()
         {
             var repoDir = @"x:\repo";
             var dte = Substitute.For<DTE>();
-            dte.Solution.When(s => s.Create(Arg.Any<string>(), Arg.Any<string>())).Do(
-                ci => { throw new COMException(); });
-            var target = CreateVSServices(repoDir, dte: dte);
+            var log = Substitute.For<ILogger>();
+            var ex = new COMException();
+            dte.Solution.When(s => s.Create(Arg.Any<string>(), Arg.Any<string>())).Do(ci => { throw ex; });
+            var target = CreateVSServices(repoDir, dte: dte, log: log);
 
-            var success = target.TryOpenRepository("");
+            var success = target.TryOpenRepository(repoDir);
 
             Assert.False(success);
+            log.Received(1).Error(ex, "Error opening repository");
         }
 
         [Test]
         public void RepoDirExistsFalse_ReturnFalse()
         {
             var repoDir = @"x:\repo";
-            var os = Substitute.For<IOperatingSystem>();
-            //var directoryInfo = Substitute.For<IDirectoryInfo>();
-            //directoryInfo.Exists.Returns(false);
-            //os.Directory.GetDirectory(repoDir).Returns(directoryInfo);
-            var target = CreateVSServices(null, os: os);
+            var target = CreateVSServices(repoDir, repoDirExists: false);
 
             var success = target.TryOpenRepository(repoDir);
 
@@ -85,22 +84,23 @@ public void SolutionCreate_DeleteVsSolutionSubdir()
             directoryInfo.Received().Delete(true);
         }
 
-        VSServices CreateVSServices(string repoDir, IOperatingSystem os = null, DTE dte = null)
+        VSServices CreateVSServices(string repoDir, IOperatingSystem os = null, DTE dte = null, bool repoDirExists = true, ILogger log = null)
         {
             os = os ?? Substitute.For<IOperatingSystem>();
             dte = dte ?? Substitute.For<DTE>();
+            log = log ?? Substitute.For<ILogger>();
 
             if (repoDir != null)
             {
                 var directoryInfo = Substitute.For<IDirectoryInfo>();
-                directoryInfo.Exists.Returns(true);
+                directoryInfo.Exists.Returns(repoDirExists);
                 os.Directory.GetDirectory(repoDir).Returns(directoryInfo);
             }
 
             var provider = Substitute.For<IGitHubServiceProvider>();
             provider.TryGetService<DTE>().Returns(dte);
             provider.TryGetService<IOperatingSystem>().Returns(os);
-            return new VSServices(provider);
+            return new VSServices(provider, log);
         }
     }
 

test/UnitTests/UnitTests.csproj

@@ -173,6 +173,7 @@
     <Reference Include="PresentationFramework" />
     <Reference Include="Serilog, Version=2.0.0.0, Culture=neutral, PublicKeyToken=24c2f752a8e58a10, processorArchitecture=MSIL">
       <HintPath>..\..\packages\Serilog.2.5.0\lib\net46\Serilog.dll</HintPath>
+      <Private>True</Private>
     </Reference>
     <Reference Include="rothko, Version=0.0.3.0, Culture=neutral, PublicKeyToken=9f664c41f503810a, processorArchitecture=MSIL">
       <HintPath>..\..\packages\Rothko.0.0.3-ghfvs\lib\net45\rothko.dll</HintPath>