Refactor plugin update checker for better testability

Replaced concrete metadata providers with interfaces ILocalPluginMetadataProvider and IOnlinePluginMetadataProvider in PluginUpdateChecker. Constructor overloading is used to maintain the original behavior along with the new behavior where the PluginUpdateChecker accepts the mentioned interfaces as arguments. This change allows better unit testing.

Additional packages were added to the project for the creation of substitute objects. Unit tests were written to test new and old behavior of PluginUpdateChecker.

The "No newline at end of file" error was also corrected.

src/Test/Core/PluginUpdateCheckerTest.cs

@@ -1,7 +1,10 @@
-using System;
+using System.Collections.Generic;
+using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Nikse.SubtitleEdit.Plugins;
+using NSubstitute;
+using NSubstitute.Extensions;
 
 namespace Test.Core
 {
@@ -9,17 +12,70 @@ namespace Test.Core
     public class PluginUpdateCheckerTest
     {
         [TestMethod]
-        public async Task CheckAsyncTest()
+        public async Task CheckUpdateTest()
         {
-            var pluginPath = Environment.GetEnvironmentVariable("plugin_directory", EnvironmentVariableTarget.User);
+            // Arrange
-            const string githubUrl = "https://raw.githubusercontent.com/SubtitleEdit/plugins/master/Plugins4.xml";
+            var onlinePluginProvider = Substitute.For<IOnlinePluginMetadataProvider>();
-            var sut = new PluginUpdateChecker(new PluginUpdateCheckerOptions()
+            var localPluginProvider = Substitute.For<ILocalPluginMetadataProvider>();
-            {
+
-                GithubUrl = githubUrl, PluginDirectory = pluginPath
+            // configure
-            });
+            onlinePluginProvider.Configure()
+                .GetPluginsAsync()
+                .Returns(new List<PluginInfo>()
+                {
+                    new LocalPlugin("foobar", "foobar", 2.0m)
+                });
+            localPluginProvider.Configure()
+                .GetInstalledPlugins()
+                .Returns(new List<LocalPlugin>()
+                {
+                    new LocalPlugin("foobar", "foobar", 1.0m)
+                });
+
+            var sut = new PluginUpdateChecker(localPluginProvider, onlinePluginProvider);
+
+            // Act 
+            var updateCheckResult = await sut.CheckAsync().ConfigureAwait(false);
+
+            // Assert 
+            Assert.AreEqual(true, updateCheckResult.Available);
+        }
+        
+        [TestMethod]
+        public async Task NoPluginInstalledTest()
+        {
+            // Arrange
+            var onlinePluginProvider = Substitute.For<IOnlinePluginMetadataProvider>();
+            var localPluginProvider = Substitute.For<ILocalPluginMetadataProvider>();
+            var sut = new PluginUpdateChecker(localPluginProvider, onlinePluginProvider);
+            
+            // Act 
             var updateCheckResult = await sut.CheckAsync();
 
+            // Assert 
             Assert.AreEqual(false, updateCheckResult.Available);
+            Assert.AreEqual(false, updateCheckResult.PluginUpdates.Any());
+        }
+
+        [TestMethod]
+        public async Task NoOnlinePluginAvailableTest()
+        {
+            // Arrange
+            var onlinePluginProvider = Substitute.For<IOnlinePluginMetadataProvider>();
+            var localPluginProvider = Substitute.For<ILocalPluginMetadataProvider>();
+            localPluginProvider.Configure().GetInstalledPlugins().Returns(new List<LocalPlugin>()
+            {
+                new LocalPlugin("foobar", "foobar", 1m)
+            });
+
+            var sut = new PluginUpdateChecker(localPluginProvider, onlinePluginProvider);
+            var updateCheckResult = await sut.CheckAsync();
+
+            // Act 
+
+            // Assert 
+            Assert.AreEqual(false, updateCheckResult.Available);
+            Assert.AreEqual(false, updateCheckResult.PluginUpdates.Any());
         }
     }
 }

src/Test/Test.csproj

@@ -40,13 +40,20 @@
     <Prefer32Bit>false</Prefer32Bit>
   </PropertyGroup>
   <ItemGroup>
+    <Reference Include="Castle.Core, Version=5.0.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\Castle.Core.5.0.0\lib\net462\Castle.Core.dll</HintPath>
+    </Reference>
     <Reference Include="Microsoft.VisualStudio.QualityTools.UnitTestFramework, Version=10.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
       <Private>False</Private>
     </Reference>
     <Reference Include="NHunspell, Version=1.2.5554.16953, Culture=neutral, PublicKeyToken=1ac793ea843b4366, processorArchitecture=MSIL">
       <HintPath>..\..\packages\NHunspell.1.2.5554.16953\lib\net\NHunspell.dll</HintPath>
     </Reference>
+    <Reference Include="NSubstitute, Version=5.0.0.0, Culture=neutral, PublicKeyToken=92dd2e9066daa5ca, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\NSubstitute.5.0.0\lib\net462\NSubstitute.dll</HintPath>
+    </Reference>
     <Reference Include="System" />
+    <Reference Include="System.Configuration" />
     <Reference Include="System.Drawing" />
     <Reference Include="System.Net.Http.Extensions, Version=2.2.29.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
       <HintPath>..\..\packages\Microsoft.Net.Http.2.2.29\lib\net45\System.Net.Http.Extensions.dll</HintPath>
