SEBWIN-674: Extended unit test coverage for third-party application logic.

SafeExamBrowser.Applications.UnitTests/ApplicationFactoryTests.cs

@@ -13,6 +13,7 @@ using SafeExamBrowser.Applications.Contracts;
 using SafeExamBrowser.Logging.Contracts;
 using SafeExamBrowser.Monitoring.Contracts.Applications;
 using SafeExamBrowser.Settings.Applications;
+using SafeExamBrowser.SystemComponents.Contracts.Registry;
 using SafeExamBrowser.WindowsApi.Contracts;
 
 namespace SafeExamBrowser.Applications.UnitTests
@@ -24,6 +25,7 @@ namespace SafeExamBrowser.Applications.UnitTests
 		private Mock<IModuleLogger> logger;
 		private Mock<INativeMethods> nativeMethods;
 		private Mock<IProcessFactory> processFactory;
+		private Mock<IRegistry> registry;
 
 		private ApplicationFactory sut;
 
@@ -34,8 +36,9 @@ namespace SafeExamBrowser.Applications.UnitTests
 			logger = new Mock<IModuleLogger>();
 			nativeMethods = new Mock<INativeMethods>();
 			processFactory = new Mock<IProcessFactory>();
+			registry = new Mock<IRegistry>();
 
-			sut = new ApplicationFactory(applicationMonitor.Object, logger.Object, nativeMethods.Object, processFactory.Object);
+			sut = new ApplicationFactory(applicationMonitor.Object, logger.Object, nativeMethods.Object, processFactory.Object, registry.Object);
 		}
 
 		[TestMethod]
@@ -54,12 +57,32 @@ namespace SafeExamBrowser.Applications.UnitTests
 			Assert.IsInstanceOfType<ExternalApplication>(application);
 		}
 