@@ -54,6 +61,9 @@
     <Reference Include="System.Net.Http.Primitives, Version=4.2.29.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
       <HintPath>..\..\packages\Microsoft.Net.Http.2.2.29\lib\net45\System.Net.Http.Primitives.dll</HintPath>
     </Reference>
+    <Reference Include="System.Threading.Tasks.Extensions, Version=4.1.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\System.Threading.Tasks.Extensions.4.3.0\lib\portable-net45+win8+wp8+wpa81\System.Threading.Tasks.Extensions.dll</HintPath>
+    </Reference>
     <Reference Include="System.Windows.Forms" />
     <Reference Include="System.Xml" />
     <Reference Include="System.Xml.Linq" />

src/Test/app.config

@@ -6,6 +6,42 @@
         <assemblyIdentity name="Microsoft.Win32.Registry" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
         <bindingRedirect oldVersion="0.0.0.0-5.0.0.0" newVersion="5.0.0.0" />
       </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="mscorlib" publicKeyToken="b77a5c561934e089" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.0.0" newVersion="4.0.0.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="netstandard" publicKeyToken="cc7b13ffcd2ddd51" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-2.1.0.0" newVersion="2.1.0.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System" publicKeyToken="b77a5c561934e089" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.0.0" newVersion="4.0.0.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Collections" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.11.0" newVersion="4.0.11.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Net.Http" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.1.2.0" newVersion="4.1.2.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Net.Primitives" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.11.0" newVersion="4.0.11.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.1.2.0" newVersion="4.1.2.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Runtime.InteropServices" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.1.2.0" newVersion="4.1.2.0" />
+      </dependentAssembly>
+      <dependentAssembly>
+        <assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" />
+        <bindingRedirect oldVersion="0.0.0.0-4.0.11.0" newVersion="4.0.11.0" />
+      </dependentAssembly>
     </assemblyBinding>
   </runtime>
 </configuration>

src/Test/packages.config

@@ -1,7 +1,10 @@
 <?xml version="1.0" encoding="utf-8"?>
 <packages>
+  <package id="Castle.Core" version="5.0.0" targetFramework="net48" />
   <package id="Microsoft.Bcl" version="1.1.10" targetFramework="net462" />
   <package id="Microsoft.Bcl.Build" version="1.0.21" targetFramework="net462" />
   <package id="Microsoft.Net.Http" version="2.2.29" targetFramework="net462" />
   <package id="NHunspell" version="1.2.5554.16953" targetFramework="net40" />
+  <package id="NSubstitute" version="5.0.0" targetFramework="net48" />
+  <package id="System.Threading.Tasks.Extensions" version="4.3.0" targetFramework="net48" />
 </packages>

src/ui/Plugins/LocalPluginMetadataProvider.cs

@@ -6,7 +6,12 @@ using System.Reflection;
 
 namespace Nikse.SubtitleEdit.Plugins
 {
-    public class LocalPluginMetadataProvider
+    public interface ILocalPluginMetadataProvider
+    {
+        IReadOnlyCollection<LocalPlugin> GetInstalledPlugins();
+    }
+
+    public class LocalPluginMetadataProvider : ILocalPluginMetadataProvider
     {
         private const string KnownPluginExtension = "*.dll";
 

src/ui/Plugins/OnlinePluginMetadataProvider.cs

@@ -5,7 +5,12 @@ using System.Xml.Linq;
 
 namespace Nikse.SubtitleEdit.Plugins
 {
-    public class OnlinePluginMetadataProvider
+    public interface IOnlinePluginMetadataProvider
+    {
+        Task<IReadOnlyCollection<PluginInfo>> GetPluginsAsync();
+    }
+
+    public class OnlinePluginMetadataProvider : IOnlinePluginMetadataProvider
     {
         private readonly string _githubUrl;
 

src/ui/Plugins/PluginUpdateChecker.cs

@@ -7,8 +7,8 @@ namespace Nikse.SubtitleEdit.Plugins
 {
     public class PluginUpdateChecker
     {
-        private readonly LocalPluginMetadataProvider _localPluginMetadataProvider;
+        private readonly ILocalPluginMetadataProvider _localPluginMetadataProvider;
-        private readonly OnlinePluginMetadataProvider _onlinePluginMetadataProvider;
+        private readonly IOnlinePluginMetadataProvider _onlinePluginMetadataProvider;
         private DateTime _lastCheckDate;
 
         /// <summary>
@@ -17,9 +17,16 @@ namespace Nikse.SubtitleEdit.Plugins
         public DateTime LastCheckDate => _lastCheckDate;
 
         public PluginUpdateChecker(PluginUpdateCheckerOptions options)
+            : this(new LocalPluginMetadataProvider(options.PluginDirectory),
+                new OnlinePluginMetadataProvider(options.GithubUrl))
         {
-            _localPluginMetadataProvider = new LocalPluginMetadataProvider(options.PluginDirectory);
+        }
-            _onlinePluginMetadataProvider = new OnlinePluginMetadataProvider(options.GithubUrl);
+
+        public PluginUpdateChecker(ILocalPluginMetadataProvider localPluginMetadataProvider,
+            IOnlinePluginMetadataProvider onlinePluginMetadataProvider)
+        {
+            _localPluginMetadataProvider = localPluginMetadataProvider;
+            _onlinePluginMetadataProvider = onlinePluginMetadataProvider;
         }
 
         public async Task<PluginUpdateCheckResult> CheckAsync()