+		[TestMethod]
+		public void MustCorrectlyReadPathFromRegistry()
+		{
+			var o = default(object);
+			var settings = new WhitelistApplication
+			{
+				DisplayName = "Windows Command Prompt",
+				ExecutableName = "cmd.exe",
+			};
+
+			var result = sut.TryCreate(settings, out var application);
+
+			registry.Verify(r => r.TryRead(It.Is<string>(s => s.Contains(RegistryValue.MachineHive.AppPaths_Key)), It.Is<string>(s => s == "Path"), out o), Times.Once);
+
+			Assert.AreEqual(FactoryResult.Success, result);
+			Assert.IsNotNull(application);
+			Assert.IsInstanceOfType<ExternalApplication>(application);
+		}
+
 		[TestMethod]
 		public void MustIndicateIfApplicationNotFound()
 		{
 			var settings = new WhitelistApplication
 			{
-				ExecutableName = "some_random_application_which_does_not_exist_on_a_normal_system.exe"
+				ExecutableName = "some_random_application_which_does_not_exist_on_a_normal_system.exe",
+				ExecutablePath = "Some/Path/Which/Does/Not/Exist"
 			};
 
 			var result = sut.TryCreate(settings, out var application);
@@ -71,13 +94,10 @@ namespace SafeExamBrowser.Applications.UnitTests
 		[TestMethod]
 		public void MustFailGracefullyAndIndicateThatErrorOccurred()
 		{
-			var settings = new WhitelistApplication
+			var o = default(object);
-			{
+			var settings = new WhitelistApplication();
-				DisplayName = "Windows Command Prompt",
-				ExecutableName = "cmd.exe",
-			};
 
-			logger.Setup(l => l.CloneFor(It.IsAny<string>())).Throws<Exception>();
+			registry.Setup(r => r.TryRead(It.IsAny<string>(), It.IsAny<string>(), out o)).Throws<Exception>();
 
 			var result = sut.TryCreate(settings, out var application);
 

SafeExamBrowser.Applications.UnitTests/ExternalApplicationTests.cs

@@ -0,0 +1,167 @@
+/*
+ * Copyright (c) 2023 ETH Zürich, Educational Development and Technology (LET)
+ * 
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
+using SafeExamBrowser.Core.Contracts.Resources.Icons;
+using SafeExamBrowser.Logging.Contracts;
+using SafeExamBrowser.Monitoring.Contracts.Applications;
+using SafeExamBrowser.Monitoring.Contracts.Applications.Events;
+using SafeExamBrowser.Settings.Applications;
+using SafeExamBrowser.WindowsApi.Contracts;
+
+namespace SafeExamBrowser.Applications.UnitTests
+{
+	[TestClass]
+	public class ExternalApplicationTests
+	{
+		private Mock<IApplicationMonitor> applicationMonitor;
+		private string executablePath;
+		private Mock<IModuleLogger> logger;
+		private Mock<INativeMethods> nativeMethods;
+		private Mock<IProcessFactory> processFactory;
+		private WhitelistApplication settings;
+
+		private ExternalApplication sut;
+
+		[TestInitialize]
+		public void Initialize()
+		{
+			applicationMonitor = new Mock<IApplicationMonitor>();
+			executablePath = @"C:\Some\Random\Path\Application.exe";
+			logger = new Mock<IModuleLogger>();
+			nativeMethods = new Mock<INativeMethods>();
+			processFactory = new Mock<IProcessFactory>();
+			settings = new WhitelistApplication();
+
+			logger.Setup(l => l.CloneFor(It.IsAny<string>())).Returns(new Mock<IModuleLogger>().Object);
+
+			sut = new ExternalApplication(applicationMonitor.Object, executablePath, logger.Object, nativeMethods.Object, processFactory.Object, settings, 1);
+		}
+
+		[TestMethod]
+		public void GetWindows_MustCorrectlyReturnOpenWindows()
+		{
+			var sync = new AutoResetEvent(false);
+			var process1 = new Mock<IProcess>();
+			var process2 = new Mock<IProcess>();
+			var openWindows = new List<IntPtr> { new IntPtr(123), new IntPtr(234), new IntPtr(456), new IntPtr(345), new IntPtr(567), new IntPtr(789), };
+
+			nativeMethods.Setup(n => n.GetOpenWindows()).Returns(openWindows);
+			nativeMethods.Setup(n => n.GetProcessIdFor(It.Is<IntPtr>(p => p == new IntPtr(234)))).Returns(1234);
+			nativeMethods.Setup(n => n.GetProcessIdFor(It.Is<IntPtr>(p => p == new IntPtr(345)))).Returns(1234);
+			nativeMethods.Setup(n => n.GetProcessIdFor(It.Is<IntPtr>(p => p == new IntPtr(567)))).Returns(5678);
+			process1.Setup(p => p.TryClose(It.IsAny<int>())).Returns(false);
+			process1.Setup(p => p.TryKill(It.IsAny<int>())).Returns(true);
+			process1.SetupGet(p => p.Id).Returns(1234);
+			process2.Setup(p => p.TryClose(It.IsAny<int>())).Returns(true);
+			process2.SetupGet(p => p.Id).Returns(5678);
+			processFactory.Setup(f => f.StartNew(It.IsAny<string>(), It.IsAny<string[]>())).Returns(process1.Object);
+
+			sut.WindowsChanged += () => sync.Set();
+			sut.Initialize();
+			sut.Start();
+
+			applicationMonitor.Raise(m => m.InstanceStarted += null, sut.Id, process2.Object);
+
+			sync.WaitOne();
+			sync.WaitOne();
+
+			var windows = sut.GetWindows();
+
+			Assert.AreEqual(3, windows.Count());
+			Assert.IsTrue(windows.Any(w => w.Handle == new IntPtr(234)));
+			Assert.IsTrue(windows.Any(w => w.Handle == new IntPtr(345)));
+			Assert.IsTrue(windows.Any(w => w.Handle == new IntPtr(567)));
+		}
+
+		[TestMethod]
+		public void Initialize_MustInitializeCorrectly()
+		{
+			settings.AutoStart = new Random().Next(2) == 1;
+
+			sut.Initialize();
+
+			applicationMonitor.VerifyAdd(a => a.InstanceStarted += It.IsAny<InstanceStartedEventHandler>(), Times.Once);
+
+			Assert.AreEqual(settings.AutoStart, sut.AutoStart);
+			Assert.AreEqual(executablePath, (sut.Icon as EmbeddedIconResource).FilePath);
+			Assert.AreEqual(settings.Id, settings.Id);
+			Assert.AreEqual(settings.DisplayName, sut.Name);
+			Assert.AreEqual(settings.Description ?? settings.DisplayName, sut.Tooltip);
+		}
+
+		[TestMethod]
+		public void Start_MustCreateInstanceCorrectly()
+		{
+			settings.Arguments.Add("some_parameter");
+			settings.Arguments.Add("another_parameter");
+			settings.Arguments.Add("yet another parameter");
+
+			sut.Start();
+
+			processFactory.Verify(f => f.StartNew(executablePath, It.Is<string[]>(args => args.All(a => settings.Arguments.Contains(a)))), Times.Once);
+		}
+
+		[TestMethod]
+		public void Start_MustHandleFailureGracefully()
+		{
+			processFactory.Setup(f => f.StartNew(It.IsAny<string>(), It.IsAny<string[]>())).Throws<Exception>();
+
+			sut.Start();
+
+			logger.Verify(l => l.Error(It.IsAny<string>(), It.IsAny<Exception>()), Times.AtLeastOnce);
+			processFactory.Verify(f => f.StartNew(It.IsAny<string>(), It.IsAny<string[]>()), Times.Once);
+		}
+
+		[TestMethod]
+		public void Terminate_MustStopAllInstancesCorrectly()
+		{
+			var process1 = new Mock<IProcess>();
+			var process2 = new Mock<IProcess>();
+
+			process1.Setup(p => p.TryClose(It.IsAny<int>())).Returns(false);
+			process1.Setup(p => p.TryKill(It.IsAny<int>())).Returns(true);
+			process1.SetupGet(p => p.Id).Returns(1234);
+			process2.Setup(p => p.TryClose(It.IsAny<int>())).Returns(true);
+			process2.SetupGet(p => p.Id).Returns(5678);
+			processFactory.Setup(f => f.StartNew(It.IsAny<string>(), It.IsAny<string[]>())).Returns(process1.Object);
+
+			sut.Initialize();
+			sut.Start();
+
+			applicationMonitor.Raise(m => m.InstanceStarted += null, sut.Id, process2.Object);
+			sut.Terminate();
+
+			process1.Verify(p => p.TryClose(It.IsAny<int>()), Times.AtLeastOnce);
+			process1.Verify(p => p.TryKill(It.IsAny<int>()), Times.Once);
+			process2.Verify(p => p.TryClose(It.IsAny<int>()), Times.Once);
+			process2.Verify(p => p.TryKill(It.IsAny<int>()), Times.Never);
+		}
+
+		[TestMethod]
+		public void Terminate_MustHandleFailureGracefully()
+		{
+			var process = new Mock<IProcess>();
+
+			process.Setup(p => p.TryClose(It.IsAny<int>())).Throws<Exception>();
+			processFactory.Setup(f => f.StartNew(It.IsAny<string>(), It.IsAny<string[]>())).Returns(process.Object);
+
+			sut.Initialize();
+			sut.Start();
+			sut.Terminate();
+
+			process.Verify(p => p.TryClose(It.IsAny<int>()), Times.AtLeastOnce);
+			process.Verify(p => p.TryKill(It.IsAny<int>()), Times.Never);
+		}
+	}
+}

SafeExamBrowser.Applications.UnitTests/SafeExamBrowser.Applications.UnitTests.csproj

@@ -83,6 +83,7 @@
   <ItemGroup>
     <Compile Include="ApplicationFactoryTests.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
+    <Compile Include="ExternalApplicationTests.cs" />
   </ItemGroup>
   <ItemGroup>
     <None Include="app.config" />
@@ -113,6 +114,10 @@
       <Project>{30b2d907-5861-4f39-abad-c4abf1b3470e}</Project>
       <Name>SafeExamBrowser.Settings</Name>
     </ProjectReference>
+    <ProjectReference Include="..\SafeExamBrowser.SystemComponents.Contracts\SafeExamBrowser.SystemComponents.Contracts.csproj">
+      <Project>{903129c6-e236-493b-9ad6-c6a57f647a3a}</Project>
+      <Name>SafeExamBrowser.SystemComponents.Contracts</Name>
+    </ProjectReference>
     <ProjectReference Include="..\SafeExamBrowser.WindowsApi.Contracts\SafeExamBrowser.WindowsApi.Contracts.csproj">
       <Project>{7016f080-9aa5-41b2-a225-385ad877c171}</Project>
       <Name>SafeExamBrowser.WindowsApi.Contracts</Name>

SafeExamBrowser.Applications/ApplicationFactory.cs

@@ -9,11 +9,11 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
-using Microsoft.Win32;
 using SafeExamBrowser.Applications.Contracts;
 using SafeExamBrowser.Logging.Contracts;
 using SafeExamBrowser.Monitoring.Contracts.Applications;
 using SafeExamBrowser.Settings.Applications;
+using SafeExamBrowser.SystemComponents.Contracts.Registry;
 using SafeExamBrowser.WindowsApi.Contracts;
 
 namespace SafeExamBrowser.Applications
@@ -24,17 +24,20 @@ namespace SafeExamBrowser.Applications
 		private readonly IModuleLogger logger;
 		private readonly INativeMethods nativeMethods;
 		private readonly IProcessFactory processFactory;
+		private readonly IRegistry registry;
 
 		public ApplicationFactory(
 			IApplicationMonitor applicationMonitor,
 			IModuleLogger logger,
 			INativeMethods nativeMethods,
-			IProcessFactory processFactory)
+			IProcessFactory processFactory,
+			IRegistry registry)
 		{
 			this.applicationMonitor = applicationMonitor;
 			this.logger = logger;
 			this.nativeMethods = nativeMethods;
 			this.processFactory = processFactory;
+			this.registry = registry;
 		}
 
 		public FactoryResult TryCreate(WhitelistApplication settings, out IApplication application)
@@ -71,8 +74,10 @@ namespace SafeExamBrowser.Applications
 
 		private IApplication BuildApplication(string executablePath, WhitelistApplication settings)
 		{
+			const int ONE_SECOND = 1000;
+
 			var applicationLogger = logger.CloneFor(settings.DisplayName);
-			var application = new ExternalApplication(applicationMonitor, executablePath, applicationLogger, nativeMethods, processFactory, settings);
+			var application = new ExternalApplication(applicationMonitor, executablePath, applicationLogger, nativeMethods, processFactory, settings, ONE_SECOND);
 
 			return application;
 		}
@@ -131,19 +136,9 @@ namespace SafeExamBrowser.Applications
 
 		private string QueryPathFromRegistry(WhitelistApplication settings)
 		{
-			try
+			if (registry.TryRead($@"{RegistryValue.MachineHive.AppPaths_Key}\{settings.ExecutableName}", "Path", out var value))
 			{
-				using (var key = Registry.LocalMachine.OpenSubKey($@"SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths\{settings.ExecutableName}"))
+				return value as string;
-				{
-					if (key != null)
-					{
-						return key.GetValue("Path") as string;
-					}
-				}
-			}
-			catch (Exception e)
-			{
-				logger.Error($"Failed to query path in registry for '{settings.ExecutableName}'!", e);
 			}
 
 			return default;

SafeExamBrowser.Applications/ExternalApplication.cs

@@ -23,13 +23,14 @@ namespace SafeExamBrowser.Applications
 	{
 		private readonly object @lock = new object();
 
-		private IApplicationMonitor applicationMonitor;
+		private readonly IApplicationMonitor applicationMonitor;
-		private string executablePath;
+		private readonly string executablePath;
-		private IModuleLogger logger;
+		private readonly IList<ExternalApplicationInstance> instances;
-		private INativeMethods nativeMethods;
+		private readonly IModuleLogger logger;
-		private IList<ExternalApplicationInstance> instances;
+		private readonly INativeMethods nativeMethods;
-		private IProcessFactory processFactory;
+		private readonly IProcessFactory processFactory;
-		private WhitelistApplication settings;
+		private readonly WhitelistApplication settings;
+		private readonly int windowMonitoringInterval;
 
 		public bool AutoStart { get; private set; }
 		public IconResource Icon { get; private set; }
@@ -45,7 +46,8 @@ namespace SafeExamBrowser.Applications
 			IModuleLogger logger,
 			INativeMethods nativeMethods,
 			IProcessFactory processFactory,
-			WhitelistApplication settings)
+			WhitelistApplication settings,
+			int windowMonitoringInterval_ms)
 		{
 			this.applicationMonitor = applicationMonitor;
 			this.executablePath = executablePath;
@@ -54,6 +56,7 @@ namespace SafeExamBrowser.Applications
 			this.instances = new List<ExternalApplicationInstance>();
 			this.processFactory = processFactory;
 			this.settings = settings;
+			this.windowMonitoringInterval = windowMonitoringInterval_ms;
 		}
 
 		public IEnumerable<IApplicationWindow> GetWindows()
@@ -89,18 +92,6 @@ namespace SafeExamBrowser.Applications
 			}
 		}
 
-		private string[] BuildArguments()
-		{
-			var arguments = new List<string>();
-
-			foreach (var argument in settings.Arguments)
-			{
-				arguments.Add(Environment.ExpandEnvironmentVariables(argument));
-			}
-
-			return arguments.ToArray();
-		}
-
 		public void Terminate()
 		{
 			applicationMonitor.InstanceStarted -= ApplicationMonitor_InstanceStarted;
@@ -153,12 +144,24 @@ namespace SafeExamBrowser.Applications
 			WindowsChanged?.Invoke();
 		}
 
+		private string[] BuildArguments()
+		{
+			var arguments = new List<string>();
+
+			foreach (var argument in settings.Arguments)
+			{
+				arguments.Add(Environment.ExpandEnvironmentVariables(argument));
+			}
+
+			return arguments.ToArray();
+		}
+
 		private void InitializeInstance(IProcess process)
 		{
 			lock (@lock)
 			{
 				var instanceLogger = logger.CloneFor($"{Name} ({process.Id})");
-				var instance = new ExternalApplicationInstance(Icon, instanceLogger, nativeMethods, process);
+				var instance = new ExternalApplicationInstance(Icon, instanceLogger, nativeMethods, process, windowMonitoringInterval);
 
 				instance.Terminated += Instance_Terminated;
 				instance.WindowsChanged += () => WindowsChanged?.Invoke();

SafeExamBrowser.Applications/ExternalApplicationInstance.cs

@@ -12,8 +12,8 @@ using System.Linq;
 using System.Timers;
 using SafeExamBrowser.Applications.Contracts;
 using SafeExamBrowser.Applications.Contracts.Events;
-using SafeExamBrowser.Core.Contracts.Resources.Icons;
 using SafeExamBrowser.Applications.Events;
+using SafeExamBrowser.Core.Contracts.Resources.Icons;
 using SafeExamBrowser.Logging.Contracts;
 using SafeExamBrowser.WindowsApi.Contracts;
 
@@ -23,24 +23,32 @@ namespace SafeExamBrowser.Applications
 	{
 		private readonly object @lock = new object();
 
-		private IconResource icon;
+		private readonly IconResource icon;
-		private ILogger logger;
+		private readonly ILogger logger;
-		private INativeMethods nativeMethods;
+		private readonly INativeMethods nativeMethods;
-		private IProcess process;
+		private readonly IProcess process;
+		private readonly int windowMonitoringInterval;
+		private readonly IList<ExternalApplicationWindow> windows;
+
 		private Timer timer;
-		private IList<ExternalApplicationWindow> windows;
 
 		internal int Id { get; private set; }
 
 		internal event InstanceTerminatedEventHandler Terminated;
 		internal event WindowsChangedEventHandler WindowsChanged;
 
-		internal ExternalApplicationInstance(IconResource icon, ILogger logger, INativeMethods nativeMethods, IProcess process)
+		internal ExternalApplicationInstance(
+			IconResource icon,
+			ILogger logger,
+			INativeMethods nativeMethods,
+			IProcess process,
+			int windowMonitoringInterval_ms)
 		{
 			this.icon = icon;
 			this.logger = logger;
 			this.nativeMethods = nativeMethods;
 			this.process = process;
+			this.windowMonitoringInterval = windowMonitoringInterval_ms;
 			this.windows = new List<ExternalApplicationWindow>();
 		}
 
@@ -145,11 +153,9 @@ namespace SafeExamBrowser.Applications
 
 		private void InitializeEvents()
 		{
-			const int ONE_SECOND = 1000;
-
 			process.Terminated += Process_Terminated;
 
-			timer = new Timer(ONE_SECOND);
+			timer = new Timer(windowMonitoringInterval);
 			timer.Elapsed += Timer_Elapsed;
 			timer.Start();
 		}

SafeExamBrowser.Applications/ExternalApplicationWindow.cs

@@ -16,7 +16,7 @@ namespace SafeExamBrowser.Applications
 {
 	internal class ExternalApplicationWindow : IApplicationWindow
 	{
-		private INativeMethods nativeMethods;
+		private readonly INativeMethods nativeMethods;
 
 		public IntPtr Handle { get; }
 		public IconResource Icon { get; private set; }

SafeExamBrowser.Applications/SafeExamBrowser.Applications.csproj

@@ -82,6 +82,10 @@
       <Project>{30b2d907-5861-4f39-abad-c4abf1b3470e}</Project>
       <Name>SafeExamBrowser.Settings</Name>
     </ProjectReference>
+    <ProjectReference Include="..\SafeExamBrowser.SystemComponents.Contracts\SafeExamBrowser.SystemComponents.Contracts.csproj">
+      <Project>{903129c6-e236-493b-9ad6-c6a57f647a3a}</Project>
+      <Name>SafeExamBrowser.SystemComponents.Contracts</Name>
+    </ProjectReference>
     <ProjectReference Include="..\SafeExamBrowser.WindowsApi.Contracts\SafeExamBrowser.WindowsApi.Contracts.csproj">
       <Project>{7016f080-9aa5-41b2-a225-385ad877c171}</Project>
       <Name>SafeExamBrowser.WindowsApi.Contracts</Name>

SafeExamBrowser.Client/CompositionRoot.cs

@@ -109,7 +109,7 @@ namespace SafeExamBrowser.Client
 
 			var processFactory = new ProcessFactory(ModuleLogger(nameof(ProcessFactory)));
 			var applicationMonitor = new ApplicationMonitor(TWO_SECONDS, ModuleLogger(nameof(ApplicationMonitor)), nativeMethods, processFactory);
-			var applicationFactory = new ApplicationFactory(applicationMonitor, ModuleLogger(nameof(ApplicationFactory)), nativeMethods, processFactory);
+			var applicationFactory = new ApplicationFactory(applicationMonitor, ModuleLogger(nameof(ApplicationFactory)), nativeMethods, processFactory, new Registry(ModuleLogger(nameof(Registry))));
 			var displayMonitor = new DisplayMonitor(ModuleLogger(nameof(DisplayMonitor)), nativeMethods, systemInfo);
 			var explorerShell = new ExplorerShell(ModuleLogger(nameof(ExplorerShell)), nativeMethods);
 			var fileSystemDialog = BuildFileSystemDialog();

SafeExamBrowser.SystemComponents.Contracts/Registry/RegistryValue.cs

@@ -19,6 +19,7 @@ namespace SafeExamBrowser.SystemComponents.Contracts.Registry
 		/// </summary>
 		public static class MachineHive
 		{
+			public const string AppPaths_Key = @"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths";
 			public const string EaseOfAccess_Key = @"HKEY_LOCAL_MACHINE\Software\Microsoft\Windows NT\CurrentVersion\Image File Execution Options\Utilman.exe";
 			public const string EaseOfAccess_Name = "Debugger";
 		}